Re: [PATCH net-next 15/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: incorrect EPROBE_DEFER handling for can_clk
--

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

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

This commit adjusts the CAN FD clock handling for the Renesas RZ/G3S to allow
sourcing from either the peripheral clock (fck) or an external clock
(can_clk). It adds a new flag fcan_pclk to the hardware info structures to
support this configuration.

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;
> +		} else {
> +			gpriv->can_clk = devm_clk_get(dev, "canfd");

[Severity: Medium]
In rcar_canfd_probe() in drivers/net/can/rcar/rcar_canfd.c, if the can_clk
provider is not yet probed, devm_clk_get() will return -EPROBE_DEFER. Does
this check incorrectly catch the probe deferral and route it to the fallback
paths instead of propagating it to the driver core?

If info->fcan_pclk is true, it appears the driver will silently swallow
the deferral and fall back to using clkp. If info->fcan_pclk is false, the
driver attempts to fetch the canfd clock and might fail with -ENOENT,
permanently failing the probe rather than waiting for the external clock to
become available.

Would it be better to use devm_clk_get_optional() here, or explicitly check
for -EPROBE_DEFER before falling back to the secondary clock sources?

> +			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);
> +		}
[ ... ]

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