Rework device probing by moving ata_getparam() call from ata_identify() to

drivers' probe routines. It allows not to sleep and so not drop Giant inside
ata_identify() critical section and so avoid crash if it reentered on
request timeout. Reentering of probe call checked inside of it.

Give device own knowledge about it's type (ata/atapi/atapicam). It is not
a good idea to ask channel status for device type inside ata_getparam().

Add softc memory deallocation on device destruction.

7 files changed, 109 insertions, 51 deletions

diff --git a/sys/dev/ata/ata-all.c b/sys/dev/ata/ata-all.c
index 232b4fe..25eefde 100644
--- a/sys/dev/ata/ata-all.c
+++ b/sys/dev/ata/ata-all.c
@@ -61,7 +61,6 @@ static struct cdevsw ata_cdevsw = {
 
 /* prototypes */
 static void ata_boot_attach(void);
-static device_t ata_add_child(device_t, struct ata_device *, int);
 static void ata_conn_event(void *, int);
 static void bswap(int8_t *, int);
 static void btrim(int8_t *, int);
@@ -180,7 +179,7 @@ ata_detach(device_t dev)
     if (!device_get_children(dev, &children, &nchildren)) {
 	for (i = 0; i < nchildren; i++)
 	    if (children[i])
-		device_delete_child(dev, children[i]);
+		ata_delete_child(dev, children[i]);
 	free(children, M_TEMP);
     } 
     taskqueue_drain(taskqueue_thread, &ch->conntask);
@@ -265,7 +264,7 @@ ata_reinit(device_t dev)
 			    ata_finish(request);
 		    request = NULL;
 		}
-		device_delete_child(dev, children[i]);
+		ata_delete_child(dev, children[i]);
 	    }
 	}
 	free(children, M_TEMP);
@@ -590,19 +589,44 @@ ata_boot_attach(void)
 /*
  * misc support functions
  */
-static device_t
-ata_add_child(device_t parent, struct ata_device *atadev, int unit)
+device_t
+ata_add_child(device_t parent, int unit, int atapi)
 {
+    struct ata_device *atadev;
     device_t child;
+    int dev_unit = -1;
 
-    if ((child = device_add_child(parent, NULL, unit))) {
+#ifdef ATA_STATIC_ID
+    if (!atapi)
+	dev_unit = (device_get_unit(parent) << 1) + unit;
+#endif
+    if ((child = device_add_child(parent, NULL, dev_unit))) {
+	if (!(atadev = malloc(sizeof(struct ata_device),
+			  M_ATA, M_NOWAIT | M_ZERO))) {
+	    device_printf(parent, "out of memory\n");
+	    device_delete_child(parent, child);
+	    return (NULL);
+	}
 	device_set_softc(child, atadev);
 	device_quiet(child);
 	atadev->dev = child;
+	atadev->unit = unit;
+	atadev->type = atapi ? ATA_T_ATAPI : ATA_T_ATA;
 	atadev->max_iosize = DEV_BSIZE;
 	atadev->mode = ATA_PIO_MAX;
     }
-    return child;
+    return (child);
+}
+
+int
+ata_delete_child(device_t parent, device_t child)
+{
+    struct ata_device *atadev = device_get_softc(child);
+    int res;
+
+    res = device_delete_child(parent, child);
+    free(atadev, M_ATA);
+    return (res);
 }
 
 int
@@ -613,11 +637,11 @@ ata_getparam(struct ata_device *atadev, int init)
     u_int8_t command = 0;
     int error = ENOMEM, retries = 2;
 
-    if (ch->devices & (ATA_ATA_MASTER << atadev->unit))
+    if (atadev->type == ATA_T_ATA)
 	command = ATA_ATA_IDENTIFY;
-    if (ch->devices & (ATA_ATAPI_MASTER << atadev->unit))
+    else if (atadev->type == ATA_T_ATAPI)
 	command = ATA_ATAPI_IDENTIFY;
-    if (!command)
+    else
 	return ENXIO;
 
     while (retries-- > 0 && error) {
@@ -663,17 +687,18 @@ ata_getparam(struct ata_device *atadev, int init)
 	btrim(atacap->serial, sizeof(atacap->serial));
 	bpack(atacap->serial, atacap->serial, sizeof(atacap->serial));
 
-	if (bootverbose)
-	    printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
+	if (init) {
+	    char buffer[64];
+
+	    if (bootverbose) {
+		printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
 		   device_get_unit(ch->dev),
 		   ata_unit2str(atadev),
 		   ata_mode2str(ata_pmode(atacap)),
 		   ata_mode2str(ata_wmode(atacap)),
 		   ata_mode2str(ata_umode(atacap)),
 		   (atacap->hwres & ATA_CABLE_ID) ? "80":"40");
-
-	if (init) {
-	    char buffer[64];
+	    }
 
 	    sprintf(buffer, "%.40s/%.8s", atacap->model, atacap->revision);
 	    device_set_desc_copy(atadev->dev, buffer);
@@ -706,7 +731,6 @@ ata_identify(device_t dev)
     struct ata_channel *ch = device_get_softc(dev);
     struct ata_device *atadev;
     device_t *children;
-    device_t child;
     int nchildren, i, n = ch->devices;
 
     if (bootverbose)
@@ -721,37 +745,18 @@ ata_identify(device_t dev)
 	}
 	free(children, M_TEMP);
     }
-    /* Create new devices. */
     if (bootverbose)
 	device_printf(dev, "New devices: %08x\n", n);
     if (n == 0) {
 	mtx_unlock(&Giant);
 	return (0);
     }
+    /* Create new devices. */
     for (i = 0; i < ATA_PM; ++i) {
-	if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) {
-	    int unit = -1;
-
-	    if (!(atadev = malloc(sizeof(struct ata_device),
-				  M_ATA, M_NOWAIT | M_ZERO))) {
-		device_printf(dev, "out of memory\n");
-		return ENOMEM;
-	    }
-	    atadev->unit = i;
-#ifdef ATA_STATIC_ID
-	    if (n & (ATA_ATA_MASTER << i))
-		unit = (device_get_unit(dev) << 1) + i;
-#endif
-	    if ((child = ata_add_child(dev, atadev, unit))) {
-		if (ata_getparam(atadev, 1)) {
-		    device_delete_child(dev, child);
-		    free(atadev, M_ATA);
-		}
-	    }
-	    else
-		free(atadev, M_ATA);
-	}
+	if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i)))
+	    ata_add_child(dev, i, n & (ATA_ATAPI_MASTER << i));
     }
+
     bus_generic_probe(dev);
     bus_generic_attach(dev);
     mtx_unlock(&Giant);
diff --git a/sys/dev/ata/ata-all.h b/sys/dev/ata/ata-all.h
index c00bb1c..8c7f0d1 100644
--- a/sys/dev/ata/ata-all.h
+++ b/sys/dev/ata/ata-all.h
@@ -410,6 +410,10 @@ struct ata_device {
 #define         ATA_MASTER              0x00
 #define         ATA_SLAVE               0x01
 #define         ATA_PM                  0x0f
+    int                         type;           /* device type */
+#define         ATA_T_ATA               0x00
+#define         ATA_T_ATAPI             0x01
+#define         ATA_T_ATAPI_CAM         0x02
 
     struct ata_params           param;          /* ata param structure */
     int                         mode;           /* current transfermode */
@@ -422,6 +426,8 @@ struct ata_device {
 #define         ATA_D_MEDIA_CHANGED     0x0002
 #define         ATA_D_ENC_PRESENT       0x0004
 #define         ATA_D_48BIT_ACTIVE      0x0008
+#define         ATA_D_PROBED            0x0010
+#define         ATA_D_VALID             0x0020
 };
 
 /* structure for holding DMA Physical Region Descriptors (PRD) entries */
@@ -559,6 +565,8 @@ void ata_interrupt(void *data);
 int ata_device_ioctl(device_t dev, u_long cmd, caddr_t data);
 int ata_getparam(struct ata_device *atadev, int init);
 int ata_identify(device_t dev);
+device_t ata_add_child(device_t, int, int);
+int ata_delete_child(device_t , device_t);
 void ata_default_registers(device_t dev);
 void ata_modify_if_48bit(struct ata_request *request);
 void ata_udelay(int interval);
diff --git a/sys/dev/ata/ata-disk.c b/sys/dev/ata/ata-disk.c
index e11f3f9..a1280ac 100644
--- a/sys/dev/ata/ata-disk.c
+++ b/sys/dev/ata/ata-disk.c
@@ -79,6 +79,18 @@ ad_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
+    if (atadev->type != ATA_T_ATA)
+	return (ENXIO);
+
+    if (!(atadev->flags & ATA_D_PROBED)) {
+	atadev->flags |= ATA_D_PROBED;
+	if (ata_getparam(atadev, 1) == 0)
+	    atadev->flags |= ATA_D_VALID;
+    }
+
+    if (!(atadev->flags & ATA_D_VALID))
+	return (ENXIO);
+
     if (!(atadev->param.config & ATA_PROTO_ATAPI) ||
 	(atadev->param.config == ATA_CFA_MAGIC1) ||
 	(atadev->param.config == ATA_CFA_MAGIC2) ||
diff --git a/sys/dev/ata/atapi-cam.c b/sys/dev/ata/atapi-cam.c
index 62e5251..5fb729d 100644
--- a/sys/dev/ata/atapi-cam.c
+++ b/sys/dev/ata/atapi-cam.c
@@ -147,7 +147,7 @@ static void
 atapi_cam_identify(driver_t *driver, device_t parent)
 {
 	struct atapi_xpt_softc *scp =
-	    malloc (sizeof (struct atapi_xpt_softc), M_ATACAM, M_NOWAIT|M_ZERO);
+	    malloc (sizeof (struct atapi_xpt_softc), M_ATA, M_NOWAIT|M_ZERO);
 	device_t child;
 
 	if (scp == NULL) {
@@ -159,10 +159,11 @@ atapi_cam_identify(driver_t *driver, device_t parent)
 	child = device_add_child(parent, "atapicam", -1);
 	if (child == NULL) {
 		printf ("atapi_cam_identify: out of memory, can't add child");
-		free (scp, M_ATACAM);
+		free (scp, M_ATA);
 		return;
 	}
 	scp->atapi_cam_dev.unit = -1;
+	scp->atapi_cam_dev.type = ATA_T_ATAPI_CAM;
 	scp->atapi_cam_dev.dev  = child;
 	device_quiet(child);
 	device_set_softc(child, scp);
@@ -174,12 +175,10 @@ atapi_cam_probe(device_t dev)
 	struct ata_device *atadev = device_get_softc (dev);
 
 	KASSERT(atadev != NULL, ("expect valid struct ata_device"));
-	if (atadev->unit < 0) {
-		device_set_desc(dev, "ATAPI CAM Attachment");
-		return (0);
-	} else {
-		return ENXIO;
-	}
+	if (atadev->type != ATA_T_ATAPI_CAM)
+		return (ENXIO);
+	device_set_desc(dev, "ATAPI CAM Attachment");
+	return (0);
 }
 
 static int
@@ -925,11 +924,8 @@ atapi_cam_event_handler(module_t mod, int what, void *arg) {
 	    if (devlist != NULL) {
 		while (devlist != NULL && devcount > 0) {
 		    device_t child = devlist[--devcount];
-		    struct atapi_xpt_softc *scp = device_get_softc(child);
 
-		    device_delete_child(device_get_parent(child),child);
-		    if (scp != NULL)
-			free(scp, M_ATACAM);
+		    ata_delete_child(device_get_parent(child),child);
 		}
 		free(devlist, M_TEMP);
 	    }
diff --git a/sys/dev/ata/atapi-cd.c b/sys/dev/ata/atapi-cd.c
index 7fd80c4..ba0c7c8 100644
--- a/sys/dev/ata/atapi-cd.c
+++ b/sys/dev/ata/atapi-cd.c
@@ -108,6 +108,18 @@ acd_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
+    if (atadev->type != ATA_T_ATAPI)
+	return (ENXIO);
+
+    if (!(atadev->flags & ATA_D_PROBED)) {
+	atadev->flags |= ATA_D_PROBED;
+	if (ata_getparam(atadev, 1) == 0)
+	    atadev->flags |= ATA_D_VALID;
+    }
+
+    if (!(atadev->flags & ATA_D_VALID))
+	return (ENXIO);
+
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_CDROM)
 	return 0;
diff --git a/sys/dev/ata/atapi-fd.c b/sys/dev/ata/atapi-fd.c
index b8434b3..f8926bd 100644
--- a/sys/dev/ata/atapi-fd.c
+++ b/sys/dev/ata/atapi-fd.c
@@ -66,6 +66,19 @@ static int
 afd_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
+
+    if (atadev->type != ATA_T_ATAPI)
+	return (ENXIO);
+
+    if (!(atadev->flags & ATA_D_PROBED)) {
+	atadev->flags |= ATA_D_PROBED;
+	if (ata_getparam(atadev, 1) == 0)
+	    atadev->flags |= ATA_D_VALID;
+    }
+
+    if (!(atadev->flags & ATA_D_VALID))
+	return (ENXIO);
+
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_DIRECT)
 	return 0;  
diff --git a/sys/dev/ata/atapi-tape.c b/sys/dev/ata/atapi-tape.c
index aa44afe..1dfdd5c 100644
--- a/sys/dev/ata/atapi-tape.c
+++ b/sys/dev/ata/atapi-tape.c
@@ -90,6 +90,18 @@ ast_probe(device_t dev)
 {
     struct ata_device *atadev = device_get_softc(dev);
 
+    if (atadev->type != ATA_T_ATAPI)
+	return (ENXIO);
+
+    if (!(atadev->flags & ATA_D_PROBED)) {
+	atadev->flags |= ATA_D_PROBED;
+	if (ata_getparam(atadev, 1) == 0)
+	    atadev->flags |= ATA_D_VALID;
+    }
+
+    if (!(atadev->flags & ATA_D_VALID))
+	return (ENXIO);
+
     if ((atadev->param.config & ATA_PROTO_ATAPI) &&
 	(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_TAPE)
 	return 0;