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

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch adds a third fCAN clock source in rcar_canfd_probe().

--

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

This commit adds a new fcan_pclk hardware feature bit to struct
rcar_canfd_hw_info. On the RZ/G3S, fCAN can then come from the peripheral
clock (fck) when no usable external can_clk is present. Existing hw_info
instances are updated to set fcan_pclk = 0 explicitly.

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: Low]
This isn't a bug, but should the comment on the extclk field in struct
rcar_canfd_global be updated as well? It still describes a two-way choice:

    bool extclk;			/* CANFD or Ext clock */

With this new branch, extclk stays false while the fCAN source is fck,
not the "canfd" clock. So on fcan_pclk SoCs, extclk == false no longer
means "CANFD clock".

Later patches in the series update the fallback comment above this block
in rcar_canfd_probe(). The commit "can: rcar_canfd: Use DT clock names in
the probe message" also updates the dev_info() string to report "fck".
The extclk field comment is still unchanged at the end of the series,
though.

> +		} 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://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110115112.3807247-2-mkl%40pengutronix.de