[PATCH v2 0/4] riscv: dts: starfive: simplify binman config

[PATCH v2 0/4] riscv: dts: starfive: simplify binman config

[Heinrich Schuchardt]

All StarFive JH7110 are supported via starfive_visionfive2_defconfig.
Up to now we have explicitly specified the multi-device-tree FIT
configuration. This more complicated than needed.

Binman can use the information from CONFIG_OF_LIST to automatically
generated a FIT image containing the necessary device-trees and
configurations.

This also removes the non-functional default configuration without a
device-tree that binman added up to now.

Before the patch set booting on an unsupported device would fail in main
U-Boot due to the lack of a device-tree in the default configuration.

With this patch set booting will fail in SPL because there is no
default configuration.

v2:
	do not use a default configuration

Heinrich Schuchardt (4):
  riscv: dts: add OF_LIST handling to binman.dtsi
  board: starfive: spl: strip off 'starfive/' prefix
  riscv: dts: starfive: remove duplicate itb entries
  riscv: dts: starfive: don't use a default configuration

 arch/riscv/dts/binman.dtsi                    |  7 +-
 .../dts/starfive-visionfive2-binman.dtsi      | 75 ++-----------------
 board/starfive/visionfive2/spl.c              |  4 +
 3 files changed, 14 insertions(+), 72 deletions(-)

-- 
2.47.1

[PATCH v2 1/4] riscv: dts: add OF_LIST handling to binman.dtsi

[Heinrich Schuchardt]

Binman can automatically generate device-tree and configuration entries in
the FIT image based on CONFIG_OF_LIST if the binman node includes the right
sub-nodes.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v2:
	no change
---
 arch/riscv/dts/binman.dtsi | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
index 0405faca574..7edc36ccac0 100644
--- a/arch/riscv/dts/binman.dtsi
+++ b/arch/riscv/dts/binman.dtsi
@@ -82,8 +82,9 @@
 					};
 				};
 
-#ifndef CONFIG_OF_BOARD
+#if !defined(CONFIG_OF_BOARD) || defined(CONFIG_OF_LIST)
 				@fdt-SEQ {
+					fit,operation = "gen-fdt-nodes";
 					description = "NAME";
 					type = "flat_dt";
 					compression = "none";
@@ -94,7 +95,7 @@
 			configurations {
 				default = "conf-1";
 
-#ifndef CONFIG_OF_BOARD
+#if !defined(CONFIG_OF_BOARD) || defined(CONFIG_OF_LIST)
 				@conf-SEQ {
 #else
 				conf-1 {
@@ -115,7 +116,7 @@
 #endif
 #endif /* CONFIG_OPTEE */
 
-#ifndef CONFIG_OF_BOARD
+#if !defined(CONFIG_OF_BOARD) || defined(CONFIG_OF_LIST)
 					fdt = "fdt-SEQ";
 #endif
 				};
-- 
2.47.1

[PATCH v2 2/4] board: starfive: spl: strip off 'starfive/' prefix

[Heinrich Schuchardt]

The configuration descriptions generated by binman contain the vendor
device-tree directory. Instead of adding it to all match strings just strip
it off.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	no change
---
 board/starfive/visionfive2/spl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index 22afd76c6b9..d63eb1abe6a 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -118,6 +118,10 @@ int board_fit_config_name_match(const char *name)
 
 	product_id = get_product_id_from_eeprom();
 
+	/* Strip off prefix */
+	if (strncmp(name, "starfive/", 9))
+		return -EINVAL;
+	name += 9;
 	if (!strncmp(product_id, "VF7110", 6)) {
 		version = get_pcb_revision_from_eeprom();
 		if ((version == 'b' || version == 'B') &&
-- 
2.47.1

[PATCH v2 3/4] riscv: dts: starfive: remove duplicate itb entries

[Heinrich Schuchardt]

As binman already creates nodes based on CONFIG_OF_LIST we don't need to
add extra nodes.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org> # StarFIve VisionFive 2
---
v2:
	no change
---
 .../dts/starfive-visionfive2-binman.dtsi      | 76 -------------------
 1 file changed, 76 deletions(-)

diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
index 4cce001e80d..05787bdb92d 100644
--- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
+++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
@@ -13,82 +13,6 @@
 };
 
 &binman {
-	itb {
-		fit {
-			images {
-				fdt-jh7110-milkv-mars {
-					description = "jh7110-milkv-mars";
-					load = <0x40400000>;
-					compression = "none";
-
-					blob-ext {
-						filename = "dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dtb";
-					};
-				};
-
-				fdt-jh7110-pine64-star64 {
-					description = "jh7110-pine64-star64";
-					load = <0x40400000>;
-					compression = "none";
-
-					blob-ext {
-						filename = "dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dtb";
-					};
-				};
-
-				fdt-jh7110-starfive-visionfive-2-v1.2a {
-					description = "jh7110-starfive-visionfive-2-v1.2a";
-					load = <0x40400000>;
-					compression = "none";
-
-					blob-ext {
-						filename = "dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
-					};
-				};
-
-				fdt-jh7110-starfive-visionfive-2-v1.3b {
-					description = "jh7110-starfive-visionfive-2-v1.3b";
-					load = <0x40400000>;
-					compression = "none";
-
-					blob-ext {
-						filename = "dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
-					};
-				};
-			};
-
-			configurations {
-				conf-jh7110-milkv-mars {
-					description = "jh7110-milkv-mars";
-					firmware = "opensbi";
-					loadables = "uboot";
-					fdt = "fdt-jh7110-milkv-mars";
-				};
-
-				conf-jh7110-pine64-star64 {
-					description = "jh7110-pine64-star64";
-					firmware = "opensbi";
-					loadables = "uboot";
-					fdt = "fdt-jh7110-pine64-star64";
-				};
-
-				conf-jh7110-starfive-visionfive-2-v1.2a {
-					description = "jh7110-starfive-visionfive-2-v1.2a";
-					firmware = "opensbi";
-					loadables = "uboot";
-					fdt = "fdt-jh7110-starfive-visionfive-2-v1.2a";
-				};
-
-				conf-jh7110-starfive-visionfive-2-v1.3b {
-					description = "jh7110-starfive-visionfive-2-v1.3b";
-					firmware = "opensbi";
-					loadables = "uboot";
-					fdt = "fdt-jh7110-starfive-visionfive-2-v1.3b";
-				};
-			};
-		};
-	};
-
 	spl-img {
 		filename = "spl/u-boot-spl.bin.normal.out";
 
-- 
2.47.1

[PATCH v2 4/4] riscv: dts: starfive: don't use a default configuration

[Heinrich Schuchardt]

Booting JH7110 systems with a VisionFive 2 device-tree used to kind of
work without causing harm to the hardware. But there is no guarantee
that this will hold true in future. So we should not rely on it.

Before the current patch series booting failed on unsupported boards due
to the lack of a device-tree in the binman generated default configuration
when reaching main U-Boot.

By setting the default configuration to non-matching value booting will
now fail on unsupported boards already in SPL. This allows SPL to
continue with the next boot source for a possible recovery.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 arch/riscv/dts/starfive-visionfive2-binman.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
index 05787bdb92d..dd0faca6824 100644
--- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
+++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
@@ -13,6 +13,19 @@
 };
 
 &binman {
+	itb {
+		fit {
+			configurations {
+				/*
+				 * Do not boot if there is no matching
+				 * configuration. We can't remove the default
+				 * here. So use a non-matching value.
+				 */
+				default = "none";
+			};
+		};
+	};
+
 	spl-img {
 		filename = "spl/u-boot-spl.bin.normal.out";
 
-- 
2.47.1

Re: [PATCH v2 4/4] riscv: dts: starfive: don't use a default configuration

[Simon Glass]

Hi Heinrich,

On Mon, 10 Feb 2025 at 04:18, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Booting JH7110 systems with a VisionFive 2 device-tree used to kind of
> work without causing harm to the hardware. But there is no guarantee
> that this will hold true in future. So we should not rely on it.
>
> Before the current patch series booting failed on unsupported boards due
> to the lack of a device-tree in the binman generated default configuration
> when reaching main U-Boot.
>
> By setting the default configuration to non-matching value booting will
> now fail on unsupported boards already in SPL. This allows SPL to
> continue with the next boot source for a possible recovery.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
>         new patch
> ---
>  arch/riscv/dts/starfive-visionfive2-binman.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
> index 05787bdb92d..dd0faca6824 100644
> --- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
> +++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
> @@ -13,6 +13,19 @@
>  };
>
>  &binman {
> +       itb {
> +               fit {
> +                       configurations {
> +                               /*
> +                                * Do not boot if there is no matching
> +                                * configuration. We can't remove the default
> +                                * here. So use a non-matching value.
> +                                */
> +                               default = "none";

The 'default' is optional [1] but the spec is silent on what happens
when the default is missing. What do you think it should do? Perhaps
we should add a property to define the behaviour?

> +                       };
> +               };
> +       };
> +
>         spl-img {
>                 filename = "spl/u-boot-spl.bin.normal.out";
>
> --
> 2.47.1
>

Regards,
SImon

[1] https://fitspec.osfw.foundation/#id4

Re: [PATCH v2 4/4] riscv: dts: starfive: don't use a default configuration

[Heinrich Schuchardt]

On 10.02.25 14:08, Simon Glass wrote:
> Hi Heinrich,
> 
> On Mon, 10 Feb 2025 at 04:18, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> Booting JH7110 systems with a VisionFive 2 device-tree used to kind of
>> work without causing harm to the hardware. But there is no guarantee
>> that this will hold true in future. So we should not rely on it.
>>
>> Before the current patch series booting failed on unsupported boards due
>> to the lack of a device-tree in the binman generated default configuration
>> when reaching main U-Boot.
>>
>> By setting the default configuration to non-matching value booting will
>> now fail on unsupported boards already in SPL. This allows SPL to
>> continue with the next boot source for a possible recovery.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v2:
>>          new patch
>> ---
>>   arch/riscv/dts/starfive-visionfive2-binman.dtsi | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> index 05787bdb92d..dd0faca6824 100644
>> --- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> +++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
>> @@ -13,6 +13,19 @@
>>   };
>>
>>   &binman {
>> +       itb {
>> +               fit {
>> +                       configurations {
>> +                               /*
>> +                                * Do not boot if there is no matching
>> +                                * configuration. We can't remove the default
>> +                                * here. So use a non-matching value.
>> +                                */
>> +                               default = "none";
> 
> The 'default' is optional [1] but the spec is silent on what happens
> when the default is missing. What do you think it should do? Perhaps
> we should add a property to define the behaviour?

If there is no match and default does not exist, we should not boot.

In our current code we don't boot if default points to a non-existing 
config. But having inconsistent FIT images might not be the best idea.

Maybe it would be better to use

#ifndef CONFIG_MULTI_DTB_FIT
	default = "conf-1";
#endif

in arch/riscv/dts/binman.dtsi to avoid having an invalid default value?

Best regards

Heinrich

> 
>> +                       };
>> +               };
>> +       };
>> +
>>          spl-img {
>>                  filename = "spl/u-boot-spl.bin.normal.out";
>>
>> --
>> 2.47.1
>>
> 
> Regards,
> SImon
> 
> [1] https://fitspec.osfw.foundation/#id4

Re: [PATCH v2 2/4] board: starfive: spl: strip off 'starfive/' prefix

[Leo Liang]

On Mon, Feb 10, 2025 at 12:18:28PM +0100, Heinrich Schuchardt wrote:
> The configuration descriptions generated by binman contain the vendor
> device-tree directory. Instead of adding it to all match strings just strip
> it off.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	no change
> ---
>  board/starfive/visionfive2/spl.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

RE: [PATCH v2 0/4] riscv: dts: starfive: simplify binman config

[Hal Feng]

> On 10.02.25 19:18, Heinrich Schuchardt wrote:
> All StarFive JH7110 are supported via starfive_visionfive2_defconfig.
> Up to now we have explicitly specified the multi-device-tree FIT configuration.
> This more complicated than needed.
> 
> Binman can use the information from CONFIG_OF_LIST to automatically
> generated a FIT image containing the necessary device-trees and
> configurations.
> 
> This also removes the non-functional default configuration without a device-
> tree that binman added up to now.
> 
> Before the patch set booting on an unsupported device would fail in main U-
> Boot due to the lack of a device-tree in the default configuration.
> 
> With this patch set booting will fail in SPL because there is no default
> configuration.
> 
> v2:
> 	do not use a default configuration
> 
> Heinrich Schuchardt (4):
>   riscv: dts: add OF_LIST handling to binman.dtsi
>   board: starfive: spl: strip off 'starfive/' prefix
>   riscv: dts: starfive: remove duplicate itb entries
>   riscv: dts: starfive: don't use a default configuration
> 
>  arch/riscv/dts/binman.dtsi                    |  7 +-
>  .../dts/starfive-visionfive2-binman.dtsi      | 75 ++-----------------
>  board/starfive/visionfive2/spl.c              |  4 +
>  3 files changed, 14 insertions(+), 72 deletions(-)

Tested-by: Hal Feng <hal.feng@starfivetech.com>

Tested successfully on VisionFive 2 1.3b, SPL can successfully enumerate
each dtb in OF_LIST.

Best regards,
Hal