On Thu, 5 May 2011, Manuel Lauss wrote: > > The only way usb_hc_died() gets called is if hcd->state is > > HC_STATE_HALT, and it gets set to that value when ehci_reset() is > > called. At the end of ehci_hcd_au1xxx_drv_resume(), hcd->state is set > > to HC_STATE_SUSPENDED. Is it possible that an interrupt arrives > > between those two assignments? (But I don't see how the commit would > > have affected that...) > > The EHCI and OHCI controller share an interrupt. As a test I disabled > the OHCI driver, and voila, resume works. Logs both with and > without built-in OHCI below. Aha -- it does indeed look like a shared interrupt causing problems. And I see why this didn't show up earlier. Prior to the commit you found, if hcd->state was set to HC_STATE_HALT when an interrupt occurred, the interrupt would be ignored. It looks like the only solution is to remove the test for HC_STATE_HALT in usb_hcd_irq(), which means each driver has to call usb_hc_died() at the appropriate times. This patch should do that. Let's see how it works (this is with that as1451 commit in place). Alan Stern Index: usb-2.6/drivers/usb/core/hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/core/hcd.c +++ usb-2.6/drivers/usb/core/hcd.c @@ -986,7 +986,7 @@ static int register_root_hub(struct usb_ spin_unlock_irq (&hcd_root_hub_lock); /* Did the HC die before the root hub was registered? */ - if (HCD_DEAD(hcd) || hcd->state == HC_STATE_HALT) + if (HCD_DEAD(hcd)) usb_hc_died (hcd); /* This time clean up */ } @@ -2128,9 +2128,6 @@ irqreturn_t usb_hcd_irq (int irq, void * set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags); if (hcd->shared_hcd) set_bit(HCD_FLAG_SAW_IRQ, &hcd->shared_hcd->flags); - - if (unlikely(hcd->state == HC_STATE_HALT)) - usb_hc_died(hcd); rc = IRQ_HANDLED; } Index: usb-2.6/drivers/usb/host/ehci-hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/host/ehci-hcd.c +++ usb-2.6/drivers/usb/host/ehci-hcd.c @@ -781,8 +781,9 @@ static irqreturn_t ehci_irq (struct usb_ goto dead; } + /* Shared IRQ? */ masked_status = status & INTR_MASK; - if (!masked_status) { /* irq sharing? */ + if (!masked_status || unlikely(hcd->state == HC_STATE_HALT)) { spin_unlock(&ehci->lock); return IRQ_NONE; } @@ -877,6 +878,7 @@ static irqreturn_t ehci_irq (struct usb_ dead: ehci_reset(ehci); ehci_writel(ehci, 0, &ehci->regs->configured_flag); + usb_hc_died(hcd); /* generic layer kills/unlinks all urbs, then * uses ehci_stop to clean up the rest */ Index: usb-2.6/drivers/usb/host/ehci-sched.c =================================================================== --- usb-2.6.orig/drivers/usb/host/ehci-sched.c +++ usb-2.6/drivers/usb/host/ehci-sched.c @@ -471,8 +471,10 @@ static int enable_periodic (struct ehci_ */ status = handshake_on_error_set_halt(ehci, &ehci->regs->status, STS_PSS, 0, 9 * 125); - if (status) + if (status) { + usb_hc_died(ehci_to_hcd(ehci)); return status; + } cmd = ehci_readl(ehci, &ehci->regs->command) | CMD_PSE; ehci_writel(ehci, cmd, &ehci->regs->command); @@ -510,8 +512,10 @@ static int disable_periodic (struct ehci */ status = handshake_on_error_set_halt(ehci, &ehci->regs->status, STS_PSS, STS_PSS, 9 * 125); - if (status) + if (status) { + usb_hc_died(ehci_to_hcd(ehci)); return status; + } cmd = ehci_readl(ehci, &ehci->regs->command) & ~CMD_PSE; ehci_writel(ehci, cmd, &ehci->regs->command); Index: usb-2.6/drivers/usb/host/isp116x-hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/host/isp116x-hcd.c +++ usb-2.6/drivers/usb/host/isp116x-hcd.c @@ -612,6 +612,7 @@ static irqreturn_t isp116x_irq(struct us /* IRQ's are off, we do no DMA, perfectly ready to die ... */ hcd->state = HC_STATE_HALT; + usb_hc_died(hcd); ret = IRQ_HANDLED; goto done; } Index: usb-2.6/drivers/usb/host/oxu210hp-hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/host/oxu210hp-hcd.c +++ usb-2.6/drivers/usb/host/oxu210hp-hcd.c @@ -1884,6 +1884,7 @@ static int enable_periodic(struct oxu_hc status = handshake(oxu, &oxu->regs->status, STS_PSS, 0, 9 * 125); if (status != 0) { oxu_to_hcd(oxu)->state = HC_STATE_HALT; + usb_hc_died(oxu_to_hcd(oxu)); return status; } @@ -1909,6 +1910,7 @@ static int disable_periodic(struct oxu_h status = handshake(oxu, &oxu->regs->status, STS_PSS, STS_PSS, 9 * 125); if (status != 0) { oxu_to_hcd(oxu)->state = HC_STATE_HALT; + usb_hc_died(oxu_to_hcd(oxu)); return status; } @@ -2449,8 +2451,9 @@ static irqreturn_t oxu210_hcd_irq(struct goto dead; } + /* Shared IRQ? */ status &= INTR_MASK; - if (!status) { /* irq sharing? */ + if (!status || unlikely(hcd->state == HC_STATE_HALT)) { spin_unlock(&oxu->lock); return IRQ_NONE; } @@ -2516,6 +2519,7 @@ static irqreturn_t oxu210_hcd_irq(struct dead: ehci_reset(oxu); writel(0, &oxu->regs->configured_flag); + usb_hc_died(hcd); /* generic layer kills/unlinks all urbs, then * uses oxu_stop to clean up the rest */ Index: usb-2.6/drivers/usb/host/ohci-hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/host/ohci-hcd.c +++ usb-2.6/drivers/usb/host/ohci-hcd.c @@ -764,6 +764,7 @@ static irqreturn_t ohci_irq (struct usb_ if (ints == ~(u32)0) { disable (ohci); ohci_dbg (ohci, "device removed!\n"); + usb_hc_died(hcd); return IRQ_HANDLED; } @@ -771,7 +772,7 @@ static irqreturn_t ohci_irq (struct usb_ ints &= ohci_readl(ohci, ®s->intrenable); /* interrupt for some other device? */ - if (ints == 0) + if (ints == 0 || unlikely(hcd->state == HC_STATE_HALT)) return IRQ_NOTMINE; if (ints & OHCI_INTR_UE) { @@ -788,6 +789,7 @@ static irqreturn_t ohci_irq (struct usb_ } else { disable (ohci); ohci_err (ohci, "OHCI Unrecoverable Error, disabled\n"); + usb_hc_died(hcd); } ohci_dump (ohci, 1); -- 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