On Sat, 2 Mar 2013, Felipe Balbi wrote: > > > @@ -174,6 +177,10 @@ static int xhci_plat_remove(struct platform_device *dev) > > > struct usb_hcd *hcd = platform_get_drvdata(dev); > > > struct xhci_hcd *xhci = hcd_to_xhci(hcd); > > > > > > + if (!pm_runtime_suspended(&dev->dev)) > > > + pm_runtime_put(&dev->dev); > > > + pm_runtime_disable(&dev->dev); > > > + > > > usb_remove_hcd(xhci->shared_hcd); > > > usb_put_hcd(xhci->shared_hcd); > > > > This is very strange. Why have a pm_runtime_put with no balancing > > pm_runtime_get? > > this is good point and, in fact, a doubt I have myself. How are we > supposed to check if device is suspended ? In case it _is_ suspended we > might not be able to read device's registers due to clocks possibly > being gated. That's really a separate question. It has a simple answer, though: If you want to access a device's registers, call pm_runtime_get_sync() beforehand and pm_runtime_put() (or _put_sync()) afterward. Then it won't matter if the device was suspended originally. If you actually do want to tell whether or not a device is suspended and nothing more, call pm_runtime_status_suspended(). Of course, this is racy -- the power state might change right after you make the call. > Also, considering that some drivers are used in multiple platforms and > those might behave differently when it comes to clock handling, how do > we do that ? Should we require drivers to explicitly clk_get(); > clk_prepare_enable(); pm_runtime_set_active(); pm_runtime_enable() ? I don't know much about clock handling. In general, the pm_runtime_set_active() and pm_runtime_enable() parts should be done by the subsystem, not the driver, whenever possible. > While that's doable, I don't see how that'd be doable for OMAP since > they want to hide clock handling from drivers. > > Any tips ? Whichever piece of code is responsible for associating a clock with a device should also be responsible for making sure that neither is suspended when the driver's probe routine runs. I'm not sure how different platforms do this; using a PM domain could be one answer. All this is somewhat off the point of my original comment, however. Drivers must be sure to balance their pm_runtime_get() and _put() calls. Having an unbalanced _put() in the remove routine is almost certainly a mistake -- especially if it is conditional on the device's power state, because a device can remain unsuspended even after the driver does a pm_runtime_put(). For example, this will happen if the user wrote "on" to /sys/.../power/control. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
