Re: [PATCH 3/3] arm64: dts: nuvoton: Add I2C nodes for MA35D1 SoC

[Krzysztof Kozlowski <krzk@kernel.org>]

On Mon, Mar 02, 2026 at 02:08:22AM +0000, Zi-Yu Chen wrote:
> Add I2C controller nodes to the MA35D1 SoC dtsi.
> Also enable the I2C interfaces on the MA35D1 SOM board
> to allow communication with onboard peripherals.
> 
> Signed-off-by: Zi-Yu Chen <zychennvt@gmail.com>
> ---
>  .../boot/dts/nuvoton/ma35d1-som-256m.dts      | 14 ++++
>  arch/arm64/boot/dts/nuvoton/ma35d1.dtsi       | 65 +++++++++++++++++++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
> index f6f20a17e501..2a8f0fd90ded 100644
> --- a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
> @@ -98,6 +98,14 @@ pinctrl_uart16: uart16-pins {
>  			power-source = <1>;
>  		};
>  	};

Missing blank line

> +	i2c-grp {
> +		pinctrl_i2c1: i2c1-pins {
> +			nuvoton,pins = <1 10 12>,
> +				       <1 11 12>;
> +			bias-disable;
> +		};
> +
> +	};
>  };
>  
>  &uart0 {
> @@ -129,3 +137,9 @@ &uart16 {
>  	pinctrl-0 = <&pinctrl_uart16>;
>  	status = "okay";
>  };
> +
> +&i2c1 {

Why 'i' is after 'u'? Please read DTS coding style.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> index e51b98f5bdce..36bd19e37b57 100644
> --- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> @@ -17,6 +17,10 @@ / {
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  
> +	aliases {
> +		i2c0 = &i2c2;

Not a property of DTSI, but DTS.

> +	};
> +
>  	cpus {
>  		#address-cells = <2>;
>  		#size-cells = <0>;
> @@ -372,6 +376,66 @@ uart15: serial@407f0000 {
>  			status = "disabled";
>  		};
>  
> +		i2c1: i2c@40810000 {
> +			compatible = "nuvoton,ma35d1-i2c";
> +			reg = <0x0 0x40810000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk I2C1_GATE>;
> +			clock-frequency = <100000>;
> +			resets = <&sys MA35D1_RESET_I2C1>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c2: i2c@40820000 {
> +			compatible = "nuvoton,ma35d1-i2c";
> +			reg = <0x0 0x40820000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk I2C2_GATE>;
> +			clock-frequency = <100000>;
> +			resets = <&sys MA35D1_RESET_I2C2>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c3: i2c@40830000 {
> +			compatible = "nuvoton,ma35d1-i2c";
> +			reg = <0x0 0x40830000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk I2C3_GATE>;
> +			clock-frequency = <100000>;
> +			resets = <&sys MA35D1_RESET_I2C3>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		i2c4: i2c@40840000 {
> +			compatible = "nuvoton,ma35d1-i2c";
> +			reg = <0x0 0x40840000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk I2C4_GATE>;
> +			clock-frequency = <100000>;
> +			resets = <&sys MA35D1_RESET_I2C4>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		i2c5: i2c@40850000 {
> +			compatible = "nuvoton,ma35d1-i2c";
> +			reg = <0x0 0x40850000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk I2C5_GATE>;
> +			clock-frequency = <100000>;
> +			resets = <&sys MA35D1_RESET_I2C5>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
>  		uart16: serial@40880000 {
>  			compatible = "nuvoton,ma35d1-uart";
>  			reg = <0x0 0x40880000 0x0 0x100>;
> @@ -379,5 +443,6 @@ uart16: serial@40880000 {
>  			clocks = <&clk UART16_GATE>;
>  			status = "disabled";
>  		};
> +

Why? Do not introduce random changes.

Best regards,
Krzysztof