| Message ID | 20240802214612.434179-1-detlev.casanova@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add device tree for ArmSoM Sige 5 board | expand |
On Fri, 02 Aug 2024 17:45:27 -0400, Detlev Casanova wrote: > Add the rk3576-armsom-sige5 device tree as well as its rk3576.dtsi base > and pinctrl information in rk3576-pinctrl.dtsi. > > The other commits add DT bindings documentation for the devices that > already work with the current corresponding drivers. > > The other bindings and driver implementations are in other patch sets: > - PMIC: https://lore.kernel.org/lkml/20240802134736.283851-1-detlev.casanova@collabora.com/ > - CRU: https://lore.kernel.org/lkml/20240802214053.433493-1-detlev.casanova@collabora.com/ > - PINCTRL: https://lore.kernel.org/lkml/20240802145458.291890-1-detlev.casanova@collabora.com/ > - PM DOMAIN: https://lore.kernel.org/lkml/20240802151647.294307-1-detlev.casanova@collabora.com/ > - DW-MMC: https://lore.kernel.org/lkml/20240802153609.296197-1-detlev.casanova@collabora.com/ > - GMAC: https://lore.kernel.org/lkml/20240802173918.301668-1-detlev.casanova@collabora.com/ > > Detlev Casanova (10): > dt-bindings: arm: rockchip: Add ArmSoM Sige 5 > dt-bindings: arm: rockchip: Add rk576 compatible string to pmu.yaml > dt-bindings: i2c: i2c-rk3x: Add rk3576 compatible > dt-bindings: iio: adc: Add rockchip,rk3576-saradc string > dt-bindings: mfd: syscon: Add rk3576 QoS register compatible > dt-bindings: serial: snps-dw-apb-uart: Add Rockchip RK3576 > dt-bindings: soc: rockchip: Add rk3576 syscon compatibles > dt-bindings: timer: rockchip: Add rk3576 compatible > arm64: dts: rockchip: Add rk3576 SoC base DT > arm64: dts: rockchip: Add rk3576-armsom-sige5 board > > .../devicetree/bindings/arm/rockchip.yaml | 5 + > .../devicetree/bindings/arm/rockchip/pmu.yaml | 2 + > .../devicetree/bindings/i2c/i2c-rk3x.yaml | 1 + > .../bindings/iio/adc/rockchip-saradc.yaml | 3 + > .../devicetree/bindings/mfd/syscon.yaml | 2 + > .../bindings/serial/snps-dw-apb-uart.yaml | 1 + > .../devicetree/bindings/soc/rockchip/grf.yaml | 16 + > .../bindings/timer/rockchip,rk-timer.yaml | 1 + > arch/arm64/boot/dts/rockchip/Makefile | 1 + > .../boot/dts/rockchip/rk3576-armsom-sige5.dts | 613 ++ > .../boot/dts/rockchip/rk3576-pinctrl.dtsi | 5775 +++++++++++++++++ > arch/arm64/boot/dts/rockchip/rk3576.dtsi | 1635 +++++ > 12 files changed, 8055 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts > create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-pinctrl.dtsi > create mode 100644 arch/arm64/boot/dts/rockchip/rk3576.dtsi > > -- > 2.46.0 > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y rockchip/rk3576-armsom-sige5.dtb' for 20240802214612.434179-1-detlev.casanova@collabora.com: In file included from arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts:14: arch/arm64/boot/dts/rockchip/rk3576.dtsi:6:10: fatal error: dt-bindings/clock/rockchip,rk3576-cru.h: No such file or directory 6 | #include <dt-bindings/clock/rockchip,rk3576-cru.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[3]: *** [scripts/Makefile.lib:434: arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtb] Error 1 make[2]: *** [scripts/Makefile.build:485: arch/arm64/boot/dts/rockchip] Error 2 make[2]: Target 'arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtb' not remade because of errors. make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1389: rockchip/rk3576-armsom-sige5.dtb] Error 2 make: *** [Makefile:224: __sub-make] Error 2 make: Target 'rockchip/rk3576-armsom-sige5.dtb' not remade because of errors.
On Fri, 02 Aug 2024 17:45:36 -0400, Detlev Casanova wrote: > This device tree contains all devices necessary for booting from network > or SD Card. > > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and > SDHCI (everything necessary to boot Linux on this system on chip) as > well as Ethernet, I2C, SPI and OTP. > > Also add the necessary DT bindings for the SoC. > > Signed-off-by: Liang Chen <cl@rock-chips.com> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > [rebase, squash and reword commit message] > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > --- > .../boot/dts/rockchip/rk3576-pinctrl.dtsi | 5775 +++++++++++++++++ > arch/arm64/boot/dts/rockchip/rk3576.dtsi | 1635 +++++ > 2 files changed, 7410 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-pinctrl.dtsi > create mode 100644 arch/arm64/boot/dts/rockchip/rk3576.dtsi > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y rockchip/rk3576-armsom-sige5.dtb' for 20240802214612.434179-10-detlev.casanova@collabora.com: In file included from arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts:14: arch/arm64/boot/dts/rockchip/rk3576.dtsi:6:10: fatal error: dt-bindings/clock/rockchip,rk3576-cru.h: No such file or directory 6 | #include <dt-bindings/clock/rockchip,rk3576-cru.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[3]: *** [scripts/Makefile.lib:434: arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtb] Error 1 make[2]: *** [scripts/Makefile.build:485: arch/arm64/boot/dts/rockchip] Error 2 make[2]: Target 'arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtb' not remade because of errors. make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1389: rockchip/rk3576-armsom-sige5.dtb] Error 2 make: *** [Makefile:224: __sub-make] Error 2 make: Target 'rockchip/rk3576-armsom-sige5.dtb' not remade because of errors.
Hi Detlev, Am Freitag, 2. August 2024, 23:45:36 CEST schrieb Detlev Casanova: > This device tree contains all devices necessary for booting from network > or SD Card. > > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and > SDHCI (everything necessary to boot Linux on this system on chip) as > well as Ethernet, I2C, SPI and OTP. > > Also add the necessary DT bindings for the SoC. > > Signed-off-by: Liang Chen <cl@rock-chips.com> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > [rebase, squash and reword commit message] > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> looks like (since 2019) there is a strong suggestion for having a soc node. See Krzysztof's mail in https://lore.kernel.org/all/6320e4f3-e737-4787-8a72-7bd314ba883c@kernel.org/ that references Documentation/devicetree/bindings/writing-bindings.rst [0] So I guess we should probably follow that - at least for new socs for now. Heiko [0] https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90
Some comments below. Whenever useful. On 8/2/24 23:45, Detlev Casanova wrote: > This device tree contains all devices necessary for booting from network > or SD Card. > > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and > SDHCI (everything necessary to boot Linux on this system on chip) as > well as Ethernet, I2C, SPI and OTP. > > Also add the necessary DT bindings for the SoC. > > Signed-off-by: Liang Chen <cl@rock-chips.com> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > [rebase, squash and reword commit message] > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > --- [..] > diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi > new file mode 100644 > index 0000000000000..00c4d2a153ced > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi > @@ -0,0 +1,1635 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2023 Rockchip Electronics Co., Ltd. > + */ > + > +#include <dt-bindings/clock/rockchip,rk3576-cru.h> > +#include <dt-bindings/reset/rockchip,rk3576-cru.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/phy/phy.h> > +#include <dt-bindings/power/rk3576-power.h> > +#include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/soc/rockchip,boot-mode.h> Sort includes. > + > +/ { > + compatible = "rockchip,rk3576"; > + > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + aliases { > + ethernet0 = &gmac0; > + ethernet1 = &gmac1; > + gpio0 = &gpio0; > + gpio1 = &gpio1; > + gpio2 = &gpio2; > + gpio3 = &gpio3; > + gpio4 = &gpio4; > + i2c0 = &i2c0; > + i2c1 = &i2c1; > + i2c2 = &i2c2; > + i2c3 = &i2c3; > + i2c4 = &i2c4; > + i2c5 = &i2c5; > + i2c6 = &i2c6; > + i2c7 = &i2c7; > + i2c8 = &i2c8; > + i2c9 = &i2c9; > + serial0 = &uart0; > + serial1 = &uart1; > + serial2 = &uart2; > + serial3 = &uart3; > + serial4 = &uart4; > + serial5 = &uart5; > + serial6 = &uart6; > + serial7 = &uart7; > + serial8 = &uart8; > + serial9 = &uart9; > + serial10 = &uart10; > + serial11 = &uart11; > + spi0 = &spi0; > + spi1 = &spi1; > + spi2 = &spi2; > + spi3 = &spi3; > + spi4 = &spi4; > + }; > + [..] For uart0..uart11: > + > + uart1: serial@27310000 { > + compatible = "rockchip,rk3576-uart", "snps,dw-apb-uart"; > + reg = <0x0 0x27310000 0x0 0x100>; > + interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>; "interrupts" are sort just like other properties. A mix of sort styles exists, so check all nodes. > + clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>; > + clock-names = "baudclk", "apb_pclk"; > + reg-shift = <2>; > + reg-io-width = <4>; Move below "reg". > + dmas = <&dmac0 8>, <&dmac0 9>; > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1m0_xfer>; > + status = "disabled"; > + }; > + > + pmu: power-management@27380000 { > + compatible = "rockchip,rk3576-pmu", "syscon", "simple-mfd"; > + reg = <0x0 0x27380000 0x0 0x800>; > + > + power: power-controller { > + compatible = "rockchip,rk3576-power-controller"; > + #power-domain-cells = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; Remove. > + > + power-domain@RK3576_PD_NPU { > + reg = <RK3576_PD_NPU>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes providing multiple PM domains. > + #address-cells = <1>; > + #size-cells = <0>; > + > + power-domain@RK3576_PD_NPUTOP { > + reg = <RK3576_PD_NPUTOP>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes providing multiple PM domains. > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&cru ACLK_RKNN0>, > + <&cru ACLK_RKNN1>, > + <&cru ACLK_RKNN_CBUF>, > + <&cru CLK_RKNN_DSU0>, > + <&cru HCLK_RKNN_CBUF>, > + <&cru HCLK_RKNN_ROOT>, > + <&cru HCLK_NPU_CM0_ROOT>, > + <&cru PCLK_NPUTOP_ROOT>; > + pm_qos = <&qos_npu_mcu>, > + <&qos_npu_nsp0>, > + <&qos_npu_nsp1>, > + <&qos_npu_m0ro>, > + <&qos_npu_m1ro>; > + > + power-domain@RK3576_PD_NPU0 { > + reg = <RK3576_PD_NPU0>; > + #power-domain-cells = <0>; > + clocks = <&cru HCLK_RKNN_ROOT>, > + <&cru ACLK_RKNN0>; > + pm_qos = <&qos_npu_m0>; > + }; > + power-domain@RK3576_PD_NPU1 { > + reg = <RK3576_PD_NPU1>; > + #power-domain-cells = <0>; > + clocks = <&cru HCLK_RKNN_ROOT>, > + <&cru ACLK_RKNN1>; > + pm_qos = <&qos_npu_m1>; > + }; > + }; > + }; > + > + power-domain@RK3576_PD_GPU { > + reg = <RK3576_PD_GPU>; > + #power-domain-cells = <0>; > + clocks = <&cru CLK_GPU>; > + pm_qos = <&qos_gpu>; > + }; > + > + power-domain@RK3576_PD_NVM { > + reg = <RK3576_PD_NVM>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes providing multiple PM domains. > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&cru ACLK_EMMC>, <&cru HCLK_EMMC>; > + pm_qos = <&qos_emmc>, > + <&qos_fspi0>; > + > + power-domain@RK3576_PD_SDGMAC { > + reg = <RK3576_PD_SDGMAC>; > + #power-domain-cells = <0>; TAB ?? > + clocks = <&cru ACLK_HSGPIO>, > + <&cru ACLK_GMAC0>, > + <&cru ACLK_GMAC1>, > + <&cru CCLK_SRC_SDIO>, > + <&cru CCLK_SRC_SDMMC0>, > + <&cru HCLK_HSGPIO>, > + <&cru HCLK_SDIO>, > + <&cru HCLK_SDMMC0>; > + pm_qos = <&qos_fspi1>, > + <&qos_gmac0>, > + <&qos_gmac1>, > + <&qos_sdio>, > + <&qos_sdmmc>, > + <&qos_flexbus>; > + }; > + }; > + power-domain@RK3576_PD_PHP { > + reg = <RK3576_PD_PHP>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes providing multiple PM domains. > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&cru ACLK_PHP_ROOT>, > + <&cru PCLK_PHP_ROOT>, > + <&cru ACLK_MMU0>, > + <&cru ACLK_MMU1>; > + pm_qos = <&qos_mmu0>, > + <&qos_mmu1>; > + > + power-domain@RK3576_PD_SUBPHP { > + reg = <RK3576_PD_SUBPHP>; > + #power-domain-cells = <0>; TAB?? > + }; > + }; > + power-domain@RK3576_PD_AUDIO { > + reg = <RK3576_PD_AUDIO>; > + #power-domain-cells = <0>; > + }; > + power-domain@RK3576_PD_VEPU1 { > + clocks = <&cru ACLK_VEPU1>, > + <&cru HCLK_VEPU1>; > + reg = <RK3576_PD_VEPU1>; sort > + #power-domain-cells = <0>; > + pm_qos = <&qos_vepu1>; > + }; > + power-domain@RK3576_PD_VPU { > + reg = <RK3576_PD_VPU>; > + #power-domain-cells = <0>; > + clocks = <&cru ACLK_EBC>, > + <&cru HCLK_EBC>, > + <&cru ACLK_RGA2E_0>, > + <&cru HCLK_RGA2E_0>, > + <&cru ACLK_RGA2E_1>, > + <&cru HCLK_RGA2E_1>, > + <&cru ACLK_JPEG>, > + <&cru HCLK_JPEG>, > + <&cru ACLK_VDPP>, > + <&cru HCLK_VDPP>; > + pm_qos = <&qos_ebc>, > + <&qos_rga0>, > + <&qos_rga1>, > + <&qos_jpeg>, > + <&qos_vdpp>; > + }; > + power-domain@RK3576_PD_VDEC { > + reg = <RK3576_PD_VDEC>; > + #power-domain-cells = <0>; > + clocks = <&cru ACLK_RKVDEC_ROOT>, > + <&cru HCLK_RKVDEC>; > + pm_qos = <&qos_rkvdec>; > + }; > + power-domain@RK3576_PD_VI { > + reg = <RK3576_PD_VI>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&cru ACLK_VICAP>, > + <&cru HCLK_VICAP>, > + <&cru DCLK_VICAP>, > + <&cru ACLK_VI_ROOT>, > + <&cru HCLK_VI_ROOT>, > + <&cru PCLK_VI_ROOT>, > + <&cru CLK_ISP_CORE>, > + <&cru ACLK_ISP>, > + <&cru HCLK_ISP>, > + <&cru CLK_CORE_VPSS>, > + <&cru ACLK_VPSS>, > + <&cru HCLK_VPSS>; > + pm_qos = <&qos_isp_mro>, > + <&qos_isp_mwo>, > + <&qos_vicap_m0>, > + <&qos_vpss_mro>, > + <&qos_vpss_mwo>; > + > + power-domain@RK3576_PD_VEPU0 { > + reg = <RK3576_PD_VEPU0>; > + #power-domain-cells = <0>; > + clocks = <&cru ACLK_VEPU0>, > + <&cru HCLK_VEPU0>; > + pm_qos = <&qos_vepu0>; > + }; > + }; > + power-domain@RK3576_PD_VOP { > + reg = <RK3576_PD_VOP>; > + #power-domain-cells = <0>; #power-domain-cells = <1>; "#power-domain-cells": enum: [0, 1] description: Must be 0 for nodes representing a single PM domain and 1 for nodes providing multiple PM domains. > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&cru ACLK_VOP>, > + <&cru HCLK_VOP>, > + <&cru HCLK_VOP_ROOT>; > + pm_qos = <&qos_vop_m0>, > + <&qos_vop_m1ro>; > + > + power-domain@RK3576_PD_USB { Since when is USB part of VOP? Recheck? > + reg = <RK3576_PD_USB>; > + #power-domain-cells = <0>; > + clocks = <&cru PCLK_PHP_ROOT>, > + <&cru ACLK_USB_ROOT>, > + <&cru ACLK_MMU2>, > + <&cru ACLK_SLV_MMU2>, > + <&cru ACLK_UFS_SYS>; > + pm_qos = <&qos_mmu2>, > + <&qos_ufshc>; > + }; > + power-domain@RK3576_PD_VO0 { > + reg = <RK3576_PD_VO0>; > + #power-domain-cells = <0>; > + clocks = <&cru ACLK_HDCP0>, > + <&cru HCLK_HDCP0>, > + <&cru ACLK_VO0_ROOT>, > + <&cru PCLK_VO0_ROOT>, > + <&cru HCLK_VOP_ROOT>; > + pm_qos = <&qos_hdcp0>; > + }; > + power-domain@RK3576_PD_VO1 { > + reg = <RK3576_PD_VO1>; > + #power-domain-cells = <0>; > + clocks = <&cru ACLK_HDCP1>, > + <&cru HCLK_HDCP1>, > + <&cru ACLK_VO1_ROOT>, > + <&cru PCLK_VO1_ROOT>, > + <&cru HCLK_VOP_ROOT>; > + pm_qos = <&qos_hdcp1>; > + }; > + }; > + }; > + }; > + [..] > + > + rktimer: timer@2acc0000 { timer0: ..... > + compatible = "rockchip,rk3576-timer", "rockchip,rk3288-timer"; > + reg = <0x0 0x2acc0000 0x0 0x20>; > + interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; sort > + clocks = <&cru PCLK_BUSTIMER0>, <&cru CLK_TIMER0>; > + clock-names = "pclk", "timer"; > + }; > + > + wdt: watchdog@2ace0000 { > + compatible = "snps,dw-wdt"; Must be SoC related. Add something to snps,dw-wdt.yaml > + reg = <0x0 0x2ace0000 0x0 0x100>; > + clocks = <&cru TCLK_WDT0>, <&cru PCLK_WDT0>; > + clock-names = "tclk", "pclk"; > + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + }; > + For spi0..spi4: > + spi0: spi@2acf0000 { > + compatible = "rockchip,rk3066-spi"; Must be SoC related. Add something to spi-rockchip.yaml > + reg = <0x0 0x2acf0000 0x0 0x1000>; > + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; sort > + #address-cells = <1>; > + #size-cells = <0>; Move things that start with # down the list as posible. > + clocks = <&cru CLK_SPI0>, <&cru PCLK_SPI0>; > + clock-names = "spiclk", "apb_pclk"; > + dmas = <&dmac0 14>, <&dmac0 15>; > + dma-names = "tx", "rx"; > + pinctrl-names = "default"; > + pinctrl-0 = <&spi0m0_csn0 &spi0m0_csn1 &spi0m0_pins>; > + num-cs = <2>; > + status = "disabled"; > + }; > + > + spi1: spi@2ad00000 { > + spi2: spi@2ad10000 { > + spi3: spi@2ad20000 { > + spi4: spi@2ad30000 { > + uart0: serial@2ad40000 { > + uart2: serial@2ad50000 { > + uart3: serial@2ad60000 { > + uart4: serial@2ad70000 { > + uart5: serial@2ad80000 { > + uart6: serial@2ad90000 { > + uart7: serial@2ada0000 { > + uart8: serial@2adb0000 { > + uart9: serial@2adc0000 { > + saradc: adc@2ae00000 { > + compatible = "rockchip,rk3576-saradc", "rockchip,rk3588-saradc"; > + reg = <0x0 0x2ae00000 0x0 0x10000>; > + interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>; sort > + #io-channel-cells = <1>; Things that start with "#" are used to interpreted the DT, so down the list as possible. > + clocks = <&cru CLK_SARADC>, <&cru PCLK_SARADC>; > + clock-names = "saradc", "apb_pclk"; > + resets = <&cru SRST_P_SARADC>; > + reset-names = "saradc-apb"; > + status = "disabled"; > + }; > + [..] > + > + uart10: serial@2afc0000 { > + > + uart11: serial@2afd0000 { > + [..] > + > + pinctrl: pinctrl { > + compatible = "rockchip,rk3576-pinctrl"; > + rockchip,grf = <&ioc_grf>; > + rockchip,sys-grf = <&sys_grf>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + gpio0: gpio@27320000 { The use of gpio nodes as subnode of pinctrl is deprecated. patternProperties: "gpio@[0-9a-f]+$": type: object $ref: /schemas/gpio/rockchip,gpio-bank.yaml# deprecated: true unevaluatedProperties: false > + compatible = "rockchip,gpio-bank"; When in use as separate node the compatible must be SoC related. Question for the maintainers: Extra entry to rockchip,gpio-bank.yaml ?? > + reg = <0x0 0x27320000 0x0 0x200>; > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru PCLK_GPIO0>, <&cru DBCLK_GPIO0>; > + > + gpio-controller; > + #gpio-cells = <2>; > + gpio-ranges = <&pinctrl 0 0 32>; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + > + gpio1: gpio@2ae10000 { > + > + gpio2: gpio@2ae20000 { > + > + gpio3: gpio@2ae30000 { > + > + gpio4: gpio@2ae40000 { > + }; > +}; > + > +#include "rk3576-pinctrl.dtsi"
Hello Detlev, Please see a few comments below. On 2024-08-02 23:45, Detlev Casanova wrote: > This device tree contains all devices necessary for booting from > network > or SD Card. > > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and > SDHCI (everything necessary to boot Linux on this system on chip) as > well as Ethernet, I2C, SPI and OTP. > > Also add the necessary DT bindings for the SoC. > > Signed-off-by: Liang Chen <cl@rock-chips.com> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > [rebase, squash and reword commit message] > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > --- [snip] > diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi > b/arch/arm64/boot/dts/rockchip/rk3576.dtsi > new file mode 100644 > index 0000000000000..00c4d2a153ced > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi > @@ -0,0 +1,1635 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2023 Rockchip Electronics Co., Ltd. > + */ > + > +#include <dt-bindings/clock/rockchip,rk3576-cru.h> > +#include <dt-bindings/reset/rockchip,rk3576-cru.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/phy/phy.h> > +#include <dt-bindings/power/rk3576-power.h> > +#include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/soc/rockchip,boot-mode.h> > + > +/ { > + compatible = "rockchip,rk3576"; > + > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + aliases { > + ethernet0 = &gmac0; > + ethernet1 = &gmac1; Please remove ethernetX aliases from the SoC dtsi. The consensus is that those aliases need to be defined at the board level instead. See the commit 5d90cb1edcf7 (arm64: dts: rockchip: Remove ethernet0 alias from the SoC dtsi for RK3399, 2023-12-12), for example, for more details. > + gpio0 = &gpio0; > + gpio1 = &gpio1; > + gpio2 = &gpio2; > + gpio3 = &gpio3; > + gpio4 = &gpio4; > + i2c0 = &i2c0; > + i2c1 = &i2c1; > + i2c2 = &i2c2; > + i2c3 = &i2c3; > + i2c4 = &i2c4; > + i2c5 = &i2c5; > + i2c6 = &i2c6; > + i2c7 = &i2c7; > + i2c8 = &i2c8; > + i2c9 = &i2c9; > + serial0 = &uart0; > + serial1 = &uart1; > + serial2 = &uart2; > + serial3 = &uart3; > + serial4 = &uart4; > + serial5 = &uart5; > + serial6 = &uart6; > + serial7 = &uart7; > + serial8 = &uart8; > + serial9 = &uart9; > + serial10 = &uart10; > + serial11 = &uart11; > + spi0 = &spi0; > + spi1 = &spi1; > + spi2 = &spi2; > + spi3 = &spi3; > + spi4 = &spi4; > + }; > + > + xin32k: clock-32k { Please use "xin32k: clock-xin32k { ... }" instead, because that follows the recently established revised pattern for clock names. We should have come consistency in the new SoC dtsi additions. > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + clock-output-names = "xin32k"; > + }; > + > + xin24m: clock-24m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + clock-output-names = "xin24m"; > + }; Please use "xin24m: clock-xin24m { ... }" instead, for the same reasons as already described above. > + spll: clock-702m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <702000000>; > + clock-output-names = "spll"; > + }; Perhaps using "spll: clock-spll { ... }" instead would also be a good idea, because it would improve the overall consistency.
Am Montag, 19. August 2024, 19:59:45 CEST schrieb Detlev Casanova: > On Wednesday, 14 August 2024 11:31:04 EDT Heiko Stübner wrote: > > Hi Detlev, > > > > Am Freitag, 2. August 2024, 23:45:36 CEST schrieb Detlev Casanova: > > > This device tree contains all devices necessary for booting from network > > > or SD Card. > > > > > > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and > > > SDHCI (everything necessary to boot Linux on this system on chip) as > > > well as Ethernet, I2C, SPI and OTP. > > > > > > Also add the necessary DT bindings for the SoC. > > > > > > Signed-off-by: Liang Chen <cl@rock-chips.com> > > > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > > > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > > > [rebase, squash and reword commit message] > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > > looks like (since 2019) there is a strong suggestion for having a soc node. > > > > See Krzysztof's mail in > > > > https://lore.kernel.org/all/6320e4f3-e737-4787-8a72-7bd314ba883c@kernel.org > > / that references > > Documentation/devicetree/bindings/writing-bindings.rst [0] > > > > So I guess we should probably follow that - at least for new socs for now. > > That make sense, but what is exactly covered by MMIO devices ? everything > except cpus, firmware, psci and timer ? if your node has a foo@mmio-address naming then it goes in there I guess
On 8/19/24 22:06, Detlev Casanova wrote: > Hi Johan, > > On Thursday, 15 August 2024 05:30:25 EDT Johan Jonker wrote: >> Some comments below. Whenever useful. >> >> On 8/2/24 23:45, Detlev Casanova wrote: >>> This device tree contains all devices necessary for booting from network >>> or SD Card. >>> >>> It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and >>> SDHCI (everything necessary to boot Linux on this system on chip) as >>> well as Ethernet, I2C, SPI and OTP. >>> >>> Also add the necessary DT bindings for the SoC. >>> >>> Signed-off-by: Liang Chen <cl@rock-chips.com> >>> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> >>> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> >>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >>> [rebase, squash and reword commit message] >>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> >>> --- >> >> [..] >> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi >>> b/arch/arm64/boot/dts/rockchip/rk3576.dtsi new file mode 100644 >>> index 0000000000000..00c4d2a153ced >>> --- /dev/null >>> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi >> [..] >> >> For uart0..uart11: >>> + >>> + uart1: serial@27310000 { >>> + compatible = "rockchip,rk3576-uart", "snps,dw-apb- > uart"; >>> + reg = <0x0 0x27310000 0x0 0x100>; >>> >>> + interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>; >> >> "interrupts" are sort just like other properties. A mix of sort styles >> exists, so check all nodes. > > Ok, so it should be sorted alphabetically with the following exceptions: > - 'compatible' and 'reg.*' on top > - "#.*" at the end, sorted > - "status" last. > > Is that right ? The dts-coding-style.rst does not say much about things with "#", so below a property they refer to or at the end looks nicer. No strict rule, but do it in a consistent style in file. Original comment by robh for things with "reg": "It makes more sense to keep reg-io-width together with reg." https://lore.kernel.org/all/20240131135955.GA966672-robh@kernel.org/ > >>> + clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>; >>> + clock-names = "baudclk", "apb_pclk"; >>> >>> + reg-shift = <2>; >>> + reg-io-width = <4>; >> >> Move below "reg". >> >>> + dmas = <&dmac0 8>, <&dmac0 9>; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&uart1m0_xfer>; >>> + status = "disabled"; >>> + }; >>> + >>> + pmu: power-management@27380000 { > > [...] > >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + clocks = <&cru ACLK_VOP>, >>> + <&cru HCLK_VOP>, >>> + <&cru HCLK_VOP_ROOT>; >>> + pm_qos = <&qos_vop_m0>, >>> + <&qos_vop_m1ro>; >>> + >>> + power-domain@RK3576_PD_USB { >> >> Since when is USB part of VOP? >> Recheck? > > The TRM doesn't tell me anything, but If I don't put it as a child of VOP, it > just hangs when the kernel tries to shut it down. Could the people from Rockchip disclose the USB PD location? > > [...] > >>> + >>> + pinctrl: pinctrl { >>> + compatible = "rockchip,rk3576-pinctrl"; >>> + rockchip,grf = <&ioc_grf>; >>> + rockchip,sys-grf = <&sys_grf>; >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges; >>> + >>> >>> + gpio0: gpio@27320000 { >> >> The use of gpio nodes as subnode of pinctrl is deprecated. >> >> patternProperties: >> "gpio@[0-9a-f]+$": >> type: object >> >> $ref: /schemas/gpio/rockchip,gpio-bank.yaml# >> deprecated: true >> >> unevaluatedProperties: false > > I tried putting the gpio nodes out of the pinctrl node, they should work > because they already have a gpio-ranges field. > But unfortunately, that seem to break the pinctrl driver which hangs at some > point. Maybe some adaptations are needed to support this, or am I missing > something ? The aliases that we added to the DT files are a work around to prevent damage when we moved to generic gpio node names. There just happened to be some code for it in the driver... https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpio/gpio-rockchip.c#n719 Comment by robh: "GPIO shouldn't really have an alias either IMO." https://lore.kernel.org/linux-arm-kernel/20230118153236.GA33699-robh@kernel.org/ Mainline Rockchip gpio driver is not so to the standard. The file gpio-rockchip.c currently does nothing with "gpio-ranges" vs. bank and node relation. My simple patch was acked, but never applied. There's no public maintainer response of what to improve. Guess, probably something more complicated idiot prove "gpio-ranges" parsing/bank linking is needed? https://lore.kernel.org/linux-arm-kernel/890be9a0-8e82-a8f4-bc15-d5d1597343c2@gmail.com/ I leave this subject up to the experts to find out what is needed to improve. Don't ask me. Johan > >>> + compatible = "rockchip,gpio-bank"; >> >> When in use as separate node the compatible must be SoC related. >> >> Question for the maintainers: Extra entry to rockchip,gpio-bank.yaml ?? >> >>> + reg = <0x0 0x27320000 0x0 0x200>; >>> + interrupts = <GIC_SPI 153 > IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&cru PCLK_GPIO0>, <&cru > DBCLK_GPIO0>; >>> + >>> + gpio-controller; >>> + #gpio-cells = <2>; >>> + gpio-ranges = <&pinctrl 0 0 32>; >>> + interrupt-controller; >>> + #interrupt-cells = <2>; >>> + }; >>> + >>> + gpio1: gpio@2ae10000 { >>> + >>> + gpio2: gpio@2ae20000 { >>> + >>> + gpio3: gpio@2ae30000 { >>> + >>> + gpio4: gpio@2ae40000 { >>> + }; >>> +}; >>> + >>> +#include "rk3576-pinctrl.dtsi" > > Regards, > > Detlev > > >