Re: 回复: Re: [PATCH v3] phy: ti: pipe3: Fix clock resource leak on probe errors

[Vinod Koul <vkoul@kernel.org>]

On 15-05-26, 13:14, 曾红玲 wrote:

Please send text only emails on the list!!!

> <div style="font-family:Microsoft YaHei;font-size:14px;color:#000000;" class="l_node_has_color"><div>Hi,</div>
> <div>Thanks for pointing out the probe deferral problems.</div>
> <div>Both issues you mentioned are valid:</div>
> <ul>
> <li>
> <div>sysclk should not convert <code>-EPROBE_DEFER</code>to <code>-EINVAL</code>.</div>
> </li>
> <li>
> <div>SATA refclk must not ignore <code>-EPROBE_DEFER</code>, although <code>-ENOENT</code>can still be tolerated for compatibility.</div>
> </li>
> </ul>
> <div>I have fix them together with the clock leak fix in .</div>
> <div>[PATCH 1/2 v3] phy: ti: pipe3: Fix clock resource leak on probe errors</div>
> <div>[PATCH 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources</div>
> <div>&nbsp;</div>
> <div>Appreciate the careful review.</div>
> <div>&nbsp;</div>
> <div>Regards,</div>
> <div>Hongling</div>
> <p>&nbsp;</p>
> <div id="signatureTop"></div>
> <div id="cs2c_re" style="margin-left: 0.5em; padding-left: 0.5em; border-left: 1px solid #cacaca; -webkit-user-modify: read-only; -moz-user-modify: read-only; -ms-user-modify: read-only; -o-user-modify: read-only;"><br><br><br>
> <div style="background-color: #f8f8f8; padding: 8px 10px; font-family: Microsoft YaHei; font-size: 14px; color: #000;"><strong>主&nbsp;&nbsp;&nbsp;题：</strong><span id="cs2c_subject">Re: [PATCH v3] phy: ti: pipe3: Fix clock resource leak on probe errors</span> <br><strong>日&nbsp;&nbsp;&nbsp;期：</strong><span id="cs2c_date">2026年05月15日10:32</span> <br><strong>发件人：</strong><span id="cs2c_from">sashiko-bot</span> <br><strong>收件人：</strong><span id="cs2c_to" style="word-break: break-all;">sashiko-bot</span> <br><strong>抄送人：</strong><span id="cs2c_to" style="word-break: break-all;">linux-phy,neil.armstrong,olteanv,vkoul</span></div>
> <br>
> <div id="cs2c_content">Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Masking of -EPROBE_DEFER to -EINVAL when fetching sysclk. - [High] Ignoring -EPROBE_DEFER for the SATA refclk. -- commit 4f2fbd627ee80455cac194a4ae1b559d48180075 Author: Hongling Zeng &lt;zenghongling@kylinos.cn&gt; phy: ti: pipe3: Fix clock resource leak on probe errors 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. &gt; diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c &gt; index b5543b5c674c8..58fbc3b278130 100644 &gt; --- a/drivers/phy/ti/phy-ti-pipe3.c &gt; +++ b/drivers/phy/ti/phy-ti-pipe3.c [ ... ] This isn't a regression introduced by this patch, but looking at ti_pipe3_get_clk() called earlier in the probe sequence: drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_get_clk() { ... if (phy-&gt;mode != PIPE3_MODE_PCIE || phy-&gt;phy_power_syscon) { phy-&gt;sys_clk = devm_clk_get(dev, "sysclk"); if (IS_ERR(phy-&gt;sys_clk)) { dev_err(dev, "unable to get sysclk\n"); return -EINVAL; } } ... } Does this code mask -EPROBE_DEFER to -EINVAL when fetching sysclk? If devm_clk_get() for the sysclk returns -EPROBE_DEFER, the function explicitly converts the error and returns -EINVAL. This completely breaks the lifecycle of probe deferral for the sysclk resource. Instead of retrying when the clock provider becomes available later in the boot process, the driver initialization fails permanently, rendering the PHY device unusable on systems with non-deterministic probe orders. Similarly, looking at the SATA refclk handling in the same function: drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_get_clk() { ... phy-&gt;refclk = devm_clk_get(dev, "refclk"); if (IS_ERR(phy-&gt;refclk)) { dev_err(dev, "unable to get refclk\n"); /* older DTBs have missing refclk in SATA PHY * so don't bail out in case of SATA PHY. */ if (phy-&gt;mode != PIPE3_MODE_SATA) return PTR_ERR(phy-&gt;refclk); } ... } Are errors returned by devm_clk_get() for the refclk ignored when the mode is PIPE3_MODE_SATA? While ignoring -ENOENT is intentionally done to support older DTBs missing the clock, indiscriminately ignoring -EPROBE_DEFER breaks the probe deferral mechanism. The driver will proceed immediately without waiting for the clock provider to probe, resulting in the SATA PHY being initialized without its required reference clock and subsequently failing to function properly. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260515021635.13444-1-zenghongling@kylinos.cn?part=1 &lt;/zenghongling@kylinos.cn&gt;</div>
> </div>
> <p>&nbsp;</p>
> <p>&nbsp;</p>
> <div id="signatureBom">
> <div>
> <p>&nbsp;</p>
> <br>
> <p style="color: #000; font-size: 16px;">---</p>
> <div id="cs2c_mail_sigature" style="color: #000; font-size: 16px; font-family: Microsoft YaHei;"></div>
> <p>&nbsp;</p>
> </div>
> </div></div>

-- 
~Vinod

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