ChangeSet 1.1608.24.5, 2004/02/26 14:09:03-08:00, stern@rowland.harvard.edu

[PATCH] USB: Return better result codes in UHCI

This patch changes the result code returned by the UHCI driver for a
certain class of errors.  Under a number of circumstances a USB device is
obliged to send a response packet within a fairly short turn-around time,
typically 1 - 10 microseconds depending on the bus speed.  Failure to do
so is a protocol error and should be reported as such, not as a timeout,
which is really a higher-level concept.  I believe the EHCI driver already
does this.

I trust nobody will object to the update this patch adds to
Documentation/usb/error-codes.txt, making this more explicit.

In a vaguely related change, the patch corrects the terminology in a few
comments.  The parts of a control transfer are called "stages", not
"phases".


 Documentation/usb/error-codes.txt |    4 +++-
 drivers/usb/host/uhci-hcd.c       |   29 ++++++++++++++---------------
 drivers/usb/host/uhci-hcd.h       |    2 +-
 3 files changed, 18 insertions(+), 17 deletions(-)


diff -Nru a/Documentation/usb/error-codes.txt b/Documentation/usb/error-codes.txt
--- a/Documentation/usb/error-codes.txt	Tue Mar 16 15:06:55 2004
+++ b/Documentation/usb/error-codes.txt	Tue Mar 16 15:06:55 2004
@@ -70,7 +70,9 @@
 			(That is, if drivers see this it's a bug.)
 
 -EPROTO (*)		a) bitstuff error
-			b) unknown USB error 
+			b) no response packet received within the
+			   prescribed bus turn-around time
+			c) unknown USB error 
 
 -EILSEQ (*)		CRC mismatch
 
diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c	Tue Mar 16 15:06:55 2004
+++ b/drivers/usb/host/uhci-hcd.c	Tue Mar 16 15:06:55 2004
@@ -781,7 +781,8 @@
 /*
  * Map status to standard result codes
  *
- * <status> is (td->status & 0xFE0000) [a.k.a. uhci_status_bits(td->status)]
+ * <status> is (td->status & 0xF60000) [a.k.a. uhci_status_bits(td->status)]
+ * Note: status does not include the TD_CTRL_NAK bit.
  * <dir_out> is True for output TDs and False for input TDs.
  */
 static int uhci_map_status(int status, int dir_out)
@@ -792,22 +793,18 @@
 		return -EPROTO;
 	if (status & TD_CTRL_CRCTIMEO) {		/* CRC/Timeout */
 		if (dir_out)
-			return -ETIMEDOUT;
+			return -EPROTO;
 		else
 			return -EILSEQ;
 	}
-	if (status & TD_CTRL_NAK)			/* NAK */
-		return -ETIMEDOUT;
 	if (status & TD_CTRL_BABBLE)			/* Babble */
 		return -EOVERFLOW;
 	if (status & TD_CTRL_DBUFERR)			/* Buffer error */
 		return -ENOSR;
 	if (status & TD_CTRL_STALLED)			/* Stalled */
 		return -EPIPE;
-	if (status & TD_CTRL_ACTIVE)			/* Active */
-		return 0;
-
-	return -EINVAL;
+	WARN_ON(status & TD_CTRL_ACTIVE);		/* Active */
+	return 0;
 }
 
 /*
@@ -832,7 +829,7 @@
 		status |= TD_CTRL_LS;
 
 	/*
-	 * Build the TD for the control request
+	 * Build the TD for the control request setup packet
 	 */
 	td = uhci_alloc_td(uhci, urb->dev);
 	if (!td)
@@ -990,13 +987,13 @@
 
 	if (urbp->short_control_packet) {
 		tmp = head->prev;
-		goto status_phase;
+		goto status_stage;
 	}
 
 	tmp = head->next;
 	td = list_entry(tmp, struct uhci_td, list);
 
-	/* The first TD is the SETUP phase, check the status, but skip */
+	/* The first TD is the SETUP stage, check the status, but skip */
 	/*  the count */
 	status = uhci_status_bits(td_status(td));
 	if (status & TD_CTRL_ACTIVE)
@@ -1037,10 +1034,10 @@
 		}
 	}
 
-status_phase:
+status_stage:
 	td = list_entry(tmp, struct uhci_td, list);
 
-	/* Control status phase */
+	/* Control status stage */
 	status = td_status(td);
 
 #ifdef I_HAVE_BUGGY_APC_BACKUPS
@@ -1053,10 +1050,11 @@
 		return 0;
 #endif
 
+	status = uhci_status_bits(status);
 	if (status & TD_CTRL_ACTIVE)
 		return -EINPROGRESS;
 
-	if (uhci_status_bits(status))
+	if (status)
 		goto td_error;
 
 	return 0;
@@ -1403,7 +1401,8 @@
 		urb->iso_frame_desc[i].actual_length = actlength;
 		urb->actual_length += actlength;
 
-		status = uhci_map_status(uhci_status_bits(td_status(td)), usb_pipeout(urb->pipe));
+		status = uhci_map_status(uhci_status_bits(td_status(td)),
+				usb_pipeout(urb->pipe));
 		urb->iso_frame_desc[i].status = status;
 		if (status) {
 			urb->error_count++;
diff -Nru a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
--- a/drivers/usb/host/uhci-hcd.h	Tue Mar 16 15:06:55 2004
+++ b/drivers/usb/host/uhci-hcd.h	Tue Mar 16 15:06:55 2004
@@ -141,7 +141,7 @@
 				 TD_CTRL_BABBLE | TD_CTRL_CRCTIME | TD_CTRL_BITSTUFF)
 
 #define uhci_maxerr(err)		((err) << TD_CTRL_C_ERR_SHIFT)
-#define uhci_status_bits(ctrl_sts)	((ctrl_sts) & 0xFE0000)
+#define uhci_status_bits(ctrl_sts)	((ctrl_sts) & 0xF60000)
 #define uhci_actual_length(ctrl_sts)	(((ctrl_sts) + 1) & TD_CTRL_ACTLEN_MASK) /* 1-based */
 
 /*