On Sun, 15 Apr 2007, David Brownell wrote:

> Assuming the downstream ports are inactive (suspended or disconnected)
> the controller itself probably has at least three states:
> 
>  - on, fully functional
>  - off, needs complete restart
>  - low-power, some clocks off (and no DMA) but retains power sessions
>       * wakeup capable (fully functional)
>       * no wakeups possible, but won't trash the sessions
> 
> The generic USB stack (usbcore) should be capable of getting the HCD
> to the point where the downstream ports are inactive, at which point
> the transition between "on" and "lowpower, wakeup capable" will be
> either internal to the HCD or else a side effect of a suspend() from
> system PM infrastructure.

Yes.  The PCI drivers don't make this transition automatically because of
problems with PME#, ACPI, and remote wakeup.  Perhaps some of the other
drivers do; you're more familiar with them than I am.

> > That is, would the non-PCI  
> > drivers want to store different, more structured values?  Or would they go 
> > somewhere else, in the driver's private data?
> 
> My first inclination is that usbcore should only care about the state
> of the downstream links.  That will help to make sure that the details
> associated with any given platform's bus glue (PCI etc) don't start to
> infect usbcore ... the details are very system-specific.

It's interesting to survey usbcore to see where it does check the state of 
the upstream link.  Here's what I found:

    (1) There are two more-or-less redundant tests in the submit_urb
        pathway, one in urb.c and the other in hcd.c:

                The test in urb.c gets hit first; it returns -EHOSTUNREACH
                if either the target USB device is suspended or the 
                controller device's power_state.event isn't set to 
                PM_EVENT_ON.

                The test in hcd.c then allows URBs to be submitted for the 
                root hub even when hcd->state is HC_STATE_SUSPENDED, 
                provided the controller's power_state.event is set to
                PM_EVENT_ON.  Otherwise, if the HC isn't running it 
                returns -ESHUTDOWN.

        That second test is confusing at best.  It's another example of
        why I dislike hcd->state.  My feeling is that at all times we
        should either allow URBs for any device or allow URBs for none.
        Under what circumstances would you ever want to accept an URB
        for the root hub but reject it for any other device?

        The question remains: How should we fail URB submissions when the
        controller isn't available?  Perhaps we should add a flag:
        hcd->okay_to_submit.  It could be turned on and off in
        hcd_bus_{resume,suspend}.

        The first test should restrict itself to considering the state of 
        the target USB device.  If the controller isn't running, the
        second test will then cause the URB to fail.

    (2) There is a test in hub.c.  It is used for checking that all the
        child devices below a hub are suspended before allowing the hub
        to be suspended, and it is conditional on !CONFIG_USB_SUSPEND.
        Once the deprecated /sys/device/.../power/state attribute is 
        removed it will never be possible for the test to fail, so we
        will be able to take it out.

    (3) There is a test in driver.c.  It's part of the autoresume code;
        it checks that the controller is running before trying to 
        autoresume a root hub.  It seems to me the test could be removed.
        If the controller isn't running then the HCD should fail the
        bus_resume call, and the final effect would be the same.

If we make all these changes then usbcore will be completely divorced from
the state of the upstream controller link.  Of course the HCDs themselves
(and hcd-pci.c) will need to be intimately aware of it.  There still are a
few places where they test the value, mostly in debugging code or to check
that the controller really is suspended before trying to resume it.  We
ought to be able to replace them with something like

        test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)

Do you think this will be good enough for those drivers, or should we add 
something more flexible?

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to