kern/103602: drive gets wedged on READ CD CAPACITY if no disc is in

[Thomas Quinot]

* Josh Carroll, 2007-04-25 :

> I used the following patch (attached) against a RELENG_6_2 src tree,
> and it's working brilliantly. No problems to speak of. I can boot with
> atapicam in the kernel without a disk in the drive and there were no
> hangs at all.

Good! It's relieving that we have finally zeroed in on this one!

I have committed the DMA change to HEAD; I'll wait a bit for things to
stabilize before merging it into RELENG_6.

Now it would be interesting to test whether the scsi_cd and cam_xpt
changes are still required in your configuration, or whether they were
just working around the real underlying issue.

You'll find attached to this message a patch against RELENG_6
atapi-cam.c (which includes the DMA fix and also a related error
handling fix from kern/112119). Could you please test whether
a kernel with:
  - just this patch;
  - this patch + the cam_xpt change (CAM_QUIRK_NOSERIAL)
works for you?

Thanks!
Thomas.

Index: atapi-cam.c
===================================================================
RCS file: /space/mirror/ncvs/src/sys/dev/ata/atapi-cam.c,v
retrieving revision 1.42.2.3
diff -u -r1.42.2.3 atapi-cam.c
--- atapi-cam.c	29 Mar 2007 20:08:32 -0000	1.42.2.3
+++ atapi-cam.c	30 Apr 2007 09:39:46 -0000
@@ -513,10 +513,10 @@
 
 	switch (ccb_h->flags & CAM_DIR_MASK) {
 	case CAM_DIR_IN:
-	     request_flags |= ATA_R_READ|ATA_R_DMA;
+	     request_flags |= ATA_R_READ;
 	     break;
 	case CAM_DIR_OUT:
-	     request_flags |= ATA_R_WRITE|ATA_R_DMA;
+	     request_flags |= ATA_R_WRITE;
 	     break;
 	case CAM_DIR_NONE:
 	     /* No flags need to be set */
@@ -525,8 +525,6 @@
 	     device_printf(softc->dev, "unknown IO operation\n");
 	     goto action_invalid;
 	}
-	if (softc->atadev[tid]->mode < ATA_DMA)
-	    request_flags &= ~ATA_R_DMA;
 
 	if ((hcb = allocate_hcb(softc, unit, bus, ccb)) == NULL) {
 	    printf("cannot allocate ATAPI/CAM hcb\n");
@@ -591,7 +589,24 @@
 	    request->u.atapi.ccb[3] = request->u.atapi.ccb[1] & 0x1f;
 	    request->u.atapi.ccb[2] = 0;
 	    request->u.atapi.ccb[1] = 0;
+	    /* FALLTHROUGH */
+
+	case READ_10:
+	    /* FALLTHROUGH */
+	case WRITE_10:
+	    /* FALLTHROUGH */
+	case READ_12:
+	    /* FALLTHROUGH */
+	case WRITE_12:
+	    /*
+	     * Enable DMA (if target supports it) for READ and WRITE commands
+	     * only, as some combinations of drive, controller and chipset do
+	     * not behave correctly when DMA is enabled for other commands.
+	     */
+	    if (softc->atadev[tid]->mode >= ATA_DMA)
+		request_flags |= ATA_R_DMA;
 	    break;
+
 	}
 
 	if ((ccb_h->flags & CAM_DIR_MASK) == CAM_DIR_IN && (len & 1)) {
@@ -613,7 +628,7 @@
 
 	/*
 	 * no retries are to be performed at the ATA level; any retries
-	 * will be done by CAM .
+	 * will be done by CAM.
 	 */
 	request->retries = 0;
 
@@ -726,7 +741,7 @@
 		 * issued a REQUEST SENSE automatically and that operation
 		 * returned without error.
 		 */
-		if (request->u.atapi.saved_cmd != 0 && request->error == 0) {
+		if (request->u.atapi.sense.key != 0 && request->error == 0) {
 		    bcopy (&request->u.atapi.sense, &csio->sense_data, sizeof(struct atapi_sense));
 		    csio->ccb_h.status |= CAM_AUTOSNS_VALID;
 		}