On 23/01/2023 08:32, Li Chen wrote: > This patch adds s6lm's devicetree. > > Signed-off-by: Li Chen <lchen@xxxxxxxxxxxxx> > Change-Id: Idc7a04cc64a53b3296bc9fcf9dd3cd648ebefeae > --- > MAINTAINERS | 2 + > .../boot/dts/ambarella/ambarella-s6lm.dtsi | 332 ++++++++++++++++++ > .../boot/dts/ambarella/s6lm_pineapple.dts | 29 ++ > 3 files changed, 363 insertions(+) > create mode 100644 arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi > create mode 100644 arch/arm64/boot/dts/ambarella/s6lm_pineapple.dts > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1360fe2f4236..d9d1033b6452 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1894,6 +1894,8 @@ ARM/Ambarella SoC support > M: Li Chen <me@linux.beauty> > L: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx (moderated for non-subscribers) > S: Supported > +F: arch/arm64/boot/dts/ambarella/ambarella-s6lm.dts > +F: arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi > F: Documentation/devicetree/bindings/arm/ambarella.yaml > F: Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml > F: Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml > diff --git a/arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi b/arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi > new file mode 100644 > index 000000000000..c3fa77b33e04 > --- /dev/null > +++ b/arch/arm64/boot/dts/ambarella/ambarella-s6lm.dtsi > @@ -0,0 +1,332 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2023 Ambarella,Inc. - http://www.ambarella.com/ > + * Author: Cao Rongrong <rrcao@xxxxxxxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. Drop boilerplate. > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > + > +/ { > + compatible = "ambarella,s6lm"; > + interrupt-parent = <&gic>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + aliases { > + serial0 = &uart0; > + nand = &nand0; > + }; > + > + chosen { > + stdout-path = &uart0; > + }; > + > + /* > + * the memory node will be overwritten in Amboot, > + * here is just the default value. > + */ > + memory { > + device_type = "memory"; > + reg = <0x00200000 0x07e00000>; /* 126M */ > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + }; > + > + psci { > + compatible = "arm,psci-0.2"; > + method = "smc"; > + }; > + > + pmu { > + compatible = "arm,cortex-a53-pmu"; > + interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, > + <0 5 0x4>, ??? Just compare these two lines above... > + <0 6 0x4>, > + <0 7 0x4>; > + }; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + cpu@0 { > + compatible = "arm,cortex-a53", "arm,armv8"; > + device_type = "cpu"; > + reg = <0x0>; > + enable-method = "psci"; > + }; > + > + cpu@1 { > + compatible = "arm,cortex-a53", "arm,armv8"; > + device_type = "cpu"; > + reg = <0x1>; > + enable-method = "psci"; > + }; > + > + cpu@2 { > + compatible = "arm,cortex-a53", "arm,armv8"; > + device_type = "cpu"; > + reg = <0x2>; > + enable-method = "psci"; > + }; > + > + cpu@3 { > + compatible = "arm,cortex-a53", "arm,armv8"; > + device_type = "cpu"; > + reg = <0x3>; > + enable-method = "psci"; > + }; > + }; > + > + gic: interrupt-controller@f3000000 { > + compatible = "arm,gic-400"; reg should be second property. > + #interrupt-cells = <3>; > + #address-cells = <0>; > + interrupt-controller; > + reg = <0xf3001000 0x1000>, /* GIC Dist */ > + <0xf3002000 0x2000>, /* GIC CPU */ > + /* following are not used if no virtulization */ > + <0xf3004000 0x2000>, /* GIC VCPU Control */ > + <0xf3006000 0x2000>; /* GIC VCPU */ > + interrupts = <GIC_PPI 9 0xf04>; /* GIC Maintenence IRQ */ > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 13 0xf08>, /* Secure Phys IRQ */ > + <1 14 0xf08>, /* Non-secure Phys IRQ */ And you do not see here anything odd? > + <1 11 0xf08>, /* Virt IRQ */ > + <1 10 0xf08>; /* Hyp IRQ */ > + }; > + > + n_apb@e4000000 { /* Non-Secure APB, but configurable */ No underscores in node names. Generic node names, so just "bus" > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0xe4000000 0x01000000>; > + ranges; > + > + uart0: uart@e4000000 { serial > + compatible = "ambarella,uart"; > + reg = <0xe4000000 0x1000>; > + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&uart0_pins>; > + clocks = <&gclk_uart0>; > + amb,ignore-fe; > + status = "ok"; > + }; > + }; > + > + s_apb@ec000000 { /* Secure APB, but configurable */ bus > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0xec000000 0x01000000>; reg is second property > + ranges; > + > + pinctrl: pinctrl@0xec003000 { > + compatible = "ambarella,pinctrl"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0xec003000 0x1000>, > + <0xec004000 0x1000>, > + <0xec005000 0x1000>, > + <0xec000000 0x1000>; reg is second > + reg-names = "gpio0", "gpio1", "gpio2", "iomux"; > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; > + amb,pull-regmap = <&scratchpad_syscon 0x14 0x30>; > + amb,ds-regmap = <&rct_syscon>; > + > + gpio: gpio@0 { > + reg = <0>; > + gpio-controller; > + #gpio-cells = <2>; > + gpio-ranges = <&pinctrl 0 0 86>; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + > + uart0_pins: uart0@0 { > + reg = <0>; Why do you have unit addresses here? What is this? > + amb,pinmux-ids = <0x100a 0x100b>; This will need more explanation... your bindings are not ready for review, so we will get to this later when you rewrite your bindings and test them before sending. > + }; > + > + snand0_pins_a: snand0@0 { > + reg = <0>; > + amb,pinmux-ids = <0x2024 0x2025 0x202a 0x202b > + 0x202c 0x202d>; > + }; > + > + snand0_pins_b: snand0@1 { > + reg = <1>; > + amb,pinmux-ids = <0x4036 0x4037 0x4038 0x4039 > + 0x403a 0x403b>; > + }; > + }; > + }; > + > + n_ahb@e0000000 { /* Non-Secure AHB, but configurable */ bus > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0xe0000000 0x01000000>; reg is a second property. > + ranges; > + > + cpuid_syscon: cpuid@e0000000 { > + compatible = "ambarella,cpuid", "syscon"; > + reg = <0xe0000000 0x1000>; > + }; > + > + nand0: nand@e0002000 { > + compatible = "ambarella,nand"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0xe0002000 0x1000>; ditto > + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>; /* fio_irq */ > + pinctrl-names = "default"; > + pinctrl-0 = <&snand0_pins_a>; > + clocks = <&pll_out_enet>; > + nand-on-flash-bbt; > + /* amb,soft-ecc = <6>; */ Drop dead code > + }; > + > + scratchpad_syscon: scratchpad_syscon@e0022000 { > + compatible = "ambarella,scratchpad", "syscon"; > + reg = <0xe0022000 0x100>; > + }; > + }; > + > + rct@ed080000 { bus > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0xed080000 0x1000>; > + ranges; > + > + rct_syscon: rct_syscon@ed080000 { syscon (no underscores in node names, generic node names) > + compatible = "ambarella,rct", "syscon"; > + reg = <0xed080000 0x1000>; > + }; > + }; > + > + clocks { > + compatible = "ambarella,clkpll-v0"; Nope, nope. Register proper clock controller. > + #address-cells = <1>; > + #size-cells = <1>; Why? You do not have any children with unit addresses. > + > + /* > + * This is a dummy clock, to be used as placeholder on other > + * mux clocks when corresponding bits in register don't > + * represent real parent clock. > + */ > + gclk_dummy: gclk_dummy { NAK. Register proper clock controller. > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <0>; > + }; > + > + /* > + * Fixed 24MHz oscillator inputs to SoC > + */ > + osc: oscillator { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; This is actually the only clock which makes sense, but it is not a property of SoC. Put it with the board DTS. At least frequency goes there to note it. > + clock-output-names = "osc"; > + }; > + > + gclk_core: gclk-core { NAK here and for all other weird nodes below > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "gclk_core"; > + amb,clk-regmap = <&rct_syscon 0x000 0x004 0x100 0x104 0x000 0x000>; > + }; > + > + gclk_ddr: gclk-ddr { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "gclk_ddr"; > + amb,clk-regmap = <&rct_syscon 0x0dc 0x0e0 0x110 0x114 0x000 0x000>; > + amb,fix-divider = <2>; > + }; > + > + gclk_cortex: gclk-cortex { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "gclk_cortex"; > + amb,clk-regmap = <&rct_syscon 0x264 0x268 0x26c 0x270 0x000 0x000>; > + }; > + > + gclk_axi: gclk-axi { > + #clock-cells = <0>; > + compatible = "fixed-factor-clock"; > + clocks = <&gclk_cortex>; > + clock-output-names = "gclk_axi"; > + clock-mult = <1>; > + clock-div = <3>; > + }; > + > + gclk_so: gclk-so { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "gclk_so"; > + assigned-clocks = <&gclk_so>; > + assigned-clock-rates = <24000000>; > + amb,clk-regmap = <&rct_syscon 0x024 0x028 0x11c 0x120 0x04c 0x030>; > + amb,frac-mode; > + }; > + > + pll_out_vo2: pll-out-vo2 { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "pll_out_vo2"; > + assigned-clocks = <&pll_out_vo2>; > + assigned-clock-rates = <24000000>; > + amb,clk-regmap = <&rct_syscon 0x0c0 0x0c4 0x13c 0x140 0x0c8 0x000>; > + amb,frac-mode; > + }; > + > + pll_out_sd: pll-out-sd { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "pll_out_sd"; > + amb,clk-regmap = <&rct_syscon 0x4ac 0x4b0 0x4b4 0x4b8 0x000 0x000>; > + }; > + pll_out_enet: pll-out-enet { > + #clock-cells = <0>; > + compatible = "ambarella,pll-clock"; > + clocks = <&osc>; > + clock-output-names = "pll_out_enet"; > + amb,clk-regmap = <&rct_syscon 0x520 0x524 0x528 0x52c 0x000 0x000>; > + }; > + gclk_uart0: gclk-uart0 { > + #clock-cells = <0>; > + compatible = "ambarella,composite-clock"; > + clocks = <&osc>, <&gclk_core>, <&pll_out_enet>, <&pll_out_sd>; > + clock-output-names = "gclk_uart0"; > + assigned-clocks = <&gclk_uart0>; > + assigned-clock-parents = <&osc>; > + assigned-clock-rates = <24000000>; > + amb,mux-regmap = <&rct_syscon 0x1c8>; > + amb,div-regmap = <&rct_syscon 0x038>; > + amb,div-width = <24>; > + amb,div-shift = <0>; > + }; > + }; > +}; > diff --git a/arch/arm64/boot/dts/ambarella/s6lm_pineapple.dts b/arch/arm64/boot/dts/ambarella/s6lm_pineapple.dts > new file mode 100644 > index 000000000000..2d5d6d18e738 > --- /dev/null > +++ b/arch/arm64/boot/dts/ambarella/s6lm_pineapple.dts > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (C) 2023 Ambarella,Inc. - http://www.ambarella.com/ > + * Author: Cao Rongrong <rrcao@xxxxxxxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +/dts-v1/; > + > +#include "ambarella-s6lm.dtsi" > + > +/ { > + model = "Ambarella S6LM Pineapple Board"; > + compatible = "ambarella,s6lm"; Missing board compatible. > + > + chosen { > + bootargs = "console=ttyS0 ubi.mtd=lnx root=ubi0:rootfs rw > + rootfstype=ubifs init=/linuxrc"; Drop bootargs. These should not be needed in DTS. Otherwise - why do you need them? Why bootloader cannot provide these? > + }; > + > + n_apb@e4000000 { > + i2c1: i2c@e4009000 { No, use override by phandle/label. > + status = "ok"; > + }; > + }; > +}; Best regards, Krzysztof
