pull-request: can-next 2026-11-10

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 371

[Severity: Low]
This isn't a bug, but adding renesas,r9a08g045-canfd requires touching three separate if-then blocks in allOf and adding a 14th conditional block with an else branch to move assigned-clocks and assigned-clock-rates out of the top-level required list. With allOf now spanning 14 per-compatible conditional blocks to toggle interrupts, resets, channels, operating modes, and clock properties across variants, would it be cleaner to split the diverging variants into a separate binding schema document instead of adding more conditionals here?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml properties: · line 18

[Severity: Low]
At this point in the series, does anything bind to this compatible?

The patch adds it as a standalone string with no fallback. However, rcar_canfd_of_table[] in drivers/net/can/rcar/rcar_canfd.c has no renesas,r9a08g045-canfd entry yet. An RZ/G3S node that passes validation would therefore not probe.

A later patch in the series, "can: rcar_canfd: Add support for Renesas RZ/G3S", appears to fix this by adding:

    { .compatible = "renesas,r9a08g045-canfd", .data = &r9a08g045_hw_info },

[ ... ]

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 367

[Severity: Low]
This is a pre-existing issue, but should this block also narrow clock-names for RZ/G3S? The top-level clock-names still accepts both forms:

clock-names:
oneOf:
- items:
- const: fck
- const: canfd
- const: can_clk
- items:
- const: fck
- const: ram_clk
- const: can_clk

No allOf entry picks one of these forms per compatible.

The RZ/G3S CPG only supplies the peripheral and RAM clocks. Even so, an RZ/G3S node that uses the canfd form would still pass dt_binding_check. The driver would then get a NULL clk_ram from devm_clk_get_optional(), and the RAM clock would never be enabled.

The renesas,rzg2l-canfd and renesas,r9a09g047-canfd compatibles already have the same gap.

Requiring the third can_clk entry looks fine, though. The internal divider setup can be described with a 0 Hz can_clk fixed-clock, as in r9a07g044.dtsi. The fck fallback that "can: rcar_canfd: Allow the CAN FD clock to be sourced from fck" adds to rcar_canfd_probe() then handles that case.

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

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_start(struct net_device *ndev) · line 1510

[Severity: Medium]
This problem wasn't introduced by this patch, but does the error path in rcar_canfd_start() [1] leave RCANFD_CCTR_CHMDC_COPM pending in hardware if readl_poll_timeout() times out?

Neither the fail_mode_change path in rcar_canfd_start() nor the out_close unwind path in rcar_canfd_open() transitions the channel back to RCANFD_CCTR_CHMDC_CRESET:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_start() {
	...
	err = readl_poll_timeout((priv->base + RCANFD_CSTS(ch)), sts,
				 (sts & RCANFD_CSTS_COMSTS), 2, 500000);
	if (err) {
		netdev_err(ndev, "channel %u communication state failed\n", ch);
		goto fail_mode_change;
	}
	...
fail_mode_change:
	rcar_canfd_disable_channel_interrupts(priv);
	return err;
}

On a later open attempt, rcar_canfd_set_bittiming() runs while the channel is no longer in reset mode, so the hardware ignores the bit-timing configuration writes.

[1] https://sashiko.dev/#/bug/linux-f4b05dc8-33ba-4ce4-8f99-3c391b0fc79a

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_probe(struct platform_device *pdev) · line 2215

[Severity: Critical]
This problem wasn't introduced by this patch, but can a failure in rcar_canfd_channel_probe() lead to a use-after-free and double free of ndev
[1] during fail_channel cleanup in rcar_canfd_probe()?

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_probe() {
    ...
	gpriv->ch[priv->channel] = priv;
	err = register_candev(ndev);
	if (err) {
		dev_err(dev, "register_candev() failed: %pe\n", ERR_PTR(err));
		goto fail_candev;
	}
    ...
fail_candev:
	netif_napi_del(&priv->napi);
fail:
	free_candev(ndev);
	return err;
}

When register_candev() fails, rcar_canfd_channel_probe() frees ndev (which contains priv) without clearing gpriv->ch[priv->channel]. rcar_canfd_probe() then jumps to fail_channel and calls rcar_canfd_channel_remove(), which sees the dangling gpriv->ch[ch] pointer and passes priv->ndev to unregister_candev() and free_candev() a second time.

[1] https://sashiko.dev/#/bug/linux-5ba990d8-8cef-4735-85ff-107958a3daf2

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_probe(struct platform_device *pdev) · line 2201

[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.

drivers/net/can/rcar/rcar_canfd.c static const struct rcar_canfd_hw_info rzg2l_hw_info = { · line 682

[Severity: Low]
This isn't a bug as long as the DT follows the binding, but should rcar_canfd_probe() ignore or reject "renesas,no-can-fd" and "renesas,fd-only" when info->fixed_canfd is set?

This is the first hw_info with fixed_canfd = 1. Both properties are still read without checking that flag:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_probe() {
    ...
	if (of_property_read_bool(dev->of_node, "renesas,no-can-fd"))
		fdmode = false;			/* Classical CAN only mode */
    ...
	if (of_property_read_bool(dev->of_node, "renesas,fd-only"))
		gpriv->fd_only_mode = true; /* FD-Only mode for CAN-FD */
    ...
}

Meanwhile rcar_canfd_reset_controller() skips the CLOE/FDOE writes, so the controller stays in CAN FD mode:

	/* Set the controller into appropriate mode */
	if (gpriv->info->fixed_canfd)
		continue;

With "renesas,no-can-fd", fdmode == false makes the rest of the driver set up classical CAN while the controller is still in CAN FD mode:

- 8 byte RFPLS/CFPLS in rcar_canfd_configure_rx() and rcar_canfd_configure_tx()
- no RCANFD_GCFG_CMPOC in rcar_canfd_configure_controller()
- no RCANFD_GCTR_CFMPOFIE in rcar_canfd_enable_global_interrupts()
- only nominal bit timing in rcar_canfd_channel_probe()

FD frames on the bus would then be dropped silently instead of being reported as errors.

With "renesas,fd-only", the probe message reports "fd-only" mode, but the hardware cannot enter that mode.

The binding change for this compatible sets "renesas,no-can-fd: false" and "renesas,fd-only: false". Only a DT that fails schema validation can hit this, and the compatible is new, so no existing DTs carry these properties.

As hardening, could probe force fdmode = true and clear fd_only_mode when fixed_canfd is set, or fail probe instead?

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_probe(struct platform_device *pdev) · line 2228

[Severity: Low]
This isn't a bug, but should this format string end with "\n"? All the other dev_err_probe() calls in rcar_canfd_probe() have one ("cannot get peripheral clock\n", "cannot get canfd clock\n", "cannot get ram clock\n", and so on).

With -EPROBE_DEFER, the case this patch is about, __dev_probe_failed() does not print the message. It stores it as-is through device_set_deferred_probe_reason().

The stored string is then printed with no newline added. One place is deferred_devs_show() in drivers/base/dd.c:

    seq_printf(s, "%s\t%s", dev_name(curr->device),
               curr->deferred_probe_reason ?: "\n");

The other is deferred_probe_timeout_work_func():

    dev_warn(p->device, "deferred probe pending: %s",
             p->deferred_probe_reason ?: "(reason unknown)\n");

So when can_clk defers, the next entry in /sys/kernel/debug/devices_deferred would run onto the same line. The deferred probe timeout warning would also have no line ending.

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_probe(struct platform_device *pdev) · line 2316

[Severity: Low]
Does checking gpriv->extclk here log "canfd" instead of "can_clk" on SoCs using r9a09g047_hw_info (such as RZ/G3E, RZ/V2N, and RZ/V2H)?

Those SoCs define their fCAN clock under the DT name "can_clk", so devm_clk_get_optional(dev, "can_clk") succeeds with a non-zero rate and rcar_canfd_probe() sets:

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;
	}
}

Because r9a09g047_hw_info has both .external_clk = 0 and .fcan_pclk = 0, gpriv->extclk is false even though the "can_clk" DT clock was selected, causing dev_info() to report "canfd clk".

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

drivers/net/can/rcar/rcar_canfd.c static int rcar_canfd_probe(struct platform_device *pdev) · line 2315

[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?

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 235

[Severity: Low]
Does this duplicate the reset-names: false constraint already present in a second if-then block for the same compatible strings further down in allOf?

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml:
- if:
properties:
compatible:
contains:
enum:
- renesas,rcar-gen3-canfd
- renesas,rcar-gen4-canfd
then:
required:
- resets
properties:
reset-names: false

These two identical if conditions for renesas,rcar-gen3-canfd and renesas,rcar-gen4-canfd could be merged, or the binding could be split into separate schema documents to avoid accumulating fragmented per-compatible conditionals.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 236

[Severity: Low]
This isn't a bug, but isn't this rule already in the binding?

Further down in the same allOf list there is a separate block for the same two compatibles. It already disallows reset-names:

- if:
properties:
compatible:
contains:
enum:
- renesas,rcar-gen3-canfd
- renesas,rcar-gen4-canfd
then:
required:
- resets
properties:
reset-names: false

So the new line duplicates that rule, and validation results for gen3 and gen4 device trees are the same before and after this hunk.

The commit message says:

Since top-level part says "reset-names" cannot accept one item, disallow it for the two variants which do not use it in practice.

That reads as if this patch adds a new restriction. Could the extra line be dropped, or the two gen3/gen4 blocks be merged into one, and the commit message updated to match?

drivers/net/can/rcar/rcar_canfd.c line 12

[Severity: Low]
This isn't a bug, but should the paragraph just below this one in the same header comment get the RZ/G3S exception too? The patch leaves it as is:

drivers/net/can/rcar/rcar_canfd.c:
* To switch the controller to Classical CAN (CAN 2.0) only mode, add
* "renesas,no-can-fd" optional property to the device tree node. A h/w reset is
* also required to switch modes.

The renesas,rcar-canfd.yaml binding does not allow this property on RZ/G3S. The allOf branch for renesas,r9a08g045-canfd has:

then:
properties:
renesas,no-can-fd: false
renesas,fd-only: false

So the comment now says Classical CAN mode is not available on RZ/G3S, but the next paragraph still tells readers to add renesas,no-can-fd, without naming any SoC that cannot use it.

Later in the series, "can: rcar_canfd: Add support for Renesas RZ/G3L" changes only the line above to "(Except RZ/G3{L,S})" and leaves this paragraph as it is.

Could the instruction paragraph also say that renesas,no-can-fd must not be used on RZ/G3S?

[ ... ]

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

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 387

[Severity: Low]
This isn't a bug, but allOf now has 15 if-then conditional blocks to handle differences in interrupt counts, channel counts, resets, clocks, and CAN modes across R-Car and RZ variants, with constraints for renesas,r9a08g046-canfd split across three separate conditionals. Would it make sense to split the binding into separate schema documents instead of adding more per-compatible conditionals to this file?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11

Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml allOf: · line 387

[Severity: Low]
This isn't a bug, but with this patch applied the binding and the driver comments disagree about RZ/G3L.

The binding now treats RZ/G3L as having no Classical CAN mode. It forbids both renesas,no-can-fd and renesas,fd-only for r9a08g045-canfd and r9a08g046-canfd.

The header comment in drivers/net/can/rcar/rcar_canfd.c still lists only RZ/G3S as the exception:

drivers/net/can/rcar/rcar_canfd.c:
* - Classical CAN (CAN 2.0) only mode (Except RZ/G3S)

The comment above the mode handling has the same wording:

/* This controller supports either Classical CAN only mode(Except RZ/G3S) or

The next patch in the series, "can: rcar_canfd: Add support for Renesas RZ/G3L", changes both comments to "(Except RZ/G3{L,S})". So once the full series is applied, the comments and the binding agree, and this only affects this one commit.

Would it be worth updating those comments here, or noting in the commit message that the driver side follows in the next patch?

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