Re: [PATCH] arm64: dts: nuvoton: ma35d1: add CAN nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Device Tree clock configuration conflict: CAN nodes improperly force the rate of the shared system PLL (`APLL`) to 200MHz, overriding board-level settings, and fail to explicitly configure the parent mux for the CAN clock.
--

commit f23799073b0fc964077e998ef98a6998b5225d4d
Author: Zi-Yu Chen <zychennvt@gmail.com>

arm64: dts: nuvoton: ma35d1: add CAN nodes

This commit adds controller nodes for the four Bosch M_CAN blocks found
on the Nuvoton MA35D1 SoC. It also configures pinctrl and enables CAN1
and CAN3 on the MA35D1 SOM board.

> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> index e51b98f5bdce4..584dd8d44701b 100644
> --- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> @@ -244,6 +244,66 @@ gpion: gpio@340 {
>  			};
>  		};
>  
> +		can0: can@403c0000 {
> +			compatible = "bosch,m_can";
> +			reg = <0x0 0x403c0000 0x0 0x200>, <0x0 0x403c0200 0x0 0x2000>;
> +			reg-names = "m_can", "message_ram";
> +			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "int0", "int1";
> +			clocks = <&clk HCLK3>, <&clk CAN0_GATE>;
> +			clock-names = "hclk", "cclk";
> +			assigned-clocks = <&clk APLL>, <&clk CAN0_DIV>;
> +			assigned-clock-rates = <200000000>, <50000000>;

[Severity: High]
Does setting the APLL rate here override the board-level clock configuration?

In ma35d1-som-256m.dts, the top-level &clk node explicitly configures APLL:

&clk {
        assigned-clocks = <&clk CAPLL>,
                          <&clk DDRPLL>,
                          <&clk APLL>,
                          ...
        assigned-clock-rates = <800000000>,
                               <266000000>,
                               <180000000>,
                          ...
}

Since APLL is a shared system PLL, probing this CAN node will silently
overwrite that 180MHz setting with 200MHz, which could alter the
frequencies of other active peripherals relying on APLL.

Also, does this node need to explicitly configure the parent mux?

There is no assigned-clock-parents property here to route APLL to the CAN
mux. If the bootloader or hardware defaults leave the mux parented to VPLL
(which runs at 102MHz on this board), forcing APLL to 200MHz won't affect
the CAN controllers.

CAN0_DIV would then incorrectly attempt to derive 50MHz from 102MHz, causing
inaccurate CAN baud rates.

These questions also apply to the can1, can2, and can3 nodes below.

> +			bosch,mram-cfg = <0x0 4 4 32 32 32 8 8>;
> +			status = "disabled";
> +		};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604060104.1662700-1-zychennvt@gmail.com?part=1