Messages in this thread | | | Date | Wed, 02 Apr 2008 14:08:17 -0400 | From | Mark Lord <> | Subject | Re: [PATCH] usb ehci_iaa_watchdog fix |
| |
Mark Lord wrote: > Alan Stern wrote: >> On Wed, 2 Apr 2008, Mark Lord wrote: >> >>> Yup, that does indeed cure it. >>> Here's a patch, in case you didn't already generate one: >>> >>> Signed-off-by: Mark Lord <mlord@pobox.com> >>> >>> --- rc8/drivers/usb/host/ehci-hcd.c 2008-03-11 11:18:40.000000000 >>> -0400 >>> +++ linux/drivers/usb/host/ehci-hcd.c 2008-04-02 >>> 12:16:40.000000000 -0400 >>> @@ -289,9 +289,7 @@ >>> * (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)) { >>> + if (ehci->reclaim && !timer_pending(&ehci->iaa_watchdog)) { >>> u32 cmd, status; >>> >>> /* If we get here, IAA is *REALLY* late. It's barely >> >> Okay, I'm puzzled. How could this make any difference? >> ehci_bus_suspend() calls end_unlink_async() anyway, whenever reclaim >> is set. >> >> Is the real problem that it does so before calling ehci_work() instead >> of after calling ehci_halt()? >> >> Mark, if you want to experiment some more, try reverting your patch >> above and moving: > .. > > Here's what I did, and yes, this also works. Pick one. > > This patch, suggested by Alan Stern, fixes the hung USB issues > on my notebook from suspend/resume cycles. > > Signed-off-by: Mark Lord <mlord@pobox.com> > > --- rc8/drivers/usb/host/ehci-hub.c 2008-03-11 11:18:40.000000000 -0400 > +++ linux/drivers/usb/host/ehci-hub.c 2008-04-02 13:28:50.000000000 > -0400 > @@ -135,8 +135,6 @@ > hcd->state = HC_STATE_QUIESCING; > } > ehci->command = ehci_readl(ehci, &ehci->regs->command); > - if (ehci->reclaim) > - end_unlink_async(ehci); > ehci_work(ehci); > > /* Unlike other USB host controller types, EHCI doesn't have > @@ -180,6 +178,9 @@ > ehci_halt (ehci); > hcd->state = HC_STATE_SUSPENDED; > > + if (ehci->reclaim) > + end_unlink_async(ehci); > + > /* allow remote wakeup */ > mask = INTR_MASK; > if (!device_may_wakeup(&hcd->self.root_hub->dev)) ..
Either patch is stable here, and have survived reboots and suspend/resumes, with and without the 2.2Kohm resistor to drain residual USB voltage.
Which one should Andrew/Linus take?
-ml
| |