| From b12412794399e6f2a5f0a229b50e1ee60891d5c0 Mon Sep 17 00:00:00 2001 |
| From: Alan Stern <stern@rowland.harvard.edu> |
| Date: Tue, 5 Jul 2011 12:34:05 -0400 |
| Subject: [PATCH] USB: EHCI: go back to using the system clock for QH unlinks |
| |
| commit 004c19682884d4f40000ce1ded53f4a1d0b18206 upstream. |
| |
| This patch (as1477) fixes a problem affecting a few types of EHCI |
| controller. Contrary to what one might expect, these controllers |
| automatically stop their internal frame counter when no ports are |
| enabled. Since ehci-hcd currently relies on the frame counter for |
| determining when it should unlink QHs from the async schedule, those |
| controllers run into trouble: The frame counter stops and the QHs |
| never get unlinked. |
| |
| Some systems have also experienced other problems traced back to |
| commit b963801164618e25fbdc0cd452ce49c3628b46c8 (USB: ehci-hcd unlink |
| speedups), which made the original switch from using the system clock |
| to using the frame counter. It never became clear what the reason was |
| for these problems, but evidently it is related to use of the frame |
| counter. |
| |
| To fix all these problems, this patch more or less reverts that commit |
| and goes back to using the system clock. But this can't be done |
| cleanly because other changes have since been made to the scan_async() |
| subroutine. One of these changes involved the tricky logic that tries |
| to avoid rescanning QHs that have already been seen when the scanning |
| loop is restarted, which happens whenever an URB is given back. |
| Switching back to clock-based unlinks would make this logic even more |
| complicated. |
| |
| Therefore the new code doesn't rescan the entire async list whenever a |
| giveback occurs. Instead it rescans only the current QH and continues |
| on from there. This requires the use of a separate pointer to keep |
| track of the next QH to scan, since the current QH may be unlinked |
| while the scanning is in progress. That new pointer must be global, |
| so that it can be adjusted forward whenever the _next_ QH gets |
| unlinked. (uhci-hcd uses this same trick.) |
| |
| Simplification of the scanning loop removes a level of indentation, |
| which accounts for the size of the patch. The amount of code changed |
| is relatively small, and it isn't exactly a reversion of the |
| b963801164 commit. |
| |
| This fixes Bugzilla #32432. |
| |
| Signed-off-by: Alan Stern <stern@rowland.harvard.edu> |
| Tested-by: Matej Kenda <matejken@gmail.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| --- |
| drivers/usb/host/ehci-hcd.c | 8 ++--- |
| drivers/usb/host/ehci-q.c | 82 ++++++++++++++++++++++----------------------- |
| drivers/usb/host/ehci.h | 3 +- |
| 3 files changed, 45 insertions(+), 48 deletions(-) |
| |
| diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c |
| index 5a320236ee3a..1674039cf683 100644 |
| --- a/drivers/usb/host/ehci-hcd.c |
| +++ b/drivers/usb/host/ehci-hcd.c |
| @@ -84,7 +84,8 @@ static const char hcd_name [] = "ehci_hcd"; |
| #define EHCI_IAA_MSECS 10 /* arbitrary */ |
| #define EHCI_IO_JIFFIES (HZ/10) /* io watchdog > irq_thresh */ |
| #define EHCI_ASYNC_JIFFIES (HZ/20) /* async idle timeout */ |
| -#define EHCI_SHRINK_FRAMES 5 /* async qh unlink delay */ |
| +#define EHCI_SHRINK_JIFFIES (DIV_ROUND_UP(HZ, 200) + 1) |
| + /* 200-ms async qh unlink delay */ |
| |
| /* Initial IRQ latency: faster than hw default */ |
| static int log2_irq_thresh = 0; // 0 to 6 |
| @@ -139,10 +140,7 @@ timer_action(struct ehci_hcd *ehci, enum ehci_timer_action action) |
| break; |
| /* case TIMER_ASYNC_SHRINK: */ |
| default: |
| - /* add a jiffie since we synch against the |
| - * 8 KHz uframe counter. |
| - */ |
| - t = DIV_ROUND_UP(EHCI_SHRINK_FRAMES * HZ, 1000) + 1; |
| + t = EHCI_SHRINK_JIFFIES; |
| break; |
| } |
| mod_timer(&ehci->watchdog, t + jiffies); |
| diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c |
| index 9b46a1ee616f..1992abbe223d 100644 |
| --- a/drivers/usb/host/ehci-q.c |
| +++ b/drivers/usb/host/ehci-q.c |
| @@ -1226,6 +1226,8 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh) |
| |
| prev->hw->hw_next = qh->hw->hw_next; |
| prev->qh_next = qh->qh_next; |
| + if (ehci->qh_scan_next == qh) |
| + ehci->qh_scan_next = qh->qh_next.qh; |
| wmb (); |
| |
| /* If the controller isn't running, we don't have to wait for it */ |
| @@ -1251,53 +1253,49 @@ static void scan_async (struct ehci_hcd *ehci) |
| struct ehci_qh *qh; |
| enum ehci_timer_action action = TIMER_IO_WATCHDOG; |
| |
| - ehci->stamp = ehci_readl(ehci, &ehci->regs->frame_index); |
| timer_action_done (ehci, TIMER_ASYNC_SHRINK); |
| -rescan: |
| stopped = !HC_IS_RUNNING(ehci_to_hcd(ehci)->state); |
| - qh = ehci->async->qh_next.qh; |
| - if (likely (qh != NULL)) { |
| - do { |
| - /* clean any finished work for this qh */ |
| - if (!list_empty(&qh->qtd_list) && (stopped || |
| - qh->stamp != ehci->stamp)) { |
| - int temp; |
| - |
| - /* unlinks could happen here; completion |
| - * reporting drops the lock. rescan using |
| - * the latest schedule, but don't rescan |
| - * qhs we already finished (no looping) |
| - * unless the controller is stopped. |
| - */ |
| - qh = qh_get (qh); |
| - qh->stamp = ehci->stamp; |
| - temp = qh_completions (ehci, qh); |
| - if (qh->needs_rescan) |
| - unlink_async(ehci, qh); |
| - qh_put (qh); |
| - if (temp != 0) { |
| - goto rescan; |
| - } |
| - } |
| |
| - /* unlink idle entries, reducing DMA usage as well |
| - * as HCD schedule-scanning costs. delay for any qh |
| - * we just scanned, there's a not-unusual case that it |
| - * doesn't stay idle for long. |
| - * (plus, avoids some kind of re-activation race.) |
| + ehci->qh_scan_next = ehci->async->qh_next.qh; |
| + while (ehci->qh_scan_next) { |
| + qh = ehci->qh_scan_next; |
| + ehci->qh_scan_next = qh->qh_next.qh; |
| + rescan: |
| + /* clean any finished work for this qh */ |
| + if (!list_empty(&qh->qtd_list)) { |
| + int temp; |
| + |
| + /* |
| + * Unlinks could happen here; completion reporting |
| + * drops the lock. That's why ehci->qh_scan_next |
| + * always holds the next qh to scan; if the next qh |
| + * gets unlinked then ehci->qh_scan_next is adjusted |
| + * in start_unlink_async(). |
| */ |
| - if (list_empty(&qh->qtd_list) |
| - && qh->qh_state == QH_STATE_LINKED) { |
| - if (!ehci->reclaim && (stopped || |
| - ((ehci->stamp - qh->stamp) & 0x1fff) |
| - >= EHCI_SHRINK_FRAMES * 8)) |
| - start_unlink_async(ehci, qh); |
| - else |
| - action = TIMER_ASYNC_SHRINK; |
| - } |
| + qh = qh_get(qh); |
| + temp = qh_completions(ehci, qh); |
| + if (qh->needs_rescan) |
| + unlink_async(ehci, qh); |
| + qh->unlink_time = jiffies + EHCI_SHRINK_JIFFIES; |
| + qh_put(qh); |
| + if (temp != 0) |
| + goto rescan; |
| + } |
| |
| - qh = qh->qh_next.qh; |
| - } while (qh); |
| + /* unlink idle entries, reducing DMA usage as well |
| + * as HCD schedule-scanning costs. delay for any qh |
| + * we just scanned, there's a not-unusual case that it |
| + * doesn't stay idle for long. |
| + * (plus, avoids some kind of re-activation race.) |
| + */ |
| + if (list_empty(&qh->qtd_list) |
| + && qh->qh_state == QH_STATE_LINKED) { |
| + if (!ehci->reclaim && (stopped || |
| + time_after_eq(jiffies, qh->unlink_time))) |
| + start_unlink_async(ehci, qh); |
| + else |
| + action = TIMER_ASYNC_SHRINK; |
| + } |
| } |
| if (action == TIMER_ASYNC_SHRINK) |
| timer_action (ehci, TIMER_ASYNC_SHRINK); |
| diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h |
| index 1bb7a7f1ae77..d32d26e27f53 100644 |
| --- a/drivers/usb/host/ehci.h |
| +++ b/drivers/usb/host/ehci.h |
| @@ -74,6 +74,7 @@ struct ehci_hcd { /* one per controller */ |
| /* async schedule support */ |
| struct ehci_qh *async; |
| struct ehci_qh *reclaim; |
| + struct ehci_qh *qh_scan_next; |
| unsigned scanning : 1; |
| |
| /* periodic schedule support */ |
| @@ -116,7 +117,6 @@ struct ehci_hcd { /* one per controller */ |
| struct timer_list iaa_watchdog; |
| struct timer_list watchdog; |
| unsigned long actions; |
| - unsigned stamp; |
| unsigned random_frame; |
| unsigned long next_statechange; |
| ktime_t last_periodic_enable; |
| @@ -336,6 +336,7 @@ struct ehci_qh { |
| struct ehci_qh *reclaim; /* next to reclaim */ |
| |
| struct ehci_hcd *ehci; |
| + unsigned long unlink_time; |
| |
| /* |
| * Do NOT use atomic operations for QH refcounting. On some CPUs |
| -- |
| 1.8.5.2 |
| |