| Message ID | 20240802214612.434179-1-detlev.casanova@collabora.com |
|---|---|
| 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.
Hi Johan, On Tuesday, 20 August 2024 09:34:58 EDT Johan Jonker wrote: > 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/ Ok, I'll fix the consistency in the file then, thanks for the clarification. > >>> + 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/dri > vers/gpio/gpio-rockchip.c#n719 Just above that, it tries to get the pinctrl_dev from the parent node. So the rockchip,gpio-bank nodes have to be children of the pinctrl node or the gpio driver will just return -EPROBE_DEFER. > 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-d5d1597343 > c2@gmail.com/ Indeed, the driver could use the gpio-ranges to determine the node's bank number. Currently it uses the aliases or falls back to the order of parsing nodes. > I leave this subject up to the experts to find out what is needed to > improve. Don't ask me. My opinion on this is that the rockchip driver needs to be adapted if we want the gpio nodes out of the pinctrl. I'm willing to have a look at this, but I don't think it is in the scope for this patch set and don't think it should be a blocker. > Johan > Regards, Detlev.