> There are a few things in here I don't like. > > > * PM updates for usbcore. This is the stuff that simplifies > > things by making CONFIG_USB_SUSPEND mean a lot less. > > > > - pm-root1.patch ... a handful of PM updates that don't > > directly affect much else; e.g. a new method that > > can make OHCI autosuspend work again. > > > > - pm-root2.patch ... gets rid of lots of special cases > > and merges most of the USB_SUSPEND logic in > > with normal PM support. > > Mostly in these, although maybe in a few of the others as well.
Those are in fact the guts of the fixes, notably pm-root2.patch ... > The main thing is moving hcd->hub_suspend into hub.c:hub_suspend. I > firmly believe that the model you're using for root hubs isn't as logical > or as consistent with external hubs as the one I outlined. Seems plenty logical to me: hub suspend code handles hub suspend, whether or not it's a root hub. Ditto with resume code. The externally testable invariants then become the same in both cases, which certainly improves consistency ... In fact doing it the other way has always bothered me as being internally inconsistent. The only excuse for taking that route in the first place was that the ONLY starting point we ever had for this was some not-very-functional PCI suspend/resume support in some HCDs. Code that clearly didn't help non-PCI drivers, and didn't even begin to touch on selective suspend and wakeup. So it's no surprise that it kept piling on the special cases. > Another thing is the way you're doing autosuspend in OHCI (and probably > EHCI as well -- I haven't checked). There's nothing new about this, but > these patches just seem to move farther down the wrong path. That's just making things work again the way they used to work. Some of the recent usbcore patches broke OHCI resume horribly ... EHCI too, though it doesn't do that sort of autosuspend. If by "wrong path" you mean "not the khubd-driven approach that's been slightly discussed" ... well, we don't have any such code yet, and it's not OK to leave OHCI broken until we do. > Also, what goes wrong when you try to use the FREEZE/SUSPEND distinction? > You mentioned only that it doesn't work. Consider that a pure FREEZE doesn't go through all the gyrations to shut down the hardware. So its in-memory state doesn't reflect any such gyrations ... that's the _entire_ point of using FREEZE instead of suspend: avoiding extra work, just idling the hardware instead of shutting it down like the SUSPEND state(s) do. But that means swusp will write garbage state out to disk, with device state marked as "frozen". Then the resume paths will restore that garbage. Unhealthy. I've commented before that the whole swsusp "freeze" thing seemed to be not-well-thought-out, and ill-defined. I had hoped not to be proven right on that; in vain, it seems. > For the sake of making forward progress, I won't object to having these > patches applied. But there are a few parts I will want to alter or change > back. There will things that need to change a bit; there are cleanups I've not sent yet, for example. And I expect the usual sort of glitches to appear. But when you say "change back", that fires up my trouble radar. Things are working sanely now in all my tests, so "change back" implies that some now-finally-fixed bugs will reappear ... - Dave ------------------------------------------------------- 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 _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
