Re: [PATCH 1/2 v3] phy: ti: pipe3: Fix clock resource leak on probe errors

[Vladimir Oltean]

On Fri, May 15, 2026 at 12:05:22PM +0800, Hongling Zeng wrote:
> 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@xxxxxxxxxx>
> 

Your patch numbering broke Patchwork. Please do not name patches 1/2 if
2/2 is not in the same thread. Neither this patch nor "[2/2] phy:
ti-pipe3: Fix EPROBE_DEFER handling for clock resources" can be applied.

Also, wasn't v3 this? https://patchwork.kernel.org/project/linux-phy/patch/20260515021635.13444-1-zenghongling@xxxxxxxxxx/
shouldn't this be v4?

pw-bot: cr

> ---
>  Change in v2:
>   -Add pm_runtime_disable() in error path (reported by Sashiko AI).
> ---
>  change in v3:
>   -Fix unbalanced clock disable by checking clk_prepare_enable()return value and
> setting sata_refclk_enabledonly on success.
>   -Fix error path teardown order to match ti_pipe3_remove()(disable clock before
> disabling runtime PM).
> ---
>  drivers/phy/ti/phy-ti-pipe3.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c..58fbc3b27813 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -831,21 +831,39 @@ static int ti_pipe3_probe(struct platform_device *pdev)
>  	 */
>  	if (phy->mode == PIPE3_MODE_SATA) {
>  		if (!IS_ERR(phy->refclk)) {
> -			clk_prepare_enable(phy->refclk);
> +			ret = clk_prepare_enable(phy->refclk);
> +			if (ret) {
> +				dev_err(dev, "Failed to enable refclk %d\n", ret);
> +				goto err_pm_disable;
> +			}
>  			phy->sata_refclk_enabled = true;
>  		}
>  	}
>  
>  	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:
> +	if (phy->sata_refclk_enabled && !IS_ERR(phy->refclk))
> +		clk_disable_unprepare(phy->refclk);
> +err_pm_disable:
> +	pm_runtime_disable(dev);
> +	return ret;
>  }
>  
>  static void ti_pipe3_remove(struct platform_device *pdev)
> -- 
> 2.25.1
> 
>