On 12/09/2024 20:25, Arturs Artamonovs via B4 Relay wrote: > From: Arturs Artamonovs <arturs.artamonovs@xxxxxxxxxx> > > Add ADI SC598-EZKIT device tree. > Support UART console as output. > > Signed-off-by: Arturs Artamonovs <Arturs.Artamonovs@xxxxxxxxxx> > Signed-off-by: Utsav Agarwal <Utsav.Agarwal@xxxxxxxxxx> > Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@xxxxxxxxxxx> > Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@xxxxxxxxxxx> > Co-developed-by: Greg Malysa <greg.malysa@xxxxxxxxxxx> > Signed-off-by: Greg Malysa <greg.malysa@xxxxxxxxxxx> > --- > arch/arm64/boot/dts/Makefile | 1 + > arch/arm64/boot/dts/adi/Makefile | 2 + > arch/arm64/boot/dts/adi/sc598-som-ezkit.dts | 14 ++ > arch/arm64/boot/dts/adi/sc598-som.dtsi | 58 +++++ > arch/arm64/boot/dts/adi/sc59x-64.dtsi | 367 ++++++++++++++++++++++++++++ > 5 files changed, 442 insertions(+) > > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile > index 21cd3a87f385309c3a655a67a3bee5f0abed7545..9b3996a8e01d8e7d264c44c075d7a50ee350ba44 100644 > --- a/arch/arm64/boot/dts/Makefile > +++ b/arch/arm64/boot/dts/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > subdir-y += actions > +subdir-y += adi > subdir-y += airoha > subdir-y += allwinner > subdir-y += altera > diff --git a/arch/arm64/boot/dts/adi/Makefile b/arch/arm64/boot/dts/adi/Makefile > new file mode 100644 > index 0000000000000000000000000000000000000000..1bf54bc97095e1ea3577953d379746fbc0ea02a9 > --- /dev/null > +++ b/arch/arm64/boot/dts/adi/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +dtb-$(CONFIG_ARCH_SC59X_64) += sc598-som-ezkit.dtb > diff --git a/arch/arm64/boot/dts/adi/sc598-som-ezkit.dts b/arch/arm64/boot/dts/adi/sc598-som-ezkit.dts > new file mode 100644 > index 0000000000000000000000000000000000000000..a8db6d5ea764f917faa6839d3d4f0b5217b927b8 > --- /dev/null > +++ b/arch/arm64/boot/dts/adi/sc598-som-ezkit.dts > @@ -0,0 +1,14 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright 2021-2024 - Analog Devices Inc. > + * Author: Nathan Barrett-Morrison <nathan.morrison@xxxxxxxxxxx> > + */ > + > +/dts-v1/; > + > +#include "sc598-som.dtsi" > + > +/ { > + model = "ADI 64-bit SC598 SOM EZ Kit"; > + compatible = "adi,sc598-som-ezkit", "adi,sc59x-64"; Where is adi,sc598-som-ezlite? > +}; > diff --git a/arch/arm64/boot/dts/adi/sc598-som.dtsi b/arch/arm64/boot/dts/adi/sc598-som.dtsi > new file mode 100644 > index 0000000000000000000000000000000000000000..3b90f367db1a24de1e1dddc4db3c219736c5b90f > --- /dev/null > +++ b/arch/arm64/boot/dts/adi/sc598-som.dtsi > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright 2021-2024 - Analog Devices Inc. > + * Author: Nathan Barrett-Morrison <nathan.morrison@xxxxxxxxxxx> > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/adi-adsp.h> > +#include "sc59x-64.dtsi" > + > +/ { > + chosen { > + stdout-path = &uart1; > + bootargs = "earlycon=adi_uart,0x31003000 console=ttySC0,115200 mem=224M"; Drop entire bootargs. Look how other SoCs do it, if you need port speed. > + }; > + > + memory@90000000 { > + device_type = "memory"; > + reg = <0x90000000 0x0e000000>; > + }; > + > + memory@20040000 { > + device_type = "memory"; > + reg = <0x20040000 0x40000>; > + }; > + > + scb: scb-bus { What is this? > + sec: sec@31089000 { And this? > + adi,sharc-cores = <2>; > + }; > + }; Drop entire node. > + Fix redundant blank lines. > +}; > + > +&uart0 { > + pinctrl-0 = <&uart0_default>; > + pinctrl-names = "default"; > + status = "okay"; > +}; > + > +&i2c0 { > + status = "okay"; > +}; > + > +&i2c1 { > + status = "disabled"; > +}; > + > +&pinctrl0 { > + uart0_default: uart0-default-pins { > + pins { > + pinmux = <ADI_ADSP_PINMUX('A', 6, ADI_ADSP_PINFUNC_ALT1)>, > + <ADI_ADSP_PINMUX('A', 7, ADI_ADSP_PINFUNC_ALT1)>; > + }; > + }; > +}; > diff --git a/arch/arm64/boot/dts/adi/sc59x-64.dtsi b/arch/arm64/boot/dts/adi/sc59x-64.dtsi > new file mode 100644 > index 0000000000000000000000000000000000000000..4a9aa08b4acb0936c97e683562e05da063a4e193 > --- /dev/null > +++ b/arch/arm64/boot/dts/adi/sc59x-64.dtsi > @@ -0,0 +1,367 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright 2021-2024 - Analog Devices Inc. > + * Author: Nathan Barrett-Morrison <nathan.morrison@xxxxxxxxxxx> > + */ > + > +#include <dt-bindings/clock/adi-sc5xx-clock.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/interrupt-controller/irq.h> > + > +/ { > + model = "ADI 64-bit SC59X"; > + compatible = "adi,sc59x-64"; > + > + interrupt-parent = <&gic>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + chosen { }; Drop > + > + aliases { > + serial0 = &uart0; > + serial2 = &uart2; > + serial3 = &uart3; > + }; Drop or move to board DTS. Not a property of the SoC. > + > + cpus { > + #address-cells = <0x2>; > + #size-cells = <0x0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a55"; > + reg = <0x0 0x0>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0xdeadbeef>; > + clocks = <&clk ADSP_SC598_CLK_ARM>, <&clk ADSP_SC598_CLK_DDR>; > + }; > + }; > + > + pmu { Order nodes alphabetically. See DTS coding style. > + compatible = "arm,armv8-pmuv3"; > + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-parent = <&gic>; > + }; > + > + gic: interrupt-controller@31200000 { This cannot be outside of SoC. See writing-bindings and DTS coding style. > + compatible = "arm,gic-v3"; > + #interrupt-cells = <3>; > + interrupt-controller; > + reg = <0x31200000 0x40000>, /* GIC Dist */ > + <0x31240000 0x40000>; /* GICR */ > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>, /* Physical Secure */ > + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, /* Physical Non-Secure */ > + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, /* Virtual */ > + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>; /* Hypervisor */ > + }; > + > + clocks { > + sys_clkin0: oscillator@1 { There is no way you tested it. It's obvious W=1 warning. > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <25000000>; > + clock-output-names = "sys_clkin0"; > + }; > + > + sys_clkin1: oscillator@2 { How are these properties of the SoC? Where are they located physically? See DTS coding style. > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <25000000>; > + clock-output-names = "sys_clkin1"; > + }; > + }; > + > + clk: clocks@3108d000 { > + compatible = "adi,sc598-clocks"; > + reg = <0x3108d000 0x1000>, > + <0x3108e000 0x1000>, > + <0x3108f000 0x1000>, > + <0x310a9000 0x1000>; > + #clock-cells = <1>; > + clocks = <&sys_clkin0>, <&sys_clkin1>; > + clock-names = "sys_clkin0", "sys_clkin1"; > + status = "okay"; Drop... everywhere. > + }; > + > + scb: scb-bus { What is scb-bus? See DTS coding style or any other SoC. This is supposed to be just sco@ with proper unit address. > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + rcu: rcu@3108c000 { > + compatible = "adi,reset-controller"; > + reg = <0x3108c000 0x1000>; > + status = "okay"; Oh... > + }; > + > + sec: sec@31089000 { Random order of nodes? See DTS coding style. > + compatible = "adi,system-event-controller"; > + reg = <0x31089000 0x1000>; > + adi,rcu = <&rcu>; > + status = "okay"; > + }; > + > + uart0: uart@31003000 { Never tested. 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). > + compatible = "adi,uart"; > + reg = <0x31003000 0x40>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + interrupt-parent = <&gic>; > + interrupt-names = "tx", "rx", "status"; > + interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 139 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; > + adi,use-edbo; > + status = "disabled"; > + }; > + > + uart1: uart@31003400 { > + compatible = "adi,uart"; > + reg = <0x31003400 0x40>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + interrupt-parent = <&gic>; > + interrupt-names = "tx", "rx", "status"; > + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>; > + adi,use-edbo; > + status = "disabled"; > + }; > + > + uart2: uart@31003800 { > + compatible = "adi,uart"; > + reg = <0x31003800 0x40>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + interrupt-parent = <&gic>; > + interrupt-names = "tx", "rx", "status"; > + interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>; > + adi,use-edbo; > + status = "disabled"; > + }; > + > + uart3: uart@31003c00 { > + compatible = "adi,uart"; > + reg = <0x31003C00 0x40>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + interrupt-parent = <&gic>; > + interrupt-names = "tx", "rx", "status"; > + interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>; > + adi,use-edbo; > + status = "disabled"; > + }; > + > + i2c0: twi@31001400 { 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 Completely wrong order of properties. Please follow DTS coding style. > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "adi,twi"; You cannot have generic compatibles in the SoC. > + reg = <0x31001400 0xFF>; I already commented on lower case hex. > + interrupts = <GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>; > + clock-khz = <100>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + status = "disabled"; > + }; > + > + i2c1: twi@31001500 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "adi,twi"; > + reg = <0x31001500 0xFF>; > + interrupts = <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>; > + clock-khz = <100>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + status = "disabled"; > + }; > + > + i2c3: twi@31001000 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "adi,twi"; > + reg = <0x31001000 0xFF>; > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > + clock-khz = <100>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + status = "disabled"; > + }; > + > + i2c4: twi@31001100 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "adi,twi"; > + reg = <0x31001100 0xFF>; > + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>; > + clock-khz = <100>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + status = "disabled"; > + }; > + > + i2c5: twi@31001200 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "adi,twi"; > + reg = <0x31001200 0xFF>; > + interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; > + clock-khz = <100>; > + clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>; > + clock-names = "sclk0"; > + status = "disabled"; > + }; > + > + pinctrl0: pinctrl@31004600 { > + compatible = "adi,adsp-pinctrl"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x31004600 0x400>; > + adi,port-sizes = <16 16 16 16 16 16 16 16 7>; > + }; > + > + pint0: pint@31005000 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005000 0xFF>; > + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint1: pint@31005100 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005100 0xFF>; > + interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint2: pint@31005200 { 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 > + compatible = "adi,adsp-pint"; > + reg = <0x31005200 0xFF>; > + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint3: pint@31005300 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005300 0xFF>; > + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint4: pint@31005400 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005400 0xFF>; > + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint5: pint@31005500 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005500 0xFF>; > + interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint6: pint@31005600 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005600 0xFF>; > + interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + pint7: pint@31005700 { > + compatible = "adi,adsp-pint"; > + reg = <0x31005700 0xFF>; > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + gpa: gport@31004000 { 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 > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004000 0x7F>; > + gpio-ranges = <&pinctrl0 0 0 16>; > + adi,pint = <&pint0 1>; > + status = "okay"; > + }; > + > + gpb: gport@31004080 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004080 0x7F>; > + gpio-ranges = <&pinctrl0 0 16 16>; > + adi,pint = <&pint0 0>; > + status = "okay"; > + }; > + > + gpc: gport@31004100 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004100 0x7F>; > + gpio-ranges = <&pinctrl0 0 32 16>; > + adi,pint = <&pint2 1>; > + status = "okay"; > + }; > + > + gpd: gport@31004180 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004180 0x7F>; > + gpio-ranges = <&pinctrl0 0 48 16>; > + adi,pint = <&pint2 0>; > + }; > + > + gpe: gport@31004200 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004200 0x7F>; > + gpio-ranges = <&pinctrl0 0 64 16>; > + adi,pint = <&pint4 1>; > + }; > + > + gpf: gport@31004280 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004280 0x7F>; > + gpio-ranges = <&pinctrl0 0 80 16>; > + adi,pint = <&pint4 0>; > + }; > + > + gpg: gport@31004300 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004300 0x7F>; > + gpio-ranges = <&pinctrl0 0 96 16>; > + adi,pint = <&pint6 1>; > + }; > + > + gph: gport@31004380 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004380 0x7F>; > + gpio-ranges = <&pinctrl0 0 112 16>; > + adi,pint = <&pint6 0>; > + }; > + > + gpi: gport@31004400 { > + compatible = "adi,adsp-port-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0x31004400 0x7F>; > + gpio-ranges = <&pinctrl0 0 128 7>; > + adi,pint = <&pint7 1>; > + }; > + All your patches have such sloppy blank lines here and there. > + }; > +}; > Best regards, Krzysztof