Re: [PATCH v3 8/8] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] CAN-FD channels are enabled unconditionally despite their transceivers being physically hardwired to standby mode by default.
--

--- Patch [8]: [PATCH v3 8/8] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD ---
commit 63f365e669fda8daca391467cd1aa1fe6a6efb02
Author: Claudiu Beznea <[email protected]>

arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD

This commit enables the two CAN FD channels available on the Renesas RZ/G3S
SoC, routing them through the SMARC Carrier II board. It adds nodes for the
transceivers and configures pinmuxing, noting that the standby pins share
GPIOs with PMOD interfaces via hardware switches.

> diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h

[ ... ]

> +/*
> + * SW_GPIO_CAN_PMOD[x] switches' states:
> + * @SW_GPIO_CAN_PMOD1:
> + *	SW_OFF - GPIO8 connected to CAN0_STB (position 1-2)
> + *	SW_ON  - GPIO8 connected to PMOD1 (position 2-3)
> + * @SW_GPIO_CAN_PMOD2:
> + *	SW_OFF - GPIO9 connected to CAN1_STB (position 4-5)
> + *	SW_ON  - GPIO9 connected to PMOD1 (position 5-6)
> + */
> +#define SW_GPIO_CAN_PMOD1	SW_ON
> +#define SW_GPIO_CAN_PMOD2	SW_ON
> +
>  #endif /* __RZG3S_SMARC_SWITCHES_H__ */

[ ... ]

> diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi

[ ... ]

> +	can_transceiver0: can-phy0 {
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <8000000>;
> +#if SW_GPIO_CAN_PMOD1 == SW_OFF
> +		standby-gpios = <&pinctrl RZG2L_GPIO(13, 0) GPIO_ACTIVE_HIGH>;
> +#endif
> +	};
> +
> +	can_transceiver1: can-phy1 {
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <8000000>;
> +#if SW_GPIO_CAN_PMOD2 == SW_OFF
> +		standby-gpios = <&pinctrl RZG2L_GPIO(13, 1) GPIO_ACTIVE_HIGH>;
> +#endif
> +	};

[ ... ]

> +&canfd {
> +	pinctrl-0 = <&canfd_pins>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +
> +	channel0 {
> +		phys = <&can_transceiver0>;
> +		status = "okay";
> +	};
> +
> +	channel1 {
> +		phys = <&can_transceiver1>;
> +		status = "okay";
> +	};
> +};

[Severity: Medium]
Since SW_GPIO_CAN_PMOD1 and SW_GPIO_CAN_PMOD2 default to SW_ON, the
standby-gpios nodes are omitted for the transceivers.

Does unconditionally setting status = "okay" here expose broken CAN interfaces
to userspace by default? If the transceivers fall back to internal pull-ups and
enter standby mode, won't any transmission attempt blackhole and result in
immediate bus-off errors?

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