Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 7 Jun 2021 09:27:09 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Guo Ren <guoren@kernel.org>
Cc:     Jernej =?utf-8?Q?=C5=A0krabec?= <jernej.skrabec@gmail.com>,
        Anup Patel <anup.patel@wdc.com>,
        Palmer Dabbelt <palmerdabbelt@google.com>,
        Arnd Bergmann <arnd@arndb.de>, Chen-Yu Tsai <wens@csie.org>,
        Drew Fustini <drew@beagleboard.org>, liush@allwinnertech.com,
        Wei Wu =?utf-8?B?KOWQtOS8nyk=?= <lazyparser@gmail.com>,
        wefu@redhat.com, linux-riscv <linux-riscv@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        linux-sunxi@lists.linux.dev, Guo Ren <guoren@linux.alibaba.com>,
        Atish Patra <atish.patra@wdc.com>,
        Christoph Hellwig <hch@lst.de>
Subject: Re: [RFC PATCH v2 09/11] riscv: soc: Initial DTS for Allwinner D1
 NeZha board
Message-ID: <20210607072709.ul4jdvtyspj6t4c6@gilmour>
References: <1622970249-50770-1-git-send-email-guoren@kernel.org>
 <1622970249-50770-13-git-send-email-guoren@kernel.org>
 <2490489.OUOj5N01qN@jernej-laptop>
 <CAJF2gTTDVy89R-gvWUS0sgpX=B14LnH5rDoGP7pv1=OSyJq28Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="dkq7vhizxit3oxmd"
Content-Disposition: inline
In-Reply-To: <CAJF2gTTDVy89R-gvWUS0sgpX=B14LnH5rDoGP7pv1=OSyJq28Q@mail.gmail.com>
Precedence: bulk


--dkq7vhizxit3oxmd
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 07, 2021 at 11:44:03AM +0800, Guo Ren wrote:
> On Mon, Jun 7, 2021 at 12:26 AM Jernej =C5=A0krabec <jernej.skrabec@gmail=
=2Ecom> 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 napis=
al(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/Makef=
ile
> > > index fe996b8..3e7b264 100644
> > > --- a/arch/riscv/boot/dts/Makefile
> > > +++ b/arch/riscv/boot/dts/Makefile
> > > @@ -2,5 +2,6 @@
> > >  subdir-y +=3D sifive
> > >  subdir-$(CONFIG_SOC_CANAAN_K210_DTB_BUILTIN) +=3D canaan
> > >  subdir-y +=3D microchip
> > > +subdir-y +=3D allwinner
> > >
> > >  obj-$(CONFIG_BUILTIN_DTB) :=3D $(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) +=3D 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 m=
ode
> > > 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 i=
t 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 =3D <2>;
> > > +     #size-cells =3D <2>;
> >
> > This should be part of SoC level DTSI.
> >
> > > +     model =3D "Allwinner D1 NeZha Kit";
> > > +     compatible =3D "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 =3D "console=3DttyS0,115200";
> >
> > Above line doesn't belong here. If anything, it should be added dynamic=
ally 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 =3D &serial0;
> > > +     };
> > > +
> > > +     memory@40000000 {
> > > +             device_type =3D "memory";
> > > +             reg =3D <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

--dkq7vhizxit3oxmd
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYL3KTQAKCRDj7w1vZxhR
xWFdAQDLzaFiCNqj57hi5iIYQrv16nkQoboc0eB4T/McOPR6fAD+IUjgmFf62NP1
oefDLUF5Vp/JYH2eWZYcHp5G9m79Sg0=
=eB6L
-----END PGP SIGNATURE-----

--dkq7vhizxit3oxmd--