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

[Hal Feng]

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";

Best regards,
Hal