Skip to content

Commit

Permalink
USB: fix ehci unlink regressions
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
David Brownell authored and Greg Kroah-Hartman committed Mar 10, 2008
1 parent 11171d1 commit e82cc12
Showing 1 changed file with 46 additions and 16 deletions.
62 changes: 46 additions & 16 deletions drivers/usb/host/ehci-hcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -281,23 +281,44 @@ static void ehci_iaa_watchdog(unsigned long param)
{
struct ehci_hcd *ehci = (struct ehci_hcd *) param;
unsigned long flags;
u32 status, cmd;

spin_lock_irqsave (&ehci->lock, flags);
WARN_ON(!ehci->reclaim);

status = ehci_readl(ehci, &ehci->regs->status);
cmd = ehci_readl(ehci, &ehci->regs->command);
ehci_dbg(ehci, "IAA watchdog: status %x cmd %x\n", status, cmd);

/* lost IAA irqs wedge things badly; seen first with a vt8235 */
if (ehci->reclaim) {
if (status & STS_IAA) {
ehci_vdbg (ehci, "lost IAA\n");
/* Lost IAA irqs wedge things badly; seen first with a vt8235.
* So we need this watchdog, but must protect it against both
* (a) SMP races against real IAA firing and retriggering, and
* (b) clean HC shutdown, when IAA watchdog was pending.
*/
if (ehci->reclaim
&& !timer_pending(&ehci->iaa_watchdog)
&& HC_IS_RUNNING(ehci_to_hcd(ehci)->state)) {
u32 cmd, status;

/* If we get here, IAA is *REALLY* late. It's barely
* conceivable that the system is so busy that CMD_IAAD
* is still legitimately set, so let's be sure it's
* clear before we read STS_IAA. (The HC should clear
* CMD_IAAD when it sets STS_IAA.)
*/
cmd = ehci_readl(ehci, &ehci->regs->command);
if (cmd & CMD_IAAD)
ehci_writel(ehci, cmd & ~CMD_IAAD,
&ehci->regs->command);

/* If IAA is set here it either legitimately triggered
* before we cleared IAAD above (but _way_ late, so we'll
* still count it as lost) ... or a silicon erratum:
* - VIA seems to set IAA without triggering the IRQ;
* - IAAD potentially cleared without setting IAA.
*/
status = ehci_readl(ehci, &ehci->regs->status);
if ((status & STS_IAA) || !(cmd & CMD_IAAD)) {
COUNT (ehci->stats.lost_iaa);
ehci_writel(ehci, STS_IAA, &ehci->regs->status);
}
ehci_writel(ehci, cmd & ~CMD_IAAD, &ehci->regs->command);

ehci_vdbg(ehci, "IAA watchdog: status %x cmd %x\n",
status, cmd);
end_unlink_async(ehci);
}

Expand Down Expand Up @@ -631,7 +652,7 @@ static int ehci_run (struct usb_hcd *hcd)
static irqreturn_t ehci_irq (struct usb_hcd *hcd)
{
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
u32 status, pcd_status = 0;
u32 status, pcd_status = 0, cmd;
int bh;

spin_lock (&ehci->lock);
Expand All @@ -652,7 +673,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)

/* clear (just) interrupts */
ehci_writel(ehci, status, &ehci->regs->status);
ehci_readl(ehci, &ehci->regs->command); /* unblock posted write */
cmd = ehci_readl(ehci, &ehci->regs->command);
bh = 0;

#ifdef EHCI_VERBOSE_DEBUG
Expand All @@ -673,8 +694,17 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)

/* complete the unlinking of some qh [4.15.2.3] */
if (status & STS_IAA) {
COUNT (ehci->stats.reclaim);
end_unlink_async(ehci);
/* guard against (alleged) silicon errata */
if (cmd & CMD_IAAD) {
ehci_writel(ehci, cmd & ~CMD_IAAD,
&ehci->regs->command);
ehci_dbg(ehci, "IAA with IAAD still set?\n");
}
if (ehci->reclaim) {
COUNT(ehci->stats.reclaim);
end_unlink_async(ehci);
} else
ehci_dbg(ehci, "IAA with nothing to reclaim?\n");
}

/* remote wakeup [4.3.1] */
Expand Down Expand Up @@ -781,7 +811,7 @@ static int ehci_urb_enqueue (
static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
{
/* failfast */
if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state))
if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state) && ehci->reclaim)
end_unlink_async(ehci);

/* if it's not linked then there's nothing to do */
Expand Down

0 comments on commit e82cc12

Please sign in to comment.