From owner-freebsd-current@FreeBSD.ORG Mon Jul 26 22:23:18 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0361F16A4CE; Mon, 26 Jul 2004 22:23:18 +0000 (GMT) Received: from www.cryptography.com (li-22.members.linode.com [64.5.53.22]) by mx1.FreeBSD.org (Postfix) with ESMTP id A939543D2F; Mon, 26 Jul 2004 22:23:17 +0000 (GMT) (envelope-from nate@root.org) Received: from [10.0.0.34] (adsl-67-127-84-57.dsl.snfc21.pacbell.net [67.127.84.57]) by www.cryptography.com (8.12.8/8.12.8) with ESMTP id i6QMNFra008789 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Mon, 26 Jul 2004 15:23:16 -0700 Message-ID: <410583B3.4000104@root.org> Date: Mon, 26 Jul 2004 15:20:35 -0700 From: Nate Lawson User-Agent: Mozilla Thunderbird 0.6 (X11/20040518) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Tim Robbins References: <20040726021326.GA23697@cat.robbins.dropbear.id.au> <20040726095929.GA30092@cat.robbins.dropbear.id.au> In-Reply-To: <20040726095929.GA30092@cat.robbins.dropbear.id.au> Content-Type: multipart/mixed; boundary="------------090108010800030208080306" cc: current@freebsd.org cc: Marcel Moolenaar Subject: Re: Floppy disk drive no longer detected X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Jul 2004 22:23:18 -0000 This is a multi-part message in MIME format. --------------090108010800030208080306 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Tim Robbins wrote: > On Mon, Jul 26, 2004 at 12:13:26PM +1000, Tim Robbins wrote: >>My floppy disk drive is no longer probed correctly by -CURRENT. I get the >>following message, but none about fd0 (and /dev/fd0 does not exist): >> >>fdc0: port 0x3f7,0x3f0-0x3f5 irq 6 drq 2 on acpi0 >> >>Previously it probed as: >> >>fdc0: port 0x3f7,0x3f0-0x3f5 irq 6 drq 2 on acpi0 >>fdc0: FIFO enabled, 8 bytes threshold >>fd0: <1440-KB 3.5" drive> on fdc0 drive 0 >> >>The ACPI DSDT for this system is available at: >>http://people.freebsd.org/~tjr/k8v.txt.bz2 > > Ok, it looks like the FDC device should have a child device for each drive, > but my system's DSDT has none. Could we work around this in the driver by > not using _FDE/_FDI to probe unless the FDC device has at least 1 child with > an _FDI object, or should we add an entry to the quirks table for this > particular BIOS instead? Ok, below is a patch to address this. There are two things this fixes. The first, which I think Marcel's laptop had from the dmesg I saw, is that _FDI is not required so a system may have child devices of FDC that lack an _FDI method. The second is Tim's bug, where _FDE is present on the FDC but there are no children. We handle this with a second pass after the namespace walk to add missing devices indicated as present by _FDE but not represented in the namespace. I also noticed that his ASL uses a non-standard approach to _FDE but one that is compatible with the standard so I fixed this also. Please test. -Nate --------------090108010800030208080306 Content-Type: text/plain; name="fdc_acpi_fix.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="fdc_acpi_fix.diff" Index: fdc_acpi.c =================================================================== RCS file: /home/ncvs/src/sys/dev/fdc/fdc_acpi.c,v retrieving revision 1.1 diff -u -r1.1 fdc_acpi.c --- fdc_acpi.c 15 Jul 2004 16:38:07 -0000 1.1 +++ fdc_acpi.c 26 Jul 2004 22:12:06 -0000 @@ -42,21 +42,31 @@ static int fdc_acpi_probe(device_t dev); static int fdc_acpi_attach(device_t dev); +static int fdc_acpi_probe_children(device_t bus, device_t dev, + void *fde); static ACPI_STATUS fdc_acpi_probe_child(ACPI_HANDLE h, device_t *dev, int level, void *arg); static void fdctl_wr_acpi(fdc_p fdc, u_int8_t v); -/* Parameters for the tape drive (5th device). */ -#define ACPI_TAPE_UNKNOWN 0 -#define ACPI_TAPE_PRESENT 1 -#define ACPI_TAPE_NEVER_PRESENT 2 +/* Maximum number of child devices of a controller (4 floppy + 1 tape.) */ +#define ACPI_FDC_MAXDEVS 5 + +/* + * Parameters for the tape drive (5th device). Some BIOS authors use this + * for all drives, not just the tape drive (e.g., ASUS K8V). This isn't + * grossly incompatible with the spec since it says the first four devices + * are simple booleans. + */ +#define ACPI_FD_UNKNOWN 0 +#define ACPI_FD_PRESENT 1 +#define ACPI_FD_NEVER_PRESENT 2 /* Temporary buf length for evaluating _FDE and _FDI. */ #define ACPI_FDC_BUFLEN 1024 /* Context for walking FDC child devices. */ struct fdc_walk_ctx { - uint32_t fd_present[5]; + uint32_t fd_present[ACPI_FDC_MAXDEVS]; int index; device_t acpi_dev; device_t dev; @@ -88,7 +98,6 @@ static int fdc_acpi_attach(device_t dev) { - struct fdc_walk_ctx *ctx; struct fdc_data *sc; ACPI_BUFFER buf; device_t bus; @@ -144,30 +153,10 @@ * this fails, fall back to the ISA hints-based probe method. */ bus = device_get_parent(dev); - if (ACPI_SUCCESS(ACPI_EVALUATE_OBJECT(bus, dev, "_FDE", NULL, &buf))) { - /* Setup the context and walk all child devices. */ - ctx = malloc(sizeof(struct fdc_walk_ctx), M_TEMP, M_NOWAIT); - if (ctx == NULL) { - device_printf(dev, "no memory for walking children\n"); - goto out; - } - bcopy(buf.Pointer, ctx->fd_present, sizeof(ctx->fd_present)); - ctx->index = 0; - ctx->dev = dev; - ctx->acpi_dev = bus; - ACPI_SCAN_CHILDREN(ctx->acpi_dev, dev, 1, fdc_acpi_probe_child, - ctx); - free(ctx, M_TEMP); - - /* Attach any children found during the probe. */ - error = bus_generic_attach(dev); - if (error != 0) - goto out; - } else { + if (ACPI_SUCCESS(ACPI_EVALUATE_OBJECT(bus, dev, "_FDE", NULL, &buf))) + error = fdc_acpi_probe_children(bus, dev, buf.Pointer); + else error = fdc_hints_probe(dev); - if (error != 0) - goto out; - } out: if (buf.Pointer) @@ -178,6 +167,40 @@ return (error); } +static int +fdc_acpi_probe_children(device_t bus, device_t dev, void *fde) +{ + struct fdc_walk_ctx *ctx; + devclass_t fd_dc; + int i; + + /* Setup the context and walk all child devices. */ + ctx = malloc(sizeof(struct fdc_walk_ctx), M_TEMP, M_NOWAIT); + if (ctx == NULL) { + device_printf(dev, "no memory for walking children\n"); + return (ENOMEM); + } + bcopy(fde, ctx->fd_present, sizeof(ctx->fd_present)); + ctx->index = 0; + ctx->dev = dev; + ctx->acpi_dev = bus; + ACPI_SCAN_CHILDREN(ctx->acpi_dev, dev, 1, fdc_acpi_probe_child, + ctx); + + /* Add any devices not represented by an AML Device handle/node. */ + fd_dc = devclass_find("fd"); + for (i = 0; i < ACPI_FDC_MAXDEVS; i++) + if (ctx->fd_present[i] == ACPI_FD_PRESENT && + devclass_get_device(fd_dc, i) == NULL) { + if (fdc_add_child(ctx->dev, "fd", i) == NULL) + device_printf(dev, "fd add failed\n"); + } + free(ctx, M_TEMP); + + /* Attach any children found during the probe. */ + return (bus_generic_attach(dev)); +} + static ACPI_STATUS fdc_acpi_probe_child(ACPI_HANDLE h, device_t *dev, int level, void *arg) { @@ -187,29 +210,38 @@ ACPI_OBJECT *pkg, *obj; ACPI_STATUS status; + ctx = (struct fdc_walk_ctx *)arg; + buf.Pointer = NULL; + /* * The first four ints are booleans that indicate whether fd0-3 are * present or not. The last is for a tape device, which we don't * bother supporting for now. */ - ctx = (struct fdc_walk_ctx *)arg; if (ctx->index > 3) return (AE_OK); + /* This device is not present, move on to the next. */ + if (ctx->fd_present[ctx->index] != ACPI_FD_PRESENT) + goto out; + + /* Create a device for the child with the given index. */ + child = fdc_add_child(ctx->dev, "fd", ctx->index); + if (child == NULL) + goto out; + *dev = child; + /* Get temporary buffer for _FDI probe. */ buf.Length = ACPI_FDC_BUFLEN; buf.Pointer = malloc(buf.Length, M_TEMP, M_NOWAIT | M_ZERO); if (buf.Pointer == NULL) goto out; - /* This device is not present, move on to the next. */ - if (ctx->fd_present[ctx->index] == 0) - goto out; - /* Evaluate _FDI to get drive type to pass to the child. */ status = ACPI_EVALUATE_OBJECT(ctx->acpi_dev, *dev, "_FDI", NULL, &buf); if (ACPI_FAILURE(status)) { - device_printf(ctx->dev, "_FDI not available - %#x\n", status); + if (status != AE_NOT_FOUND) + device_printf(ctx->dev, "_FDI failed - %#x\n", status); goto out; } pkg = (ACPI_OBJECT *)buf.Pointer; @@ -222,12 +254,6 @@ device_printf(ctx->dev, "invalid type object in _FDI\n"); goto out; } - - /* Create a device for the child with the given index and set ivars. */ - child = fdc_add_child(ctx->dev, "fd", ctx->index); - if (child == NULL) - goto out; - *dev = child; fdc_set_fdtype(child, obj->Integer.Value); out: --------------090108010800030208080306--