David:
Ownership of hcd->state is unclear. In hcd-pci.c, I see the following:
See also the patch I just posted, which includes part of what BenH sent by recently along with changes based on your comments.
That state machine does still need some cleanup.
usb_hcd_pci_probe() calls driver->reset() but doesn't set hcd->state. It later calls driver->start() but still doesn't set hcd->state.
It's behaving correctly for now because driver->hcd_alloc() returns zeroed memory, and HALTED is zero, and that step "never" gets errors anyway. (Patch explicitly sets to HALTED.)
usb_hcd_pci_remove() sets hcd->state to QUIESCING -- even if it was previously equal to HALT(!) -- and calls driver->stop(). Lastly it sets hcd->state to HALT.
This updated patch only changes state there if the hcd's running. State is set to HALT at the end purely out of paranoia; everything's about to be freed, anyway.
usb_hcd_pci_suspend() checks that hcd->state isn't already SUSPENDED or HALT. If not, it calls driver->suspend() and then sets hcd->state to SUSPENDED.
That does sort of "know" that the only non-running states are SUSPENDED and HALT, yes ... and also, that the request isn't going to be vetoed.
I can understand wanting either the glue or the hcd to be able to veto suspending. Does vetoing work yet? I can't suspend at all, because of at least the IDE problems.
usb_hcd_pci_resume() checks that hcd->state is SUSPENDED, sets
hcd->state to RESUMING, calls driver->resume(), and then checks that hcd->state is either RUNNING or READY.
The latest stuff changes it to just a 'not running' test at the end.
So who is responsible for setting hcd->state? Sometimes the glue layer does it and sometimes it doesn't. To work properly, the driver's start() and resume() methods have to set it to RUNNING, the reset() method should set it to HALT, and the suspend(), and stop() methods don't have to set it at all.
Sounds right. The start() and resume() rules are right; that's what's most likely to fail. We're optimistic about reset() for now, since we don't have an "unknown" state ... and stop() is for bettor worse a "can't fail".
When we veto suspends, then the reset() call should work a bit differently. But we're not there yet.
Also, what does USB_STATE_READY mean? It doesn't appear anywhere in the code AFAICT, only in hcd.h.
Think of it as a nuance of USB_STATE_RUNNING, turned out to be of no value. (Removed in that patch.)
And why doesn't pci_suspend() set hcd->state to QUIESCING during the time that driver->suspend() is underway?
Bug -- this was also fixed in Ben's patch.
- Dave
Alan Stern
------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
