from netbsd, umass.c 1.100 1.113 1.115, umass_scsipi.c 1.11, umassvar.h 1.21

log message for umass 1.98, umass_scsipi.c 1.11, umassvar.h 1.21:
Fix several problems with CCI handling, and enable it: * We were never calling
the callback function when we got a UFI CCI(!). * We were passing a bogus
residual count on non-UFI CCIs. * After a REQUEST SENSE, the UFI CCI may
include the ASC/ASCQ from the sense  information -- don't consider this a
failure.

In addition, remove a horrible hack that was causing us to drop sense
information on the floor and return empty INQUIRY responses, often in the
case where there is a UNIT ATTENTION pending during the INQUIRY.  (Some drives
send the data and then fail, some don't.)

log message for umass.c 1.113:
Do not return STATUS_WIRE_FAILED in response to a CBI stall.  This is perfectly
normal, and is used to report errors.  Instead, use STATUS_CMD_FAILED, which
causes us to do a REQUEST SENSE.  Should address PR 22960.

log message for umass.c 1.115:
Do not do a CBI reset before returning STATUS_CMD_FAILED.  This causes any
sense information to be thrown away, and is therefore nonsensical.

ok krw@

4 files changed, 67 insertions, 68 deletions

diff --git a/sys/dev/usb/umass.c b/sys/dev/usb/umass.c
index af3244924fc..a0fe1177e9a 100644
--- a/sys/dev/usb/umass.c
+++ b/sys/dev/usb/umass.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umass.c,v 1.28 2004/07/17 06:04:00 dlg Exp $ */
+/*	$OpenBSD: umass.c,v 1.29 2004/07/21 07:43:41 dlg Exp $ */
 /*	$NetBSD: umass.c,v 1.98 2003/09/08 19:30:59 mycroft Exp $	*/
 /*-
  * Copyright (c) 1999 MAEKAWA Masahide <bishop@rr.iij4u.or.jp>,
@@ -319,10 +319,6 @@ USB_ATTACH(umass)
 		}
 	}
 
-	/* XXX - Now unsupported CBI with CCI */
-	if (sc->sc_wire == UMASS_WPROTO_CBI_I)
-		sc->sc_wire = UMASS_WPROTO_CBI;
-
 	if (sc->sc_cmd == UMASS_CPROTO_UNSPEC) {
 		switch (id->bInterfaceSubClass) {
 		case UISUBCLASS_SCSI:
@@ -453,6 +449,9 @@ USB_ATTACH(umass)
 	}
 
 	/* Open the bulk-in and -out pipe */
+	DPRINTF(UDMASS_USB, ("%s: opening iface %p epaddr %d for BULKOUT\n",
+	    USBDEVNAME(sc->sc_dev), sc->sc_iface,
+	    sc->sc_epaddr[UMASS_BULKOUT]));
 	err = usbd_open_pipe(sc->sc_iface, sc->sc_epaddr[UMASS_BULKOUT],
 				USBD_EXCLUSIVE_USE,
 				&sc->sc_pipe[UMASS_BULKOUT]);
@@ -462,6 +461,9 @@ USB_ATTACH(umass)
 		umass_disco(sc);
 		USB_ATTACH_ERROR_RETURN;
 	}
+	DPRINTF(UDMASS_USB, ("%s: opening iface %p epaddr %d for BULKIN\n",
+	    USBDEVNAME(sc->sc_dev), sc->sc_iface,
+	    sc->sc_epaddr[UMASS_BULKIN]));
 	err = usbd_open_pipe(sc->sc_iface, sc->sc_epaddr[UMASS_BULKIN],
 				USBD_EXCLUSIVE_USE, &sc->sc_pipe[UMASS_BULKIN]);
 	if (err) {
@@ -483,6 +485,9 @@ USB_ATTACH(umass)
 	 * arriving concurrently.
 	 */
 	if (sc->sc_wire == UMASS_WPROTO_CBI_I) {
+		DPRINTF(UDMASS_USB, ("%s: opening iface %p epaddr %d for INTRIN\n",
+		    USBDEVNAME(sc->sc_dev), sc->sc_iface,
+		    sc->sc_epaddr[UMASS_INTRIN]));
 		err = usbd_open_pipe(sc->sc_iface, sc->sc_epaddr[UMASS_INTRIN],
 				USBD_EXCLUSIVE_USE, &sc->sc_pipe[UMASS_INTRIN]);
 		if (err) {
@@ -1450,27 +1455,11 @@ umass_cbi_state(usbd_xfer_handle xfer, usbd_private_handle priv,
 					sc->transfer_xfer[XFER_CBI_DATA]))
 				umass_cbi_reset(sc, STATUS_WIRE_FAILED);
 
-		} else if (sc->sc_wire == UMASS_WPROTO_CBI_I) {
-			DPRINTF(UDMASS_CBI, ("%s: no data phase\n",
-				USBDEVNAME(sc->sc_dev)));
-			sc->transfer_state = TSTATE_CBI_STATUS;
-			if (umass_setup_transfer(sc, sc->sc_pipe[UMASS_INTRIN],
-					&sc->sbl, sizeof(sc->sbl),
-					0,	/* fixed length transfer */
-					sc->transfer_xfer[XFER_CBI_STATUS])){
-				umass_cbi_reset(sc, STATUS_WIRE_FAILED);
-			}
 		} else {
 			DPRINTF(UDMASS_CBI, ("%s: no data phase\n",
 				USBDEVNAME(sc->sc_dev)));
-			/* No command completion interrupt. Request
-			 * sense data.
-			 */
-			sc->transfer_state = TSTATE_IDLE;
-			sc->transfer_cb(sc, sc->transfer_priv,
-			       0, STATUS_CMD_UNKNOWN);
+			goto dostatus;
 		}
-
 		return;
 
 	case TSTATE_CBI_DATA:
@@ -1503,15 +1492,15 @@ umass_cbi_state(usbd_xfer_handle xfer, usbd_private_handle priv,
 					umass_dump_buffer(sc, sc->transfer_data,
 						sc->transfer_actlen, 48));
 
+	dostatus:
 		if (sc->sc_wire == UMASS_WPROTO_CBI_I) {
 			sc->transfer_state = TSTATE_CBI_STATUS;
 			memset(&sc->sbl, 0, sizeof(sc->sbl));
 			if (umass_setup_transfer(sc, sc->sc_pipe[UMASS_INTRIN],
 				    &sc->sbl, sizeof(sc->sbl),
 				    0,	/* fixed length transfer */
-				    sc->transfer_xfer[XFER_CBI_STATUS])){
+				    sc->transfer_xfer[XFER_CBI_STATUS]))
 				umass_cbi_reset(sc, STATUS_WIRE_FAILED);
-			}
 		} else {
 			/* No command completion interrupt. Request
 			 * sense to get status of command.
@@ -1542,6 +1531,15 @@ umass_cbi_state(usbd_xfer_handle xfer, usbd_private_handle priv,
 
 		/* Dissect the information in the buffer */
 
+		{
+			u_int32_t actlen;
+			usbd_get_xfer_status(xfer, NULL, NULL, &actlen, NULL);
+			DPRINTF(UDMASS_CBI, ("%s: CBI_STATUS actlen=%d\n",
+			    USBDEVNAME(sc->sc_dev), actlen));
+			if (actlen != 2)
+				break;
+		}
+
 		if (sc->sc_cmd == UMASS_CPROTO_UFI) {
 			int status;
 
@@ -1555,37 +1553,44 @@ umass_cbi_state(usbd_xfer_handle xfer, usbd_private_handle priv,
 				USBDEVNAME(sc->sc_dev),
 				sc->sbl.ufi.asc, sc->sbl.ufi.ascq));
 
-			if (sc->sbl.ufi.asc == 0 && sc->sbl.ufi.ascq == 0)
+			if ((sc->sbl.ufi.asc == 0 && sc->sbl.ufi.ascq == 0) ||
+			    sc->sc_sense)
 				status = STATUS_CMD_OK;
 			else
 				status = STATUS_CMD_FAILED;
 
-			/* No sense, command successful */
+			/* No autosense, command successful */
+			sc->transfer_state = TSTATE_IDLE;
+			sc->transfer_cb(sc, sc->transfer_priv,
+			    sc->transfer_datalen - sc->transfer_actlen, status);
 		} else {
+			int status;
+
 			/* Command Interrupt Data Block */
+
 			DPRINTF(UDMASS_CBI, ("%s: type=0x%02x, value=0x%02x\n",
 				USBDEVNAME(sc->sc_dev),
 				sc->sbl.common.type, sc->sbl.common.value));
 
 			if (sc->sbl.common.type == IDB_TYPE_CCI) {
-				int err;
-
-				if ((sc->sbl.common.value&IDB_VALUE_STATUS_MASK)
-							== IDB_VALUE_PASS) {
-					err = STATUS_CMD_OK;
-				} else if ((sc->sbl.common.value & IDB_VALUE_STATUS_MASK)
-							== IDB_VALUE_FAIL ||
-					   (sc->sbl.common.value & IDB_VALUE_STATUS_MASK)
-						== IDB_VALUE_PERSISTENT) {
-					err = STATUS_CMD_FAILED;
-				} else {
-					err = STATUS_WIRE_FAILED;
-				}
+				switch (sc->sbl.common.value &
+				    IDB_VALUE_STATUS_MASK) {
+				case IDB_VALUE_PASS:
+					status = STATUS_CMD_OK;
+					break;
+				case IDB_VALUE_FAIL:
+				case IDB_VALUE_PERSISTENT:
+					status = STATUS_CMD_FAILED;
+					break;
+				case IDB_VALUE_PHASE:
+					status = STATUS_WIRE_FAILED;
+					break;
+ 				}
 
 				sc->transfer_state = TSTATE_IDLE;
 				sc->transfer_cb(sc, sc->transfer_priv,
-						sc->transfer_datalen,
-						err);
+				    sc->transfer_datalen - sc->transfer_actlen,
+				    status);
 			}
 		}
 		return;
@@ -1595,21 +1600,23 @@ umass_cbi_state(usbd_xfer_handle xfer, usbd_private_handle priv,
 			printf("%s: CBI bulk-in/out stall clear failed, %s\n",
 			       USBDEVNAME(sc->sc_dev), usbd_errstr(err));
 			umass_cbi_reset(sc, STATUS_WIRE_FAILED);
+		} else {
+			sc->transfer_state = TSTATE_IDLE;
+			sc->transfer_cb(sc, sc->transfer_priv,
+			    sc->transfer_datalen, STATUS_CMD_FAILED);
 		}
-
-		sc->transfer_state = TSTATE_IDLE;
-		sc->transfer_cb(sc, sc->transfer_priv,
-				sc->transfer_datalen,
-				STATUS_CMD_FAILED);
 		return;
 
 	case TSTATE_CBI_SCLEAR:
-		if (err)	/* should not occur */
+		if (err) {	/* should not occur */
 			printf("%s: CBI intr-in stall clear failed, %s\n",
 			       USBDEVNAME(sc->sc_dev), usbd_errstr(err));
-
-		/* Something really bad is going on. Reset the device */
-		umass_cbi_reset(sc, STATUS_CMD_FAILED);
+			umass_cbi_reset(sc, STATUS_WIRE_FAILED);
+		} else {
+			sc->transfer_state = TSTATE_IDLE;
+			sc->transfer_cb(sc, sc->transfer_priv,
+			    sc->transfer_datalen, STATUS_CMD_FAILED);
+		}
 		return;
 
 	/***** CBI Reset *****/
diff --git a/sys/dev/usb/umass_quirks.c b/sys/dev/usb/umass_quirks.c
index eaa0eb21303..bf2b3b65d9b 100644
--- a/sys/dev/usb/umass_quirks.c
+++ b/sys/dev/usb/umass_quirks.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umass_quirks.c,v 1.14 2004/07/17 06:04:00 dlg Exp $	*/
+/*	$OpenBSD: umass_quirks.c,v 1.15 2004/07/21 07:43:41 dlg Exp $	*/
 /*	$NetBSD: umass_quirks.c,v 1.67 2004/06/28 07:49:16 mycroft Exp $	*/
 
 /*
@@ -400,7 +400,7 @@ Static const struct umass_quirk umass_quirks[] = {
 
 	{ { USB_VENDOR_SONY, USB_PRODUCT_SONY_MSC },
 	  UMASS_WPROTO_CBI, UMASS_CPROTO_UFI,
-	  UMASS_QUIRK_RS_NO_CLEAR_UA,
+	  0,
 	  PQUIRK_NOMODESENSE,
 	  UMATCH_DEVCLASS_DEVSUBCLASS_DEVPROTO,
 	  NULL, NULL
@@ -440,7 +440,7 @@ Static const struct umass_quirk umass_quirks[] = {
 
 	{ { USB_VENDOR_YEDATA, USB_PRODUCT_YEDATA_FLASHBUSTERU },
 	  UMASS_WPROTO_UNSPEC, UMASS_CPROTO_UFI,
-	  UMASS_QUIRK_RS_NO_CLEAR_UA,
+	  0,
 	  PQUIRK_NOMODESENSE,
 	  UMATCH_VENDOR_PRODUCT_REV,
 	  NULL, umass_fixup_yedata
diff --git a/sys/dev/usb/umass_scsi.c b/sys/dev/usb/umass_scsi.c
index f7b3a8a6203..0b5b38c85de 100644
--- a/sys/dev/usb/umass_scsi.c
+++ b/sys/dev/usb/umass_scsi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umass_scsi.c,v 1.7 2004/02/21 00:47:42 krw Exp $ */
+/*	$OpenBSD: umass_scsi.c,v 1.8 2004/07/21 07:43:41 dlg Exp $ */
 /*	$NetBSD: umass_scsipi.c,v 1.9 2003/02/16 23:14:08 augustss Exp $	*/
 /*
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -356,6 +356,7 @@ umass_scsi_cb(struct umass_softc *sc, void *priv, int residue, int status)
 		DPRINTF(UDMASS_CMD, ("umass_scsi_cb: status cmd failed for "
 		    "scsi op 0x%02x\n", xs->cmd->opcode));
 		/* fetch sense data */
+		sc->sc_sense = 1;
 		memset(&scbus->sc_sense_cmd, 0, sizeof(scbus->sc_sense_cmd));
 		scbus->sc_sense_cmd.opcode = REQUEST_SENSE;
 		scbus->sc_sense_cmd.byte2 = link->lun << SCSI_CMD_LUN_SHIFT;
@@ -405,20 +406,11 @@ umass_scsi_sense_cb(struct umass_softc *sc, void *priv, int residue,
 	DPRINTF(UDMASS_CMD,("umass_scsi_sense_cb: xs=%p residue=%d "
 		"status=%d\n", xs, residue, status));
 
+	sc->sc_sense = 0;
 	switch (status) {
 	case STATUS_CMD_OK:
 	case STATUS_CMD_UNKNOWN:
 		/* getting sense data succeeded */
-		if (xs->cmd->opcode == INQUIRY && (xs->resid < xs->datalen ||
-		    (sc->sc_quirks & UMASS_QUIRK_RS_NO_CLEAR_UA /* XXX */))) {
-			/*
-			 * Some drivers return SENSE errors even after INQUIRY.
-			 * The upper layer doesn't like that.
-			 */
-			xs->error = XS_NOERROR;
-			break;
-		}
-		/* XXX look at residue */
 		if (residue == 0 || residue == 14)/* XXX */
 			xs->error = XS_SENSE;
 		else
diff --git a/sys/dev/usb/umassvar.h b/sys/dev/usb/umassvar.h
index 1b65d874ea9..3f12e39003a 100644
--- a/sys/dev/usb/umassvar.h
+++ b/sys/dev/usb/umassvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umassvar.h,v 1.6 2004/07/17 06:04:00 dlg Exp $ */
+/*	$OpenBSD: umassvar.h,v 1.7 2004/07/21 07:43:41 dlg Exp $ */
 /*	$NetBSD: umassvar.h,v 1.20 2003/09/08 19:31:01 mycroft Exp $	*/
 /*-
  * Copyright (c) 1999 MAEKAWA Masahide <bishop@rr.iij4u.or.jp>,
@@ -177,9 +177,8 @@ struct umass_softc {
 #define UMASS_CPROTO_ISD_ATA	5
 
 	u_int32_t		sc_quirks;
-#define UMASS_QUIRK_RS_NO_CLEAR_UA	0x00000002
-#define UMASS_QUIRK_WRONG_CSWSIG	0x00000010
-#define UMASS_QUIRK_WRONG_CSWTAG	0x00000020
+#define UMASS_QUIRK_WRONG_CSWSIG	0x00000001
+#define UMASS_QUIRK_WRONG_CSWTAG	0x00000002
 
 	u_int32_t		sc_busquirks;
 
@@ -262,6 +261,7 @@ struct umass_softc {
 	int			sc_xfer_flags;
 	char			sc_dying;
 	int			sc_refcnt;
+	int			sc_sense;
 
 	struct umassbus_softc	*bus;		 /* bus dependent data */
 };