[Jaunty][PATCH] USB EHCI: slow down ITD reuse

Federico Briata federicobriata at gmail.com
Sat Mar 7 16:05:44 UTC 2009


Hi,

the current patch let snd_usb_us122l driver work on EHCI bus, it has
been applied on 2.6.29-rc3

also a bug as been opened
https://bugs.launchpad.net/bugs/329437

federico

---------- Forwarded message ----------
From: David Brownell <david-b at pacbell.net>
Date: 2009/2/9
Subject: [patch 2.6.29-rc3] USB: EHCI: slow down ITD reuse
To: Greg KH <greg at kroah.com>
Cc: Karsten Wiese <fzu at wemgehoertderstaat.de>, linux-usb at vger.kernel.org


From: Karsten Wiese <fzu at wemgehoertderstaat.de>

Currently ITDs are immediately recycled whenever their URB completes.
However, EHCI hardware can sometimes remember some ITD state.  This
means that when the ITD is reused before end-of-frame it may sometimes
cause the hardware to reference bogus state.

This patch defers reusing such ITDs by moving them into a new ehci member
cached_itd_list. ITDs resting in cached_itd_list are moved back into their
stream's free_list once scan_periodic() detects that the active frame has
elapsed.

This makes the snd_usb_us122l driver (in kernel since .28) work right
when it's hooked up through EHCI.

[ dbrownell at users.sourceforge.net: comment fixups ]

Signed-off-by: Karsten Wiese <fzu at wemgehoertderstaat.de>
Tested-by: Philippe Carriere <philippe-f.carriere at wanadoo.fr>
Tested-by: Federico Briata <federicobriata at gmail.com>
Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
---
Suitable for 2.6.28-stable too, it seems ...

 drivers/usb/host/ehci-hcd.c   |    2 +
 drivers/usb/host/ehci-mem.c   |    1
 drivers/usb/host/ehci-sched.c |   56 ++++++++++++++++++++++++++++++++++------
 drivers/usb/host/ehci.h       |    6 ++++
 4 files changed, 57 insertions(+), 8 deletions(-)

--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -485,6 +485,7 @@ static int ehci_init(struct usb_hcd *hcd
        * periodic_size can shrink by USBCMD update if hcc_params allows.
        */
       ehci->periodic_size = DEFAULT_I_TDPS;
+       INIT_LIST_HEAD(&ehci->cached_itd_list);
       if ((retval = ehci_mem_init(ehci, GFP_KERNEL)) < 0)
               return retval;

@@ -497,6 +498,7 @@ static int ehci_init(struct usb_hcd *hcd

       ehci->reclaim = NULL;
       ehci->next_uframe = -1;
+       ehci->clock_frame = -1;

       /*
        * dedicate a qh for the async ring head, since we couldn't unlink
--- a/drivers/usb/host/ehci-mem.c
+++ b/drivers/usb/host/ehci-mem.c
@@ -128,6 +128,7 @@ static inline void qh_put (struct ehci_q

 static void ehci_mem_cleanup (struct ehci_hcd *ehci)
 {
+       free_cached_itd_list(ehci);
       if (ehci->async)
               qh_put (ehci->async);
       ehci->async = NULL;
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1006,7 +1006,8 @@ iso_stream_put(struct ehci_hcd *ehci, st

               is_in = (stream->bEndpointAddress & USB_DIR_IN) ? 0x10 : 0;
               stream->bEndpointAddress &= 0x0f;
-               stream->ep->hcpriv = NULL;
+               if (stream->ep)
+                       stream->ep->hcpriv = NULL;

               if (stream->rescheduled) {
                       ehci_info (ehci, "ep%d%s-iso rescheduled "
@@ -1657,14 +1658,28 @@ 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);
-
+       if (ehci->clock_frame != itd->frame || itd->index[7] != -1) {
+               /* OK to recycle this ITD now. */
+               itd->stream = NULL;
+               list_move(&itd->itd_list, &stream->free_list);
+               iso_stream_put(ehci, stream);
+       } else {
+               /* HW might remember this ITD, so we can't recycle it yet.
+                * Move it to a safe place until a new frame starts.
+                */
+               list_move(&itd->itd_list, &ehci->cached_itd_list);
+               if (stream->refcount == 2) {
+                       /* If iso_stream_put() were called here, stream
+                        * would be freed.  Instead, just prevent reuse.
+                        */
+                       stream->ep->hcpriv = NULL;
+                       stream->ep = NULL;
+               }
+       }
       return retval;
 }

@@ -2105,6 +2120,20 @@ done:

 /*-------------------------------------------------------------------------*/

+static void free_cached_itd_list(struct ehci_hcd *ehci)
+{
+       struct ehci_itd *itd, *n;
+
+       list_for_each_entry_safe(itd, n, &ehci->cached_itd_list, itd_list) {
+               struct ehci_iso_stream  *stream = itd->stream;
+               itd->stream = NULL;
+               list_move(&itd->itd_list, &stream->free_list);
+               iso_stream_put(ehci, stream);
+       }
+}
+
+/*-------------------------------------------------------------------------*/
+
 static void
 scan_periodic (struct ehci_hcd *ehci)
 {
@@ -2119,10 +2148,17 @@ scan_periodic (struct ehci_hcd *ehci)
        * Touches as few pages as possible:  cache-friendly.
        */
       now_uframe = ehci->next_uframe;
-       if (HC_IS_RUNNING (ehci_to_hcd(ehci)->state))
+       if (HC_IS_RUNNING(ehci_to_hcd(ehci)->state)) {
               clock = ehci_readl(ehci, &ehci->regs->frame_index);
-       else
+               clock_frame = (clock >> 3) % ehci->periodic_size;
+       } else  {
               clock = now_uframe + mod - 1;
+               clock_frame = -1;
+       }
+       if (ehci->clock_frame != clock_frame) {
+               free_cached_itd_list(ehci);
+               ehci->clock_frame = clock_frame;
+       }
       clock %= mod;
       clock_frame = clock >> 3;

@@ -2281,6 +2317,10 @@ restart:
                       /* rescan the rest of this frame, then ... */
                       clock = now;
                       clock_frame = clock >> 3;
+                       if (ehci->clock_frame != clock_frame) {
+                               free_cached_itd_list(ehci);
+                               ehci->clock_frame = clock_frame;
+                       }
               } else {
                       now_uframe++;
                       now_uframe %= mod;
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -87,6 +87,10 @@ struct ehci_hcd {                    /* one per controlle
       int                     next_uframe;    /* scan periodic, start here */
       unsigned                periodic_sched; /* periodic activity count */

+       /* list of itds completed while clock_frame was still active */
+       struct list_head        cached_itd_list;
+       unsigned                clock_frame;
+
       /* per root hub port */
       unsigned long           reset_done [EHCI_MAX_ROOT_PORTS];

@@ -220,6 +224,8 @@ timer_action (struct ehci_hcd *ehci, enu
       }
 }

+static void free_cached_itd_list(struct ehci_hcd *ehci);
+
 /*-------------------------------------------------------------------------*/

 #include <linux/usb/ehci_def.h>
--




More information about the kernel-team mailing list