[PATCH] ehci-hcd, urb queuing

In doing some more extensive testing of the urb queueing behavior,
I noticed that (a) IOC wasn't always being set for each urb, while
for now it needs to be set; (b) a qh patchup wasn't done quite
where it should be.  This resolves those two issues, as well
as making it a bit less noisy to unlink lots of urbs at the once.

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index f19a5fdd9fd689585c7110f0c9eea6d94e03aada..ed92a80f66d0c1922263ba4b6310e87cb899f864 100644 (file)
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -749,7 +749,7 @@ static int ehci_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
        default:
                spin_lock_irqsave (&ehci->lock, flags);
                if (ehci->reclaim) {
-                       dbg ("dq %p: reclaim = %p, %s",
+                       vdbg ("dq %p: reclaim = %p, %s",
                                qh, ehci->reclaim, RUN_CONTEXT);
                        if (qh == ehci->reclaim) {
                                /* unlinking qh for another queued urb? */

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 0fd8b3bdae382be4f5c8b5e6fe8a6c339c82a218..c24ed3b70c2f81b958c74e80da7b298ef3eba287 100644 (file)
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -255,23 +255,23 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
                struct urb      *urb = qtd->urb;
                u32             token = 0;
 
+               /* hc's on-chip qh overlay cache can overwrite our idea of
+                * next qtd ptrs, if we appended a qtd while the queue was
+                * advancing.  (because we don't use dummy qtds.)
+                */
+               if (cpu_to_le32 (qtd->qtd_dma) == qh->hw_current
+                               && qtd->hw_next != qh->hw_qtd_next) {
+                       qh->hw_alt_next = qtd->hw_alt_next;
+                       qh->hw_qtd_next = qtd->hw_next;
+                       COUNT (ehci->stats.qpatch);
+               }
+
                /* clean up any state from previous QTD ...*/
                if (last) {
                        if (likely (last->urb != urb)) {
                                ehci_urb_done (ehci, last->urb);
                                count++;
                        }
-
-                       /* qh overlays can have HC's old cached copies of
-                        * next qtd ptrs, if an URB was queued afterwards.
-                        */
-                       if (cpu_to_le32 (last->qtd_dma) == qh->hw_current
-                                       && last->hw_next != qh->hw_qtd_next) {
-                               qh->hw_alt_next = last->hw_alt_next;
-                               qh->hw_qtd_next = last->hw_next;
-                               COUNT (ehci->stats.qpatch);
-                       }
-
                        ehci_qtd_free (ehci, last);
                        last = 0;
                }
@@ -529,7 +529,8 @@ qh_urb_transaction (
        }
 
        /* by default, enable interrupt on urb completion */
-       if (likely (!(urb->transfer_flags & URB_NO_INTERRUPT)))
+// ... do it always, unless we switch over to dummy qtds
+//     if (likely (!(urb->transfer_flags & URB_NO_INTERRUPT)))
                qtd->hw_token |= __constant_cpu_to_le32 (QTD_IOC);
        return head;
 
@@ -785,7 +786,6 @@ static struct ehci_qh *qh_append_tds (
                /* append to tds already queued to this qh? */
                if (unlikely (!list_empty (&qh->qtd_list) && qtd)) {
                        struct ehci_qtd         *last_qtd;
-                       int                     short_rx = 0;
                        u32                     hw_next;
 
                        /* update the last qtd's "next" pointer */
@@ -800,23 +800,16 @@ static struct ehci_qh *qh_append_tds (
                                        && (epnum & 0x10)) {
                                // only the last QTD for now
                                last_qtd->hw_alt_next = hw_next;
-                               short_rx = 1;
                        }
 
-                       /* Adjust any old copies in qh overlay too.
-                        * Interrupt code must cope with case of HC having it
-                        * cached, and clobbering these updates.
-                        * ... complicates getting rid of extra interrupts!
-                        * (Or:  use dummy td, so cache always stays valid.)
+                       /* qh_completions() may need to patch the qh overlay if
+                        * the hc was advancing this queue while we appended.
+                        * we know it can: last_qtd->hw_token has IOC set.
+                        *
+                        * or:  use a dummy td (so the overlay gets the next td
+                        * only when we set its active bit); fewer irqs.
                         */
-                       if (qh->hw_current == cpu_to_le32 (last_qtd->qtd_dma)) {
-                               wmb ();
-                               qh->hw_qtd_next = hw_next;
-                               if (short_rx)
-                                       qh->hw_alt_next = hw_next
-                                               | (qh->hw_alt_next & 0x1e);
-                               vdbg ("queue to qh %p, patch", qh);
-                       }
+                       wmb ();
 
                /* no URB queued */
                } else {