Re: [PATCH net-next 15/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-10-16 02:37:08
Also in:
linux-can
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] The patch adds a third fCAN source (fck/pclk, used when `info->fcan_pclk` is set), but the clock comments and the boot banner… -- 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@wanadoo.fr (local) Link: https://patch.msgid.link/20260902141544.565763-4-claudiu.beznea+renesas@tuxon.dev (local)
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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 {
[Severity: Low] 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? [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015015336.816543-2-mkl%40pengutronix.de