Re: [PATCH] usb ehci_iaa_watchdog fix

Mark Lord
2008-04-02T18:08:28Z

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