Re: [PATCH net-next 11/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] can: rcar_canfd: silent override of -EPROBE_DEFER for can_clk
--

commit 5f73d63bd97b58e10882cb62531a376bbe264f50
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

can: rcar_canfd: Allow the CAN FD clock to be sourced from fck

This patch allows the Renesas RZ/G3S CAN FD clock to be sourced from either
the peripheral clock (fck) or an external clock (can_clk). It adjusts the
rcar_canfd_hw_info configurations and the probe function to support this
fallback routing.

Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
Link: https://patch.msgid.link/20260902141544.565763-4-claudiu.beznea+renesas@tuxon.dev

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index b7c4fe3477ac5..f9173774387fa 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c

[ ... ]

> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	 */
>  	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>  	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> -		gpriv->can_clk = devm_clk_get(dev, "canfd");
> -		if (IS_ERR(gpriv->can_clk))
> -			return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
> -					     "cannot get canfd clock\n");
> +		if (info->fcan_pclk) {
> +			fcan_freq = clk_get_rate(gpriv->clkp);
> +			gpriv->can_clk = NULL;

[Severity: Medium]
Does this unintentionally swallow -EPROBE_DEFER if the external clock
provider is simply not ready yet?

If devm_clk_get() for "can_clk" returns -EPROBE_DEFER, IS_ERR() will evaluate
to true. For devices where info->fcan_pclk is true, this error appears to be
silently discarded since gpriv->can_clk is set to NULL.

Could this cause the CAN controller to incorrectly bind to the peripheral
clock (clkp) with the wrong frequency, rather than deferring the probe to wait
for the intended external clock?

> +		} else {
> +			gpriv->can_clk = devm_clk_get(dev, "canfd");
> +			if (IS_ERR(gpriv->can_clk))
> +				return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
> +						     "cannot get canfd clock\n");
> +
> +			fcan_freq = clk_get_rate(gpriv->can_clk);
> +		}
>  
>  		/* CANFD clock may be further divided within the IP */
> -		fcan_freq = clk_get_rate(gpriv->can_clk) / info->postdiv;
> +		fcan_freq /= info->postdiv;
>  	} else {
>  		fcan_freq = clk_get_rate(gpriv->can_clk);
>  		gpriv->extclk = gpriv->info->external_clk;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009134323.64064-1-mkl@pengutronix.de?part=11