Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Dmitry Baryshkov]

On Tue, Jul 21, 2026 at 03:05:46PM +0200, Loic Poulain wrote:
> Runtime PM has to be enabled before creating the PHYs, since phy_create()
> only enables runtime PM on the PHY devices if it is already enabled on
> this parent device. This opens a small window where the device can be
> runtime suspended after pm_runtime_enable() and before the later
> pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
> the PHYs are not yet registered.
> 
> Take a runtime PM usage reference with pm_runtime_get_noresume() before
> enabling runtime PM and release it once the PHYs have been created to
> prevent the device from being runtime suspended during that window.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> +	/*
> +	 * Enable runtime PM before creating the PHYs, phy_create() only enables
> +	 * it on the PHY devices if already enabled on the parent. Hold a usage
> +	 * reference so callbacks cannot run until the PHY is ready.
> +	 */
> +	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  	/*
>  	 * Prevent runtime pm from being ON by default. Users can enable
>  	 * it using power/control in sysfs.
> @@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  
>  	ret = qmp_usbc_register_clocks(qmp, np);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  
>  	qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
>  	if (IS_ERR(qmp->usb_phy)) {
>  		ret = PTR_ERR(qmp->usb_phy);
>  		dev_err(dev, "failed to create PHY: %d\n", ret);
> -		goto err_node_put;
> +		goto err_pm_put;
>  	}
>  
>  	phy_set_drvdata(qmp->usb_phy, qmp);
> @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  		if (IS_ERR(qmp->dp_phy)) {
>  			ret = PTR_ERR(qmp->dp_phy);
>  			dev_err(dev, "failed to create PHY: %d\n", ret);
> -			goto err_node_put;
> +			goto err_pm_put;
>  		}
>  		phy_set_drvdata(qmp->dp_phy, qmp);
>  	}
>  
> +	pm_runtime_put(dev);

Should be _put_sync() ?

> +
>  	of_node_put(np);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  
> +err_pm_put:
> +	pm_runtime_put_noidle(dev);

Why is it _noidle()?

>  err_node_put:
>  	of_node_put(np);
>  	return ret;
> 
> -- 
> 2.34.1
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@xxxxxxxxxxxxxxxxxxx
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry