[PATCH v2] phy: ti: pipe3: Fix clock resource leak on probe errors

[PATCH v2] phy: ti: pipe3: Fix clock resource leak on probe errors

[Hongling Zeng]

When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.

Fix this by adding an error handling path to disable the clock
when these functions fail.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v2:
  -Add pm_runtime_disable() in error path (reported by Sashiko AI).
---
 drivers/phy/ti/phy-ti-pipe3.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..5f4a81f6d2b9 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -837,15 +837,28 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	}
 
 	generic_phy = devm_phy_create(dev, NULL, &ops);
-	if (IS_ERR(generic_phy))
-		return PTR_ERR(generic_phy);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		goto err_clk_disable;
+	}
 
 	phy_set_drvdata(generic_phy, phy);
 
 	ti_pipe3_power_off(generic_phy);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	return PTR_ERR_OR_ZERO(phy_provider);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	pm_runtime_disable(dev);
+	if (phy->sata_refclk_enabled)
+		clk_disable_unprepare(phy->refclk);
+	return ret;
 }
 
 static void ti_pipe3_remove(struct platform_device *pdev)
-- 
2.25.1


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

Re: [PATCH v2] phy: ti: pipe3: Fix clock resource leak on probe errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unchecked clk_prepare_enable() return value leads to unbalanced clock disable.
- [Medium] Inverted resource teardown order in the error path.
--

commit 34a06b14102bc841b336d48ee9651b777cd39eb7
Author: Hongling Zeng <zenghongling@kylinos.cn>

phy: ti: pipe3: Fix clock resource leak on probe errors

When devm_phy_create() or devm_of_phy_provider_register() fails,
this commit adds an error handling path to disable the refclk that
was enabled earlier, preventing a resource leak.

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -837,15 +837,28 @@ static int ti_pipe3_probe(struct platform_device *pdev)

[ ... ]

> +err_clk_disable:
> +	pm_runtime_disable(dev);
> +	if (phy->sata_refclk_enabled)
> +		clk_disable_unprepare(phy->refclk);
> +	return ret;
>  }

Is the teardown order in this new error path correct?

During probe, pm_runtime_enable() is called before clk_prepare_enable().
Should clk_disable_unprepare() be called before pm_runtime_disable() to
follow reverse-initialization order, similar to the existing logic in
ti_pipe3_remove()?

Additionally, looking at the clock initialization earlier in
ti_pipe3_probe():

        if (phy->mode == PIPE3_MODE_SATA) {
                if (!IS_ERR(phy->refclk)) {
                        clk_prepare_enable(phy->refclk);
                        phy->sata_refclk_enabled = true;
                }
        }

The return value of clk_prepare_enable() is not checked, and
phy->sata_refclk_enabled is set to true regardless of whether the clock
was successfully enabled.

If clk_prepare_enable() fails, and a subsequent step like
devm_phy_create() also fails, the code will jump out to err_clk_disable
and call clk_disable_unprepare() because phy->sata_refclk_enabled is
true.

Does this introduce a regression by causing an unbalanced clock disable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514023424.23815-1-zenghongling@kylinos.cn?part=1

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