Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

[Emil Renner Berthing]

On Wed, 15 Feb 2023 at 15:04, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote:
>
> On 2/14/2023 5:53 PM, Emil Renner Berthing wrote:
> > On Tue, 20 Dec 2022 at 02:12, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote:
> >>
> >> From: Emil Renner Berthing <kernel@xxxxxxxx>
> >>
> >> Add a minimal device tree for StarFive JH7110 VisionFive 2 board
> >> which has version A and version B. Support booting and basic
> >> clock/reset/pinctrl/uart drivers.
> >>
> >> Signed-off-by: Emil Renner Berthing <kernel@xxxxxxxx>
> >> Co-developed-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>
> >> Co-developed-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
> >> ---
> >>  arch/riscv/boot/dts/starfive/Makefile         |   1 +
> >>  .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
> >>  .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
> >>  .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
> >>  4 files changed, 138 insertions(+)
> >>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
> >>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
> >>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/Makefile b/arch/riscv/boot/dts/starfive/Makefile
> >> index 0ea1bc15ab30..79e925a4a227 100644
> >> --- a/arch/riscv/boot/dts/starfive/Makefile
> >> +++ b/arch/riscv/boot/dts/starfive/Makefile
> >> @@ -1,2 +1,3 @@
> >>  # SPDX-License-Identifier: GPL-2.0
> >>  dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
> >> +dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
> >> new file mode 100644
> >> index 000000000000..188d3fddbe88
> >> --- /dev/null
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
> >> @@ -0,0 +1,13 @@
> >> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> >> +/*
> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> + * Copyright (C) 2022 Emil Renner Berthing <kernel@xxxxxxxx>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +#include "jh7110-starfive-visionfive-2.dtsi"
> >> +
> >> +/ {
> >> +       model = "StarFive VisionFive 2 VA";
> >> +       compatible = "starfive,visionfive-2-va", "starfive,jh7110";
> >> +};
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
> >> new file mode 100644
> >> index 000000000000..f75c10536f84
> >> --- /dev/null
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
> >> @@ -0,0 +1,13 @@
> >> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> >> +/*
> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> + * Copyright (C) 2022 Emil Renner Berthing <kernel@xxxxxxxx>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +#include "jh7110-starfive-visionfive-2.dtsi"
> >> +
> >> +/ {
> >> +       model = "StarFive VisionFive 2 VB";
> >> +       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";
> >
> > Hi Hal,
> >
> > I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va"
> > and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I
> > still think having these names match what is printed on the silkscreen
> > makes it a lot easier for everybody. Even your own releases calls the
> > boards "v1.2A" and "v1.3B":
> > https://github.com/starfive-tech/VisionFive2/releases/
> >
> > So I'd suggest
> > model = "StarFive VisionFive 2 v1.3B";
> > compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";
> >
> > I haven't seen these "VA" and "VB" anywhere else, so if you don't want
> > the version numbers and can promise that there will be no incompatible
> > future revisions of the boards then maybe just drop the "V". Eg.
> > model = "StarFive VisionFive 2 B";
> > compatible = "starfive,visionfive-2-b", "starfive,jh7110";
>
> The version A board has reached the end of life. As far as I know, the
> version B board will not update also unless there are some important
> requirements and StarFive decides to update. Furthermore, it's too late
> to change the compatible as patch 1 was already accepted. Will it be
> easier to read if I modify it as below?
>
>         model = "StarFive VisionFive 2 vB";
>         compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards
something that's used nowhere else, even by StarFive :/
But yeah, I guess vA and vB are a little easier to read.

> Best regards,
> Hal