Re: [PATCH 06/10] usb: chipidea: msm: keep the glue device runtime-active

[Peter Chen]

On 26-08-10 18:42:10, Dmitry Baryshkov wrote:
> The msm chipidea glue enables runtime PM with pm_runtime_no_callbacks()
> and nothing pinning it active. Right after a successful probe the
> driver core's pm_request_idle() runtime-suspends the glue (the ci core
> child has runtime PM disabled, so it does not hold its parent), and
> when the glue sits in a power domain - the HSIC instances live in the
> USB_HS_HSIC GDSC on msm8974 - genpd then powers the domain off
> underneath the running controller. The EHCI register file reads back
> zeroes, an onboard hub's connect is never seen, and subsequent bus
> accesses into the unpowered block wedge the interconnect, eventually
> taking the whole SoC down.

If you want runtime-pm support, you may set flag CI_HDRC_SUPPORTS_RUNTIME_PM,
The USB core (EHCI) would resume the controller (CI core).

> 
> Pin the glue runtime-active so the domain stays powered for the
> lifetime of the controller. Proper low-power handling can later be
> built on the child's runtime PM together with the HSIC wakeup
> interrupt.

If you would need HSIC wakeup interrupt, the CI MSM should support
runtime PM per my understanding.

Peter
> 
> Fixes: 2d0cdcc5a0e4 ("USB: gadget: Implement runtime PM for MSM bus glue driver")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> ---
>  drivers/usb/chipidea/ci_hdrc_msm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index e28ea87f9898..9a9410a814e5 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -276,7 +276,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  			dev_warn(&pdev->dev, "no wakeup interrupt: %d\n", ret);
>  	}
>  
> +	/*
> +	 * The glue owns the controller's power domain (the HSIC instances sit
> +	 * in a GDSC): if it runtime-suspends, the domain is powered off
> +	 * underneath the running controller. Pin it active.
> +	 */
>  	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_get_noresume(&pdev->dev);
>  	pm_runtime_no_callbacks(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> @@ -294,6 +300,7 @@ static void ci_hdrc_msm_remove(struct platform_device *pdev)
>  	struct ci_hdrc_msm *ci = platform_get_drvdata(pdev);
>  
>  	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
>  	dev_pm_clear_wake_irq(&ci->ci->dev);
>  	ci_hdrc_remove_device(ci->ci);
>  	clk_disable_unprepare(ci->iface_clk);
> 
> -- 
> 2.47.3
> 

-- 

Thanks,
Peter Chen