Hi Guo:
On 16:22 Mon 29 May , Guo Ren wrote:
On Mon, May 29, 2023 at 3:54 PM Yixun Lan dlan@gentoo.org wrote:
Hi Guo:
On 14:50 Mon 29 May , Guo Ren wrote:
On Mon, May 29, 2023 at 11:01 AM Yixun Lan dlan@gentoo.org wrote:
Hi Guo:
see my comment below.
On 09:19 Mon 29 May , Guo Ren wrote:
On Sat, May 27, 2023 at 5:17 PM Yixun Lan dlan@gentoo.org wrote:
Hi Guo:
On 09:43 Sat 27 May , Guo Ren wrote: > Sorry, why we need dts here? If we put dts here, we could delete the > one in Linux. No, I think it's more than a historical reason for why we have two dts both in u-boot and kernel. And this dts here is merely used by u-boot, it could be a simplified version comparing to kernel's dts.
> > We shouldn't put it with two places, that would be bad for maintanice. I can totally understand your concern, in fact, we are trying to keep them sync, so I will probably wait the dts of kernel settle down, before take action here. so, please conside this patch as RFC, and may change in next revisions..
Thx for clarification. But I still concern the necessary of the patch, could you move this from this series first, because we've argument on it. Your other patches looks good, I'm looking forward your next v2.
But for this one, let's talk it after kernel side merged.
I'd like to include dts in the first series, otherwise it won't be a complete working series, e.g. - not only it's unable to build, also can't do run-time test
I couldn't make sense how it block your build, The build operation is controlled by you Makefile modification. right?
you mostly right, but in patch "[ 3/4 ] configs: th1520_lpi4a_defconfig: Add initial config", we will enable device tree, and require CONFIG_DEFAULT_DEVICE_TREE="th1520-lichee-pi-4a.dtb" to be provided, also since UART's base register is parsed from dts (run time), so I see no reason why the dts patch should be separated
Oh, I see; u-boot needs a seperate dtb to boot indepedent. Maybe, we just keep a minimum dts here, then we needn't syncronize with the kernel side.
yes, that's the plan, we will keep necessary dts here, and try to keep it sync with kernel when needed..
diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile index 79a58694f5..72fd815a40 100644 --- a/arch/riscv/dts/Makefile +++ b/arch/riscv/dts/Makefile @@ -9,6 +9,7 @@ dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb dtb-$(CONFIG_TARGET_STARFIVE_VISIONFIVE2) += jh7110-starfive-visionfive-2-v1.3b.dtb dtb-$(CONFIG_TARGET_STARFIVE_VISIONFIVE2) += jh7110-starfive-visionfive-2-v1.2a.dtb +dtb-$(CONFIG_TARGET_TH1520_LPI4A) += th1520-lichee-pi-4a.dtb include $(srctree)/scripts/Makefile.dts
I'm totally fine with kernel dts merged first, then submit this later, so no push..
> > On Fri, May 26, 2023 at 8:41 PM Yixun Lan dlan@gentoo.org wrote: > > > > Only add basic support for CPU, PLIC UART and Timer. > > > > Reviewed-by: Wei Fu wefu@redhat.com > > Signed-off-by: Yixun Lan dlan@gentoo.org > > --- > > arch/riscv/dts/Makefile | 1 + > > arch/riscv/dts/th1520-lichee-module-4a.dtsi | 34 ++ > > arch/riscv/dts/th1520-lichee-pi-4a.dts | 32 ++ > > arch/riscv/dts/th1520.dtsi | 435 ++++++++++++++++++++ > > 4 files changed, 502 insertions(+) > > create mode 100644 arch/riscv/dts/th1520-lichee-module-4a.dtsi > > create mode 100644 arch/riscv/dts/th1520-lichee-pi-4a.dts > > create mode 100644 arch/riscv/dts/th1520.dtsi > > > > diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile > > index 79a58694f5..72fd815a40 100644 > > --- a/arch/riscv/dts/Makefile > > +++ b/arch/riscv/dts/Makefile > > @@ -9,6 +9,7 @@ dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb > > dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb > > dtb-$(CONFIG_TARGET_STARFIVE_VISIONFIVE2) += jh7110-starfive-visionfive-2-v1.3b.dtb > > dtb-$(CONFIG_TARGET_STARFIVE_VISIONFIVE2) += jh7110-starfive-visionfive-2-v1.2a.dtb > > +dtb-$(CONFIG_TARGET_TH1520_LPI4A) += th1520-lichee-pi-4a.dtb > > include $(srctree)/scripts/Makefile.dts > > > > targets += $(dtb-y) > > diff --git a/arch/riscv/dts/th1520-lichee-module-4a.dtsi b/arch/riscv/dts/th1520-lichee-module-4a.dtsi > > new file mode 100644 > > index 0000000000..dc00e3dfa0 > > --- /dev/null > > +++ b/arch/riscv/dts/th1520-lichee-module-4a.dtsi > > @@ -0,0 +1,34 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2023 Jisheng Zhang jszhang@kernel.org > > + */ > > + > > +/dts-v1/; > > + > > +#include "th1520.dtsi" > > + > > +/ { > > + model = "Sipeed Lichee Module 4A"; > > + compatible = "sipeed,lichee-module-4a", "thead,th1520"; > > + > > + memory@0 { > > + device_type = "memory"; > > + reg = <0x0 0x00000000 0x2 0x00000000>; > > + }; > > +}; > > + > > +&osc { > > + clock-frequency = <24000000>; > > +}; > > + > > +&osc_32k { > > + clock-frequency = <32768>; > > +}; > > + > > +&apb_clk { > > + clock-frequency = <62500000>; > > +}; > > + > > +&uart_sclk { > > + clock-frequency = <100000000>; > > +}; > > diff --git a/arch/riscv/dts/th1520-lichee-pi-4a.dts b/arch/riscv/dts/th1520-lichee-pi-4a.dts > > new file mode 100644 > > index 0000000000..a1248b2ee3 > > --- /dev/null > > +++ b/arch/riscv/dts/th1520-lichee-pi-4a.dts > > @@ -0,0 +1,32 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2023 Jisheng Zhang jszhang@kernel.org > > + */ > > + > > +#include "th1520-lichee-module-4a.dtsi" > > + > > +/ { > > + model = "Sipeed Lichee Pi 4A"; > > + compatible = "sipeed,lichee-pi-4a", "sipeed,lichee-module-4a", "thead,th1520"; > > + > > + aliases { > > + gpio0 = &gpio0; > > + gpio1 = &gpio1; > > + gpio2 = &gpio2; > > + gpio3 = &gpio3; > > + serial0 = &uart0; > > + serial1 = &uart1; > > + serial2 = &uart2; > > + serial3 = &uart3; > > + serial4 = &uart4; > > + serial5 = &uart5; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > +}; > > + > > +&uart0 { > > + status = "okay"; > > +}; > > diff --git a/arch/riscv/dts/th1520.dtsi b/arch/riscv/dts/th1520.dtsi > > new file mode 100644 > > index 0000000000..f62a62da6e > > --- /dev/null > > +++ b/arch/riscv/dts/th1520.dtsi > > @@ -0,0 +1,435 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > + * Copyright (C) 2023 Jisheng Zhang jszhang@kernel.org > > + */ > > + > > +#include <dt-bindings/interrupt-controller/irq.h> > > + > > +/ { > > + compatible = "thead,th1520"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + cpus: cpus {
We needn't multi-cpus definitions.
> > + #address-cells = <1>; > > + #size-cells = <0>; > > + timebase-frequency = <3000000>; > > + > > + c910_0: cpu@0 { > > + compatible = "thead,c910", "riscv"; > > + device_type = "cpu"; > > + riscv,isa = "rv64imafdc"; > > + reg = <0>; > > + i-cache-block-size = <64>; > > + i-cache-size = <65536>; > > + i-cache-sets = <512>; > > + d-cache-block-size = <64>; > > + d-cache-size = <65536>; > > + d-cache-sets = <512>; > > + next-level-cache = <&l2_cache>; > > + mmu-type = "riscv,sv39"; > > + > > + cpu0_intc: interrupt-controller { > > + compatible = "riscv,cpu-intc"; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + }; > > + }; > > + > > + c910_1: cpu@1 { > > + compatible = "thead,c910", "riscv"; > > + device_type = "cpu"; > > + riscv,isa = "rv64imafdc"; > > + reg = <1>; > > + i-cache-block-size = <64>; > > + i-cache-size = <65536>; > > + i-cache-sets = <512>; > > + d-cache-block-size = <64>; > > + d-cache-size = <65536>; > > + d-cache-sets = <512>; > > + next-level-cache = <&l2_cache>; > > + mmu-type = "riscv,sv39"; > > + > > + cpu1_intc: interrupt-controller { > > + compatible = "riscv,cpu-intc"; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + }; > > + }; > > + > > + c910_2: cpu@2 { > > + compatible = "thead,c910", "riscv"; > > + device_type = "cpu"; > > + riscv,isa = "rv64imafdc"; > > + reg = <2>; > > + i-cache-block-size = <64>; > > + i-cache-size = <65536>; > > + i-cache-sets = <512>; > > + d-cache-block-size = <64>; > > + d-cache-size = <65536>; > > + d-cache-sets = <512>; > > + next-level-cache = <&l2_cache>; > > + mmu-type = "riscv,sv39"; > > + > > + cpu2_intc: interrupt-controller { > > + compatible = "riscv,cpu-intc"; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + }; > > + }; > > + > > + c910_3: cpu@3 { > > + compatible = "thead,c910", "riscv"; > > + device_type = "cpu"; > > + riscv,isa = "rv64imafdc"; > > + reg = <3>; > > + i-cache-block-size = <64>; > > + i-cache-size = <65536>; > > + i-cache-sets = <512>; > > + d-cache-block-size = <64>; > > + d-cache-size = <65536>; > > + d-cache-sets = <512>; > > + next-level-cache = <&l2_cache>; > > + mmu-type = "riscv,sv39"; > > + > > + cpu3_intc: interrupt-controller { > > + compatible = "riscv,cpu-intc"; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + }; > > + }; > > + > > + cpu-map { > > + cluster0 { > > + core0 { > > + cpu = <&c910_0>; > > + }; > > + > > + core1 { > > + cpu = <&c910_1>; > > + }; > > + > > + core2 { > > + cpu = <&c910_2>; > > + }; > > + > > + core3 { > > + cpu = <&c910_3>; > > + }; > > + }; > > + }; > > + > > + l2_cache: l2-cache {
We also needn't l2_cache description for u-boot.
> > + compatible = "cache"; > > + cache-block-size = <64>; > > + cache-level = <2>; > > + cache-size = <1048576>; > > + cache-sets = <1024>; > > + cache-unified; > > + }; > > + }; > > + > > + osc: oscillator {
Does u-boot need it?
> > + compatible = "fixed-clock"; > > + clock-output-names = "osc_24m"; > > + #clock-cells = <0>; > > + }; > > + > > + osc_32k: 32k-oscillator {
Does u-boot need it?
> > + compatible = "fixed-clock"; > > + clock-output-names = "osc_32k"; > > + #clock-cells = <0>; > > + }; > > + > > + apb_clk: apb-clk-clock {
Does u-boot need it?
> > + compatible = "fixed-clock"; > > + clock-output-names = "apb_clk"; > > + #clock-cells = <0>; > > + }; > > + > > + uart_sclk: uart-sclk-clock { > > + compatible = "fixed-clock"; > > + clock-output-names = "uart_sclk"; > > + #clock-cells = <0>; > > + }; > > + > > + soc { > > + compatible = "simple-bus"; > > + interrupt-parent = <&plic>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + cpurst: cpurst {
The u-boot needn't it
> > + compatible = "thead,reset-th1520"; > > + entry-reg = <0xff 0xff019050>; > > + entry-cnt = <4>; > > + control-reg = <0xff 0xff015004>; > > + control-val = <0x1c>; > > + csr-copy = <0x7f3 0x7c0 0x7c1 0x7c2 0x7c3 0x7c5 0x7cc>; > > + }; > This part has been changed. > noted, will update accordingly
> > + > > + plic: interrupt-controller@ffd8000000 { > > + compatible = "thead,th1520-plic", "thead,c900-plic"; > > + reg = <0xff 0xd8000000 0x0 0x01000000>; > > + interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>, > > + <&cpu1_intc 11>, <&cpu1_intc 9>, > > + <&cpu2_intc 11>, <&cpu2_intc 9>, > > + <&cpu3_intc 11>, <&cpu3_intc 9>; > > + interrupt-controller; > > + #address-cells = <0>; > > + #interrupt-cells = <2>; > > + riscv,ndev = <240>; > > + }; > > + > > + clint: timer@ffdc000000 { > > + compatible = "thead,th1520-clint", "thead,c900-clint"; > > + reg = <0xff 0xdc000000 0x0 0x00010000>; > > + interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>, > > + <&cpu1_intc 3>, <&cpu1_intc 7>, > > + <&cpu2_intc 3>, <&cpu2_intc 7>, > > + <&cpu3_intc 3>, <&cpu3_intc 7>; > > + }; > > + > > + uart0: serial@ffe7014000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xe7014000 0x0 0x4000>; > > + interrupts = <36 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + uart1: serial@ffe7f00000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xe7f00000 0x0 0x4000>; > > + interrupts = <37 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + uart3: serial@ffe7f04000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xe7f04000 0x0 0x4000>; > > + interrupts = <39 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + gpio2: gpio@ffe7f34000 { > > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xe7f34000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + portc: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <58 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + > > + gpio3: gpio@ffe7f38000 {
Do we enable the gpio driver in u-boot?
> > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xe7f38000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + portd: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <59 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + > > + gpio0: gpio@ffec005000 { > > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xec005000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + porta: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <56 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + > > + gpio1: gpio@ffec006000 { > > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xec006000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + portb: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <57 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + > > + uart2: serial@ffec010000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xec010000 0x0 0x4000>; > > + interrupts = <38 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + timer0: timer@ffefc32000 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xefc32000 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer1: timer@ffefc32014 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xefc32014 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer2: timer@ffefc32028 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xefc32028 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <18 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer3: timer@ffefc3203c { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xefc3203c 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <19 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + uart4: serial@fff7f08000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xf7f08000 0x0 0x4000>; > > + interrupts = <40 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + uart5: serial@fff7f0c000 { > > + compatible = "snps,dw-apb-uart"; > > + reg = <0xff 0xf7f0c000 0x0 0x4000>; > > + interrupts = <41 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&uart_sclk>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + status = "disabled"; > > + }; > > + > > + timer4: timer@ffffc33000 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xffc33000 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer5: timer@ffffc33014 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xffc33014 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer6: timer@ffffc33028 { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xffc33028 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <22 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + timer7: timer@ffffc3303c { > > + compatible = "snps,dw-apb-timer"; > > + reg = <0xff 0xffc3303c 0x0 0x14>; > > + clocks = <&apb_clk>; > > + clock-names = "timer"; > > + interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; > > + status = "disabled"; > > + }; > > + > > + ao_gpio0: gpio@fffff41000 { > > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xfff41000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + porte: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <76 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + > > + ao_gpio1: gpio@fffff52000 { > > + compatible = "snps,dw-apb-gpio"; > > + reg = <0xff 0xfff52000 0x0 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + portf: gpio-controller@0 { > > + compatible = "snps,dw-apb-gpio-port"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + ngpios = <32>; > > + reg = <0>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + interrupts = <55 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > + }; > > +}; > > -- > > 2.40.0 > > > > > -- > Best Regards > Guo Ren
-- Yixun Lan (dlan) Gentoo Linux Developer GPG Key ID AABEFD55
-- Best Regards Guo Ren
-- Yixun Lan (dlan) Gentoo Linux Developer GPG Key ID AABEFD55
-- Best Regards Guo Ren
-- Yixun Lan (dlan) Gentoo Linux Developer GPG Key ID AABEFD55
-- Best Regards Guo Ren