Re: [PATCH v5 3/4] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch correctly fixes -EPROBE_DEFER propagation for `sysclk` and `refclk` but retains `dev_err()` for logging. This introduces dmesg spam because the probe will now be repeatedly deferred and retried, printing the error on every attempt.
--

commit 3f69854dd952e4e5aee73b24e6d27c10a31a39dc
Author: Hongling Zeng <zenghongling@kylinos.cn>

phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources

This commit fixes probe deferral handling by propagating the actual error
codes for sysclk and refclk instead of returning -EINVAL or ignoring errors
completely.

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index 2d36fe4c4218c..9ec228c2a940c 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -608,14 +608,20 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>  	struct clk *clk;
>  	struct device *dev = phy->dev;
>  
> -	phy->refclk = devm_clk_get(dev, "refclk");
> +	/*
> +	 * refclk is optional for SATA PHY to support older DTBs, but
> +	 * required for other modes. Use devm_clk_get_optional() for SATA
> +	 * which returns NULL for -ENOENT, allowing us to propagate all
> +	 * other errors including -EPROBE_DEFER.
> +	 */
> +	if (phy->mode == PIPE3_MODE_SATA)
> +		phy->refclk = devm_clk_get_optional(dev, "refclk");
> +	else
> +		phy->refclk = devm_clk_get(dev, "refclk");
> +
>  	if (IS_ERR(phy->refclk)) {
>  		dev_err(dev, "unable to get refclk\n");

Now that the code correctly propagates -EPROBE_DEFER, will this dev_err()
call cause dmesg spam on systems where the clock provider is deferred and
retried multiple times?

Would it be better to replace dev_err() with dev_err_probe() here to cleanly
suppress the error message on deferrals and record the deferral reason?

> -		/* older DTBs have missing refclk in SATA PHY
> -		 * so don't bail out in case of SATA PHY.
> -		 */
> -		if (phy->mode != PIPE3_MODE_SATA)
> -			return PTR_ERR(phy->refclk);
> +		return PTR_ERR(phy->refclk);
>  	}

[ ... ]

> @@ -632,7 +638,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>  		phy->sys_clk = devm_clk_get(dev, "sysclk");
>  		if (IS_ERR(phy->sys_clk)) {
>  			dev_err(dev, "unable to get sysclk\n");

Does this sysclk error handling also have the same logging issue when
returning -EPROBE_DEFER?

> -			return -EINVAL;
> +			return PTR_ERR(phy->sys_clk);
>  		}
>  	}

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

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