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

From: Hal Feng <hal.feng@starfivetech.com>
To: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: <linux-riscv@lists.infradead.org>, <devicetree@vger.kernel.org>,
	"Conor Dooley" <conor@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Ben Dooks <ben.dooks@sifive.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>, Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	Linus Walleij <linus.walleij@linaro.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree
Date: Wed, 15 Feb 2023 22:03:43 +0800	[thread overview]
Message-ID: <af42ed91-95aa-014a-1efb-6f70ee5a0433@starfivetech.com> (raw)
In-Reply-To: <CAJM55Z9Y_fF+4Dtu++C_jVS0+ohXp5U0GyuJCBpUh-SpTMGrVA@mail.gmail.com>

On 2/14/2023 5:53 PM, Emil Renner Berthing wrote:
> On Tue, 20 Dec 2022 at 02:12, Hal Feng <hal.feng@starfivetech.com> wrote:
>>
>> From: Emil Renner Berthing <kernel@esmil.dk>
>>
>> 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@esmil.dk>
>> Co-developed-by: Jianlong Huang <jianlong.huang@starfivetech.com>
>> Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com>
>> Co-developed-by: Hal Feng <hal.feng@starfivetech.com>
>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  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@esmil.dk>
>> + */
>> +
>> +/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@esmil.dk>
>> + */
>> +
>> +/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";

Best regards,
Hal

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv