public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH v2 0/2] riscv: starfive: generate u-boot-spl.bin.normal.out
@ 2023-09-06 12:00 Heinrich Schuchardt
  2023-09-06 12:00 ` [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support Heinrich Schuchardt
  2023-09-06 12:00 ` [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
  0 siblings, 2 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2023-09-06 12:00 UTC (permalink / raw)
  To: Rick Chen, Leo, Yanhong Wang
  Cc: Simon Glass, Marc Kleine-Budde, Chanho Park, u-boot,
	Heinrich Schuchardt

The StarFive JH7110 base boards require a header to be prefixed to the SPL
binary image. This has previously done with a vendor tool 'spl_tool'
published under a GPL-2-or-later license. Integrate this capability into
mkimage.

Add a binman task into the VisionFive 2 build to build the prefixed
U-Boot SPL file u-boot-spl.bin.normal.out.

v2:
	Fix a typo in a comment in tools/sfspl.c
	Add Tested-by credits

Heinrich Schuchardt (2):
  tools: mkimage: Add StarFive SPL image support
  riscv: dts: starfive: generate u-boot-spl.bin.normal.out

 .../jh7110-starfive-visionfive-2-u-boot.dtsi  |  10 +
 boot/image.c                                  |   1 +
 doc/board/starfive/visionfive2.rst            |  14 +-
 include/image.h                               |   1 +
 tools/Makefile                                |   1 +
 tools/sfspl.c                                 | 174 ++++++++++++++++++
 6 files changed, 189 insertions(+), 12 deletions(-)
 create mode 100644 tools/sfspl.c

-- 
2.40.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support
  2023-09-06 12:00 [PATCH v2 0/2] riscv: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
@ 2023-09-06 12:00 ` Heinrich Schuchardt
  2023-09-16 19:47   ` Milan P. Stanić
  2023-09-06 12:00 ` [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
  1 sibling, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2023-09-06 12:00 UTC (permalink / raw)
  To: Rick Chen, Leo, Yanhong Wang
  Cc: Simon Glass, Marc Kleine-Budde, Chanho Park, u-boot,
	Heinrich Schuchardt

The StarFive JH7110 base boards require a header to be prefixed to the SPL
binary image. This has previously done with a vendor tool 'spl_tool'
published under a GPL-2-or-later license. Integrate this capability into
mkimage.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Chanho Park <chanho61.park@samsung.com>
---
v2:
	no change except for Tested-by credit
---
 boot/image.c    |   1 +
 include/image.h |   1 +
 tools/Makefile  |   1 +
 tools/sfspl.c   | 174 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 tools/sfspl.c

diff --git a/boot/image.c b/boot/image.c
index 5c4f9b807d..3a99d2e897 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -182,6 +182,7 @@ static const table_entry_t uimage_type[] = {
 	{	IH_TYPE_SUNXI_TOC0, "sunxi_toc0",  "Allwinner TOC0 Boot Image" },
 	{	IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", },
 	{	IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" },
+	{	IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
 	{	-1,		    "",		  "",			},
 };
 
diff --git a/include/image.h b/include/image.h
index 01a6787d21..5f85bf84a2 100644
--- a/include/image.h
+++ b/include/image.h
@@ -231,6 +231,7 @@ enum image_type_t {
 	IH_TYPE_SUNXI_TOC0,		/* Allwinner TOC0 Boot Image */
 	IH_TYPE_FDT_LEGACY,		/* Binary Flat Device Tree Blob	in a Legacy Image */
 	IH_TYPE_RENESAS_SPKG,		/* Renesas SPKG image */
+	IH_TYPE_STARFIVE_SPL,		/* StarFive SPL image */
 
 	IH_TYPE_COUNT,			/* Number of image types */
 };
diff --git a/tools/Makefile b/tools/Makefile
index 3d0c4b0dd6..1aa1e36137 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -123,6 +123,7 @@ dumpimage-mkimage-objs := aisimage.o \
 			pblimage.o \
 			pbl_crc32.o \
 			renesas_spkgimage.o \
+			sfspl.o \
 			vybridimage.o \
 			stm32image.o \
 			$(ROCKCHIP_OBS) \
diff --git a/tools/sfspl.c b/tools/sfspl.c
new file mode 100644
index 0000000000..ec18a0a77e
--- /dev/null
+++ b/tools/sfspl.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
+ *
+ * The StarFive JH7110 requires to prepend a header to u-boot-spl.bin describing
+ * the payload length and CRC32.
+ *
+ * This module implements support in mkimage and dumpimage for this file format.
+ *
+ * StarFive's spl_tool available under GPL-2.0-and-later at
+ * https://github.com/starfive-tech/Tools implements writing the same file
+ * format and served as a reference.
+ */
+
+#include <compiler.h>
+#include <fcntl.h>
+#include <u-boot/crc.h>
+#include <unistd.h>
+#include "imagetool.h"
+
+#define DEFAULT_VERSION 0x01010101
+#define DEFAULT_BACKUP 0x200000U
+#define DEFAULT_OFFSET 0x240
+
+/**
+ * struct spl_hdr - header for SPL on JH7110
+ *
+ * All fields are low-endian.
+ */
+struct spl_hdr {
+	/** @offset:	offset to SPL header (0x240) */
+	unsigned int offset;
+	/** @bkp_offs:	address of backup SPL, defaults to DEFAULT_BACKUP */
+	unsigned int bkp_offs;
+	/** @zero1:	set to zero */
+	unsigned int zero1[159];
+	/** @version:	header version, defaults to DEFAULT_VERSION */
+	unsigned int version;
+	/** @file_size:	file size */
+	unsigned int file_size;
+	/** @hdr_size:	size of the file header (0x400) */
+	unsigned int hdr_size;
+	/** @crc32:	CRC32 */
+	unsigned int crc32;
+	/** @zero2:	set to zero */
+	unsigned int zero2[91];
+};
+
+static int sfspl_check_params(struct image_tool_params *params)
+{
+	/* Only the RISC-V architecture is supported */
+	if (params->Aflag && params->arch != IH_ARCH_RISCV)
+		return EXIT_FAILURE;
+
+	return EXIT_SUCCESS;
+}
+
+static int sfspl_verify_header(unsigned char *buf, int size,
+			       struct image_tool_params *params)
+{
+	struct spl_hdr *hdr = (void *)buf;
+	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
+	unsigned int file_size = le32_to_cpu(hdr->file_size);
+	unsigned int crc = le32_to_cpu(hdr->crc32);
+	unsigned int crc_check;
+
+	if (size < 0 ||
+	    (size_t)size < sizeof(struct spl_hdr) ||
+	    (size_t)size < hdr_size + file_size) {
+		printf("Truncated file\n");
+		return EXIT_FAILURE;
+	}
+	if (hdr->version != DEFAULT_VERSION) {
+		printf("Unknown file format version\n");
+		return EXIT_FAILURE;
+	}
+	crc_check = crc32(0, &buf[hdr_size], size - hdr_size);
+	if (crc_check != crc) {
+		printf("Incorrect CRC32\n");
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
+
+static void sfspl_print_header(const void *buf,
+			       struct image_tool_params *params)
+{
+	struct spl_hdr *hdr = (void *)buf;
+	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
+	unsigned int file_size = le32_to_cpu(hdr->file_size);
+
+	printf("Header size: %u\n", hdr_size);
+	printf("Payload size: %u\n", file_size);
+}
+
+static int sfspl_image_extract_subimage(void *ptr,
+					struct image_tool_params *params)
+{
+	struct spl_hdr *hdr = (void *)ptr;
+	unsigned char *buf = ptr;
+	int fd;
+	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
+	unsigned int file_size = le32_to_cpu(hdr->file_size);
+
+	if (params->pflag) {
+		printf("Invalid image index %d\n", params->pflag);
+		return EXIT_FAILURE;
+	}
+
+	fd = open(params->outfile, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	if (fd == -1) {
+		perror("Can write file");
+		return EXIT_FAILURE;
+	}
+	if (write(fd, &buf[hdr_size], file_size) != file_size) {
+		perror("Cannot write file");
+		return EXIT_FAILURE;
+	}
+	close(fd);
+
+	return EXIT_SUCCESS;
+}
+
+static int sfspl_check_image_type(uint8_t type)
+{
+	if (type == IH_TYPE_STARFIVE_SPL)
+		return EXIT_SUCCESS;
+
+	return EXIT_FAILURE;
+}
+
+static void sfspl_set_header(void *buf, struct stat *sbuf, int infd,
+			     struct image_tool_params *params)
+{
+	struct spl_hdr *hdr = buf;
+	unsigned int file_size;
+	unsigned int crc;
+
+	file_size = params->file_size - sizeof(struct spl_hdr);
+	crc = crc32(0, &((unsigned char *)buf)[sizeof(struct spl_hdr)],
+		    file_size);
+
+	hdr->offset = cpu_to_le32(DEFAULT_OFFSET);
+	hdr->bkp_offs = cpu_to_le32(DEFAULT_BACKUP);
+	hdr->version = cpu_to_le32(DEFAULT_VERSION);
+	hdr->file_size = cpu_to_le32(file_size);
+	hdr->hdr_size = cpu_to_le32(sizeof(struct spl_hdr));
+	hdr->crc32 = cpu_to_le32(crc);
+}
+
+static int sfspl_vrec_header(struct image_tool_params *params,
+			     struct image_type_params *tparams)
+{
+	tparams->hdr = calloc(sizeof(struct spl_hdr), 1);
+
+	/* No padding */
+	return 0;
+}
+
+U_BOOT_IMAGE_TYPE(
+	sfspl, /* id */
+	"StarFive SPL Image", /* name */
+	sizeof(struct spl_hdr), /* header_size */
+	NULL, /* header */
+	sfspl_check_params, /* check_params */
+	sfspl_verify_header, /* verify header */
+	sfspl_print_header, /* print header */
+	sfspl_set_header, /* set header */
+	sfspl_image_extract_subimage, /* extract_subimage */
+	sfspl_check_image_type, /* check_image_type */
+	NULL, /* fflag_handle */
+	sfspl_vrec_header /* vrec_header */
+);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-06 12:00 [PATCH v2 0/2] riscv: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
  2023-09-06 12:00 ` [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support Heinrich Schuchardt
@ 2023-09-06 12:00 ` Heinrich Schuchardt
  2023-09-16 19:47   ` Milan P. Stanić
  2023-09-16 22:19   ` Jonas Karlman
  1 sibling, 2 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2023-09-06 12:00 UTC (permalink / raw)
  To: Rick Chen, Leo, Yanhong Wang
  Cc: Simon Glass, Marc Kleine-Budde, Chanho Park, u-boot,
	Heinrich Schuchardt

The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
prefixed header. We have referring to a vendor tool (spl_tool) for this
task. 'mkimage -T sfspl' can generate the prefixed file.

Use binman to invoke mkimage for the generation of file
spl/u-boot-spl.bin.normal.out.

Update the documentation.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Chanho Park <chanho61.park@samsung.com>
---
v2:
	Fix a typo in a comment in tools/sfspl.c
	Add Tested-by credit
---
 .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
 doc/board/starfive/visionfive2.rst                 | 14 ++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
index 13f69da31e..defe2b605f 100644
--- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
+++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
@@ -103,4 +103,14 @@
 			};
 		};
 	};
+	u-boot-spl {
+		filename = "spl/u-boot-spl.bin.normal.out";
+
+		mkimage {
+			args = "-T sfspl";
+			blob {
+				filename = "spl/u-boot-spl.bin";
+			};
+		};
+	};
 };
diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
index 941899a0a4..f5575ab68b 100644
--- a/doc/board/starfive/visionfive2.rst
+++ b/doc/board/starfive/visionfive2.rst
@@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
 	make starfive_visionfive2_defconfig
 	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
 
-This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
-
-u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
-to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
-the below command:
-
-	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
-
-More detailed description of spl_tool,please refer spl_tool documenation.
-(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
-
-This will generate u-boot-spl.bin.normal.out file.
+This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
+as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
 
 Flashing
 ~~~~~~~~
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-06 12:00 ` [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
@ 2023-09-16 19:47   ` Milan P. Stanić
  2023-09-16 22:19   ` Jonas Karlman
  1 sibling, 0 replies; 10+ messages in thread
From: Milan P. Stanić @ 2023-09-16 19:47 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Rick Chen, Leo, Yanhong Wang, Simon Glass, Marc Kleine-Budde,
	Chanho Park, u-boot

On Wed, 2023-09-06 at 14:00, Heinrich Schuchardt wrote:
> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
> prefixed header. We have referring to a vendor tool (spl_tool) for this
> task. 'mkimage -T sfspl' can generate the prefixed file.
> 
> Use binman to invoke mkimage for the generation of file
> spl/u-boot-spl.bin.normal.out.
> 
> Update the documentation.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Chanho Park <chanho61.park@samsung.com>

Tested-by: Milan P. Stanić <mps@arvanta.net>

> ---
> v2:
> 	Fix a typo in a comment in tools/sfspl.c
> 	Add Tested-by credit
> ---
>  .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
>  doc/board/starfive/visionfive2.rst                 | 14 ++------------
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> index 13f69da31e..defe2b605f 100644
> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> @@ -103,4 +103,14 @@
>  			};
>  		};
>  	};
> +	u-boot-spl {
> +		filename = "spl/u-boot-spl.bin.normal.out";
> +
> +		mkimage {
> +			args = "-T sfspl";
> +			blob {
> +				filename = "spl/u-boot-spl.bin";
> +			};
> +		};
> +	};
>  };
> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
> index 941899a0a4..f5575ab68b 100644
> --- a/doc/board/starfive/visionfive2.rst
> +++ b/doc/board/starfive/visionfive2.rst
> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
>  	make starfive_visionfive2_defconfig
>  	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
>  
> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
> -
> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
> -the below command:
> -
> -	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
> -
> -More detailed description of spl_tool,please refer spl_tool documenation.
> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
> -
> -This will generate u-boot-spl.bin.normal.out file.
> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
>  
>  Flashing
>  ~~~~~~~~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support
  2023-09-06 12:00 ` [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support Heinrich Schuchardt
@ 2023-09-16 19:47   ` Milan P. Stanić
  0 siblings, 0 replies; 10+ messages in thread
From: Milan P. Stanić @ 2023-09-16 19:47 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Rick Chen, Leo, Yanhong Wang, Simon Glass, Marc Kleine-Budde,
	Chanho Park, u-boot

On Wed, 2023-09-06 at 14:00, Heinrich Schuchardt wrote:
> The StarFive JH7110 base boards require a header to be prefixed to the SPL
> binary image. This has previously done with a vendor tool 'spl_tool'
> published under a GPL-2-or-later license. Integrate this capability into
> mkimage.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Chanho Park <chanho61.park@samsung.com>

Tested-by: Milan P. Stanić <mps@arvanta.net>

> ---
> v2:
> 	no change except for Tested-by credit
> ---
>  boot/image.c    |   1 +
>  include/image.h |   1 +
>  tools/Makefile  |   1 +
>  tools/sfspl.c   | 174 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 177 insertions(+)
>  create mode 100644 tools/sfspl.c
> 
> diff --git a/boot/image.c b/boot/image.c
> index 5c4f9b807d..3a99d2e897 100644
> --- a/boot/image.c
> +++ b/boot/image.c
> @@ -182,6 +182,7 @@ static const table_entry_t uimage_type[] = {
>  	{	IH_TYPE_SUNXI_TOC0, "sunxi_toc0",  "Allwinner TOC0 Boot Image" },
>  	{	IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", },
>  	{	IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" },
> +	{	IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
>  	{	-1,		    "",		  "",			},
>  };
>  
> diff --git a/include/image.h b/include/image.h
> index 01a6787d21..5f85bf84a2 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -231,6 +231,7 @@ enum image_type_t {
>  	IH_TYPE_SUNXI_TOC0,		/* Allwinner TOC0 Boot Image */
>  	IH_TYPE_FDT_LEGACY,		/* Binary Flat Device Tree Blob	in a Legacy Image */
>  	IH_TYPE_RENESAS_SPKG,		/* Renesas SPKG image */
> +	IH_TYPE_STARFIVE_SPL,		/* StarFive SPL image */
>  
>  	IH_TYPE_COUNT,			/* Number of image types */
>  };
> diff --git a/tools/Makefile b/tools/Makefile
> index 3d0c4b0dd6..1aa1e36137 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -123,6 +123,7 @@ dumpimage-mkimage-objs := aisimage.o \
>  			pblimage.o \
>  			pbl_crc32.o \
>  			renesas_spkgimage.o \
> +			sfspl.o \
>  			vybridimage.o \
>  			stm32image.o \
>  			$(ROCKCHIP_OBS) \
> diff --git a/tools/sfspl.c b/tools/sfspl.c
> new file mode 100644
> index 0000000000..ec18a0a77e
> --- /dev/null
> +++ b/tools/sfspl.c
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> + *
> + * The StarFive JH7110 requires to prepend a header to u-boot-spl.bin describing
> + * the payload length and CRC32.
> + *
> + * This module implements support in mkimage and dumpimage for this file format.
> + *
> + * StarFive's spl_tool available under GPL-2.0-and-later at
> + * https://github.com/starfive-tech/Tools implements writing the same file
> + * format and served as a reference.
> + */
> +
> +#include <compiler.h>
> +#include <fcntl.h>
> +#include <u-boot/crc.h>
> +#include <unistd.h>
> +#include "imagetool.h"
> +
> +#define DEFAULT_VERSION 0x01010101
> +#define DEFAULT_BACKUP 0x200000U
> +#define DEFAULT_OFFSET 0x240
> +
> +/**
> + * struct spl_hdr - header for SPL on JH7110
> + *
> + * All fields are low-endian.
> + */
> +struct spl_hdr {
> +	/** @offset:	offset to SPL header (0x240) */
> +	unsigned int offset;
> +	/** @bkp_offs:	address of backup SPL, defaults to DEFAULT_BACKUP */
> +	unsigned int bkp_offs;
> +	/** @zero1:	set to zero */
> +	unsigned int zero1[159];
> +	/** @version:	header version, defaults to DEFAULT_VERSION */
> +	unsigned int version;
> +	/** @file_size:	file size */
> +	unsigned int file_size;
> +	/** @hdr_size:	size of the file header (0x400) */
> +	unsigned int hdr_size;
> +	/** @crc32:	CRC32 */
> +	unsigned int crc32;
> +	/** @zero2:	set to zero */
> +	unsigned int zero2[91];
> +};
> +
> +static int sfspl_check_params(struct image_tool_params *params)
> +{
> +	/* Only the RISC-V architecture is supported */
> +	if (params->Aflag && params->arch != IH_ARCH_RISCV)
> +		return EXIT_FAILURE;
> +
> +	return EXIT_SUCCESS;
> +}
> +
> +static int sfspl_verify_header(unsigned char *buf, int size,
> +			       struct image_tool_params *params)
> +{
> +	struct spl_hdr *hdr = (void *)buf;
> +	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
> +	unsigned int file_size = le32_to_cpu(hdr->file_size);
> +	unsigned int crc = le32_to_cpu(hdr->crc32);
> +	unsigned int crc_check;
> +
> +	if (size < 0 ||
> +	    (size_t)size < sizeof(struct spl_hdr) ||
> +	    (size_t)size < hdr_size + file_size) {
> +		printf("Truncated file\n");
> +		return EXIT_FAILURE;
> +	}
> +	if (hdr->version != DEFAULT_VERSION) {
> +		printf("Unknown file format version\n");
> +		return EXIT_FAILURE;
> +	}
> +	crc_check = crc32(0, &buf[hdr_size], size - hdr_size);
> +	if (crc_check != crc) {
> +		printf("Incorrect CRC32\n");
> +		return EXIT_FAILURE;
> +	}
> +
> +	return EXIT_SUCCESS;
> +}
> +
> +static void sfspl_print_header(const void *buf,
> +			       struct image_tool_params *params)
> +{
> +	struct spl_hdr *hdr = (void *)buf;
> +	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
> +	unsigned int file_size = le32_to_cpu(hdr->file_size);
> +
> +	printf("Header size: %u\n", hdr_size);
> +	printf("Payload size: %u\n", file_size);
> +}
> +
> +static int sfspl_image_extract_subimage(void *ptr,
> +					struct image_tool_params *params)
> +{
> +	struct spl_hdr *hdr = (void *)ptr;
> +	unsigned char *buf = ptr;
> +	int fd;
> +	unsigned int hdr_size = le32_to_cpu(hdr->hdr_size);
> +	unsigned int file_size = le32_to_cpu(hdr->file_size);
> +
> +	if (params->pflag) {
> +		printf("Invalid image index %d\n", params->pflag);
> +		return EXIT_FAILURE;
> +	}
> +
> +	fd = open(params->outfile, O_WRONLY | O_CREAT | O_TRUNC, 0644);
> +	if (fd == -1) {
> +		perror("Can write file");
> +		return EXIT_FAILURE;
> +	}
> +	if (write(fd, &buf[hdr_size], file_size) != file_size) {
> +		perror("Cannot write file");
> +		return EXIT_FAILURE;
> +	}
> +	close(fd);
> +
> +	return EXIT_SUCCESS;
> +}
> +
> +static int sfspl_check_image_type(uint8_t type)
> +{
> +	if (type == IH_TYPE_STARFIVE_SPL)
> +		return EXIT_SUCCESS;
> +
> +	return EXIT_FAILURE;
> +}
> +
> +static void sfspl_set_header(void *buf, struct stat *sbuf, int infd,
> +			     struct image_tool_params *params)
> +{
> +	struct spl_hdr *hdr = buf;
> +	unsigned int file_size;
> +	unsigned int crc;
> +
> +	file_size = params->file_size - sizeof(struct spl_hdr);
> +	crc = crc32(0, &((unsigned char *)buf)[sizeof(struct spl_hdr)],
> +		    file_size);
> +
> +	hdr->offset = cpu_to_le32(DEFAULT_OFFSET);
> +	hdr->bkp_offs = cpu_to_le32(DEFAULT_BACKUP);
> +	hdr->version = cpu_to_le32(DEFAULT_VERSION);
> +	hdr->file_size = cpu_to_le32(file_size);
> +	hdr->hdr_size = cpu_to_le32(sizeof(struct spl_hdr));
> +	hdr->crc32 = cpu_to_le32(crc);
> +}
> +
> +static int sfspl_vrec_header(struct image_tool_params *params,
> +			     struct image_type_params *tparams)
> +{
> +	tparams->hdr = calloc(sizeof(struct spl_hdr), 1);
> +
> +	/* No padding */
> +	return 0;
> +}
> +
> +U_BOOT_IMAGE_TYPE(
> +	sfspl, /* id */
> +	"StarFive SPL Image", /* name */
> +	sizeof(struct spl_hdr), /* header_size */
> +	NULL, /* header */
> +	sfspl_check_params, /* check_params */
> +	sfspl_verify_header, /* verify header */
> +	sfspl_print_header, /* print header */
> +	sfspl_set_header, /* set header */
> +	sfspl_image_extract_subimage, /* extract_subimage */
> +	sfspl_check_image_type, /* check_image_type */
> +	NULL, /* fflag_handle */
> +	sfspl_vrec_header /* vrec_header */
> +);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-06 12:00 ` [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
  2023-09-16 19:47   ` Milan P. Stanić
@ 2023-09-16 22:19   ` Jonas Karlman
  2023-09-17  0:00     ` Heinrich Schuchardt
  1 sibling, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2023-09-16 22:19 UTC (permalink / raw)
  To: Heinrich Schuchardt, Rick Chen, Leo, Yanhong Wang
  Cc: Simon Glass, Marc Kleine-Budde, Chanho Park, u-boot

On 2023-09-06 14:00, Heinrich Schuchardt wrote:
> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
> prefixed header. We have referring to a vendor tool (spl_tool) for this
> task. 'mkimage -T sfspl' can generate the prefixed file.
> 
> Use binman to invoke mkimage for the generation of file
> spl/u-boot-spl.bin.normal.out.
> 
> Update the documentation.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Chanho Park <chanho61.park@samsung.com>
> ---
> v2:
> 	Fix a typo in a comment in tools/sfspl.c
> 	Add Tested-by credit
> ---
>  .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
>  doc/board/starfive/visionfive2.rst                 | 14 ++------------
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> index 13f69da31e..defe2b605f 100644
> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> @@ -103,4 +103,14 @@
>  			};
>  		};
>  	};
> +	u-boot-spl {
> +		filename = "spl/u-boot-spl.bin.normal.out";
> +
> +		mkimage {
> +			args = "-T sfspl";
> +			blob {
> +				filename = "spl/u-boot-spl.bin";
> +			};
> +		};
> +	};

This should probably be:

mkimage {
	filename = "spl/u-boot-spl.bin.normal.out";
	args = "-T sfspl";

	u-boot-spl {
	};
};

Regards,
Jonas

>  };
> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
> index 941899a0a4..f5575ab68b 100644
> --- a/doc/board/starfive/visionfive2.rst
> +++ b/doc/board/starfive/visionfive2.rst
> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
>  	make starfive_visionfive2_defconfig
>  	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
>  
> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
> -
> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
> -the below command:
> -
> -	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
> -
> -More detailed description of spl_tool,please refer spl_tool documenation.
> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
> -
> -This will generate u-boot-spl.bin.normal.out file.
> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
>  
>  Flashing
>  ~~~~~~~~


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-16 22:19   ` Jonas Karlman
@ 2023-09-17  0:00     ` Heinrich Schuchardt
  2023-09-17  7:14       ` Massimo Pegorer
  2023-09-17  9:02       ` Jonas Karlman
  0 siblings, 2 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2023-09-17  0:00 UTC (permalink / raw)
  To: Jonas Karlman, Simon Glass
  Cc: Simon Glass, Marc Kleine-Budde, Chanho Park, u-boot, Rick Chen,
	Leo, Yanhong Wang



On 9/17/23 00:19, Jonas Karlman wrote:
> On 2023-09-06 14:00, Heinrich Schuchardt wrote:
>> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
>> prefixed header. We have referring to a vendor tool (spl_tool) for this
>> task. 'mkimage -T sfspl' can generate the prefixed file.
>>
>> Use binman to invoke mkimage for the generation of file
>> spl/u-boot-spl.bin.normal.out.
>>
>> Update the documentation.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> Tested-by: Chanho Park <chanho61.park@samsung.com>
>> ---
>> v2:
>> 	Fix a typo in a comment in tools/sfspl.c
>> 	Add Tested-by credit
>> ---
>>   .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
>>   doc/board/starfive/visionfive2.rst                 | 14 ++------------
>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>> index 13f69da31e..defe2b605f 100644
>> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>> @@ -103,4 +103,14 @@
>>   			};
>>   		};
>>   	};
>> +	u-boot-spl {
>> +		filename = "spl/u-boot-spl.bin.normal.out";
>> +
>> +		mkimage {
>> +			args = "-T sfspl";
>> +			blob {
>> +				filename = "spl/u-boot-spl.bin";
>> +			};
>> +		};
>> +	};
> 
> This should probably be:
> 
> mkimage {
> 	filename = "spl/u-boot-spl.bin.normal.out";
> 	args = "-T sfspl";
> 
> 	u-boot-spl {
> 	};
> };

@Jonas
If I replace the node u-boot-spl by the suggested mkimage node, I get a 
file spl/u-boot-spl.bin.normal.out which is identical to 
spl/u-boot-spl.bin. It lacks the header that mkimage should create.

Replacing the blob node by a u-boot-spl node is possible. Whether it is 
better readable is a matter of taste.

@Simon:
Could you, please, have a look at doc/develop/package/entries.rst. It 
seems not to fully describe how binman is controlled via the device-tree.

* A blob sub-node for the mkimage node is not described.
* A mkimage node which is not a direct subnode of binman is not mentioned.
* Why the target filename must be outside of the mkimage node in my case 
is not evident.

Maybe a more formal description of the schema in a yaml file would help.

Best regards

Heinrich

> 
> Regards,
> Jonas
> 
>>   };
>> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
>> index 941899a0a4..f5575ab68b 100644
>> --- a/doc/board/starfive/visionfive2.rst
>> +++ b/doc/board/starfive/visionfive2.rst
>> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
>>   	make starfive_visionfive2_defconfig
>>   	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
>>   
>> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
>> -
>> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
>> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
>> -the below command:
>> -
>> -	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
>> -
>> -More detailed description of spl_tool,please refer spl_tool documenation.
>> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
>> -
>> -This will generate u-boot-spl.bin.normal.out file.
>> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
>> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
>>   
>>   Flashing
>>   ~~~~~~~~
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-17  0:00     ` Heinrich Schuchardt
@ 2023-09-17  7:14       ` Massimo Pegorer
  2023-09-17  8:48         ` Massimo Pegorer
  2023-09-17  9:02       ` Jonas Karlman
  1 sibling, 1 reply; 10+ messages in thread
From: Massimo Pegorer @ 2023-09-17  7:14 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Jonas Karlman, Simon Glass, Marc Kleine-Budde, Chanho Park,
	u-boot, Rick Chen, Leo, Yanhong Wang

Il giorno dom 17 set 2023 alle ore 02:01 Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> ha scritto:
>
>
>
> On 9/17/23 00:19, Jonas Karlman wrote:
> > On 2023-09-06 14:00, Heinrich Schuchardt wrote:
> >> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
> >> prefixed header. We have referring to a vendor tool (spl_tool) for this
> >> task. 'mkimage -T sfspl' can generate the prefixed file.
> >>
> >> Use binman to invoke mkimage for the generation of file
> >> spl/u-boot-spl.bin.normal.out.
> >>
> >> Update the documentation.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> Tested-by: Chanho Park <chanho61.park@samsung.com>
> >> ---
> >> v2:
> >>      Fix a typo in a comment in tools/sfspl.c
> >>      Add Tested-by credit
> >> ---
> >>   .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
> >>   doc/board/starfive/visionfive2.rst                 | 14 ++------------
> >>   2 files changed, 12 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> >> index 13f69da31e..defe2b605f 100644
> >> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> >> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> >> @@ -103,4 +103,14 @@
> >>                      };
> >>              };
> >>      };
> >> +    u-boot-spl {
> >> +            filename = "spl/u-boot-spl.bin.normal.out";
> >> +
> >> +            mkimage {
> >> +                    args = "-T sfspl";
> >> +                    blob {
> >> +                            filename = "spl/u-boot-spl.bin";
> >> +                    };
> >> +            };
> >> +    };
> >
> > This should probably be:
> >
> > mkimage {
> >       filename = "spl/u-boot-spl.bin.normal.out";
> >       args = "-T sfspl";
> >
> >       u-boot-spl {
> >       };
> > };
>
> @Jonas
> If I replace the node u-boot-spl by the suggested mkimage node, I get a
> file spl/u-boot-spl.bin.normal.out which is identical to
> spl/u-boot-spl.bin. It lacks the header that mkimage should create.
>
> Replacing the blob node by a u-boot-spl node is possible. Whether it is
> better readable is a matter of taste.

It is also a matter of default/standard assumptions and conventions,
and thus opportunities. If SPL binary filename will change in the
future for any reason, a single fix to binman would be enough for
everybody using u-boot-spl node, while blob node users should fix each
one of the -u-boot.dtsi files.

>
> @Simon:
> Could you, please, have a look at doc/develop/package/entries.rst. It
> seems not to fully describe how binman is controlled via the device-tree.
>
> * A blob sub-node for the mkimage node is not described.
> * A mkimage node which is not a direct subnode of binman is not mentioned.
> * Why the target filename must be outside of the mkimage node in my case
> is not evident.
>
> Maybe a more formal description of the schema in a yaml file would help.
>
> Best regards
>
> Heinrich
>
> >
> > Regards,
> > Jonas
> >
> >>   };
> >> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
> >> index 941899a0a4..f5575ab68b 100644
> >> --- a/doc/board/starfive/visionfive2.rst
> >> +++ b/doc/board/starfive/visionfive2.rst
> >> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
> >>      make starfive_visionfive2_defconfig
> >>      make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
> >>
> >> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
> >> -
> >> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
> >> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
> >> -the below command:
> >> -
> >> -    ./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
> >> -
> >> -More detailed description of spl_tool,please refer spl_tool documenation.
> >> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
> >> -
> >> -This will generate u-boot-spl.bin.normal.out file.
> >> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
> >> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
> >>
> >>   Flashing
> >>   ~~~~~~~~
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-17  7:14       ` Massimo Pegorer
@ 2023-09-17  8:48         ` Massimo Pegorer
  0 siblings, 0 replies; 10+ messages in thread
From: Massimo Pegorer @ 2023-09-17  8:48 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Jonas Karlman, Simon Glass, Marc Kleine-Budde, Chanho Park,
	u-boot, Rick Chen, Leo, Yanhong Wang

Il giorno dom 17 set 2023 alle ore 09:14 Massimo Pegorer
<massimo.pegorer+oss@gmail.com> ha scritto:
>
> Il giorno dom 17 set 2023 alle ore 02:01 Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> ha scritto:
> >
> >
> >
> > On 9/17/23 00:19, Jonas Karlman wrote:
> > > On 2023-09-06 14:00, Heinrich Schuchardt wrote:
> > >> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
> > >> prefixed header. We have referring to a vendor tool (spl_tool) for this
> > >> task. 'mkimage -T sfspl' can generate the prefixed file.
> > >>
> > >> Use binman to invoke mkimage for the generation of file
> > >> spl/u-boot-spl.bin.normal.out.
> > >>
> > >> Update the documentation.
> > >>
> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >> Tested-by: Chanho Park <chanho61.park@samsung.com>
> > >> ---
> > >> v2:
> > >>      Fix a typo in a comment in tools/sfspl.c
> > >>      Add Tested-by credit
> > >> ---
> > >>   .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
> > >>   doc/board/starfive/visionfive2.rst                 | 14 ++------------
> > >>   2 files changed, 12 insertions(+), 12 deletions(-)
> > >>
> > >> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> > >> index 13f69da31e..defe2b605f 100644
> > >> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> > >> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> > >> @@ -103,4 +103,14 @@
> > >>                      };
> > >>              };
> > >>      };
> > >> +    u-boot-spl {
> > >> +            filename = "spl/u-boot-spl.bin.normal.out";
> > >> +
> > >> +            mkimage {
> > >> +                    args = "-T sfspl";
> > >> +                    blob {
> > >> +                            filename = "spl/u-boot-spl.bin";
> > >> +                    };
> > >> +            };
> > >> +    };
> > >
> > > This should probably be:
> > >
> > > mkimage {
> > >       filename = "spl/u-boot-spl.bin.normal.out";
> > >       args = "-T sfspl";
> > >
> > >       u-boot-spl {
> > >       };
> > > };
> >
> > @Jonas
> > If I replace the node u-boot-spl by the suggested mkimage node, I get a
> > file spl/u-boot-spl.bin.normal.out which is identical to
> > spl/u-boot-spl.bin. It lacks the header that mkimage should create.

Images and entries are different things for binman. It expects an
image at first level, while mkimage is an entry. If you wrap Jonas'
suggestion in an image declaration, you will get what you need.
Something like:

anyname {
    mkimage {
        filename = "spl/u-boot-spl.bin.normal.out";
        args = "-T sfspl";

        u-boot-spl {
        };
    };
};

My personal and questionable suggestions:
 - Do not make binman/mkimage working in spl subfolder.
 - Do not use a generic term like "normal" in filename.
 - Do not use .out filename extension instead of .bin

Thus I would suggest something like:

u-boot-spl.starfive {
    mkimage {
        filename = "u-boot-spl.starfive.bin";
        args = "-T sfspl";

        u-boot-spl {
        };
    };
};

Regards,
Massimo


> >
> > Replacing the blob node by a u-boot-spl node is possible. Whether it is
> > better readable is a matter of taste.
>
> It is also a matter of default/standard assumptions and conventions,
> and thus opportunities. If SPL binary filename will change in the
> future for any reason, a single fix to binman would be enough for
> everybody using u-boot-spl node, while blob node users should fix each
> one of the -u-boot.dtsi files.
>
> >
> > @Simon:
> > Could you, please, have a look at doc/develop/package/entries.rst. It
> > seems not to fully describe how binman is controlled via the device-tree.
> >
> > * A blob sub-node for the mkimage node is not described.
> > * A mkimage node which is not a direct subnode of binman is not mentioned.
> > * Why the target filename must be outside of the mkimage node in my case
> > is not evident.
> >
> > Maybe a more formal description of the schema in a yaml file would help.
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > Regards,
> > > Jonas
> > >
> > >>   };
> > >> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
> > >> index 941899a0a4..f5575ab68b 100644
> > >> --- a/doc/board/starfive/visionfive2.rst
> > >> +++ b/doc/board/starfive/visionfive2.rst
> > >> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
> > >>      make starfive_visionfive2_defconfig
> > >>      make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
> > >>
> > >> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
> > >> -
> > >> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
> > >> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
> > >> -the below command:
> > >> -
> > >> -    ./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
> > >> -
> > >> -More detailed description of spl_tool,please refer spl_tool documenation.
> > >> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool)
> > >> -
> > >> -This will generate u-boot-spl.bin.normal.out file.
> > >> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
> > >> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
> > >>
> > >>   Flashing
> > >>   ~~~~~~~~
> > >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out
  2023-09-17  0:00     ` Heinrich Schuchardt
  2023-09-17  7:14       ` Massimo Pegorer
@ 2023-09-17  9:02       ` Jonas Karlman
  1 sibling, 0 replies; 10+ messages in thread
From: Jonas Karlman @ 2023-09-17  9:02 UTC (permalink / raw)
  To: Heinrich Schuchardt, Simon Glass
  Cc: Marc Kleine-Budde, Chanho Park, u-boot, Rick Chen, Leo,
	Yanhong Wang

On 2023-09-17 02:00, Heinrich Schuchardt wrote:
> On 9/17/23 00:19, Jonas Karlman wrote:
>> On 2023-09-06 14:00, Heinrich Schuchardt wrote:
>>> The StarFive VisionFive 2 board cannot load spl/u-boot-spl.bin but needs a
>>> prefixed header. We have referring to a vendor tool (spl_tool) for this
>>> task. 'mkimage -T sfspl' can generate the prefixed file.
>>>
>>> Use binman to invoke mkimage for the generation of file
>>> spl/u-boot-spl.bin.normal.out.
>>>
>>> Update the documentation.
>>>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> Tested-by: Chanho Park <chanho61.park@samsung.com>
>>> ---
>>> v2:
>>> 	Fix a typo in a comment in tools/sfspl.c
>>> 	Add Tested-by credit
>>> ---
>>>   .../dts/jh7110-starfive-visionfive-2-u-boot.dtsi   | 10 ++++++++++
>>>   doc/board/starfive/visionfive2.rst                 | 14 ++------------
>>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> index 13f69da31e..defe2b605f 100644
>>> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
>>> @@ -103,4 +103,14 @@
>>>   			};
>>>   		};
>>>   	};
>>> +	u-boot-spl {
>>> +		filename = "spl/u-boot-spl.bin.normal.out";
>>> +
>>> +		mkimage {
>>> +			args = "-T sfspl";
>>> +			blob {
>>> +				filename = "spl/u-boot-spl.bin";
>>> +			};
>>> +		};
>>> +	};
>>
>> This should probably be:
>>
>> mkimage {
>> 	filename = "spl/u-boot-spl.bin.normal.out";
>> 	args = "-T sfspl";
>>
>> 	u-boot-spl {
>> 	};
>> };
> 
> @Jonas
> If I replace the node u-boot-spl by the suggested mkimage node, I get a 
> file spl/u-boot-spl.bin.normal.out which is identical to 
> spl/u-boot-spl.bin. It lacks the header that mkimage should create.

You are correct, with the multiple-images prop in binman node each
subnode is treated as an image and not as an entry type node.
See tools/binman/image.py for some description of image type.

This means we need to have a wrapping image node and cannot directly
use an entry type node at top level under binman node.

> 
> Replacing the blob node by a u-boot-spl node is possible. Whether it is 
> better readable is a matter of taste.

The u-boot-spl entry type is meant to be used for referencing the U-Boot
SPL binary and should be preferred over a plain blob entry.

I would also suggest you name the image node something other then
u-boot-spl because it was easily confused with the entry type of same
name.

E.g. something like:

spl {
	filename = "spl/u-boot-spl.bin.normal.out";

	mkimage {
		args = "-T sfspl";

		u-boot-spl {
		};
	};
};

Regards,
Jonas

> 
> @Simon:
> Could you, please, have a look at doc/develop/package/entries.rst. It 
> seems not to fully describe how binman is controlled via the device-tree.
> 
> * A blob sub-node for the mkimage node is not described.
> * A mkimage node which is not a direct subnode of binman is not mentioned.
> * Why the target filename must be outside of the mkimage node in my case 
> is not evident.
> 
> Maybe a more formal description of the schema in a yaml file would help.
> 
> Best regards
> 
> Heinrich
> 
>>
>> Regards,
>> Jonas
>>
>>>   };
>>> diff --git a/doc/board/starfive/visionfive2.rst b/doc/board/starfive/visionfive2.rst
>>> index 941899a0a4..f5575ab68b 100644
>>> --- a/doc/board/starfive/visionfive2.rst
>>> +++ b/doc/board/starfive/visionfive2.rst
>>> @@ -65,18 +65,8 @@ Now build the U-Boot SPL and U-Boot proper
>>>   	make starfive_visionfive2_defconfig
>>>   	make OPENSBI=$(opensbi_dir)/opensbi/build/platform/generic/firmware/fw_dynamic.bin
>>>   
>>> -This will generate spl/u-boot-spl.bin and FIT image (u-boot.itb)
>>> -
>>> -u-boot-spl.bin cannot be used directly on StarFive VisionFive2,we need
>>> -to convert the u-boot-spl.bin to u-boot-spl.bin.normal.out with
>>> -the below command:
>>> -
>>> -	./spl_tool -c -f $(Uboot_PATH)/spl/u-boot-spl.bin
>>> -
>>> -More detailed description of spl_tool,please refer spl_tool documenation.
>>> -(Note: spl_tool git repo is at https://github.com/starfive-tech/Tools/tree/master/spl_tool
>>> -
>>> -This will generate u-boot-spl.bin.normal.out file.
>>> +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as well
>>> +as the FIT image (u-boot.itb) with OpenSBI and U-Boot.
>>>   
>>>   Flashing
>>>   ~~~~~~~~
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-09-17  9:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-06 12:00 [PATCH v2 0/2] riscv: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
2023-09-06 12:00 ` [PATCH v2 1/2] tools: mkimage: Add StarFive SPL image support Heinrich Schuchardt
2023-09-16 19:47   ` Milan P. Stanić
2023-09-06 12:00 ` [PATCH v2 2/2] riscv: dts: starfive: generate u-boot-spl.bin.normal.out Heinrich Schuchardt
2023-09-16 19:47   ` Milan P. Stanić
2023-09-16 22:19   ` Jonas Karlman
2023-09-17  0:00     ` Heinrich Schuchardt
2023-09-17  7:14       ` Massimo Pegorer
2023-09-17  8:48         ` Massimo Pegorer
2023-09-17  9:02       ` Jonas Karlman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox