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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Asynchronous pm_runtime_put() before devm_of_phy_provider_register() can cause an async suspend race if registration fails.
--

--- Patch [2]: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot ---
commit 4e67cff2d4e3ab76976662b7c3a8f55faa25f2bd
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

This commit takes a runtime PM usage reference with pm_runtime_get_noresume()
before enabling runtime PM. It then releases the reference once the PHYs
have been created to prevent the device from being runtime suspended during
the creation window.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index ab3055bb5b0c1..4317224070fd8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c

[ ... ]

> @@ -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);
> +
>  	of_node_put(np);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);

[Severity: High]
Since pm_runtime_put() queues an asynchronous idle notification, is there a
race condition here if devm_of_phy_provider_register() fails?

If devm_of_phy_provider_register() returns an error, the function returns
immediately and triggers devm unwinding. This unwinding will call
pm_runtime_disable() due to the earlier devm_pm_runtime_enable() call.

If pm_runtime_disable() cancels the pending asynchronous idle/suspend work,
could the hardware be left in an incorrect active power state?

Would it be safer to move devm_of_phy_provider_register() before
pm_runtime_put(dev), and use the err_pm_put label (which safely uses the
synchronous pm_runtime_put_noidle()) if it fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy