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