On Fri, 16 Sep 2005, David Brownell wrote: > > > The operative change being testing udev->parent. I think I prefer > > > the solution of having the hub suspend code call the root hub suspend > > > code, to make layering a bit more clear. > > > > Do you mean having the HCD suspend code call the root hub suspend code? > > No, I mean having the hub_suspend() code call bus->hub_suspend(), > as in the attached compiles-but-untested patch.
Okay, now I understand. > It could be followed with patches trimming code out of HCDs, although > depending on how defensively those paths are coded, that may not be > needed right away. Well, let's get this right first. > > Either way could work. I prefer to do it this way, though. > > It's certainly a good quick-fix for that issue. But it leaves an > inconsistency (actually, two of them) I'd rather do away with. > > With this patch, the device suspend logic _only_ deals with a device's > upstream USB link, when it has one (which root hubs don't). Which gives > a clean definition of USB_SUSPEND to use as a good stabilization target: > it enables (a) the USB suspend state, and (b) remote wakeup paths. I tend to think of the HC as the root hub's upstream connection, 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!) Likewise, resuming the root-hub device wouldn't work so long as the interface remained suspended -- whereas it would work with external hubs. However I like your clean definition of CONIFG_USB_SUSPEND, if you add the proviso that (a) applies only to physical, non-root, devices. > Or said differently: for HCDs, there'd be one set of suspend/resume > paths to test and debug. Not two. This will end up being true in any case. Perhaps it should have been all along, but we didn't think of it before. > The second inconsistency is that the hub driver should be like other > USB drivers, and have suspend/resume methods if just PM is defined. > We decided on that policy a while back, and "hub.c" is the only > driver I know about which still uses USB_SUSPEND for that not PM. Yes, that's clearly a mistake -- a holdover from the early days of the code. > So the bottom line is that we need even more code changing there ... but > are now "finally" in a position to have the changes structured in a way > that may not imply significant changes in every kernel release. (Crosses > his fingers...) I agree that we do seem to be approaching a final, stable, correct implementation, if only asymptotically! :-) 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
