[FIXED] ATAPI CD boot problems

I've found and fixed several problems in ata/acd/atapicam locally, but
I've identified several more.  Of note, here are the simple ones.  The
first one is the most important!

1. ATAPI devices come up in an inconsistent state sometimes.  Issuing
   an ATAPI reset seems to fix this completely; normally I would see,
   most of the time, these messages:
   acd0: MODE_SENSE_BIG trying to write on read buffer
   acd0: timeout waiting for ATAPI ready
   acd0: unknown transfer phase 0x1:0x50
   acd0: timeout waiting for ATAPI ready
   <lock-up or crash>
2. Unknown transfer phase what?  The old message was very undescriptive.
3. If using ATAPICAM and the attach fails, it'll crash due to freeing
   in-flight CCBs out from underneath. [not fixed]
4. If not using ATAPICAM, the timeout() function is set and never
   reset after each "timeout waiting for ATAPI ready".  I made it
   untimeout() immediately if that happens, and things are much
   smoother (at least without ATAPICAM, when failing to attach an
   acd device).
5. An acd device finishes attaching before GEOM has fully attached to it.
6. Detachment is done at the wrong places, so generally requests are
   still queued to a dying device.

There are still lots of detachment issues and furthermore I don't think
that my computer won't lock up when trying to rip one of my new CDs, but
this is a huge improvement, so see if it helps your problems with ATAPI
CD/DVD drives in current.

cvs diff: Diffing .
Index: ata-all.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/ata/ata-all.c,v
retrieving revision 1.214
diff -u -r1.214 ata-all.c
--- ata-all.c	22 Jun 2004 11:18:24 -0000	1.214
+++ ata-all.c	26 Jun 2004 03:38:25 -0000
_at__at_ -183,17 +183,20 _at__at_
     if (!dev || !(ch = device_get_softc(dev)) || !ch->r_irq)
 	return ENXIO;
 
-    /* detach devices on this channel */
     if (ch->device[MASTER].detach)
-	ch->device[MASTER].detach(&ch->device[MASTER]);
+	ch->device[MASTER].flags |= ATA_D_DETACHING;
     if (ch->device[SLAVE].detach)
-	ch->device[SLAVE].detach(&ch->device[SLAVE]);
+	ch->device[SLAVE].flags |= ATA_D_DETACHING;
+    /* fail outstanding requests on this channel */
+    ata_fail_requests(ch, NULL);
 #ifdef DEV_ATAPICAM
     atapi_cam_detach_bus(ch);
 #endif
-
-    /* fail outstanding requests on this channel */
-    ata_fail_requests(ch, NULL);
+    /* detach devices on this channel */
+    if (ch->device[MASTER].detach)
+	ch->device[MASTER].detach(&ch->device[MASTER]);
+    if (ch->device[SLAVE].detach)
+	ch->device[SLAVE].detach(&ch->device[SLAVE]);
 
     /* flush cache and powerdown device */
     if (ch->device[MASTER].param) {
_at__at_ -254,8 +257,9 _at__at_
 		request->result = ENXIO;
 		request->retries = 0;
 	    }
-	    ch->device[MASTER].detach(&ch->device[MASTER]);
+	    ch->device[MASTER].flags |= ATA_D_DETACHING;
 	    ata_fail_requests(ch, &ch->device[MASTER]);
+	    ch->device[MASTER].detach(&ch->device[MASTER]);
 	    free(ch->device[MASTER].param, M_ATA);
 	    ch->device[MASTER].param = NULL;
 	}
_at__at_ -265,8 +269,9 _at__at_
 		request->result = ENXIO;
 		request->retries = 0;
 	    }
-	    ch->device[SLAVE].detach(&ch->device[SLAVE]);
+	    ch->device[SLAVE].flags |= ATA_D_DETACHING;
 	    ata_fail_requests(ch, &ch->device[SLAVE]);
+	    ch->device[SLAVE].detach(&ch->device[SLAVE]);
 	    free(ch->device[SLAVE].param, M_ATA);
 	    ch->device[SLAVE].param = NULL;
 	}
Index: ata-disk.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/ata/ata-disk.c,v
retrieving revision 1.173
diff -u -r1.173 ata-disk.c
--- ata-disk.c	22 Jun 2004 11:18:24 -0000	1.173
+++ ata-disk.c	26 Jun 2004 03:38:25 -0000
_at__at_ -160,7 +160,6 _at__at_
 {
     struct ad_softc *adp = atadev->softc;
 
-    atadev->flags |= ATA_D_DETACHING;
 #ifdef DEV_ATARAID
     if (adp->flags & AD_F_RAID_SUBDISK)
 	ata_raiddisk_detach(adp);
Index: ata-lowlevel.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/ata/ata-lowlevel.c,v
retrieving revision 1.38
diff -u -r1.38 ata-lowlevel.c
--- ata-lowlevel.c	11 Jun 2004 07:39:15 -0000	1.38
+++ ata-lowlevel.c	26 Jun 2004 03:38:25 -0000
_at__at_ -476,7 +476,8 _at__at_
 	    break;
 
 	default:
-	    ata_prtdev(request->device, "unknown transfer phase\n");
+	    ata_prtdev(request->device, "unknown transfer phase %#x:%#x\n",
+		ATA_IDX_INB(ch, ATA_IREASON), request->status);
 	    request->status = ATA_S_ERROR;
 	}
 
Index: ata-queue.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/ata/ata-queue.c,v
retrieving revision 1.29
diff -u -r1.29 ata-queue.c
--- ata-queue.c	1 Jun 2004 12:26:08 -0000	1.29
+++ ata-queue.c	30 Jun 2004 00:15:39 -0000
_at__at_ -211,7 +211,8 _at__at_
     ATA_DEBUG_RQ(request, "taskqueue completition");
 
     /* request is done schedule it for completition */
-    if (request->device->channel->flags & ATA_IMMEDIATE_MODE) {
+    if (request->device->channel->flags & ATA_IMMEDIATE_MODE ||
+	request->result != 0) {
 	ata_completed(request, 0);
     }
     else {
Index: atapi-cd.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/ata/atapi-cd.c,v
retrieving revision 1.168
diff -u -r1.168 atapi-cd.c
--- atapi-cd.c	22 Jun 2004 11:18:24 -0000	1.168
+++ atapi-cd.c	30 Jun 2004 01:49:02 -0000
_at__at_ -113,6 +113,8 _at__at_
     }
 
     ata_set_name(atadev, "acd", cdp->lun);
+    /* perform a reset to get hardware in a known state */
+    (void)ata_controlcmd(atadev, ATA_ATAPI_RESET, 0, 0, 0);
     acd_get_cap(cdp);
 
     /* if this is a changer device, allocate the neeeded lun's */
_at__at_ -154,7 +156,7 _at__at_
 		tmpcdp->driver = cdparr;
 		tmpcdp->slot = count;
 		tmpcdp->changer_info = chp;
-		g_post_event(acd_geom_create, tmpcdp, M_WAITOK, NULL);
+		g_waitfor_event(acd_geom_create, tmpcdp, M_WAITOK, NULL);
 	    }
 	    if (!(name = malloc(strlen(atadev->name) + 2, M_ACD, M_NOWAIT))) {
 		ata_prtdev(atadev, "out of memory\n");
_at__at_ -169,7 +171,7 _at__at_
 	}
     }
     else 
-	g_post_event(acd_geom_create, cdp, M_WAITOK, NULL);
+	g_waitfor_event(acd_geom_create, cdp, M_WAITOK, NULL);
 
     /* setup the function ptrs */
     atadev->detach = acd_detach;

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green_at_FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\