On Sunday 01 May 2005 7:14 pm, Alan Stern wrote:
> David:
>
> I found part of the source for the trouble I've had with root-hub
> suspend/resume on OHCI. It's these two lines near the end of
> ohci-hub.c:ohci_hub_suspend():
>
> if (status == 0)
> hcd->state = HC_STATE_SUSPENDED;
I think I remember why that's there. It pairs the earlier line
in the same function to set hcd->state to QUIESCING. And the reason
for that is because hcd->state is the only hook we have for, erm,
quiescing the traffic going into the HCD.
That is, whenever a root hub starts to suspend, whether that's
because of (a) some internal state transition, (b) a request to
that root hub's /sys/.../power/state file, (c) a side effect
of suspending the HCD by /sys/.../power/state when USB_SUSPEND
is not enabled, or even (d) a side effect of suspending the HCD
as part of entering some system sleep state ... new URBs going
into the HCD must be blocked.
We have no other way to throttle down traffic just now...
> ...
>
> This usage reveals a significant conceptual problem, potentially affecting
> all the HCDs. Namely,
>
> Does hcd->state describe the state of the host controller
> device or the state of the root-hub device?
Yes, certainly it does. How could you doubt that it represents
at least one of those two? ;)
> I've been assuming all along that it describes the host controller --
> after all, we already have hcd->self.root_hub->state to describe the root
> hub -- but the OHCI code appears to adopt the other interpretation.
I think a better way of looking at this is that the two notions
may not yet been fully teased apart.
However, it seems I've been interpreting hcd->state as representing
the parts of "root hub suspended" that are a superset of what the
root_hub->state == USB_STATE_SUSPENDED means. And leaving the other
parts to bus-specific logic.
One complication is that there's _also_ a need to represent the
state of the HC itself. Including cases like:
[1] PCI devices with PM support, where pci_dev->current_state
can hold values like PCI_D3hot. In those cases it's easy
to distinguish two modes of root hub suspend: with and
without a PCI device power state change.
[2] PCI devices without PM support (called "legacy" though they're
still used in new board designs), where pci_dev->current_state
can never hold values other than PCI_D0. In those cases
the hcd-pci code doesn't change PCI power states, but it does
deactivate IRQs and DMA for "HC suspend".
[3] Non-PCI devices found on current SOC chips, where there's no
analogue of pci_dev->current_state except maybe something
associated with that part of the clock tree.
[4] Non-PCI chips like the sl811hs, isp1161, or isp1183, where
device states may include (a) full power-off as controlled
by on-board power switch logic, (b) chip-specific suspend
states perhaps analagous to PCI D2, (c) clock based suspend
states ... and the actual states are board-specific.
There's also the struct device dev->power.power_state field, which
has always been problematic (vergin on useless) since the PM core
doesn't manage it coherently. It's never really been usable as
anything other than a boolean ... not powerful enough to distinguish
cases like [4] in current kernels, either.
> The
> way hcd->state is used in usbcore is rather mixed up. For example,
> consider this extract from hcd-pci.c:usb_hcd_pci_suspend():
>
> switch (hcd->state) {
>
> /* entry if root hub wasn't yet suspended ... from sysfs,
> * without autosuspend, or if USB_SUSPEND isn't configured.
> */
> case HC_STATE_RUNNING:
> hcd->state = HC_STATE_QUIESCING;
> retval = hcd->driver->suspend (hcd, message);
>
> The comment indicates that hcd->state refers to the root hub, but the code
> calls the suspend routine for the host controller.
It's consistent with EHCI and OHCI managing hcd->state as part of
root hub suspend logic, though, as well as the next comment:
/* entry with CONFIG_USB_SUSPEND, or hcds that autosuspend: the
* controller and/or root hub will already have been suspended,
* but it won't be ready for a PCI resume call.
*
* FIXME only CONFIG_USB_SUSPEND guarantees hub_suspend() will
* have been called, otherwise root hub timers still run ...
*/
And if you look at how those routines work for those HCDs, all they
really do is suspend or resume the root hub ... modulo that timer issue.
Those two "host controller" suspend/resume calls are effectively
there only for the PCI based controllers, and nothing they currently
do seems HC-specific. (Other than the PMAC hooks, which seem to
reflect deficiencies in the PCI suspend/resume framework... that
stuff should be handled using generic PCI platform hooks.) I'd
be happy to see those hooks vanish.
> (And incidentally, it
> should check that the callback pointer isn't NULL...)
Actually that's a case where oopsing would be legit. It'd be
a major driver bug not to have such an entry.
> Evidently this confusion needs to be straightened out. I wouldn't be
> surprised if it's responsible for some other odd things we've been seeing.
Could be, but I'm still curious why I've never seen the problem
you saw. Could it be that you've been working under the assumption
that hcd->state is more like pci_dev->power_state than like an
augmented root_hub->state? Any changes in RC3 haven't been very
testable by me so far, though I've started to poke around with
coGITo and so on.
- Dave
-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4
opportunities to win big! Highest score wins.NEC IT Guy Games. Play to
win an NEC 61 plasma display. Visit http://www.necitguy.com/?r=20
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
