Re: [PATCH v4 08/10] riscv: dts: bouffalolab: add Sipeed M1s SoM and Dock devicetree

[Jisheng Zhang <jszhang@kernel.org>]

On Thu, May 18, 2023 at 10:55:21PM -0500, Samuel Holland wrote:
> Hi Jisheng,
> 
> On 5/18/23 10:22, Jisheng Zhang wrote:
> > Sipeed manufactures a M1s system-on-module and dock board, add basic
> > support for them.
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> > ---
> >  arch/riscv/boot/dts/Makefile                  |  1 +
> >  arch/riscv/boot/dts/bouffalolab/Makefile      |  2 ++
> >  .../dts/bouffalolab/bl808-sipeed-m1s-dock.dts | 25 +++++++++++++++++++
> >  .../dts/bouffalolab/bl808-sipeed-m1s.dtsi     | 21 ++++++++++++++++
> >  4 files changed, 49 insertions(+)
> >  create mode 100644 arch/riscv/boot/dts/bouffalolab/Makefile
> >  create mode 100644 arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s-dock.dts
> >  create mode 100644 arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s.dtsi
> > 
> > diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
> > index f0d9f89054f8..133e6c38c9b0 100644
> > --- a/arch/riscv/boot/dts/Makefile
> > +++ b/arch/riscv/boot/dts/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  subdir-y += allwinner
> > +subdir-y += bouffalolab
> >  subdir-y += sifive
> >  subdir-y += starfive
> >  subdir-y += canaan
> > diff --git a/arch/riscv/boot/dts/bouffalolab/Makefile b/arch/riscv/boot/dts/bouffalolab/Makefile
> > new file mode 100644
> > index 000000000000..5419964e892d
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/bouffalolab/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +dtb-$(CONFIG_SOC_BOUFFALOLAB) += bl808-sipeed-m1s-dock.dtb
> > diff --git a/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s-dock.dts b/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s-dock.dts
> > new file mode 100644
> > index 000000000000..aa6cf909cd4d
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s-dock.dts
> > @@ -0,0 +1,25 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > +/*
> > + * Copyright (C) 2022 Jisheng Zhang <jszhang@kernel.org>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "bl808-sipeed-m1s.dtsi"
> > +
> > +/ {
> > +	model = "Sipeed M1s Dock";
> > +	compatible = "sipeed,m1s-dock", "sipeed,m1s", "bouffalolab,bl808";
> > +
> > +	aliases {
> > +		serial3 = &uart3;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial3:2000000n8";
> > +	};
> > +};
> > +
> > +&uart3 {
> > +	status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s.dtsi b/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s.dtsi
> > new file mode 100644
> > index 000000000000..5026de768534
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/bouffalolab/bl808-sipeed-m1s.dtsi
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > +/*
> > + * Copyright (C) 2022 Jisheng Zhang <jszhang@kernel.org>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "bl808.dtsi"
> > +
> > +/ {
> > +	compatible = "sipeed,m1s", "bouffalolab,bl808";
> > +
> > +	memory@50000000 {
> > +		device_type = "memory";
> > +		reg = <0x50000000 0x04000000>;
> > +	};
> 
> Especially since the SoC contains three heterogeneous CPUs, the firmware
> may want to divide the PSRAM among them, so I do not think it is a good
> idea to define this statically. (Or would all of the DTs contain this
do you want the bootloader/firmware e.g uboot to add the memory node
dynamically?

But to be honest, nowdays most SoCs contain some heterogeneous CPUs, and
in real products some of those CPUs need to use DDR memory.
FWICT, their dtbs(in arch/arm64/boot/dts/...) still define the memory
statically. I believe this is acchieved by dynamically update the memory
node of DT. This solution doesn't make obvious difference with the uboot
adding memory node solution.

> same node, and then use reserved-memory nodes to cover the other CPUs'
> allocations?)
> 
> Regards,
> Samuel
> 
> > +};
> > +
> > +&xtal {
> > +	clock-frequency = <40000000>;
> > +};
>