The change to remove the HC_IS_RUNNING test meant that routine
would then work when hcd->state is HC_STATE_SUSPENDED or possibly
HC_STATE_RESUMING. (Since those are the two states for which
HC_IS_RUNNING fails, other than HC_STATE_HALTED which shouldn't
be happening here...)
Now, prior to the now-merged patch that was skipping some work,
and that seemed to cause resume problems. Making it not skip
the work made the resume behave (as did the now-merged patch).
Not exactly. First, ehci_bus_suspend() force the timer off all
by itself ... way too early, while things can still retrigger it.
That's a buggy idiom that should be fixed. (As you've agreed,
elsewhere.)
Second, it doesn't always call end_unlink_async() ... only when
one or more async unlink is still pending.
Third, if it *does* call end_unlink_async(), it can retriger
the timer when it needs to do another unlink. Only when the
HC is halted (HC_STATE_HALT) is that logic bypassed, scrubbing
several endpoints at once. (And a minor fourth point, direct
calls to end_unlink_async leaves the IAA IRQ machinery active.)
I think your fix handles the "one pending unlink" case, but
not the more general "N pending unlinks" ...
Removing the state check in the watchdog changed behavior, so the
HC must have been in one of the two states I listed above when
that watchdog fired (SUSPENDED or RESUMING). And it had work to
do, which (because of the state check) it didn't do.
It takes care of *ONE* pending QH unlink. If there's more than
one, it retriggers the timer, so this problem will reappear...
When the timer would be retriggered, to finish additional unlinks,
it does matter. A complete fix for this would (a) disable the IAA
watchdog timer later, once it can never be retriggered again, and
(b) make end_unlink_async handle the multiple-unlinks case when the
HC is suspended, including not retriggering the watchdog.
- Dave
--
