Re: [PATCH net-next 17/37] can: rcar_canfd: Add support for Renesas RZ/G3S

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: rcar_canfd: uninitialized DCFG register on fixed_canfd hardware if no-can-fd is specified
--

commit ef6619c0671e4d4b6f4522cdff322e81009d1151
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

can: rcar_canfd: Add support for Renesas RZ/G3S

The Renesas RZ/G3S CAN-FD controller is largely compatible with the variant
found on the Renesas RZ/G3E. The main differences are:
- the RZ/G3S provides 16 AFL pages
- the RZ/G3S supports only two channels
- the RZ/G3S supports only CAN-FD operation and does not implement the bits
  used to select between classical CAN-only and CAN FD-only modes.
- the RZ/G3S includes an internal divider that allows the peripheral clock
  to be used as the CAN FD clock source.

Link: https://patch.msgid.link/20260902141544.565763-6-claudiu.beznea+renesas@tuxon.dev

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index ac5d5959d8e4c..74c3c3f20991f 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -661,6 +661,26 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
>  	.fixed_canfd = 0,
>  };
>  
> +static const struct rcar_canfd_hw_info r9a08g045_hw_info = {
> +	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
> +	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
> +	.tdc_const = &rcar_canfd_gen4_tdc_const,
> +	.regs = &rcar_gen4_regs,
> +	.sh = &rcar_gen4_shift_data,
> +	.rnc_field_width = 16,
> +	.max_aflpn = 15,
> +	.max_cftml = 31,
> +	.max_channels = 2,
> +	.postdiv = 2,
> +	.shared_global_irqs = 0,
> +	.multi_channel_irqs = 1,
> +	.ch_interface_mode = 1,
> +	.shared_can_regs = 1,
> +	.external_clk = 1,
> +	.fcan_pclk = 1,
> +	.fixed_canfd = 1,

[Severity: High]
This problem wasn't introduced by this patch, but setting fixed_canfd here
could lead to incomplete hardware initialization if a user specifies
"renesas,no-can-fd" in the device tree.

In rcar_canfd_probe(), the driver unconditionally disables fdmode if the DT
property is present, without verifying if the hardware actually supports
classical CAN mode:

	if (of_property_read_bool(dev->of_node, "renesas,no-can-fd"))
		fdmode = false;			/* Classical CAN only mode */

Since fdmode is false, CAN_CTRLMODE_FD is missing from the interface, which
causes rcar_canfd_set_bittiming() to return early without programming the
DCFG register:

	if (!(priv->can.ctrlmode & CAN_CTRLMODE_FD))
		return;

	/* Data bit timing settings */
	brp = dbt->brp - 1;
...
	cfg = rcar_canfd_compute_data_bit_rate_cfg(gpriv->info, tseg1, tseg2, sjw, brp);
	writel(cfg, &gpriv->fcbase[ch].dcfg);

Meanwhile, rcar_canfd_reset_controller() bypasses the FDOE/CLOE register
toggle because fixed_canfd is true:

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

		if (gpriv->info->ch_interface_mode) {
			/* Do not set CLOE and FDOE simultaneously */
			if (!gpriv->fdmode) {

Could this leave the fixed CAN FD hardware with uninitialized data phase
timing while remaining as an active CAN FD node, potentially corrupting the
shared CAN bus with active error frames? Should rcar_canfd_probe() ignore
or reject the "renesas,no-can-fd" property when fixed_canfd is true?

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015015336.816543-1-mkl@pengutronix.de?part=17