On Fri, 16 Sep 2005, David Brownell wrote: > Remember that root hubs, unlike "normal" hubs, expose _three_ faces (not two) > through the driver model: the controller (e.g. a PCI or platform device); the > root hub usb_device; and the root hub usb_interfacee. Plus, no upstream USB > link exists; USB gets morphed to some other bus in silicon ... > > So if the HC (PCI/platform device) is the upstream link, then it'll be > be power managed as a non-USB device. As one would expect. > > But for the downstream link ... I think the choice between the two is > almost arbitrary. And it's less confusing to have hub->suspend call > hcd->hub_suspend than to special-case hub support into device suspend. :)
I think I see where our thinking diverges... Mostly it comes down to what "suspend the root hub" really means. > > which > > implies that the bus->op->hub_suspend method call in __usb_suspend_device > > should stay where it is. Moving the method call to the hub_suspend > > function is wrong, because it would mean that suspending a hub's interface > > has the unexpected side-effect of suspending the whole hub device (for > > root hubs)! (And thereby suspending that entire bus!) > > That's not "wrong" ... it's 100% consistent with suspending any other > hub interface! When it finishes suspending, the down-stream links are > all suspended; not the upstream one. (True for both faces of the > downstream link... though the usb_interface is always suspended first.) Consider what will happen when we remove _all_ the recursion from the USB suspend/resume routines. Suspending an external hub's interface won't have any effect on the downstream ports; they will be affected only when you call usb_suspend_device for the children. The fact that your proposed root-hub behavior is consistent with the current state of affairs is a coincidence, an artifact of the way you originally wrote the suspend code to include the recursion. What happens when an external hub's interface is suspended but the downstream ports are still active? Answer: There's no status URB queued, so usbcore won't be aware of port status changes, but the hub will continue to relay packets. Root hubs should be capable of the same sort of behavior. (Until very recently, unbinding the hub driver from a hub via sysfs would leave it in pretty much this state.) Part of the conceptual difficulty here is that our implementation of virtual root hubs isn't as accurate as it could be. When an external hub has been suspended, you can't communicate with it at all (apart from wakeup signals). It won't respond to Get-Descriptor or Get-Port-Status requests, for example. (Assuming usbcore would allow you to send such requests in the first place, rather than just returning -EHOSTUNREACH.) But root hubs _do_ respond to such requests when they are suspended. The code in hcd.c that handles Get-Descriptor for root hubs doesn't even know when the hub has been suspended, and the port-status code in HCDs (like in uhci-hub.c) doesn't bother to check. In practice this flaw in the emulation doesn't matter, since usbcore doesn't let you send URBs to suspended devices. But it does confuse our thinking. So for the sake of argument, let's pretend that we have changed the emulation and removed the -EHOSTUNREACH check. From now on, URBs sent to suspended root hubs return -EPROTO errors, as do URBs sent to suspended external hubs. Now look at how your proposal results in different behavior for root hubs and external hubs. If you suspend an external hub's interface, you can still send Get-Descriptor to the hub itself and get a reply. But if you suspend a root hub's interface, a Get-Descriptor request will return -EPROTO because hcd->hub_suspend was called. At this point, you'll probably want to say that my analysis is wrong, that virtual root hubs are effectively split into two halves. The upstream half is the HC device, and what we've been calling the root hub is really only the downstream usb_device half. To fully suspend the root hub, it's necessary to suspend both halves. I don't like this explanation. I think the HC device should embody the parent bus (e.g., the PCI) aspects of a host controller and the root-hub device should embody the USB aspects -- i.e., its virtual hub. Suspending the usb_device should cause the virtual hub to behave like a physically-suspended external hub. Suspending the HC device should cause it to behave like any other suspended PCI device. It shouldn't be necessary to put the controller in D3 (for example) just in order to fully suspend the usb_device. Nor should it be necessary to make two distinct suspend calls to suspend a root hub, seeing as how one call suffices for external hubs. Or put it another way. You can fully suspend an external hub by calling usb_suspend_device(hdev); that single call should suffice even when hdev is a root hub. Okay, that was a long speech. But these are subtle and tricky concepts, and it's important to state them clearly and agree on them. > This is easier to fix than the hub/device parenting stuff... You lost me there. What hub/device parenting stuff are you talking about? > > Likewise, resuming > > the root-hub device wouldn't work so long as the interface remained > > suspended -- whereas it would work with external hubs. > > But we had previously discussed the need to change that behavior so > that resuming a hub need **NOT** resume all the downstream links... > again (testing is the only hard part of that patch). True. But resuming a hub and calling usb_resume_device for its children _should_ cause those downstream links to become active, even if the hub's interface remains suspended. Your proposal would violate this for root hubs. Whereas my proposal is consistent with both behaviors. > > However I like your clean definition of CONIFG_USB_SUSPEND, if you add the > > proviso that (a) applies only to physical, non-root, devices. > > That is, we expect all HCDs to support "global(-to-this-bus) suspend"? Just call it "bus-wide suspend"; I won't mind the non-standard terminology. :-) I'm not sure what you're asking. An HCD doesn't need anything special to support bus-wide suspend. All it has to do is stop the controller from transmitting packets. That's an inevitable side-effect of suspending the root hub. (In fact, although the USB spec doesn't spell this out fully, what it calls "global suspend" is really nothing more than a root-hub suspend.) > Yes; usbcore will always use hcd->hub_suspend(). Today it doesn't. It doesn't if USB_SUSPEND isn't set, right. But it should, so that the HCDs don't need to take special actions. Yes. > Another way to describe the difference would then be: (a) it's no longer > just root hubs that will suspend downstream devices, all hubs will do that. Yes. Sort of. With USB_SUSPEND set, any hub will be able to suspend any child. Without it only root hubs can suspend anything, and they won't support suspending individual children -- only all their children together. It's clearer to say that with USB_SUSPEND set, usb_suspend_device will work on any device. Otherwise it will work only on root hubs. Alan Stern ------------------------------------------------------- SF.Net email is sponsored by: Tame your development challenges with Apache's Geronimo App Server. Download it for free - -and be entered to win a 42" plasma tv or your very own Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
