Re: [PATCH v2] phy: freescale: phy-fsl-imx8qm-lvds-phy: Fix missing pm_runtime_disable() on probe error path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pending asynchronous suspend scheduled by `pm_runtime_put()` can be cancelled by `pm_runtime_disable()` during probe failure, leaving the hardware permanently in an active power state.
--

commit 8ed1a4a5b60301f95aa6700ff6d900f3ad7141f0
Author: Felix Gu <ustc.gu@gmail.com>

phy: freescale: phy-fsl-imx8qm-lvds-phy: Fix missing pm_runtime_disable() on probe error path

This commit replaces manual pm_runtime_disable() calls with
devm_pm_runtime_enable() to automatically clean up runtime PM during probe
failures or unbinds.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
> index ece3574435217..c662f91e598c7 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c

[ ... ]

> @@ -355,17 +357,15 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < PHY_NUM; i++) {
>  		lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy), GFP_KERNEL);
> -		if (!lvds_phy) {
> -			ret = -ENOMEM;
> -			goto err;
> -		}
> +		if (!lvds_phy)
> +			return -ENOMEM;
>  
>  		phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
>  		if (IS_ERR(phy)) {
>  			ret = PTR_ERR(phy);
>  			dev_err(dev, "failed to create PHY for channel%d: %d\n",
>  				i, ret);
> -			goto err;
> +			return ret;

[Severity: High]
This is a pre-existing issue, but will these error paths leave the hardware
powered on indefinitely if initialization fails?

Just prior to this loop, mixel_lvds_phy_probe() calls mixel_lvds_phy_reset(),
which finishes by queuing an asynchronous suspend:

drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c:mixel_lvds_phy_reset() {
	...
	regmap_write(priv->regmap, PHY_CTRL, CTRL_RESET_VAL);

	pm_runtime_put(dev);

	return 0;
}

If a subsequent initialization step fails and returns an error, the devres
cleanup will immediately execute pm_runtime_disable() (which was also the
case previously with the goto err label).

Because pm_runtime_disable() calls __pm_runtime_barrier(), it cancels any
pending asynchronous idle or suspend work. This means the runtime suspend
callback might never execute, leaving the PHY hardware fully powered up.

Should mixel_lvds_phy_reset() use pm_runtime_put_sync() instead to
guarantee the device is properly suspended before a potential error
disables runtime PM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-lvds-v2-1-3ce7539d1104@gmail.com?part=1

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