On Mon, 25 Apr 2005, David Brownell wrote: > On Sunday 24 April 2005 9:21 am, Alan Stern wrote: > > > > There's no recursion. When the PCI device is suspended or > > resumed, the code does _not_ suspend or resume the root hub > > device. > > If it can be guaranteed that the root hub is already suspended, > including all the root hub timer wierdness, that's OK. But last > I checked, that wasn't particularly straightforward. It may well > be getting easier to do now with your updates to the hub code and > to the root hub glue; but not in the 2.6.12-rc3 code.
I don't see any problem. All you need to do is make sure that the hub_suspend routine does all the same things as the driver's internal call to suspend the root hub from within the PCI suspend routine. If the internal call makes sufficient guarantees -- which I presume it does -- then so should hub_suspend. Then the internal call becomes unnecessary. With or without my recent updates. > > This is in keeping with my attitude that the USB subsystem shouldn't do > > much (or any!) recursion for suspend/resume. > > It's hard to say; the PM device tree framework is pretty goofy, > and that's the root cause of why it works the way it does now. > > As you know, I've been leaning towards making the CONFIG_USB_SUSPEND > code get rid of recursion. And in fact I've had patches doing > just that, for a few months now (waiting for 2.6.12 to ship before > I clean them up and submit). > > The big blotch on that nice picture is that sysfs power/states are > (a) the only standard way to test suspend/resume outside of system > suspend transitions that affect every device, (b) wholly ignorant > of the basic rules that children suspend first, and parents resume > first, (c) interacting curiously with suspend states one would > think should be driver-internal, notably for root hub timers. I agree with everything you say, and you seem to be agreeing (perhaps with reservations!) with what I wrote. > > During a system sleep/wakeup > > transition it's not necessary, as the PM core will do it for us. For > > runtime power management the recursion can be left up to userspace tools. > > As I've said to you in off-list email ... although I've also said that > relying on userspace tools for that is only a debugging hack, and > the need for them is only more proof of the (increasing!) weakness > of the driver model support for anything other than system suspend > states. ;) True enough. But it still is the appropriate approach for now. Later on, when the driver model support is improved, we can reconsider. > > There's an exception made if CONFIG_USB_SUSPEND isn't set. > > In that case the PCI device's suspend/resume routines do > > automatically call the internal subroutines for suspending/ > > resuming the root hub -- otherwise the root hub would never > > change state! > > OHCI and EHCI do that too. Yes, and they also lock/unlock the root hub around the call. I suppose UHCI should do the locking too... > > Note that they don't call usb_suspend_device or usb_resume_device, since > > those routines don't do anything when CONFIG_USB_SUSPEND isn't set. > > And again, we want CONFIG_USB_SUSPEND to eventually be the default. > We need more drivers to support suspend() and resume() methods though, > and other cleanups (including in the driver model pm core stuff) > before that's fully safe. Actually I'm not clear on what would be unsafe about leaving it always on. For system sleep it wouldn't make much difference, would it? > > On the other hand, the code does do a minimal amount of > > checking. The PCI device's suspend routine checks that the > > root hub is already suspended, and the root hub's resume > > routine checks that the PCI device is already resumed. If > > the checks fail an error is returned. > > > > Note that these checks don't actually involve dereferencing > > hcd->self.root_hub. The driver keeps track internally of the state of > > the root hub. Thus there's no danger should the code run at a time > > when the root hub hasn't been registered (although in fact that can't > > happen). > > You know, those constraints actually belong in the driver core code, > not in USB at all... Yes. But until they get there we have to make do. > > Auto-suspend (called AUTO_STOP in uhci-hcd) is handled > > separately from the normal suspend pathway. In particular, > > it doesn't involve calling usb_suspend_device for the root > > hub. Likewise, resuming from AUTO_STOP doesn't involve > > calling usb_resume_device. Only internal routines are > > involved. > > > > This is partly so that auto-suspend can work even when CONFIG_USB_SUSPEND > > is off. But more importantly, it reflects a separation of > > responsibilities between the hub driver and the HCD. As far as usbcore is > > concerned, the root hub is (like every other hub) totally under the > > control of the hub driver. It shouldn't be suspended or resumed on the > > whim of the HCD, which is only supposed to manage the hardware. > > > > When the hub driver gets auto-suspend capability added, the HCD won't need > > it. Until then, an HCD-initiated auto-suspend is completely internal to > > the driver -- usbcore is unaware that anything has happened. > > That sounds fair, but also dependent on some cleanups with respect > to usbcore and root hub timers that haven't been merged yet... Everything necessary should already be present in Greg's usb-2.6.12-rc3.patch. But I suspect you don't really need anything special. If a root-hub timer fires at a bad moment, the hub_status routine need only return 0. > > An HCD should be largely unaware of the existence of > > hcd->self.root_hub. When the HCD needs to do something > > that affects the root hub, a facility should be provided > > by usbcore. In short, the HCD should never need to > > dereference hcd->self.root_hub. > > > > The facilities currently available for use by HCDs include usb_hc_died, > > usb_hcd_resume_root_hub, and usb_hcd_poll_rh_status. Normally usbcore > > will call usb_hc_died on behalf of the HCD, so the driver doesn't need to > > do so by hand. The usb_hcd_poll_rh_status routine is quite new, and > > drivers should convert to use IRQs rather than polling for port changes. > > > > From looking through the code, I see that we need another facility added: > > usb_hcd_root_hub_lost_power. This would take over the duty of marking all > > the children devices as NOTATTACHED and simulating connect-change events > > on their ports, something which clearly belongs in usbcore and not in > > HCDs. > > OHCI relies on that "lost power" capability. It's always been a > fairly nasty interaction with usbcore and the hub driver. I seem > to recall making that work may have been the first substantial OHCI > patch that I ever submitted (2.3.x I think). I'd be glad to finally > see usbcore stop getting in the way of HCDs there. :) I'll write a patch for it soon. On Tue, 26 Apr 2005, Olav Kongas wrote: > The above scheme means that if a userspace app wants to > suspend the device via sysfs, it must first explicitly > suspend root hub. And if it wants to resume usb bus, it must > first resume device and then explicitly resume root hub. That's right. And it has to explicitly suspend/resume all the USB devices below the root hub, and all their interfaces (which are registered in sysfs as devices too). Eventually I hope there will be recursive suspend/resume procedures added to the driver-model core. Until then, the recursion should be handled by userspace. > Also, remote wakeups can result in resuming of root hub only > if device is not suspended. Was that correct? It depends. If the device supports some form of remote wakeup, then it might be possible to wake up both the device and the root hub. However at the moment Linux doesn't have good support for remote wakeup when the system as a whole isn't asleep. To make matters worse, device wakeup is very platform-specific. So for example, with PCI right now you are right. > I haven't fully understood the intended relationship between > device's and root hub's suspend/resume operations. The > reason why root hub's suspend/resume are currently called > from device's suspend/resume in isp116x-hcd was exactly to > avoid this explicit double suspending/resuming via sysfs. Don't forget the world outside of USB! As far as I know there aren't any other parts of the kernel where suspending/resuming a device will automatically suspend/resume the children/parent. Why should USB be any different? In any case, this sort of recursion belongs in the driver-model core rather than in the individual device drivers. > Thanks for explaining the plans about auto-suspend. I guess > auto-suspend, when implemented in USB core, won't depend on > CONFIG_PM? Actually it probably will. That might be a good reason to keep a minimal auto-suspend capability in the HCDs, for use when CONFIG_PM is off. Alan Stern ------------------------------------------------------- SF.Net email is sponsored by: Tell us your software development plans! Take this survey and enter to win a one-year sub to SourceForge.net Plus IDC's 2005 look-ahead and a copy of this survey Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
