On Thu, 12 May 2005, David Brownell wrote: > > That's a fragile way of doing things. It requires the HCD, while avoiding > > actually calling usb_suspend_device, nevertheless to duplicate most of its > > functionality so that usb_resume_device can reverse things properly. > > The HCD parts of the functionality are in the hub_suspend() method. > Everything except (for now) turning off the timer... no duplication.
Sure... no duplication for the HCD parts. But what about everything else? Right now you're relying on inside knowledge about usb_suspend_device -- that the only thing it does beside actually calling hub_suspend is to suspend the hub driver (which you skip doing when you autosuspend, why?). What if usb_suspend_device changes in the future to include something new which usb_resume_device then expects to undo? You'd have to change the HCD's autosuspend routine to make it do the same thing. Otherwise usb_resume_device would get confused when it tried to undo something that hadn't been done. You're much better off simply not calling usb_resume_device whenever you don't call usb_suspend_device. > > Anyway, here's the problem as I understand it. OHCI (EHCI too?) needs a > > process context to resume, if not to suspend. > > Yes, and in fact the hub_resume() method is guaranteed to be called > in a task context. That's always true. In UHCI the hub_resume method calls wakeup_rh, which needs a process context iff it's not doing an autoresume. > > It does autosuspend without > > calling usb_suspend_device (which doesn't even exist if USB_SUSPEND isn't > > set). Hence it ought to autoresume without calling usb_resume_device. > > It could just call hub_resume() more directly ... As I suggested below. > > And here's a suggestion for a solution. Make usb_hcd_resume_root_hub > > conditional on CONFIG_PM rather than CONFIG_USB_SUSPEND (i.e., use your > > patch), and make khubd call the hub_resume method directly if > > root_hub->state != USB_STATE_SUSPENDED -- which will always be the case if > > !CONFIG_USB_SUSPEND. This means also changing OHCI so that it leaves the > > state equal to USB_STATE_CONFIGURED when autosuspending. How does that > > sound? > > Simplest to just _always_ call hub_resume(). After all, the HCD would > not have told usbcore to wake the hub up if it didn't need it ... :) No. I'm quite certain of this. If the root hub was suspended by usb_suspend_device then it should be awakened by usb_resume_device; if not then it should be awakened by calling hub_resume directly. Testing root_hub->state gives a simple way to tell how the root hub was suspended. > But again, the timer stuff comes up. Once the HCD has full control > over that stuff (again), that'll be no problem. You did test the original version of the new timer patch. If you still have those changes to the OHCI driver, it shouldn't be very hard to add code to disable interrupt generation in the IRQ handler and to reenable it in the new hub_irq_enable callback. Alan Stern ------------------------------------------------------- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
