diff options
| author | David Brownell <david-b@pacbell.net> | 2008-03-07 16:49:42 -0500 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@suse.de> | 2008-03-10 19:42:27 -0400 |
| commit | e82cc1288fa57857c6af8c57f3d07096d4bcd9d9 (patch) | |
| tree | c582499d6e926cf57b56cd0cdf4e85e05cc7d133 /drivers | |
| parent | 11171d1bde45eefa4fed605a5cf6ebe0e3d24395 (diff) | |
USB: fix ehci unlink regressions
The recent EHCI driver update to split the IAA watchdog timer out from
the other timers made several things work better, but not everything;
and it created a couple new issues in bugzilla. Ergo this patch:
- Handle a should-be-rare SMP race between the watchdog firing
and (very late) IAA interrupts;
- Remove a shouldn't-have-been-added WARN_ON() test;
- Guard against one observed OOPS;
- If this watchdog fires during clean HC shutdown, it should act
as a NOP instead of interfering with the shutdown sequence;
- Guard against silicon errata hypothesized by some vendors:
* IAA status latch broken, but IAAD cleared OK;
* IAAD wasn't cleared when IAA status got reported;
The WARN_ON is in bugzilla as 10168; the OOPS as 10078; these are
both regressions.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Tested-by: Gordon Farquharson <gordonfarquharson@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'drivers')
| -rw-r--r-- | drivers/usb/host/ehci-hcd.c | 62 |
1 files changed, 46 insertions, 16 deletions
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index b8ad55aff842..46ee7f4c0912 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c | |||
| @@ -281,23 +281,44 @@ static void ehci_iaa_watchdog(unsigned long param) | |||
| 281 | { | 281 | { |
| 282 | struct ehci_hcd *ehci = (struct ehci_hcd *) param; | 282 | struct ehci_hcd *ehci = (struct ehci_hcd *) param; |
| 283 | unsigned long flags; | 283 | unsigned long flags; |
| 284 | u32 status, cmd; | ||
| 285 | 284 | ||
| 286 | spin_lock_irqsave (&ehci->lock, flags); | 285 | spin_lock_irqsave (&ehci->lock, flags); |
| 287 | WARN_ON(!ehci->reclaim); | ||
| 288 | 286 | ||
| 289 | status = ehci_readl(ehci, &ehci->regs->status); | 287 | /* Lost IAA irqs wedge things badly; seen first with a vt8235. |
| 290 | cmd = ehci_readl(ehci, &ehci->regs->command); | 288 | * So we need this watchdog, but must protect it against both |
| 291 | ehci_dbg(ehci, "IAA watchdog: status %x cmd %x\n", status, cmd); | 289 | * (a) SMP races against real IAA firing and retriggering, and |
| 292 | 290 | * (b) clean HC shutdown, when IAA watchdog was pending. | |
| 293 | /* lost IAA irqs wedge things badly; seen first with a vt8235 */ | 291 | */ |
| 294 | if (ehci->reclaim) { | 292 | if (ehci->reclaim |
| 295 | if (status & STS_IAA) { | 293 | && !timer_pending(&ehci->iaa_watchdog) |
| 296 | ehci_vdbg (ehci, "lost IAA\n"); | 294 | && HC_IS_RUNNING(ehci_to_hcd(ehci)->state)) { |
| 295 | u32 cmd, status; | ||
| 296 | |||
| 297 | /* If we get here, IAA is *REALLY* late. It's barely | ||
| 298 | * conceivable that the system is so busy that CMD_IAAD | ||
| 299 | * is still legitimately set, so let's be sure it's | ||
| 300 | * clear before we read STS_IAA. (The HC should clear | ||
| 301 | * CMD_IAAD when it sets STS_IAA.) | ||
| 302 | */ | ||
| 303 | cmd = ehci_readl(ehci, &ehci->regs->command); | ||
| 304 | if (cmd & CMD_IAAD) | ||
| 305 | ehci_writel(ehci, cmd & ~CMD_IAAD, | ||
| 306 | &ehci->regs->command); | ||
| 307 | |||
| 308 | /* If IAA is set here it either legitimately triggered | ||
| 309 | * before we cleared IAAD above (but _way_ late, so we'll | ||
| 310 | * still count it as lost) ... or a silicon erratum: | ||
| 311 | * - VIA seems to set IAA without triggering the IRQ; | ||
| 312 | * - IAAD potentially cleared without setting IAA. | ||
| 313 | */ | ||
| 314 | status = ehci_readl(ehci, &ehci->regs->status); | ||
| 315 | if ((status & STS_IAA) || !(cmd & CMD_IAAD)) { | ||
| 297 | COUNT (ehci->stats.lost_iaa); | 316 | COUNT (ehci->stats.lost_iaa); |
| 298 | ehci_writel(ehci, STS_IAA, &ehci->regs->status); | 317 | ehci_writel(ehci, STS_IAA, &ehci->regs->status); |
| 299 | } | 318 | } |
| 300 | ehci_writel(ehci, cmd & ~CMD_IAAD, &ehci->regs->command); | 319 | |
| 320 | ehci_vdbg(ehci, "IAA watchdog: status %x cmd %x\n", | ||
| 321 | status, cmd); | ||
| 301 | end_unlink_async(ehci); | 322 | end_unlink_async(ehci); |
| 302 | } | 323 | } |
| 303 | 324 | ||
| @@ -631,7 +652,7 @@ static int ehci_run (struct usb_hcd *hcd) | |||
| 631 | static irqreturn_t ehci_irq (struct usb_hcd *hcd) | 652 | static irqreturn_t ehci_irq (struct usb_hcd *hcd) |
| 632 | { | 653 | { |
| 633 | struct ehci_hcd *ehci = hcd_to_ehci (hcd); | 654 | struct ehci_hcd *ehci = hcd_to_ehci (hcd); |
| 634 | u32 status, pcd_status = 0; | 655 | u32 status, pcd_status = 0, cmd; |
| 635 | int bh; | 656 | int bh; |
| 636 | 657 | ||
| 637 | spin_lock (&ehci->lock); | 658 | spin_lock (&ehci->lock); |
| @@ -652,7 +673,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) | |||
| 652 | 673 | ||
| 653 | /* clear (just) interrupts */ | 674 | /* clear (just) interrupts */ |
| 654 | ehci_writel(ehci, status, &ehci->regs->status); | 675 | ehci_writel(ehci, status, &ehci->regs->status); |
| 655 | ehci_readl(ehci, &ehci->regs->command); /* unblock posted write */ | 676 | cmd = ehci_readl(ehci, &ehci->regs->command); |
| 656 | bh = 0; | 677 | bh = 0; |
| 657 | 678 | ||
| 658 | #ifdef EHCI_VERBOSE_DEBUG | 679 | #ifdef EHCI_VERBOSE_DEBUG |
| @@ -673,8 +694,17 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) | |||
| 673 | 694 | ||
| 674 | /* complete the unlinking of some qh [4.15.2.3] */ | 695 | /* complete the unlinking of some qh [4.15.2.3] */ |
| 675 | if (status & STS_IAA) { | 696 | if (status & STS_IAA) { |
| 676 | COUNT (ehci->stats.reclaim); | 697 | /* guard against (alleged) silicon errata */ |
| 677 | end_unlink_async(ehci); | 698 | if (cmd & CMD_IAAD) { |
| 699 | ehci_writel(ehci, cmd & ~CMD_IAAD, | ||
| 700 | &ehci->regs->command); | ||
| 701 | ehci_dbg(ehci, "IAA with IAAD still set?\n"); | ||
| 702 | } | ||
| 703 | if (ehci->reclaim) { | ||
| 704 | COUNT(ehci->stats.reclaim); | ||
| 705 | end_unlink_async(ehci); | ||
| 706 | } else | ||
| 707 | ehci_dbg(ehci, "IAA with nothing to reclaim?\n"); | ||
| 678 | } | 708 | } |
| 679 | 709 | ||
| 680 | /* remote wakeup [4.3.1] */ | 710 | /* remote wakeup [4.3.1] */ |
| @@ -781,7 +811,7 @@ static int ehci_urb_enqueue ( | |||
| 781 | static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh) | 811 | static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh) |
| 782 | { | 812 | { |
| 783 | /* failfast */ | 813 | /* failfast */ |
| 784 | if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state)) | 814 | if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state) && ehci->reclaim) |
| 785 | end_unlink_async(ehci); | 815 | end_unlink_async(ehci); |
| 786 | 816 | ||
| 787 | /* if it's not linked then there's nothing to do */ | 817 | /* if it's not linked then there's nothing to do */ |
