[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.

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 41d3171fb333f821b05bc662ebe82f998c3754df..c567166757c62b1ca5bbf4b670f07cd9267eb1f9 100644 (file)
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -525,8 +525,12 @@ static void uhci_append_queued_urb(struct uhci_hcd *uhci, struct urb *eurb, stru
 
        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,39 +564,44 @@ static void uhci_delete_queued_urb(struct uhci_hcd *uhci, struct urb *urb)
 
        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);
-
-               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;
+       /*
+        * 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;
+               }
 
-               turbp = list_entry(tmp, struct urb_priv, queue_list);
+               head = &urbp->queue_list;
+               tmp = head->next;
+               while (head != tmp) {
+                       struct urb_priv *turbp;
 
-               tmp = tmp->next;
+                       turbp = list_entry(tmp, struct urb_priv, queue_list);
+                       tmp = tmp->next;
 
-               if (!turbp->queued)
-                       break;
+                       if (!turbp->queued)
+                               break;
+                       toggle = uhci_fixup_toggle(turbp->urb, toggle);
+               }
 
-               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;