Re: [PATCH v6 4/4] phy: ti-pipe3: Fix clock leak in init error path

[Hongling Zeng <zhongling0719@126.com>]

在 2026年09月09日 15:58, Manivannan Sadhasivam 写道:
> On Fri, Jun 19, 2026 at 11:02:14AM +0800, Hongling Zeng wrote:
>> When regmap_update_bits() fails in ti_pipe3_init() for PCIe mode,
>> the function returns the error without calling ti_pipe3_disable_clocks().
>> This leaves the clocks permanently enabled since the PHY framework won't
>> invoke the .exit callback on init failure.
>>
>> Fix this by adding proper clock cleanup in the PCIe error path, consistent
>> with how the DPLL program error path handles cleanup.
>>
>> Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
>> Reported-by: Sashiko AI <sashiko@kernel.org>
>> Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> Cc: stable@vger.kernel.org
>>
>> ---
>> Change in v5:
>>    -Add Fix ignored clock enable return value in init patch
>> ---
>> Change in v6:
>>   -Fix all clock leak paths comprehensively:
>>   -PCIe syscon update failure path
>>   -SATA DPLL lock check path
>>   -SATA errata path in ti_pipe3_exit()
>> ---
>>   drivers/phy/ti/phy-ti-pipe3.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
>> index 9ec228c2a940..860058f31594 100644
>> --- a/drivers/phy/ti/phy-ti-pipe3.c
>> +++ b/drivers/phy/ti/phy-ti-pipe3.c
>> @@ -518,6 +518,8 @@ static int ti_pipe3_init(struct phy *x)
>>   		val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
>>   		ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
>>   					 PCIE_PCS_MASK, val);
>> +		if (ret)
>> +			ti_pipe3_disable_clocks(phy);
>>   		return ret;
>>   	}
>>   
>> @@ -531,8 +533,9 @@ static int ti_pipe3_init(struct phy *x)
>>   
>>   	/* SATA has issues if re-programmed when locked */
>>   	val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
>> -	if ((val & PLL_LOCK) && phy->mode == PIPE3_MODE_SATA)
>> -		return ret;
>> +	if ((val & PLL_LOCK) && phy->mode == PIPE3_MODE_SATA) {
>> +		return 0;
> Why are you returning 0 here and below?
>
> - Mani
>
Hi Mani,

Thanks for pointing this out.

You're right. Changing the SATA PLL-locked path from `return ret` to
`return 0` was an unintended change and is not related to the clock leak
fix. I'll restore the original code in the next version:

     if ((val & PLL_LOCK) && phy->mode == PIPE3_MODE_SATA)
         return ret;

Thanks,
Hongling