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

[sashiko-bot@kernel.org]

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