Re: [PATCH] arm64: dts: Introduce more nodes to EN7581 SoC evaluation board

[Krzysztof Kozlowski <krzk@kernel.org>]

On 01/02/2025 15:39, Lorenzo Bianconi wrote:
> Add the following nodes to EN7581 SoC and EN7581 evaluation board:
> - clock controller
> - rng controller
> - pinctrl
> - i2c controllers
> - spi nand controller
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Missing prefix for vendor.

>  arch/arm64/boot/dts/airoha/en7581-evb.dts |  8 +++
>  arch/arm64/boot/dts/airoha/en7581.dtsi    | 90 +++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/airoha/en7581-evb.dts b/arch/arm64/boot/dts/airoha/en7581-evb.dts
> index cf58e43dd5b21dbf4f64e305a4b4a2daee100858..1126da4b795f5d5df9725ec4d75cd9353b011710 100644
> --- a/arch/arm64/boot/dts/airoha/en7581-evb.dts
> +++ b/arch/arm64/boot/dts/airoha/en7581-evb.dts
> @@ -24,3 +24,11 @@ memory@80000000 {
>  		reg = <0x0 0x80000000 0x2 0x00000000>;
>  	};
>  };
> +
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&snfi {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
> index 55eb1762fb11364877695960f5a2d3e42caf8611..b1cf650efd78c6c20d19e7f18c204cf5862215c0 100644
> --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
> +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
> @@ -2,6 +2,7 @@
>  
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/en7523-clk.h>
>  
>  / {
>  	interrupt-parent = <&gic>;
> @@ -150,5 +151,94 @@ uart1: serial@1fbf0000 {
>  			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
>  			clock-frequency = <1843200>;
>  		};
> +
> +		scuclk: clock-controller@1fa20000 {
> +			compatible = "airoha,en7581-scu";
> +			reg = <0x0 0x1fb00000 0x0 0x970>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +		};
> +
> +		rng@1faa1000 {
> +			compatible = "airoha,en7581-trng";
> +			reg = <0x0 0x1faa1000 0x0 0xc04>;
> +			interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +
> +		system-controller@1fbf0200 {
> +			compatible = "syscon", "simple-mfd";

These are never allowed alone. I am pretty sure I added proper checks
which should point it out, so I think you did not really test your DTS.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.


> +			reg = <0x0 0x1fbf0200 0x0 0xc0>;
> +
> +			en7581_pinctrl: pinctrl {
> +				compatible = "airoha,en7581-pinctrl";
> +
> +				interrupt-parent = <&gic>;
> +				interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> +
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +		};
> +
> +		i2cclock: i2cclock@0 {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +
> +			/* 20 MHz */
> +			clock-frequency = <20000000>;
> +		};
> +
> +		i2c0: i2c0@1fbf8000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

i2c@

> +			compatible = "mediatek,mt7621-i2c";
> +			reg = <0x0 0x1fbf8000 0x0 0x100>;
> +
> +			clocks = <&i2cclock>;
> +
> +			/* 100 kHz */
> +			clock-frequency = <100000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			status = "disable";
> +		};
> +
> +		i2c1: i2c1@1fbf8100 {
Best regards,
Krzysztof