Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
From: Claudiu Beznea
Date: Fri Oct 16 2026 - 14:19:47 EST
On 10/16/26 16:39, Marc Kleine-Budde wrote:
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.
This looks to me a pre-existing issue.
The newly added support does:
fcan_freq = clk_get_rate(gpriv->clkp);
At the time this is executed the clkp was already checked against any errors (above) including -EPROBE_DEFER.
Please let me know what is the procedure now. Should I re-send the series and address the fixes? Should I send follow up patches?
[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.
We can update the comment as follows to comply with the clock name in the driver:
/* fCAN clock: Pick External clock. If not available fallback to
* CANFD clock or peripheral clock in case fcan_pclk = 1
*/
or, to comply with the clock name in the bindings:
/* fCAN clock: Pick External clock. If not available fallback to
* CANFD clock or fck in case fcan_pclk = 1
*/
Please let me know what is prefered.
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 */
There is already a pointer covering the clkp:
struct clk *clkp; /* Peripheral 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?
That could be updated with "clkp" to comply with the driver name or "fck" to comply with the bindings or with "peripheral". Please let me know what is preferred.
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.
The commit message mentions: ...the CAN FD clock can be sourced either from fck
(the peripheral clock) or from an external clock (can_clk).
The driver uses clkp (peripheral clock) to refer to the fck.
fck is the clock name used in the bindings. In bindings I kept the same clock name and provided the proper phandle in the device tree. I don't consider we should change the driver and the bindings for this thing. Please let me know otherwise.
The clock diagram is something like:
+----+
pclk (fck) -> 1/2 div ->| |
| mux|---> fcan
ext (can_clk)--------->| |
++++++
b/w () are the clock names in bindings.
The full HW block with the clocks involved can be found at https://github.com/claudiubeznea/logs/blob/master/canfd-clk.png
Please let me know what is the procedure for this. Should I re-spin the series? Should I send follow up patches?
Thank you,
Claudiu