Re: [PATCH v4 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Log spam during boot due to lack of dev_err_probe() for deferred clocks.
- [Low] Unnecessary braces used for single-statement if/else branches.
--

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

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

This commit fixes probe deferral handling in the ti-pipe3 driver by returning
the actual error code for sysclk failures instead of masking it. It also
propagates non-ENOENT errors for the SATA refclk.

> 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
> @@ -605,14 +605,21 @@ 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");
> +	}

This isn't a bug, but is it necessary to use braces here? The Linux kernel
coding style typically recommends omitting braces for single-statement
if/else branches.

> +
>  	if (IS_ERR(phy->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->mode != PIPE3_MODE_SATA)
> -			return PTR_ERR(phy->refclk);
> +		return PTR_ERR(phy->refclk);
>  	}

Since PTR_ERR() now correctly propagates -EPROBE_DEFER when the clock is not
yet available, will this dev_err() call cause repetitive log spam regressions
during normal boot sequence deferrals?

Could this be replaced with dev_err_probe() to properly suppress the error
message when the clock provider is simply deferred?

[ ... ]

> @@ -629,7 +636,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");
> -			return -EINVAL;
> +			return PTR_ERR(phy->sys_clk);
>  		}
>  	}

Does the same log spam regression apply here now that sysclk errors,
including -EPROBE_DEFER, are correctly propagated?

Using dev_err_probe() here might also help keep the kernel log clean during
probe deferral.

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

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