Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck

[Marc Kleine-Budde]

On 02.09.2026 17:15:40, Claudiu Beznea wrote:
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index b7c4fe3477ac..f9173774387f 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c

[...]

>
>  /* Helper functions */
> @@ -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)) {
            ^^^^^^^^^^^^^^^^^^^^^

As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.

[1] https://lore.kernel.org/all/sashiko-outbox-173569@xxxxxxxxxx/

The netdev bot found more issues.
pw-bot: cr

regards,
Marc

--- Patch [15]: [PATCH net-next 15/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck ---
commit 4ef0042ccf34457229cdde398a008bebaab54e69
Author: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

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

This commit adds an fcan_pclk hardware info flag. With it set, the fCAN
clock falls back to the peripheral clock (fck) when the external can_clk
is unavailable, the rate is divided by info->postdiv and can_clk is set
to NULL. All existing rcar_canfd_hw_info instances now set fcan_pclk = 0
explicitly.

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

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index b7c4fe3477ac..f9173774387f 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 {

Discovered by: opus-5-5 · Missed by: gpt-6-astra, sashiko-gemini · Confirmed by: gpt-6-astra
Should the clock comments and the probe banner be updated for this new
fck source? Several places still describe only the external and canfd
clocks.

The comment just above this block in rcar_canfd_probe() still says:

	/* fCAN clock: Pick External clock. If not available fallback to
	 * CANFD clock
	 */

With fcan_pclk set, the fallback is fck (gpriv->clkp), not the canfd
clock.

In struct rcar_canfd_global, the field comments also don't cover this
case. Here can_clk is NULL and the fCAN source is clkp:

	struct clk *can_clk;		/* fCAN clock */
	...
	bool extclk;			/* CANFD or Ext clock */

The banner at the end of rcar_canfd_probe() wasn't changed either:

	dev_info(dev, "global operational state (%s clk, %s mode)\n",
		 gpriv->extclk ? "ext" : "canfd",

On this path extclk stays false. Won't the banner then say "canfd clk"
for a controller that has no canfd clock and runs from fck / postdiv?

The later "can: rcar_canfd: Add support for Renesas RZ/G3S" patch sets
.fcan_pclk = 1 in r9a08g045_hw_info. After that, an RZ/G3S board whose
can_clk is missing or has a rate of 0 would print this label.

Also, the new bitfield comment calls this clock pclk:

	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */

The commit message and the code call it fck. Could the comment use the
same name?

[ ... ]

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature