ChangeSet 1.1508, 2004/01/20 14:50:56-08:00, stern@rowland.harvard.edu

[PATCH] USB UHCI: fix broken data toggles for queued control URBs

This patch fixes a long-standing (albeit unidentified) problem in the
queueing code for the UHCI HCD.  The code propagates data toggle settings
between messages in a queue for control transfers just the same as bulk
and interrupt transfers.  That is a mistake, since control messages always
restart with data toggle 0.  With this patch, the UHCI driver now passes
test 10 (control URB queueing) in David Brownell's usbtest suite.

The patch appears to change more than it really does, because it alters
the indentation level of a large section of code.


 drivers/usb/host/uhci-hcd.c |   73 ++++++++++++++++++++++++--------------------
 1 files changed, 41 insertions(+), 32 deletions(-)


diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c	Tue Jan 20 17:33:53 2004
+++ b/drivers/usb/host/uhci-hcd.c	Tue Jan 20 17:33:53 2004
@@ -525,8 +525,12 @@
 
 	lltd = list_entry(lurbp->td_list.prev, struct uhci_td, list);
 
-	usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe), usb_pipeout(urb->pipe),
-		uhci_fixup_toggle(urb, uhci_toggle(td_token(lltd)) ^ 1));
+	/* Control transfers always start with toggle 0 */
+	if (!usb_pipecontrol(urb->pipe))
+		usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe),
+				usb_pipeout(urb->pipe),
+				uhci_fixup_toggle(urb,
+					uhci_toggle(td_token(lltd)) ^ 1));
 
 	/* All qh's in the queue need to link to the next queue */
 	urbp->qh->link = eurbp->qh->link;
@@ -560,38 +564,43 @@
 
 	nurbp = list_entry(urbp->queue_list.next, struct urb_priv, queue_list);
 
-	/* Fix up the toggle for the next URB's */
-	if (!urbp->queued)
-		/* We just set the toggle in uhci_unlink_generic */
-		toggle = usb_gettoggle(urb->dev, usb_pipeendpoint(urb->pipe), usb_pipeout(urb->pipe));
-	else {
-		/* If we're in the middle of the queue, grab the toggle */
-		/*  from the TD previous to us */
-		purbp = list_entry(urbp->queue_list.prev, struct urb_priv,
-				queue_list);
+	/*
+	 * Fix up the toggle for the following URBs in the queue.
+	 * Only needed for bulk and interrupt: control and isochronous
+	 * endpoints don't propagate toggles between messages.
+	 */
+	if (usb_pipebulk(urb->pipe) || usb_pipeint(urb->pipe)) {
+		if (!urbp->queued)
+			/* We just set the toggle in uhci_unlink_generic */
+			toggle = usb_gettoggle(urb->dev,
+					usb_pipeendpoint(urb->pipe),
+					usb_pipeout(urb->pipe));
+		else {
+			/* If we're in the middle of the queue, grab the */
+			/* toggle from the TD previous to us */
+			purbp = list_entry(urbp->queue_list.prev,
+					struct urb_priv, queue_list);
+			pltd = list_entry(purbp->td_list.prev,
+					struct uhci_td, list);
+			toggle = uhci_toggle(td_token(pltd)) ^ 1;
+		}
+
+		head = &urbp->queue_list;
+		tmp = head->next;
+		while (head != tmp) {
+			struct urb_priv *turbp;
+
+			turbp = list_entry(tmp, struct urb_priv, queue_list);
+			tmp = tmp->next;
+
+			if (!turbp->queued)
+				break;
+			toggle = uhci_fixup_toggle(turbp->urb, toggle);
+		}
 
-		pltd = list_entry(purbp->td_list.prev, struct uhci_td, list);
-
-		toggle = uhci_toggle(td_token(pltd)) ^ 1;
-	}
-
-	head = &urbp->queue_list;
-	tmp = head->next;
-	while (head != tmp) {
-		struct urb_priv *turbp;
-
-		turbp = list_entry(tmp, struct urb_priv, queue_list);
-
-		tmp = tmp->next;
-
-		if (!turbp->queued)
-			break;
-
-		toggle = uhci_fixup_toggle(turbp->urb, toggle);
+		usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe),
+				usb_pipeout(urb->pipe), toggle);
 	}
-
-	usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe),
-		usb_pipeout(urb->pipe), toggle);
 
 	if (!urbp->queued) {
 		struct uhci_qh *pqh;