Re: Floppy disk drive no longer detected

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: <floppy drive controller (FDE)> port 0x3f7,0x3f0-0x3f5 irq 6 drq 2 on acpi0
>>
>>Previously it probed as:
>>
>>fdc0: <Enhanced floppy controller (i82077, NE72065 or clone)> 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

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
_at__at_ -42,21 +42,31 _at__at_
 
 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;
_at__at_ -88,7 +98,6 _at__at_
 static int
 fdc_acpi_attach(device_t dev)
 {
-	struct fdc_walk_ctx *ctx;
 	struct fdc_data *sc;
 	ACPI_BUFFER buf;
 	device_t bus;
_at__at_ -144,30 +153,10 _at__at_
 	 * 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)
_at__at_ -178,6 +167,40 _at__at_
 	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)
 {
_at__at_ -187,29 +210,38 _at__at_
 	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;
_at__at_ -222,12 +254,6 _at__at_
 		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: