URBs for a particular endpoint should complete sequentially. That is, we shouldn't call the completion handler for one URB until the handler for the previous URB has returned. When the OHCI watchdog routine is added, there will be two paths for completing URBs: interrupt handler and watchdog routine. Their activities have to be synchronized so that completions don't occur in multiple threads concurrently. For that purpose, this patch creates an ohci_work() routine which will be responsible for calling process_done_list() and finish_unlinks(), the two routines that detect when an URB is complete. Everything will funnel through ohci_work(), and it will be careful not to run in more than one thread at a time. Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> --- [as1756] drivers/usb/host/ohci-hcd.c | 10 ++++------ drivers/usb/host/ohci-hub.c | 6 ++---- drivers/usb/host/ohci-q.c | 28 ++++++++++++++++++++++++++-- drivers/usb/host/ohci.h | 2 ++ 4 files changed, 34 insertions(+), 12 deletions(-) Index: usb-3.16/drivers/usb/host/ohci.h =================================================================== --- usb-3.16.orig/drivers/usb/host/ohci.h +++ usb-3.16/drivers/usb/host/ohci.h @@ -393,6 +393,8 @@ struct ohci_hcd { unsigned long next_statechange; /* suspend/resume */ u32 fminterval; /* saved register */ unsigned autostop:1; /* rh auto stopping/stopped */ + unsigned working:1; + unsigned restart_work:1; unsigned long flags; /* for HC bugs */ #define OHCI_QUIRK_AMD756 0x01 /* erratum #4 */ Index: usb-3.16/drivers/usb/host/ohci-hcd.c =================================================================== --- usb-3.16.orig/drivers/usb/host/ohci-hcd.c +++ usb-3.16/drivers/usb/host/ohci-hcd.c @@ -316,7 +316,7 @@ static int ohci_urb_dequeue(struct usb_h if (ohci->rh_state != OHCI_RH_RUNNING) { /* With HC dead, we can clean up right away */ - finish_unlinks(ohci, 0); + ohci_work(ohci); } } spin_unlock_irqrestore (&ohci->lock, flags); @@ -349,7 +349,7 @@ rescan: if (ohci->rh_state != OHCI_RH_RUNNING) { sanitize: ed->state = ED_IDLE; - finish_unlinks (ohci, 0); + ohci_work(ohci); } switch (ed->state) { @@ -789,9 +789,7 @@ static irqreturn_t ohci_irq (struct usb_ /* handle any pending URB/ED unlinks, leaving INTR_SF enabled * when there's still unlinking to be done (next frame). */ - process_done_list(ohci); - if (ohci->ed_rm_list) - finish_unlinks (ohci, ohci_frame_no(ohci)); + ohci_work(ohci); if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list && ohci->rh_state == OHCI_RH_RUNNING) ohci_writel (ohci, OHCI_INTR_SF, ®s->intrdisable); @@ -879,7 +877,7 @@ int ohci_restart(struct ohci_hcd *ohci) if (!urb->unlinked) urb->unlinked = -ESHUTDOWN; } - finish_unlinks (ohci, 0); + ohci_work(ohci); spin_unlock_irq(&ohci->lock); /* paranoia, in case that didn't work: */ Index: usb-3.16/drivers/usb/host/ohci-hub.c =================================================================== --- usb-3.16.orig/drivers/usb/host/ohci-hub.c +++ usb-3.16/drivers/usb/host/ohci-hub.c @@ -40,8 +40,7 @@ (OHCI_CTRL_CLE|OHCI_CTRL_BLE|OHCI_CTRL_PLE|OHCI_CTRL_IE) static void update_done_list(struct ohci_hcd *); -static void process_done_list(struct ohci_hcd *); -static void finish_unlinks (struct ohci_hcd *, u16); +static void ohci_work(struct ohci_hcd *); #ifdef CONFIG_PM static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop) @@ -89,8 +88,7 @@ __acquires(ohci->lock) spin_lock_irq (&ohci->lock); } update_done_list(ohci); - process_done_list(ohci); - finish_unlinks (ohci, ohci_frame_no(ohci)); + ohci_work(ohci); /* * Some controllers don't handle "global" suspend properly if Index: usb-3.16/drivers/usb/host/ohci-q.c =================================================================== --- usb-3.16.orig/drivers/usb/host/ohci-q.c +++ usb-3.16/drivers/usb/host/ohci-q.c @@ -964,9 +964,9 @@ static void update_done_list(struct ohci /*-------------------------------------------------------------------------*/ /* there are some urbs/eds to unlink; called in_irq(), with HCD locked */ -static void -finish_unlinks (struct ohci_hcd *ohci, u16 tick) +static void finish_unlinks(struct ohci_hcd *ohci) { + unsigned tick = ohci_frame_no(ohci); struct ed *ed, **last; rescan_all: @@ -1202,3 +1202,27 @@ static void process_done_list(struct ohc takeback_td(ohci, td); } } + +/* + * TD takeback and URB giveback must be single-threaded. + * This routine takes care of it all. + */ +static void ohci_work(struct ohci_hcd *ohci) +{ + if (ohci->working) { + ohci->restart_work = 1; + return; + } + ohci->working = 1; + + restart: + process_done_list(ohci); + if (ohci->ed_rm_list) + finish_unlinks(ohci); + + if (ohci->restart_work) { + ohci->restart_work = 0; + goto restart; + } + ohci->working = 0; +} -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html