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

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

[Felix Gu]

If mixel_lvds_phy_reset() fails in probe after pm_runtime_enable(),
the function returns directly without calling pm_runtime_disable(),
leaving runtime PM permanently enabled for the device.

Fix this by using devm_pm_runtime_enable() so that cleanup is
automatic on any probe failure or driver unbind. This also allows
removing the manual err label and the .remove callback.

Fixes: 06ff622d61d2 ("phy: freescale: Add i.MX8qm Mixel LVDS PHY support")
Acked-by: Liu Ying <victor.liu@nxp.com>
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Changes in v2:
- Change the patch's subject prefix requested by Liu Ying.
- Add Liu Ying's Acked-by.
- Link to v1: https://patch.msgid.link/20260604-lvds-v1-1-b8e1ff1bdee7@gmail.com

To: Vinod Koul <vkoul@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Frank Li <Frank.Li@nxp.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: Liu Ying <victor.liu@nxp.com>
Cc: linux-phy@lists.infradead.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
index ece357443521..c662f91e598c 100644
--- a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
+++ b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
@@ -345,7 +345,9 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, priv);
 
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
 
 	ret = mixel_lvds_phy_reset(dev);
 	if (ret) {
@@ -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;
 		}
 
 		lvds_phy->phy = phy;
@@ -379,19 +379,10 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider)) {
 		ret = PTR_ERR(phy_provider);
 		dev_err(dev, "failed to register PHY provider: %d\n", ret);
-		goto err;
+		return ret;
 	}
 
 	return 0;
-err:
-	pm_runtime_disable(dev);
-
-	return ret;
-}
-
-static void mixel_lvds_phy_remove(struct platform_device *pdev)
-{
-	pm_runtime_disable(&pdev->dev);
 }
 
 static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct device *dev)
@@ -432,7 +423,6 @@ MODULE_DEVICE_TABLE(of, mixel_lvds_phy_of_match);
 
 static struct platform_driver mixel_lvds_phy_driver = {
 	.probe = mixel_lvds_phy_probe,
-	.remove = mixel_lvds_phy_remove,
 	.driver = {
 		.pm = &mixel_lvds_phy_pm_ops,
 		.name = "mixel-lvds-phy",

---
base-commit: a225caacc36546a09586e3ece36c0313146e7da9
change-id: 20260604-lvds-d67cb619df17

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>


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

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

[sashiko-bot]

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

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

[Frank Li]

On Fri, Jun 05, 2026 at 07:57:20PM +0800, Felix Gu wrote:
> If mixel_lvds_phy_reset() fails in probe after pm_runtime_enable(),
> the function returns directly without calling pm_runtime_disable(),
> leaving runtime PM permanently enabled for the device.
>
> Fix this by using devm_pm_runtime_enable() so that cleanup is
> automatic on any probe failure or driver unbind. This also allows
> removing the manual err label and the .remove callback.
>
> Fixes: 06ff622d61d2 ("phy: freescale: Add i.MX8qm Mixel LVDS PHY support")
> Acked-by: Liu Ying <victor.liu@nxp.com>
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
> - Change the patch's subject prefix requested by Liu Ying.
> - Add Liu Ying's Acked-by.
> - Link to v1: https://patch.msgid.link/20260604-lvds-v1-1-b8e1ff1bdee7@gmail.com
>
> To: Vinod Koul <vkoul@kernel.org>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Frank Li <Frank.Li@nxp.com>
> To: Sascha Hauer <s.hauer@pengutronix.de>
> To: Pengutronix Kernel Team <kernel@pengutronix.de>
> To: Fabio Estevam <festevam@gmail.com>
> To: Liu Ying <victor.liu@nxp.com>
> Cc: linux-phy@lists.infradead.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
> index ece357443521..c662f91e598c 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
> @@ -345,7 +345,9 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
>
>  	dev_set_drvdata(dev, priv);
>
> -	pm_runtime_enable(dev);
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
>
>  	ret = mixel_lvds_phy_reset(dev);
>  	if (ret) {
> @@ -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;
>  		}
>
>  		lvds_phy->phy = phy;
> @@ -379,19 +379,10 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(phy_provider)) {
>  		ret = PTR_ERR(phy_provider);
>  		dev_err(dev, "failed to register PHY provider: %d\n", ret);
> -		goto err;
> +		return ret;
>  	}
>
>  	return 0;
> -err:
> -	pm_runtime_disable(dev);
> -
> -	return ret;
> -}
> -
> -static void mixel_lvds_phy_remove(struct platform_device *pdev)
> -{
> -	pm_runtime_disable(&pdev->dev);
>  }
>
>  static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct device *dev)
> @@ -432,7 +423,6 @@ MODULE_DEVICE_TABLE(of, mixel_lvds_phy_of_match);
>
>  static struct platform_driver mixel_lvds_phy_driver = {
>  	.probe = mixel_lvds_phy_probe,
> -	.remove = mixel_lvds_phy_remove,
>  	.driver = {
>  		.pm = &mixel_lvds_phy_pm_ops,
>  		.name = "mixel-lvds-phy",
>
> ---
> base-commit: a225caacc36546a09586e3ece36c0313146e7da9
> change-id: 20260604-lvds-d67cb619df17
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
>

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