> Date: Fri, 11 Jan 2008 17:13:46 -0800
> From: Greg KH <[EMAIL PROTECTED]>
>
> On Tue, Jan 08, 2008 at 10:34:58PM -0800, David Brownell wrote:
> > Sorry, this got accidentally filed in "trash" not "review".
> > Happened from cleaning out a thousand or so emails.  :(
> > 
> > I suspect I'll be going with a slightly different fix; you
> > can get a preview of it (depending textually on another
> > patch which will now be deferred) at
> > 
> >   http://marc.info/?l=linux-usb&m=119913406214488&w=2
> > 
> > I'll CC you on the updated patch, and ask you to verify
> > that it behaves on your system.
>
> So should I drop this from my tree for now?

Yes, please.  The appended patch is that updated version.

======= CUT HERE
From: David Brownell <[EMAIL PROTECTED]>

This adds a workaround for an issue reported with ISO transfers
on some EHCI controllers, most recently with VIA KT800 and PS3
EHCI silicon.

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.  It does so by not recycling them until after
issuing the completion callback which would reuse them by enqueueing
an URB and thus (re)allocating ISO DMA descriptors.

Signed-off-by: David Brownell <[EMAIL PROTECTED]>
---
 drivers/usb/host/ehci-sched.c |   58 ++++++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 16 deletions(-)

--- g26.orig/drivers/usb/host/ehci-sched.c      2008-01-09 01:27:25.000000000 
-0800
+++ g26/drivers/usb/host/ehci-sched.c   2008-01-09 04:18:39.000000000 -0800
@@ -1565,6 +1565,16 @@ itd_link_urb (
 
 #define        ISO_ERRS (EHCI_ISOC_BUF_ERR | EHCI_ISOC_BABBLE | 
EHCI_ISOC_XACTERR)
 
+/* Process and recycle a completed ITD.  Return true iff its urb completed,
+ * and hence its completion callback probably added things to the hardware
+ * schedule.
+ *
+ * Note that we carefully avoid recycling this descriptor until after any
+ * completion callback runs, so that it won't be reused quickly.  That is,
+ * assuming (a) no more than two urbs per frame on this endpoint, and also
+ * (b) only this endpoint's completions submit URBs.  It seems some silicon
+ * corrupts things if you reuse completed descriptors very quickly...
+ */
 static unsigned
 itd_complete (
        struct ehci_hcd *ehci,
@@ -1577,6 +1587,7 @@ itd_complete (
        int                                     urb_index = -1;
        struct ehci_iso_stream                  *stream = itd->stream;
        struct usb_device                       *dev;
+       unsigned                                retval = false;
 
        /* for each uframe with a packet */
        for (uframe = 0; uframe < 8; uframe++) {
@@ -1610,15 +1621,9 @@ itd_complete (
                }
        }
 
-       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)
@@ -1628,6 +1633,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--;
@@ -1641,8 +1647,15 @@ itd_complete (
                        (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out");
        }
        iso_stream_put (ehci, stream);
+       /* OK to recycle this ITD now that its completion callback ran. */
+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 1;
+       return retval;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1944,6 +1957,16 @@ sitd_link_urb (
 #define        SITD_ERRS (SITD_STS_ERR | SITD_STS_DBE | SITD_STS_BABBLE \
                                | SITD_STS_XACT | SITD_STS_MMF)
 
+/* Process and recycle a completed SITD.  Return true iff its urb completed,
+ * and hence its completion callback probably added things to the hardware
+ * schedule.
+ *
+ * Note that we carefully avoid recycling this descriptor until after any
+ * completion callback runs, so that it won't be reused quickly.  That is,
+ * assuming (a) no more than two urbs per frame on this endpoint, and also
+ * (b) only this endpoint's completions submit URBs.  It seems some silicon
+ * corrupts things if you reuse completed descriptors very quickly...
+ */
 static unsigned
 sitd_complete (
        struct ehci_hcd         *ehci,
@@ -1955,6 +1978,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];
@@ -1975,17 +1999,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)
@@ -1995,6 +2013,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--;
@@ -2008,8 +2027,15 @@ sitd_complete (
                        (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out");
        }
        iso_stream_put (ehci, stream);
+       /* OK to recycle this SITD now that its completion callback ran. */
+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 1;
+       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