Re: [PATCH 2/2] arm64: dts: rockchip: support CAN1-CAN2-UART4 adapter for RK3588 Jaguar

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `max-speed` property is incorrectly applied directly to the UART controller node, where it is ignored.
- [Low] Device tree compilation fails due to undefined labels `can0`, `can1`, and `can2`.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: rockchip: support CAN1-CAN2-UART4 adapter for RK3588 Jaguar ---
commit 6be47f689c18a743663d59bd2b78cb3a800f23a2
Author: Quentin Schulz <quentin.schulz@cherry.de>

arm64: dts: rockchip: support CAN1-CAN2-UART4 adapter for RK3588 Jaguar

RK3588 Jaguar has a proprietary Mezzanine connector which exposes among
other things CAN1 and CAN2 controllers as well as UART4.

Add support for the CAN1-CAN2-UART4 Mezzanine adapter for RK3588 Jaguar
which exposes two CAN busses (CAN1 and CAN2) and an RS-232 UART, limited
to 250Kbps.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar-can1-can2-uart4.dtso b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-can1-can2-uart4.dtso
> new file mode 100644
> index 0000000000000..9dfb81e5d0a0b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar-can1-can2-uart4.dtso
> @@ -0,0 +1,27 @@
[ ... ]
> +
> +&can1 {
> +	status = "okay";
> +};
> +
> +&can2 {
> +	status = "okay";
> +};

[Severity: Low]
Does this patch depend on another patch series that adds the CAN controller
nodes?

The labels &can1 and &can2 (as well as &can0 in the base file) do not appear
to exist in the upstream rk3588.dtsi base files. Compiling this causes a syntax
error because the referenced labels cannot be found.

> +
> +/* RS232 */
> +&uart4 {
> +	/* ST3232B only guarantees 250Kbps */
> +	max-speed = <250000>;
> +	status = "okay";
> +};

[Severity: Medium]
Is the max-speed property effective when placed directly on the UART
controller node?

According to the device tree bindings (serial-peripheral-props.yaml),
max-speed is meant for serial peripherals (child nodes of the UART).
Standard kernel UART drivers typically do not parse or enforce the
max-speed property on the controller node itself.

Will this actually limit the baud rate, or will it be ignored by the driver?

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> index ed6c1162bd821..0446834e84d5d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
[ ... ]
> @@ -201,6 +201,16 @@ &can0 {
>  	status = "okay";
>  };
>  
> +&can1 {
> +	assigned-clocks = <&cru CLK_CAN1>;
> +	assigned-clock-rates = <300000000>;
> +};
> +
> +&can2 {
> +	assigned-clocks = <&cru CLK_CAN2>;
> +	assigned-clock-rates = <300000000>;
> +};

[Severity: Low]
Are the &can1 and &can2 labels defined in another pending patch for the
rk3588.dtsi base tree? Without them, attempting to set these assigned-clocks
will result in the same compilation failure mentioned above.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-jaguar-can-v1-0-9b566bd785da@cherry.de?part=2