On 12.5.2020 7.03, Peter Chen wrote:
On 5/12/2020 8:05 AM, Peter Chen wrote:
Cc: Baolin Wang baolin.wang@linaro.org Cc: Mathias Nyman mathias.nyman@linux.intel.com Cc: stable@vger.kernel.org Fixes: b0c69b4bace3 ("usb: host: plat: Enable xHCI plat runtime PM") Reviewed-by: Peter Chen peter.chen@nxp.com Signed-off-by: Li Jun jun.li@nxp.com --- drivers/usb/host/xhci-plat.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 1d4f6f85f0fe..f38d53528c96 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -362,6 +362,7 @@ static int xhci_plat_remove(struct platform_device *dev) struct clk *reg_clk = xhci->reg_clk; struct usb_hcd *shared_hcd = xhci->shared_hcd;
- pm_runtime_get_sync(&dev->dev);
Where is corresponding _put() call?
At the end of this function, there is a pm_runtime_set_suspended(&dev->dev). Calling pm_runtime_put_sync(&dev->dev) will cause xhci_plat_runtime_suspend is called which is not expected.
Peter
This seems odd, I don't understand why pm_runtime_set_suspended() would call pm_runtime_put_sync()? I thought pm_runtime_set_suspended() is used to let pm core know the hardware is suspended, and inform the parent of this, possibly allowing parent to runtime suspend. Not sure if it's needed in the remove function. It makes sense to allow the parent to suspend, but xhci isn't really suspended. Yes is stopped, but we can't resume our way back to a active state.
Could it be that the runtime suspend call you are seeing is because we are removing all child devices, disconnecting and freeing everything, including roothubs and hcd's. Maybe runtime pm should be disabled a bit earlier.
Currently probe and remove looks like:
xhci_plat_probe() pm_runtime_set_active() pm_runtime_enable() pm_runtime_get_noresume() ... pm_runtime_put_noidle() pm_runtime_forbid()
xhci_plat_remove() <remove and put both hcd's> pm_runtime_set_suspended() pm_runtime_disable()
Would it make sense to change xhci_plat_remove() to
xhci_plat_remove() pm_runtime_disable() <remove and put both hcd's> pm_runtime_set_suspended()
or possibly wrapping the remove in a runtime get/put:
xhci_plat_remove() pm_runtime_get_noresume() pm_runtime_disable() <remove and put both hcd's> pm_runtime_set_suspended() pm_runtime_put_noidle()
-Mathias