diff mbox series

[RFC,v2,09/11] riscv: soc: Initial DTS for Allwinner D1 NeZha board

Message ID 1622970249-50770-13-git-send-email-guoren@kernel.org (mailing list archive)
State New, archived
Headers show
Series riscv: Add DMA_COHERENT support for Allwinner D1 | expand

Commit Message

Guo Ren June 6, 2021, 9:04 a.m. UTC
From: Guo Ren <guoren@linux.alibaba.com>

Add initial DTS for Allwinner D1 NeZha board having only essential
devices (uart, dummy, clock, reset, clint, plic, etc).

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Drew Fustini <drew@beagleboard.org>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Palmer Dabbelt <palmerdabbelt@google.com>
Cc: Wei Fu <wefu@redhat.com>
Cc: Wei Wu <lazyparser@gmail.com>
---
 arch/riscv/boot/dts/Makefile                       |  1 +
 arch/riscv/boot/dts/allwinner/Makefile             |  2 +
 .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84 ++++++++++++++++++++++
 4 files changed, 116 insertions(+)
 create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
 create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
 create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi

Comments

Jernej Škrabec June 6, 2021, 4:26 p.m. UTC | #1
Hi!

I didn't go through all details. After you fix all comments below, you should 
run "make dtbs_check" and fix all reported warnings too.

Dne nedelja, 06. junij 2021 ob 11:04:07 CEST je guoren@kernel.org napisal(a):
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> Add initial DTS for Allwinner D1 NeZha board having only essential
> devices (uart, dummy, clock, reset, clint, plic, etc).
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Drew Fustini <drew@beagleboard.org>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> Cc: Wei Fu <wefu@redhat.com>
> Cc: Wei Wu <lazyparser@gmail.com>
> ---
>  arch/riscv/boot/dts/Makefile                       |  1 +
>  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
>  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
>  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84
> ++++++++++++++++++++++ 4 files changed, 116 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
>  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> 
> diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> index fe996b8..3e7b264 100644
> --- a/arch/riscv/boot/dts/Makefile
> +++ b/arch/riscv/boot/dts/Makefile
> @@ -2,5 +2,6 @@
>  subdir-y += sifive
>  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
>  subdir-y += microchip
> +subdir-y += allwinner
> 
>  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> diff --git a/arch/riscv/boot/dts/allwinner/Makefile
> b/arch/riscv/boot/dts/allwinner/Makefile new file mode 100644
> index 00000000..4adbf4b
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts new file mode
> 100644
> index 00000000..cd9f7c9
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts

Board DT names are comprised of soc name and board name, in this case it would 
be "sun20i-d1-nezha-kit.dts"

> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)

Usually copyrights are added below spdx id.

> +
> +/dts-v1/;
> +
> +#include "allwinner-d1.dtsi"
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;

This should be part of SoC level DTSI.

> +	model = "Allwinner D1 NeZha Kit";
> +	compatible = "allwinner,d1-nezha-kit";

Board specific compatible string should be followed with SoC compatible, in 
this case "allwinner,sun20i-d1".  You should document it too.

> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200";

Above line doesn't belong here. If anything, it should be added dynamically by 
bootloader.

> +		stdout-path = &serial0;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0x0 0x20000000>;
> +	};

Ditto for whole memory node.

> +
> +	soc {
> +	};

There is no point having empty nodes.

> +};
> +
> +&serial0 {
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi new file mode 100644
> index 00000000..11cd938
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi

Current naming approach for Allwinner SoC level DTSI is "sun20i-d1.dtsi".

> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)

> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;

Since all peripherals and memory are below 4 GiB, why have 64-bit addresses 
and sizes? It just clutters DT.

> +	model = "Allwinner D1 Soc";
> +	compatible = "allwinner,d1-nezha-kit";

Compatible and model don't belong to SoC level DTSI.

> +
> +	chosen {
> +	};

Remove empty node.

> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <2400000>;
> +		cpu@0 {
> +			device_type = "cpu";
> +			reg = <0>;
> +			status = "okay";
> +			compatible = "riscv";
> +			riscv,isa = "rv64imafdcv";
> +			mmu-type = "riscv,sv39";
> +			cpu0_intc: interrupt-controller {
> +				#interrupt-cells = <1>;
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +			};
> +		};
> +	};
> +
> +	soc {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		reset: reset-sample {
> +			compatible = "thead,reset-sample";
> +			plic-delegate = <0x0 0x101ffffc>;
> +		};
> +
> +		clint: clint@14000000 {
> +			compatible = "riscv,clint0";
> +			interrupts-extended = <
> +				&cpu0_intc  3 &cpu0_intc  7
> +				>;
> +			reg = <0x0 0x14000000 0x0 0x04000000>;
> +			clint,has-no-64bit-mmio;
> +		};
> +
> +		plic: interrupt-controller@10000000 {
> +			#interrupt-cells = <1>;
> +			compatible = "riscv,plic0";
> +			interrupt-controller;
> +			interrupts-extended = <
> +				&cpu0_intc  0xffffffff &cpu0_intc  9
> +				>;
> +			reg = <0x0 0x10000000 0x0 0x04000000>;
> +			reg-names = "control";
> +			riscv,max-priority = <7>;
> +			riscv,ndev = <200>;
> +		};
> +
> +		dummy_apb: apb-clock {
> +			compatible = "fixed-clock";
> +			clock-frequency = <24000000>;
> +			clock-output-names = "dummy_apb";
> +			#clock-cells = <0>;
> +		};
> +
> +		serial0: serial@2500000 {

This should be uart0 and board should have alias for it. Check ARM based 
Allwinner DTs.

Best regards,
Jernej

> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x0 0x02500000 0x0 0x400>;
> +			reg-io-width = <4>;
> +			reg-shift = <2>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <18>;
> +			clocks = <&dummy_apb>;
> +			status = "disabled";
> +		};
> +	};
> +};
Guo Ren June 6, 2021, 5:05 p.m. UTC | #2
On Mon, Jun 7, 2021 at 12:26 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi!
>
> I didn't go through all details. After you fix all comments below, you should
> run "make dtbs_check" and fix all reported warnings too.
>
> Dne nedelja, 06. junij 2021 ob 11:04:07 CEST je guoren@kernel.org napisal(a):
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Add initial DTS for Allwinner D1 NeZha board having only essential
> > devices (uart, dummy, clock, reset, clint, plic, etc).
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> > Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Drew Fustini <drew@beagleboard.org>
> > Cc: Maxime Ripard <maxime@cerno.tech>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > Cc: Wei Fu <wefu@redhat.com>
> > Cc: Wei Wu <lazyparser@gmail.com>
> > ---
> >  arch/riscv/boot/dts/Makefile                       |  1 +
> >  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
> >  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
> >  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84
> > ++++++++++++++++++++++ 4 files changed, 116 insertions(+)
> >  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
> >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> >
> > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > index fe996b8..3e7b264 100644
> > --- a/arch/riscv/boot/dts/Makefile
> > +++ b/arch/riscv/boot/dts/Makefile
> > @@ -2,5 +2,6 @@
> >  subdir-y += sifive
> >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
> >  subdir-y += microchip
> > +subdir-y += allwinner
> >
> >  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> > diff --git a/arch/riscv/boot/dts/allwinner/Makefile
> > b/arch/riscv/boot/dts/allwinner/Makefile new file mode 100644
> > index 00000000..4adbf4b
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts new file mode
> > 100644
> > index 00000000..cd9f7c9
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
>
> Board DT names are comprised of soc name and board name, in this case it would
> be "sun20i-d1-nezha-kit.dts"
>
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> Usually copyrights are added below spdx id.
No certain copyrights, maybe let allwinner added later.

>
> > +
> > +/dts-v1/;
> > +
> > +#include "allwinner-d1.dtsi"
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
>
> This should be part of SoC level DTSI.
>
> > +     model = "Allwinner D1 NeZha Kit";
> > +     compatible = "allwinner,d1-nezha-kit";
>
> Board specific compatible string should be followed with SoC compatible, in
> this case "allwinner,sun20i-d1".  You should document it too.
Okay

>
> > +
> > +     chosen {
> > +             bootargs = "console=ttyS0,115200";
>
> Above line doesn't belong here. If anything, it should be added dynamically by
> bootloader.
Okay

>
> > +             stdout-path = &serial0;
> > +     };
> > +
> > +     memory@40000000 {
> > +             device_type = "memory";
> > +             reg = <0x0 0x40000000 0x0 0x20000000>;
> > +     };
>
> Ditto for whole memory node.
Okay remove it and let u-boot append.

>
> > +
> > +     soc {
> > +     };
>
> There is no point having empty nodes.
Okay.

>
> > +};
> > +
> > +&serial0 {
> > +     status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi new file mode 100644
> > index 00000000..11cd938
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
>
> Current naming approach for Allwinner SoC level DTSI is "sun20i-d1.dtsi".
>
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
>
> Since all peripherals and memory are below 4 GiB, why have 64-bit addresses
> and sizes? It just clutters DT.
>
> > +     model = "Allwinner D1 Soc";
> > +     compatible = "allwinner,d1-nezha-kit";
>
> Compatible and model don't belong to SoC level DTSI.
>
> > +
> > +     chosen {
> > +     };
>
> Remove empty node.
Okay.

>
> > +
> > +     cpus {
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +             timebase-frequency = <2400000>;
> > +             cpu@0 {
> > +                     device_type = "cpu";
> > +                     reg = <0>;
> > +                     status = "okay";
> > +                     compatible = "riscv";
> > +                     riscv,isa = "rv64imafdcv";
> > +                     mmu-type = "riscv,sv39";
> > +                     cpu0_intc: interrupt-controller {
> > +                             #interrupt-cells = <1>;
> > +                             compatible = "riscv,cpu-intc";
> > +                             interrupt-controller;
> > +                     };
> > +             };
> > +     };
> > +
> > +     soc {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             compatible = "simple-bus";
> > +             ranges;
> > +
> > +             reset: reset-sample {
> > +                     compatible = "thead,reset-sample";
> > +                     plic-delegate = <0x0 0x101ffffc>;
> > +             };
> > +
> > +             clint: clint@14000000 {
> > +                     compatible = "riscv,clint0";
> > +                     interrupts-extended = <
> > +                             &cpu0_intc  3 &cpu0_intc  7
> > +                             >;
> > +                     reg = <0x0 0x14000000 0x0 0x04000000>;
> > +                     clint,has-no-64bit-mmio;
> > +             };
> > +
> > +             plic: interrupt-controller@10000000 {
> > +                     #interrupt-cells = <1>;
> > +                     compatible = "riscv,plic0";
> > +                     interrupt-controller;
> > +                     interrupts-extended = <
> > +                             &cpu0_intc  0xffffffff &cpu0_intc  9
> > +                             >;
> > +                     reg = <0x0 0x10000000 0x0 0x04000000>;
> > +                     reg-names = "control";
> > +                     riscv,max-priority = <7>;
> > +                     riscv,ndev = <200>;
> > +             };
> > +
> > +             dummy_apb: apb-clock {
> > +                     compatible = "fixed-clock";
> > +                     clock-frequency = <24000000>;
> > +                     clock-output-names = "dummy_apb";
> > +                     #clock-cells = <0>;
> > +             };
> > +
> > +             serial0: serial@2500000 {
>
> This should be uart0 and board should have alias for it. Check ARM based
> Allwinner DTs.
Okay, use the uart0 alias.

>
> Best regards,
> Jernej
>
> > +                     compatible = "snps,dw-apb-uart";
> > +                     reg = <0x0 0x02500000 0x0 0x400>;
> > +                     reg-io-width = <4>;
> > +                     reg-shift = <2>;
> > +                     interrupt-parent = <&plic>;
> > +                     interrupts = <18>;
> > +                     clocks = <&dummy_apb>;
> > +                     status = "disabled";
> > +             };
> > +     };
> > +};
>
>
>
>
Guo Ren June 7, 2021, 3:44 a.m. UTC | #3
On Mon, Jun 7, 2021 at 12:26 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi!
>
> I didn't go through all details. After you fix all comments below, you should
> run "make dtbs_check" and fix all reported warnings too.
>
> Dne nedelja, 06. junij 2021 ob 11:04:07 CEST je guoren@kernel.org napisal(a):
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Add initial DTS for Allwinner D1 NeZha board having only essential
> > devices (uart, dummy, clock, reset, clint, plic, etc).
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> > Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Drew Fustini <drew@beagleboard.org>
> > Cc: Maxime Ripard <maxime@cerno.tech>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > Cc: Wei Fu <wefu@redhat.com>
> > Cc: Wei Wu <lazyparser@gmail.com>
> > ---
> >  arch/riscv/boot/dts/Makefile                       |  1 +
> >  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
> >  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
> >  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84
> > ++++++++++++++++++++++ 4 files changed, 116 insertions(+)
> >  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
> >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> >
> > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > index fe996b8..3e7b264 100644
> > --- a/arch/riscv/boot/dts/Makefile
> > +++ b/arch/riscv/boot/dts/Makefile
> > @@ -2,5 +2,6 @@
> >  subdir-y += sifive
> >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
> >  subdir-y += microchip
> > +subdir-y += allwinner
> >
> >  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> > diff --git a/arch/riscv/boot/dts/allwinner/Makefile
> > b/arch/riscv/boot/dts/allwinner/Makefile new file mode 100644
> > index 00000000..4adbf4b
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts new file mode
> > 100644
> > index 00000000..cd9f7c9
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
>
> Board DT names are comprised of soc name and board name, in this case it would
> be "sun20i-d1-nezha-kit.dts"
>
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> Usually copyrights are added below spdx id.
>
> > +
> > +/dts-v1/;
> > +
> > +#include "allwinner-d1.dtsi"
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
>
> This should be part of SoC level DTSI.
>
> > +     model = "Allwinner D1 NeZha Kit";
> > +     compatible = "allwinner,d1-nezha-kit";
>
> Board specific compatible string should be followed with SoC compatible, in
> this case "allwinner,sun20i-d1".  You should document it too.
>
> > +
> > +     chosen {
> > +             bootargs = "console=ttyS0,115200";
>
> Above line doesn't belong here. If anything, it should be added dynamically by
> bootloader.
After discussion, we still want to keep a default value here.
Sometimes we could boot with jtag and parse dtb is hard for gdbinit
script.

>
> > +             stdout-path = &serial0;
> > +     };
> > +
> > +     memory@40000000 {
> > +             device_type = "memory";
> > +             reg = <0x0 0x40000000 0x0 0x20000000>;
> > +     };
>
> Ditto for whole memory node.
Ditto

>
> > +
> > +     soc {
> > +     };
>
> There is no point having empty nodes.
>
> > +};
> > +
> > +&serial0 {
> > +     status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi new file mode 100644
> > index 00000000..11cd938
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
>
> Current naming approach for Allwinner SoC level DTSI is "sun20i-d1.dtsi".
>
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
>
> Since all peripherals and memory are below 4 GiB, why have 64-bit addresses
> and sizes? It just clutters DT.
>
> > +     model = "Allwinner D1 Soc";
> > +     compatible = "allwinner,d1-nezha-kit";
>
> Compatible and model don't belong to SoC level DTSI.
>
> > +
> > +     chosen {
> > +     };
>
> Remove empty node.
>
> > +
> > +     cpus {
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +             timebase-frequency = <2400000>;
> > +             cpu@0 {
> > +                     device_type = "cpu";
> > +                     reg = <0>;
> > +                     status = "okay";
> > +                     compatible = "riscv";
> > +                     riscv,isa = "rv64imafdcv";
> > +                     mmu-type = "riscv,sv39";
> > +                     cpu0_intc: interrupt-controller {
> > +                             #interrupt-cells = <1>;
> > +                             compatible = "riscv,cpu-intc";
> > +                             interrupt-controller;
> > +                     };
> > +             };
> > +     };
> > +
> > +     soc {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             compatible = "simple-bus";
> > +             ranges;
> > +
> > +             reset: reset-sample {
> > +                     compatible = "thead,reset-sample";
> > +                     plic-delegate = <0x0 0x101ffffc>;
> > +             };
> > +
> > +             clint: clint@14000000 {
> > +                     compatible = "riscv,clint0";
> > +                     interrupts-extended = <
> > +                             &cpu0_intc  3 &cpu0_intc  7
> > +                             >;
> > +                     reg = <0x0 0x14000000 0x0 0x04000000>;
> > +                     clint,has-no-64bit-mmio;
> > +             };
> > +
> > +             plic: interrupt-controller@10000000 {
> > +                     #interrupt-cells = <1>;
> > +                     compatible = "riscv,plic0";
> > +                     interrupt-controller;
> > +                     interrupts-extended = <
> > +                             &cpu0_intc  0xffffffff &cpu0_intc  9
> > +                             >;
> > +                     reg = <0x0 0x10000000 0x0 0x04000000>;
> > +                     reg-names = "control";
> > +                     riscv,max-priority = <7>;
> > +                     riscv,ndev = <200>;
> > +             };
> > +
> > +             dummy_apb: apb-clock {
> > +                     compatible = "fixed-clock";
> > +                     clock-frequency = <24000000>;
> > +                     clock-output-names = "dummy_apb";
> > +                     #clock-cells = <0>;
> > +             };
> > +
> > +             serial0: serial@2500000 {
>
> This should be uart0 and board should have alias for it. Check ARM based
> Allwinner DTs.
>
> Best regards,
> Jernej
>
> > +                     compatible = "snps,dw-apb-uart";
> > +                     reg = <0x0 0x02500000 0x0 0x400>;
> > +                     reg-io-width = <4>;
> > +                     reg-shift = <2>;
> > +                     interrupt-parent = <&plic>;
> > +                     interrupts = <18>;
> > +                     clocks = <&dummy_apb>;
> > +                     status = "disabled";
> > +             };
> > +     };
> > +};
>
>
>
>
Maxime Ripard June 7, 2021, 7:24 a.m. UTC | #4
Hi,

Thanks for the patches

On Sun, Jun 06, 2021 at 09:04:07AM +0000, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> Add initial DTS for Allwinner D1 NeZha board having only essential
> devices (uart, dummy, clock, reset, clint, plic, etc).
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Drew Fustini <drew@beagleboard.org>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> Cc: Wei Fu <wefu@redhat.com>
> Cc: Wei Wu <lazyparser@gmail.com>
> ---
>  arch/riscv/boot/dts/Makefile                       |  1 +
>  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
>  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
>  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84 ++++++++++++++++++++++

Can you add the riscv folder to our MAINTAINERS entry as well?

>  4 files changed, 116 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
>  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
>  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> 
> diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> index fe996b8..3e7b264 100644
> --- a/arch/riscv/boot/dts/Makefile
> +++ b/arch/riscv/boot/dts/Makefile
> @@ -2,5 +2,6 @@
>  subdir-y += sifive
>  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
>  subdir-y += microchip
> +subdir-y += allwinner

I assume they should be ordered alphabetically?

>  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> diff --git a/arch/riscv/boot/dts/allwinner/Makefile b/arch/riscv/boot/dts/allwinner/Makefile
> new file mode 100644
> index 00000000..4adbf4b
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> new file mode 100644
> index 00000000..cd9f7c9
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include "allwinner-d1.dtsi"
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	model = "Allwinner D1 NeZha Kit";
> +	compatible = "allwinner,d1-nezha-kit";
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200";
> +		stdout-path = &serial0;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0x0 0x20000000>;
> +	};
> +
> +	soc {
> +	};
> +};
> +
> +&serial0 {
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> new file mode 100644
> index 00000000..11cd938
> --- /dev/null
> +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	model = "Allwinner D1 Soc";
> +	compatible = "allwinner,d1-nezha-kit";
> +
> +	chosen {
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <2400000>;
> +		cpu@0 {
> +			device_type = "cpu";
> +			reg = <0>;
> +			status = "okay";
> +			compatible = "riscv";
> +			riscv,isa = "rv64imafdcv";
> +			mmu-type = "riscv,sv39";
> +			cpu0_intc: interrupt-controller {
> +				#interrupt-cells = <1>;
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +			};
> +		};
> +	};
> +
> +	soc {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		reset: reset-sample {
> +			compatible = "thead,reset-sample";
> +			plic-delegate = <0x0 0x101ffffc>;
> +		};

This compatible is not documented anywhere?

Maxime
Maxime Ripard June 7, 2021, 7:27 a.m. UTC | #5
On Mon, Jun 07, 2021 at 11:44:03AM +0800, Guo Ren wrote:
> On Mon, Jun 7, 2021 at 12:26 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> >
> > Hi!
> >
> > I didn't go through all details. After you fix all comments below, you should
> > run "make dtbs_check" and fix all reported warnings too.
> >
> > Dne nedelja, 06. junij 2021 ob 11:04:07 CEST je guoren@kernel.org napisal(a):
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > Add initial DTS for Allwinner D1 NeZha board having only essential
> > > devices (uart, dummy, clock, reset, clint, plic, etc).
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> > > Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> > > Cc: Anup Patel <anup.patel@wdc.com>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > Cc: Drew Fustini <drew@beagleboard.org>
> > > Cc: Maxime Ripard <maxime@cerno.tech>
> > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > Cc: Wei Fu <wefu@redhat.com>
> > > Cc: Wei Wu <lazyparser@gmail.com>
> > > ---
> > >  arch/riscv/boot/dts/Makefile                       |  1 +
> > >  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
> > >  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
> > >  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84
> > > ++++++++++++++++++++++ 4 files changed, 116 insertions(+)
> > >  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
> > >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > >
> > > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > > index fe996b8..3e7b264 100644
> > > --- a/arch/riscv/boot/dts/Makefile
> > > +++ b/arch/riscv/boot/dts/Makefile
> > > @@ -2,5 +2,6 @@
> > >  subdir-y += sifive
> > >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
> > >  subdir-y += microchip
> > > +subdir-y += allwinner
> > >
> > >  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> > > diff --git a/arch/riscv/boot/dts/allwinner/Makefile
> > > b/arch/riscv/boot/dts/allwinner/Makefile new file mode 100644
> > > index 00000000..4adbf4b
> > > --- /dev/null
> > > +++ b/arch/riscv/boot/dts/allwinner/Makefile
> > > @@ -0,0 +1,2 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> > > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts new file mode
> > > 100644
> > > index 00000000..cd9f7c9
> > > --- /dev/null
> > > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> >
> > Board DT names are comprised of soc name and board name, in this case it would
> > be "sun20i-d1-nezha-kit.dts"
> >
> > > @@ -0,0 +1,29 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >
> > Usually copyrights are added below spdx id.
> >
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "allwinner-d1.dtsi"
> > > +
> > > +/ {
> > > +     #address-cells = <2>;
> > > +     #size-cells = <2>;
> >
> > This should be part of SoC level DTSI.
> >
> > > +     model = "Allwinner D1 NeZha Kit";
> > > +     compatible = "allwinner,d1-nezha-kit";
> >
> > Board specific compatible string should be followed with SoC compatible, in
> > this case "allwinner,sun20i-d1".  You should document it too.
> >
> > > +
> > > +     chosen {
> > > +             bootargs = "console=ttyS0,115200";
> >
> > Above line doesn't belong here. If anything, it should be added dynamically by
> > bootloader.
>
> After discussion, we still want to keep a default value here.
> Sometimes we could boot with jtag and parse dtb is hard for gdbinit
> script.
>
> >
> > > +             stdout-path = &serial0;
> > > +     };
> > > +
> > > +     memory@40000000 {
> > > +             device_type = "memory";
> > > +             reg = <0x0 0x40000000 0x0 0x20000000>;
> > > +     };
> >
> > Ditto for whole memory node.
>
> Ditto

The thing is that there's never a good value for a default. Let's take
the memory node here: what would be a good default? If we want to make
it work everywhere it's going to be the lowest amount of memory
available on the D1 boards. It's going to be hard to maintain and very
likely to be overlooked, resulting in broken boards anyway.

If someone is savvy enough to use JTAG, it's not really difficult to
modify the DT for their board when they need it.

Maxime
Guo Ren June 7, 2021, 7:53 a.m. UTC | #6
Thx for the clarification.

On Mon, Jun 7, 2021 at 3:27 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Mon, Jun 07, 2021 at 11:44:03AM +0800, Guo Ren wrote:
> > On Mon, Jun 7, 2021 at 12:26 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> > >
> > > Hi!
> > >
> > > I didn't go through all details. After you fix all comments below, you should
> > > run "make dtbs_check" and fix all reported warnings too.
> > >
> > > Dne nedelja, 06. junij 2021 ob 11:04:07 CEST je guoren@kernel.org napisal(a):
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > Add initial DTS for Allwinner D1 NeZha board having only essential
> > > > devices (uart, dummy, clock, reset, clint, plic, etc).
> > > >
> > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> > > > Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> > > > Cc: Anup Patel <anup.patel@wdc.com>
> > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > Cc: Christoph Hellwig <hch@lst.de>
> > > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > > Cc: Drew Fustini <drew@beagleboard.org>
> > > > Cc: Maxime Ripard <maxime@cerno.tech>
> > > > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > > > Cc: Wei Fu <wefu@redhat.com>
> > > > Cc: Wei Wu <lazyparser@gmail.com>
> > > > ---
> > > >  arch/riscv/boot/dts/Makefile                       |  1 +
> > > >  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
> > > >  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
> > > >  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84
> > > > ++++++++++++++++++++++ 4 files changed, 116 insertions(+)
> > > >  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
> > > >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > > create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > > >
> > > > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > > > index fe996b8..3e7b264 100644
> > > > --- a/arch/riscv/boot/dts/Makefile
> > > > +++ b/arch/riscv/boot/dts/Makefile
> > > > @@ -2,5 +2,6 @@
> > > >  subdir-y += sifive
> > > >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
> > > >  subdir-y += microchip
> > > > +subdir-y += allwinner
> > > >
> > > >  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> > > > diff --git a/arch/riscv/boot/dts/allwinner/Makefile
> > > > b/arch/riscv/boot/dts/allwinner/Makefile new file mode 100644
> > > > index 00000000..4adbf4b
> > > > --- /dev/null
> > > > +++ b/arch/riscv/boot/dts/allwinner/Makefile
> > > > @@ -0,0 +1,2 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> > > > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > > > b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts new file mode
> > > > 100644
> > > > index 00000000..cd9f7c9
> > > > --- /dev/null
> > > > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > >
> > > Board DT names are comprised of soc name and board name, in this case it would
> > > be "sun20i-d1-nezha-kit.dts"
> > >
> > > > @@ -0,0 +1,29 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > >
> > > Usually copyrights are added below spdx id.
> > >
> > > > +
> > > > +/dts-v1/;
> > > > +
> > > > +#include "allwinner-d1.dtsi"
> > > > +
> > > > +/ {
> > > > +     #address-cells = <2>;
> > > > +     #size-cells = <2>;
> > >
> > > This should be part of SoC level DTSI.
> > >
> > > > +     model = "Allwinner D1 NeZha Kit";
> > > > +     compatible = "allwinner,d1-nezha-kit";
> > >
> > > Board specific compatible string should be followed with SoC compatible, in
> > > this case "allwinner,sun20i-d1".  You should document it too.
> > >
> > > > +
> > > > +     chosen {
> > > > +             bootargs = "console=ttyS0,115200";
> > >
> > > Above line doesn't belong here. If anything, it should be added dynamically by
> > > bootloader.
> >
> > After discussion, we still want to keep a default value here.
> > Sometimes we could boot with jtag and parse dtb is hard for gdbinit
> > script.
> >
> > >
> > > > +             stdout-path = &serial0;
> > > > +     };
> > > > +
> > > > +     memory@40000000 {
> > > > +             device_type = "memory";
> > > > +             reg = <0x0 0x40000000 0x0 0x20000000>;
> > > > +     };
> > >
> > > Ditto for whole memory node.
> >
> > Ditto
>
> The thing is that there's never a good value for a default. Let's take
> the memory node here: what would be a good default? If we want to make
> it work everywhere it's going to be the lowest amount of memory
> available on the D1 boards. It's going to be hard to maintain and very
> likely to be overlooked, resulting in broken boards anyway.
>
> If someone is savvy enough to use JTAG, it's not really difficult to
> modify the DT for their board when they need it.
okay, I see. I'll follow the rule in the next version of the patchset.

>
> Maxime
Guo Ren June 7, 2021, 8:07 a.m. UTC | #7
On Mon, Jun 7, 2021 at 3:24 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> Thanks for the patches
>
> On Sun, Jun 06, 2021 at 09:04:07AM +0000, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Add initial DTS for Allwinner D1 NeZha board having only essential
> > devices (uart, dummy, clock, reset, clint, plic, etc).
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Co-Developed-by: Liu Shaohua <liush@allwinnertech.com>
> > Signed-off-by: Liu Shaohua <liush@allwinnertech.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Drew Fustini <drew@beagleboard.org>
> > Cc: Maxime Ripard <maxime@cerno.tech>
> > Cc: Palmer Dabbelt <palmerdabbelt@google.com>
> > Cc: Wei Fu <wefu@redhat.com>
> > Cc: Wei Wu <lazyparser@gmail.com>
> > ---
> >  arch/riscv/boot/dts/Makefile                       |  1 +
> >  arch/riscv/boot/dts/allwinner/Makefile             |  2 +
> >  .../boot/dts/allwinner/allwinner-d1-nezha-kit.dts  | 29 ++++++++
> >  arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi    | 84 ++++++++++++++++++++++
>
> Can you add the riscv folder to our MAINTAINERS entry as well?
Yes

>
> >  4 files changed, 116 insertions(+)
> >  create mode 100644 arch/riscv/boot/dts/allwinner/Makefile
> >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> >  create mode 100644 arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> >
> > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > index fe996b8..3e7b264 100644
> > --- a/arch/riscv/boot/dts/Makefile
> > +++ b/arch/riscv/boot/dts/Makefile
> > @@ -2,5 +2,6 @@
> >  subdir-y += sifive
> >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
> >  subdir-y += microchip
> > +subdir-y += allwinner
>
> I assume they should be ordered alphabetically?
Thx for pointing it out.

>
> >  obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
> > diff --git a/arch/riscv/boot/dts/allwinner/Makefile b/arch/riscv/boot/dts/allwinner/Makefile
> > new file mode 100644
> > index 00000000..4adbf4b
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > new file mode 100644
> > index 00000000..cd9f7c9
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +
> > +/dts-v1/;
> > +
> > +#include "allwinner-d1.dtsi"
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
> > +     model = "Allwinner D1 NeZha Kit";
> > +     compatible = "allwinner,d1-nezha-kit";
> > +
> > +     chosen {
> > +             bootargs = "console=ttyS0,115200";
> > +             stdout-path = &serial0;
> > +     };
> > +
> > +     memory@40000000 {
> > +             device_type = "memory";
> > +             reg = <0x0 0x40000000 0x0 0x20000000>;
> > +     };
> > +
> > +     soc {
> > +     };
> > +};
> > +
> > +&serial0 {
> > +     status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > new file mode 100644
> > index 00000000..11cd938
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
> > +     model = "Allwinner D1 Soc";
> > +     compatible = "allwinner,d1-nezha-kit";
> > +
> > +     chosen {
> > +     };
> > +
> > +     cpus {
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +             timebase-frequency = <2400000>;
> > +             cpu@0 {
> > +                     device_type = "cpu";
> > +                     reg = <0>;
> > +                     status = "okay";
> > +                     compatible = "riscv";
> > +                     riscv,isa = "rv64imafdcv";
> > +                     mmu-type = "riscv,sv39";
> > +                     cpu0_intc: interrupt-controller {
> > +                             #interrupt-cells = <1>;
> > +                             compatible = "riscv,cpu-intc";
> > +                             interrupt-controller;
> > +                     };
> > +             };
> > +     };
> > +
> > +     soc {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             compatible = "simple-bus";
> > +             ranges;
> > +
> > +             reset: reset-sample {
> > +                     compatible = "thead,reset-sample";
> > +                     plic-delegate = <0x0 0x101ffffc>;
> > +             };
>
> This compatible is not documented anywhere?
It used by opensbi (riscv runtime firmware), not in Linux. But I think
we should keep it.

>
> Maxime
Maxime Ripard June 14, 2021, 3:33 p.m. UTC | #8
On Mon, Jun 07, 2021 at 04:07:37PM +0800, Guo Ren wrote:
> On Mon, Jun 7, 2021 at 3:24 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > +             reset: reset-sample {
> > > +                     compatible = "thead,reset-sample";
> > > +                     plic-delegate = <0x0 0x101ffffc>;
> > > +             };
> >
> > This compatible is not documented anywhere?
>
> It used by opensbi (riscv runtime firmware), not in Linux. But I think
> we should keep it.

This should have a documentation still.

Maxime
Guo Ren June 14, 2021, 4:28 p.m. UTC | #9
On Mon, Jun 14, 2021 at 11:33 PM Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Mon, Jun 07, 2021 at 04:07:37PM +0800, Guo Ren wrote:
> > On Mon, Jun 7, 2021 at 3:24 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > +             reset: reset-sample {
> > > > +                     compatible = "thead,reset-sample";
> > > > +                     plic-delegate = <0x0 0x101ffffc>;
> > > > +             };
> > >
> > > This compatible is not documented anywhere?
> >
> > It used by opensbi (riscv runtime firmware), not in Linux. But I think
> > we should keep it.
>
> This should have a documentation still.

Could we detail the above in [1]?

1: https://github.com/riscv/opensbi/blob/master/docs/platform/thead-c9xx.md

>
> Maxime
Jernej Škrabec June 14, 2021, 4:31 p.m. UTC | #10
Dne ponedeljek, 14. junij 2021 ob 18:28:28 CEST je Guo Ren napisal(a):
> On Mon, Jun 14, 2021 at 11:33 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > On Mon, Jun 07, 2021 at 04:07:37PM +0800, Guo Ren wrote:
> > > On Mon, Jun 7, 2021 at 3:24 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > +             reset: reset-sample {
> > > > > +                     compatible = "thead,reset-sample";
> > > > > +                     plic-delegate = <0x0 0x101ffffc>;
> > > > > +             };
> > > >
> > > > This compatible is not documented anywhere?
> > >
> > > It used by opensbi (riscv runtime firmware), not in Linux. But I think
> > > we should keep it.
> >
> > This should have a documentation still.
> 
> Could we detail the above in [1]?

All DT nodes must be documented in
Documentation/devicetree/bindings folder in yaml format, so DTs can be machine 
verified.

Best regards,
Jernej

> 
> 1: https://github.com/riscv/opensbi/blob/master/docs/platform/thead-c9xx.md
> 
> >
> > Maxime
> 
> 
> 
> -- 
> Best Regards
>  Guo Ren
> 
> ML: https://lore.kernel.org/linux-csky/
> 
>
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
index fe996b8..3e7b264 100644
--- a/arch/riscv/boot/dts/Makefile
+++ b/arch/riscv/boot/dts/Makefile
@@ -2,5 +2,6 @@ 
 subdir-y += sifive
 subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) += canaan
 subdir-y += microchip
+subdir-y += allwinner
 
 obj-$(CONFIG_BUILTIN_DTB) := $(addsuffix /, $(subdir-y))
diff --git a/arch/riscv/boot/dts/allwinner/Makefile b/arch/riscv/boot/dts/allwinner/Makefile
new file mode 100644
index 00000000..4adbf4b
--- /dev/null
+++ b/arch/riscv/boot/dts/allwinner/Makefile
@@ -0,0 +1,2 @@ 
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_SOC_SUNXI) += allwinner-d1-nezha-kit.dtb
diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
new file mode 100644
index 00000000..cd9f7c9
--- /dev/null
+++ b/arch/riscv/boot/dts/allwinner/allwinner-d1-nezha-kit.dts
@@ -0,0 +1,29 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include "allwinner-d1.dtsi"
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	model = "Allwinner D1 NeZha Kit";
+	compatible = "allwinner,d1-nezha-kit";
+
+	chosen {
+		bootargs = "console=ttyS0,115200";
+		stdout-path = &serial0;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0x0 0x20000000>;
+	};
+
+	soc {
+	};
+};
+
+&serial0 {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
new file mode 100644
index 00000000..11cd938
--- /dev/null
+++ b/arch/riscv/boot/dts/allwinner/allwinner-d1.dtsi
@@ -0,0 +1,84 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+
+/dts-v1/;
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	model = "Allwinner D1 Soc";
+	compatible = "allwinner,d1-nezha-kit";
+
+	chosen {
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		timebase-frequency = <2400000>;
+		cpu@0 {
+			device_type = "cpu";
+			reg = <0>;
+			status = "okay";
+			compatible = "riscv";
+			riscv,isa = "rv64imafdcv";
+			mmu-type = "riscv,sv39";
+			cpu0_intc: interrupt-controller {
+				#interrupt-cells = <1>;
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+			};
+		};
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		reset: reset-sample {
+			compatible = "thead,reset-sample";
+			plic-delegate = <0x0 0x101ffffc>;
+		};
+
+		clint: clint@14000000 {
+			compatible = "riscv,clint0";
+			interrupts-extended = <
+				&cpu0_intc  3 &cpu0_intc  7
+				>;
+			reg = <0x0 0x14000000 0x0 0x04000000>;
+			clint,has-no-64bit-mmio;
+		};
+
+		plic: interrupt-controller@10000000 {
+			#interrupt-cells = <1>;
+			compatible = "riscv,plic0";
+			interrupt-controller;
+			interrupts-extended = <
+				&cpu0_intc  0xffffffff &cpu0_intc  9
+				>;
+			reg = <0x0 0x10000000 0x0 0x04000000>;
+			reg-names = "control";
+			riscv,max-priority = <7>;
+			riscv,ndev = <200>;
+		};
+
+		dummy_apb: apb-clock {
+			compatible = "fixed-clock";
+			clock-frequency = <24000000>;
+			clock-output-names = "dummy_apb";
+			#clock-cells = <0>;
+		};
+
+		serial0: serial@2500000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x02500000 0x0 0x400>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			interrupt-parent = <&plic>;
+			interrupts = <18>;
+			clocks = <&dummy_apb>;
+			status = "disabled";
+		};
+	};
+};