> > Also, what you wrote is a little mixed up. Normally usb_suspend_device > > calls driver->suspend, not the other way around. > > No. usb_generic_suspend() does it. (the PM core callback in the bus_type > structure). It calls usb_suspend_device() when there is no driver (or > rather, the generic driver) or calls the driver's->suspend() when there > is a driver.
Remember that "usb_device" suspend is managed only by usbcore, and drivers themselves connect to a "usb_interface" of which several generally exist. Don't even _think_ of usb_suspend_device() as anything more than an internal "helper" routine. Yes, called by the "generic" suspend routine that understands the difference between the two types of usb devie nodes: usb_device, usb_interface. > What usb_suspend_device() does according to my understanding of the code > in hub.c is to call hub_port_suspend() for the parent hub, in order to > suspend the port to which the device is connected. > > That leads to 2 things unless I missed something: You did miss a few things here. > - If a device has a driver, it should call usb_suspend_device() itself > from it's suspend() routine or the hub port won't be suspended (and thus > the device). Or did I miss another code path ? No, there's a missing "autosuspend" mechanism. That's what would suspend USB devices after all of their interfaces are suspended (possibly because they have no driver) ... as part of runtime PM, rather than as part of system-wide sleep state transitions. > - If a device wants to suspend itself That concept is nonsense. A driver may choose to suspend its device. And for USB, the drivers are bound to usb_interface objects, not to usb_device objects. So drivers suspend usb_interface->dev ... and usbcore is then responsible for suspending usb_device->dev, not the driver for one of the device's interfaces. > (that is suspend the hub port > it's connected to, whatever it is, a hub or a root hub) at any time for > local power management (for example, usb storage could do that if no > request have been issued for a while, usb serial could do that when no > port is openeed, etc etc ...), it should call usb_suspend_device(). No, never. Drivers work with their interfaces, and that's it. There are a couple exceptions, notably anything calling usb_reset_device() (for DFU firmware updates or for usb-storage fault recovery), but we're trying to steer drivers away from such exceptions. > However, in order to get the locking right, usb_suspend_device() should > be called with the PM core semaphore held, right ? So according to your > own explanation previously, the right way to do it is for the driver to > call dpm_device_suspend() on itself, The driver could call that on the device object inside its usb_interface. That ought to work. But it won't suspend the parent usb_device yet, because there isn't an autosuspend framework yet. > which in turn causes > driver->suspend() to be called with the PM sem held, which can itself > call usb_suspend_device() as I wrote in my previous email. > > Now, please, let me know if I missed something in the code, I may well > have missed some subtle device driver vs. interface driver issue or > something similar here ... You missed some of those, as well as the missing "autosuspend" logic. Since that logic is missing, it's especially easy to overlook. ;) > > Drivers for USB devices? Okay. To suspend a USB device, you need to call > > dpm_runtime_suspend for each of the device's interface, and then for the > > device itself. > > Ok, so I think my walking though the code missed some subtlety with > device vs. interfaces. However, still, the point is, the only thing that > ever calls usb_suspend_device() (in order to suspend the hub port) is > usb_generic_suspend() and only if there is no driver (that is the > generic driver). Thus I suppose if there is a driver attached, that > driver should be responsible for calling usb_generic_suspend(), right ? No, that's part of the interface-vs-device thing you missed. Strict hierarchy. > > When the infrastructure is in place, it will automatically take care of > > suspending the device once all the interfaces are suspended. For now, > > however, that's your responsibility. I'd say it differently: For now don't try that at all. Because in addition to an autosuspend mechanism being missing, so is autoresume. - Dave ------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://ads.osdn.com/?ad_id=7637&alloc_id=16865&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
