Re: [PATCH net-next 07/12] can: rcar_canfd: Use DT clock names in the probe message

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In rcar_canfd_probe() (drivers/net/can/rcar/rcar_canfd.c), the
  new probe message works out the DT clock name from gpriv->extclk and…

--

can: rcar_canfd: Use DT clock names in the probe message

This commit changes the clock name in the final dev_info() of
rcar_canfd_probe(). It now prints DT binding clock names ("can_clk",
"fck" or "canfd") instead of "ext" or "canfd".

Link: https://patch.msgid.link/20261019121356.2421761-3-claudiu.beznea@kernel.org

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index bb2d03b75142a..4f5cdfa8211da 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -2312,7 +2312,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, gpriv);
>  	dev_info(dev, "global operational state (%s clk, %s mode)\n",
> -		 gpriv->extclk ? "ext" : "canfd",
> +		 gpriv->extclk ? "can_clk" : (info->fcan_pclk ? "fck" : "canfd"),

[Severity: Medium]
Is the right name printed when can_clk is in use on hardware where
external_clk is 0?

gpriv->extclk does not record which DT clock was picked. In the branch
that uses "can_clk", it is copied from the hw_info flag that controls
the GCFG DCS bit:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_probe() {
    ...
	} else {
		fcan_freq = clk_get_rate(gpriv->can_clk);
		gpriv->extclk = gpriv->info->external_clk;
	}
    ...
}

r9a09g047_hw_info has:

	.external_clk = 0,
	.fcan_pclk = 0,

RZ/G3E uses this hw_info, and so do RZ/V2N and RZ/V2H through the
r9a09g047 fallback compatible. Their device trees (r9a09g047.dtsi,
r9a09g056.dtsi and r9a09g057.dtsi) only list:

	clock-names = "fck", "ram_clk", "can_clk";

On these SoCs, devm_clk_get_optional(dev, "can_clk") returns a clock
with a non-zero rate, so the else branch runs. extclk then ends up
false, and the message prints "canfd clk".

None of those device trees has a "canfd" clock, so a successful probe on
these SoCs can only have used "can_clk".

The commit message says the names "match what users see in the device
tree". That does not seem to hold on these platforms. The same ternary
is also still there at the end of the series.

Could each branch that gets the clock set a const char * clock name
instead, rather than working it out from extclk and fcan_pclk?

>  		 gpriv->fdmode ? (gpriv->fd_only_mode ? "fd-only" : "fd") : "classical");
>  	return 0;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110115112.3807247-2-mkl%40pengutronix.de