This adds a workaround for an issue reported with ISO transfers
on some EHCI controllers, most recently with VIA KT800 silicon.
(Reported by Karsten Wiese and some others.)

The issue is that the silicon doesn't necessarily seem to be done
using ISO DMA descriptors (itd, sitd) when it marks them inactive.
(One theory is that the ill-defined mechanism where hardware caches
periodic transfer descriptors isn't invalidating their state...)
With such silicon, quick re-use of those descriptors makes trouble.
Waiting until the next frame seems to be a sufficient workaround.

This patch ensures that the relevant descriptors aren't available
for immediate re-use, by not recycling them until after issuing
the completion callback which would reuse them (by enqueueing an
URB and thus allocating ISO DMA descriptors).

Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
I'd expect this to make the KT800 behave, in conjunction with the
previous patch to complete quicker...

 drivers/usb/host/ehci-sched.c |   56 ++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 16 deletions(-)

--- g26.orig/drivers/usb/host/ehci-sched.c      2007-12-31 12:12:00.000000000 
-0800
+++ g26/drivers/usb/host/ehci-sched.c   2007-12-31 12:44:30.000000000 -0800
@@ -1560,6 +1560,9 @@ itd_link_urb (
 
 #define        ISO_ERRS (EHCI_ISOC_BUF_ERR | EHCI_ISOC_BABBLE | 
EHCI_ISOC_XACTERR)
 
+/* Return true iff urb completed, and hence its completion
+ * callback probably added things to the hardware schedule.
+ */
 static unsigned
 itd_complete (
        struct ehci_hcd *ehci,
@@ -1572,6 +1575,7 @@ itd_complete (
        unsigned                                urb_index = itd->index;
        struct ehci_iso_stream                  *stream = itd->stream;
        struct usb_device                       *dev;
+       unsigned                                retval = false;
 
        /* Caller guarantees this ITD is inactive.  We always
         * report something for each transfer it defines.
@@ -1615,15 +1619,9 @@ itd_complete (
                urb_index++;
        }
 
-       usb_put_urb (urb);
-       itd->urb = NULL;
-       itd->stream = NULL;
-       list_move (&itd->itd_list, &stream->free_list);
-       iso_stream_put (ehci, stream);
-
        /* handle completion now? */
        if (likely ((urb_index + 1) != urb->number_of_packets))
-               return 0;
+               goto done;
 
        /* ASSERT: it's really the last itd for this urb
        list_for_each_entry (itd, &stream->td_list, itd_list)
@@ -1633,6 +1631,7 @@ itd_complete (
        /* give urb back to the driver; completion often (re)submits */
        dev = urb->dev;
        ehci_urb_done(ehci, urb, 0);
+       retval = true;
        urb = NULL;
        ehci->periodic_sched--;
        ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;
@@ -1647,7 +1646,20 @@ itd_complete (
        }
        iso_stream_put (ehci, stream);
 
-       return 1;
+       /* NOTE:  we carefully avoid recycling this descriptor until
+        * after any completion callback runs, so that it can't be reused
+        * until the next frame.  It seems some silicon will cache ISO
+        * descriptors, and won't invalidate the cache state until the
+        * next frame.  That precludes immediate descriptor re-use...
+        */
+done:
+       usb_put_urb(urb);
+       itd->urb = NULL;
+       itd->stream = NULL;
+       list_move(&itd->itd_list, &stream->free_list);
+       iso_stream_put(ehci, stream);
+
+       return retval;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1949,6 +1961,9 @@ sitd_link_urb (
 #define        SITD_ERRS (SITD_STS_ERR | SITD_STS_DBE | SITD_STS_BABBLE \
                                | SITD_STS_XACT | SITD_STS_MMF)
 
+/* Return true iff urb completed, and hence its completion
+ * callback probably added things to the hardware schedule.
+ */
 static unsigned
 sitd_complete (
        struct ehci_hcd         *ehci,
@@ -1960,6 +1975,7 @@ sitd_complete (
        int                                     urb_index = -1;
        struct ehci_iso_stream                  *stream = sitd->stream;
        struct usb_device                       *dev;
+       unsigned                                        retval = false;
 
        urb_index = sitd->index;
        desc = &urb->iso_frame_desc [urb_index];
@@ -1980,17 +1996,11 @@ sitd_complete (
                desc->status = 0;
                desc->actual_length = desc->length - SITD_LENGTH (t);
        }
-
-       usb_put_urb (urb);
-       sitd->urb = NULL;
-       sitd->stream = NULL;
-       list_move (&sitd->sitd_list, &stream->free_list);
        stream->depth -= stream->interval << 3;
-       iso_stream_put (ehci, stream);
 
        /* handle completion now? */
        if ((urb_index + 1) != urb->number_of_packets)
-               return 0;
+               goto done;
 
        /* ASSERT: it's really the last sitd for this urb
        list_for_each_entry (sitd, &stream->td_list, sitd_list)
@@ -2000,6 +2010,7 @@ sitd_complete (
        /* give urb back to the driver; completion often (re)submits */
        dev = urb->dev;
        ehci_urb_done(ehci, urb, 0);
+       retval = true;
        urb = NULL;
        ehci->periodic_sched--;
        ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;
@@ -2014,7 +2025,20 @@ sitd_complete (
        }
        iso_stream_put (ehci, stream);
 
-       return 1;
+       /* NOTE:  we carefully avoid recycling this descriptor until
+        * after any completion callback runs, so that it can't be reused
+        * until the next frame.  It seems some silicon will cache ISO
+        * descriptors, and won't invalidate the cache state until the
+        * next frame.  That precludes immediate descriptor re-use...
+        */
+done:
+       usb_put_urb(urb);
+       sitd->urb = NULL;
+       sitd->stream = NULL;
+       list_move(&sitd->sitd_list, &stream->free_list);
+       iso_stream_put(ehci, stream);
+
+       return retval;
 }
 
 
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to