SPL: extend FIT loading support

Currently the SPL FIT loader uses the spl_fit_select_fdt() function to find the offset to the right DTB within the FIT image.
For this it iterates over all subnodes of the /configuration node in the FIT tree and compares all "description" strings therein using a board specific matching function.
If that finds a match, it uses the string in the "fdt" property of that subnode to locate the matching subnode in the /images node, which points to the DTB data.
Now this works very well, but is quite specific to cover this particular use case. To open up the door for a more generic usage, let's split this function into:
1) a function that just returns the node offset for the matching configuration node (spl_fit_find_config_node())
2) a function that returns the image data any given property in a given configuration node points to, additionally using a given index into a possbile list of strings (spl_fit_select_index())
This allows us to replace the specific function above by asking for the image the _first string of the "fdt" property_ in the matching configuration subnode points to.

This patch introduces no functional changes, it just refactors the code to allow reusing it later.

(diff is overly clever here and produces a hard-to-read patch, so I recommend to throw a look at the result instead).

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 83 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 31 deletions(-).

common/spl/spl_fit.c +2 -10

 {
-       const char *name, *fdt_name;
-       int conf, node, fdt_node;
-       int len;
+       const char *name;
+       int conf, node, len;
 
-       *fdt_offsetp = 0;
        conf = fdt_path_offset(fdt, FIT_CONFS_PATH);
        if (conf < 0) {
                debug("%s: Cannot find /configurations node: %d\n", __func__,

@@ -50,39 +48,61 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
                        continue;
 
                debug("Selecting config '%s'", name);
-               fdt_name = fdt_getprop(fdt, node, FIT_FDT_PROP, &len);
-               if (!fdt_name) {
-                       debug("%s: Cannot find fdt name property: %d\n",
-                             __func__, len);
-                       return -EINVAL;
-               }
 

Hi Andre,

Quoted 23 lines · Andre Przywara

Currently the SPL FIT loader uses the spl_fit_select_fdt() function to find the offset to the right DTB within the FIT image.
For this it iterates over all subnodes of the /configuration node in the FIT tree and compares all "description" strings therein using a board specific matching function.
If that finds a match, it uses the string in the "fdt" property of that subnode to locate the matching subnode in the /images node, which points to the DTB data.
Now this works very well, but is quite specific to cover this particular use case. To open up the door for a more generic usage, let's split this function into:
1) a function that just returns the node offset for the matching configuration node (spl_fit_find_config_node())
2) a function that returns the image data any given property in a given configuration node points to, additionally using a given index into a possbile list of strings (spl_fit_select_index())
This allows us to replace the specific function above by asking for the image the _first string of the "fdt" property_ in the matching configuration subnode points to.

This patch introduces no functional changes, it just refactors the code to allow reusing it later.

Quoted 2 lines

(diff is overly clever here and produces a hard-to-read patch, so I recommend to throw a look at the result instead).

First I want to commend you on your excellent commit messages. For example this one explains the current situation, the change your commit performs and the motivation for that change. With these more complicated / core pieces, it is very valuable and you are an example to us all :-)

Quoted 6 lines

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 83 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 31 deletions(-).

Reviewed-by Simon Glass <[email protected]>

I think we need a pytest for this somewhere in this series. With sandbox_spl we can run spl/u-boot-spl and it jumps to u-boot. Can we use this to check that the right thing happens in a few simple cases?

common/spl/spl_fit.c static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop) · line 25

Quoted 11 lines

common/spl/spl_fit.c +1 -1

index aae556f..ba45e65 100644
@@ -22,13 +22,11 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)
        return fdt32_to_cpu(*cell);
 }
 
-static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
+static int spl_fit_find_config_node(const void *fdt)

Can you comment this function please? I should have done this myself.

Quoted 22 lines

common/spl/spl_fit.c +2 -10

 {
-       const char *name, *fdt_name;
-       int conf, node, fdt_node;
-       int len;
+       const char *name;
+       int conf, node, len;
 
-       *fdt_offsetp = 0;
        conf = fdt_path_offset(fdt, FIT_CONFS_PATH);
        if (conf < 0) {
                debug("%s: Cannot find /configurations node: %d\n", __func__,

@@ -50,39 +48,61 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
                        continue;
 
                debug("Selecting config '%s'", name);
-               fdt_name = fdt_getprop(fdt, node, FIT_FDT_PROP, &len);
-               if (!fdt_name) {
-                       debug("%s: Cannot find fdt name property: %d\n",
-                             __func__, len);
-                       return -EINVAL;
-               }
 

common/spl/spl_fit.c static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) · line 55

Quoted 10 lines

common/spl/spl_fit.c +4 -6

-               debug(", fdt '%s'\n", fdt_name);
-               fdt_node = fdt_subnode_offset(fdt, images, fdt_name);
-               if (fdt_node < 0) {
-                       debug("%s: Cannot find fdt node '%s': %d\n",
-                             __func__, fdt_name, fdt_node);
-                       return -EINVAL;
+               return node;
+       }
+
+       return -1;

How about -ENOENT?

common/spl/spl_fit.c static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) · line 59

Quoted 4 lines

common/spl/spl_fit.c +4 -0

+}
+
+static int spl_fit_select_index(const void *fit, int images, int *offsetp,
+                               const char *type, int index)

And this one.

Hi Simon,

Quoted 26 lines · Simon Glass

Hi Andre,

Quoted 23 lines · Andre Przywara

Currently the SPL FIT loader uses the spl_fit_select_fdt() function to find the offset to the right DTB within the FIT image.
For this it iterates over all subnodes of the /configuration node in the FIT tree and compares all "description" strings therein using a board specific matching function.
If that finds a match, it uses the string in the "fdt" property of that subnode to locate the matching subnode in the /images node, which points to the DTB data.
Now this works very well, but is quite specific to cover this particular use case. To open up the door for a more generic usage, let's split this function into:
1) a function that just returns the node offset for the matching configuration node (spl_fit_find_config_node())
2) a function that returns the image data any given property in a given configuration node points to, additionally using a given index into a possbile list of strings (spl_fit_select_index())
This allows us to replace the specific function above by asking for the image the _first string of the "fdt" property_ in the matching configuration subnode points to.

This patch introduces no functional changes, it just refactors the code to allow reusing it later.

Quoted 8 lines

Quoted 2 lines

(diff is overly clever here and produces a hard-to-read patch, so I recommend to throw a look at the result instead).

First I want to commend you on your excellent commit messages. For example this one explains the current situation, the change your commit performs and the motivation for that change. With these more complicated / core pieces, it is very valuable and you are an example to us all :-)

Thank you very much, you made my day. That is a welcome departure from the usual Linux ML communication style ;-)

And yes, will fix those things you mentioned below, though have to wrap my mind about pytest first.

Now back into the rough waters of the Linux mailing lists ...

Currently the SPL FIT loader uses the spl_fit_select_fdt() function to find the offset to the right DTB within the FIT image.
For this it iterates over all subnodes of the /configuration node in the FIT tree and compares all "description" strings therein using a board specific matching function.
If that finds a match, it uses the string in the "fdt" property of that subnode to locate the matching subnode in the /images node, which points to the DTB data.
Now this works very well, but is quite specific to cover this particular use case. To open up the door for a more generic usage, let's split this function into:
1) a function that just returns the node offset for the matching configuration node (spl_fit_find_config_node())
2) a function that returns the image data any given property in a given configuration node points to, additionally using a given index into a possbile list of strings (spl_fit_select_index())
This allows us to replace the specific function above by asking for the image the _first string of the "fdt" property_ in the matching configuration subnode points to.

This patch introduces no functional changes, it just refactors the code to allow reusing it later.

(diff is overly clever here and produces a hard-to-read patch, so I recommend to throw a look at the result instead).

Signed-off-by Andre Przywara <[email protected]>

Currently the SPL FIT loader always looks only for the first image in the /images node a FIT tree, which it loads and later executes.

Generalize this by looking for a "firmware" property in the matched configuration subnode, or, if that does not exist, for the first string in the "loadables" property. Then using the string in that property, load the image of that name from the /images node.
This still loads only one image at the moment, but refactors the code to allow extending this in a following patch.
To simplify later re-usage, we also generalize the spl_fit_select_index() function to not return the image location, but just the node offset.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

Currently the SPL FIT loader always looks only for the first image in the /images node a FIT tree, which it loads and later executes.

Generalize this by looking for a "firmware" property in the matched configuration subnode, or, if that does not exist, for the first string in the "loadables" property. Then using the string in that property, load the image of that name from the /images node.
This still loads only one image at the moment, but refactors the code to allow extending this in a following patch.
To simplify later re-usage, we also generalize the spl_fit_select_index() function to not return the image location, but just the node offset.

Signed-off-by Andre Przywara <[email protected]>

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by Andre Przywara <[email protected]>

---

 common/spl/spl_fit.c | 129 +++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 72 deletions(-)

common/spl/spl_fit.c +57 -72

index 572a5db..ad5ba15 100644
@@ -18,7 +18,7 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)
 
 	cell = fdt_getprop(fdt, node, prop, &len);
 	if (len != sizeof(*cell))
-		return -1U;
+		return -1UL;
 	return fdt32_to_cpu(*cell);
 }
 

@@ -139,19 +139,63 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	return (data_size + info->bl_len - 1) / info->bl_len;
 }
 
+static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
+			      void *fit, ulong base_offset, int node,
+			      struct spl_image_info *image_info)
+{
+	ulong offset;
+	size_t length;
+	ulong load_addr, load_ptr, entry;
+	void *src;
+	ulong overhead;
+	int nr_sectors;
+	int align_len = ARCH_DMA_MINALIGN - 1;
+
+	offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;
+	length = fdt_getprop_u32(fit, node, "data-size");
+	load_addr = fdt_getprop_u32(fit, node, "load");
+	if (load_addr == -1UL && image_info)
+		load_addr = image_info->load_addr;
+	load_ptr = (load_addr + align_len) & ~align_len;
+	entry = fdt_getprop_u32(fit, node, "entry");
+
+	overhead = get_aligned_image_overhead(info, offset);
+	nr_sectors = get_aligned_image_size(info, length, offset);
+
+	if (info->read(info, sector + get_aligned_image_offset(info, offset),
+		       nr_sectors, (void*)load_ptr) != nr_sectors)
+		return -EIO;
+	debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
+	      (unsigned long)length);
+
+	src = (void *)load_ptr + overhead;
+#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
+	board_fit_image_post_process(&src, &length);
+#endif
+
+	memcpy((void*)load_addr, src, length);
+
+	if (image_info) {
+		image_info->load_addr = load_addr;
+		image_info->size = length;
+		if (entry == -1UL)
+			image_info->entry_point = load_addr;
+		else
+			image_info->entry_point = entry;
+	}
+
+	return 0;
+}
+
 int spl_load_simple_fit(struct spl_image_info *spl_image,
 			struct spl_load_info *info, ulong sector, void *fit)
 {
 	int sectors;
-	ulong size, load;
+	ulong size;
 	unsigned long count;
+	struct spl_image_info image_info;
 	int node, images;
-	void *load_ptr;
-	int fdt_offset, fdt_len;
-	int data_offset, data_size;
 	int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
-	int src_sector;
-	void *dst, *src;
 
 	/*
 	 * Figure out where the external images start. This is the base for the

@@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		return -1;
 	}
 
-	/* Get its information and set up the spl_image structure */
-	data_offset = fdt_getprop_u32(fit, node, "data-offset");
-	data_size = fdt_getprop_u32(fit, node, "data-size");
-	load = fdt_getprop_u32(fit, node, "load");
-	debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
-	spl_image->load_addr = load;
-	spl_image->entry_point = load;
+	/* Load the image and set up the spl_image structure */
+	spl_load_fit_image(info, sector, fit, base_offset, node, spl_image);
 	spl_image->os = IH_OS_U_BOOT;
 
-	/*
-	 * Work out where to place the image. We read it so that the first
-	 * byte will be at 'load'. This may mean we need to load it starting
-	 * before then, since we can only read whole blocks.
-	 */
-	data_offset += base_offset;
-	sectors = get_aligned_image_size(info, data_size, data_offset);
-	load_ptr = (void *)load;
-	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
-	dst = load_ptr;
-
-	/* Read the image */
-	src_sector = sector + get_aligned_image_offset(info, data_offset);
-	debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",
-	      dst, src_sector, sectors);
-	count = info->read(info, src_sector, sectors, dst);
-	if (count != sectors)
-		return -EIO;
-	debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
-	      data_size);
-	src = dst + get_aligned_image_overhead(info, data_offset);
-
-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
-	board_fit_image_post_process((void **)&src, (size_t *)&data_size);
-#endif
-
-	memcpy(dst, src, data_size);
-
 	/* Figure out which device tree the board wants to use */
 	node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
 	if (node < 0) {
 		debug("%s: cannot find FDT node\n", __func__);
 		return node;
 	}
-	fdt_offset = fdt_getprop_u32(fit, node, "data-offset");
-	fdt_len = fdt_getprop_u32(fit, node, "data-size");
-
-	/*
-	 * Read the device tree and place it after the image. There may be
-	 * some extra data before it since we can only read entire blocks.
-	 * And also align the destination address to ARCH_DMA_MINALIGN.
-	 */
-	dst = (void *)((load + data_size + align_len) & ~align_len);
-	fdt_offset += base_offset;
-	sectors = get_aligned_image_size(info, fdt_len, fdt_offset);
-	src_sector = sector + get_aligned_image_offset(info, fdt_offset);
-	count = info->read(info, src_sector, sectors, dst);
-	debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",
-	      dst, src_sector, sectors);
-	if (count != sectors)
-		return -EIO;
 
 	/*
-	 * Copy the device tree so that it starts immediately after the image.
-	 * After this we will have the U-Boot image and its device tree ready
-	 * for us to start.
+	 * Read the device tree and place it after the image.
+	 * Align the destination address to ARCH_DMA_MINALIGN.
 	 */
-	debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
-	      fdt_len);
-	src = dst + get_aligned_image_overhead(info, fdt_offset);
-	dst = load_ptr + data_size;
-
-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
-	board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
-#endif
-
-	memcpy(dst, src, fdt_len);
+	image_info.load_addr = spl_image->load_addr + spl_image->size;
+	spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
 
 	return 0;
 }

Quoted 7 lines · Andre Przywara

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by Andre Przywara <[email protected]>

---

Acked-by Andrew F. Davis <[email protected]>

+Franklin,

This patch, and #12 look like something you would be interested in for your 66AK2G0x work.

Andrew

Quoted 176 lines

 common/spl/spl_fit.c | 129 +++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 72 deletions(-)

common/spl/spl_fit.c +57 -72

index 572a5db..ad5ba15 100644
@@ -18,7 +18,7 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)
 
 	cell = fdt_getprop(fdt, node, prop, &len);
 	if (len != sizeof(*cell))
-		return -1U;
+		return -1UL;
 	return fdt32_to_cpu(*cell);
 }
 

@@ -139,19 +139,63 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	return (data_size + info->bl_len - 1) / info->bl_len;
 }
 
+static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
+			      void *fit, ulong base_offset, int node,
+			      struct spl_image_info *image_info)
+{
+	ulong offset;
+	size_t length;
+	ulong load_addr, load_ptr, entry;
+	void *src;
+	ulong overhead;
+	int nr_sectors;
+	int align_len = ARCH_DMA_MINALIGN - 1;
+
+	offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;
+	length = fdt_getprop_u32(fit, node, "data-size");
+	load_addr = fdt_getprop_u32(fit, node, "load");
+	if (load_addr == -1UL && image_info)
+		load_addr = image_info->load_addr;
+	load_ptr = (load_addr + align_len) & ~align_len;
+	entry = fdt_getprop_u32(fit, node, "entry");
+
+	overhead = get_aligned_image_overhead(info, offset);
+	nr_sectors = get_aligned_image_size(info, length, offset);
+
+	if (info->read(info, sector + get_aligned_image_offset(info, offset),
+		       nr_sectors, (void*)load_ptr) != nr_sectors)
+		return -EIO;
+	debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
+	      (unsigned long)length);
+
+	src = (void *)load_ptr + overhead;
+#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
+	board_fit_image_post_process(&src, &length);
+#endif
+
+	memcpy((void*)load_addr, src, length);
+
+	if (image_info) {
+		image_info->load_addr = load_addr;
+		image_info->size = length;
+		if (entry == -1UL)
+			image_info->entry_point = load_addr;
+		else
+			image_info->entry_point = entry;
+	}
+
+	return 0;
+}
+
 int spl_load_simple_fit(struct spl_image_info *spl_image,
 			struct spl_load_info *info, ulong sector, void *fit)
 {
 	int sectors;
-	ulong size, load;
+	ulong size;
 	unsigned long count;
+	struct spl_image_info image_info;
 	int node, images;
-	void *load_ptr;
-	int fdt_offset, fdt_len;
-	int data_offset, data_size;
 	int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
-	int src_sector;
-	void *dst, *src;
 
 	/*
 	 * Figure out where the external images start. This is the base for the

@@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		return -1;
 	}
 
-	/* Get its information and set up the spl_image structure */
-	data_offset = fdt_getprop_u32(fit, node, "data-offset");
-	data_size = fdt_getprop_u32(fit, node, "data-size");
-	load = fdt_getprop_u32(fit, node, "load");
-	debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
-	spl_image->load_addr = load;
-	spl_image->entry_point = load;
+	/* Load the image and set up the spl_image structure */
+	spl_load_fit_image(info, sector, fit, base_offset, node, spl_image);
 	spl_image->os = IH_OS_U_BOOT;
 
-	/*
-	 * Work out where to place the image. We read it so that the first
-	 * byte will be at 'load'. This may mean we need to load it starting
-	 * before then, since we can only read whole blocks.
-	 */
-	data_offset += base_offset;
-	sectors = get_aligned_image_size(info, data_size, data_offset);
-	load_ptr = (void *)load;
-	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
-	dst = load_ptr;
-
-	/* Read the image */
-	src_sector = sector + get_aligned_image_offset(info, data_offset);
-	debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",
-	      dst, src_sector, sectors);
-	count = info->read(info, src_sector, sectors, dst);
-	if (count != sectors)
-		return -EIO;
-	debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
-	      data_size);
-	src = dst + get_aligned_image_overhead(info, data_offset);
-
-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
-	board_fit_image_post_process((void **)&src, (size_t *)&data_size);
-#endif
-
-	memcpy(dst, src, data_size);
-
 	/* Figure out which device tree the board wants to use */
 	node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
 	if (node < 0) {
 		debug("%s: cannot find FDT node\n", __func__);
 		return node;
 	}
-	fdt_offset = fdt_getprop_u32(fit, node, "data-offset");
-	fdt_len = fdt_getprop_u32(fit, node, "data-size");
-
-	/*
-	 * Read the device tree and place it after the image. There may be
-	 * some extra data before it since we can only read entire blocks.
-	 * And also align the destination address to ARCH_DMA_MINALIGN.
-	 */
-	dst = (void *)((load + data_size + align_len) & ~align_len);
-	fdt_offset += base_offset;
-	sectors = get_aligned_image_size(info, fdt_len, fdt_offset);
-	src_sector = sector + get_aligned_image_offset(info, fdt_offset);
-	count = info->read(info, src_sector, sectors, dst);
-	debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",
-	      dst, src_sector, sectors);
-	if (count != sectors)
-		return -EIO;
 
 	/*
-	 * Copy the device tree so that it starts immediately after the image.
-	 * After this we will have the U-Boot image and its device tree ready
-	 * for us to start.
+	 * Read the device tree and place it after the image.
+	 * Align the destination address to ARCH_DMA_MINALIGN.
 	 */
-	debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
-	      fdt_len);
-	src = dst + get_aligned_image_overhead(info, fdt_offset);
-	dst = load_ptr + data_size;
-
-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
-	board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
-#endif
-
-	memcpy(dst, src, fdt_len);
+	image_info.load_addr = spl_image->load_addr + spl_image->size;
+	spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
 
 	return 0;
 }

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 129 +++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 72 deletions(-)

common/spl/spl_fit.c +0 -0

index 572a5db..ad5ba15 100644

common/spl/spl_fit.c +37 -6

+       length = fdt_getprop_u32(fit, node, "data-size");
+       load_addr = fdt_getprop_u32(fit, node, "load");
+       if (load_addr == -1UL && image_info)
+               load_addr = image_info->load_addr;
+       load_ptr = (load_addr + align_len) & ~align_len;
+       entry = fdt_getprop_u32(fit, node, "entry");
+
+       overhead = get_aligned_image_overhead(info, offset);
+       nr_sectors = get_aligned_image_size(info, length, offset);
+
+       if (info->read(info, sector + get_aligned_image_offset(info, offset),
+                      nr_sectors, (void*)load_ptr) != nr_sectors)
+               return -EIO;
+       debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
+             (unsigned long)length);
+
+       src = (void *)load_ptr + overhead;
+#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
+       board_fit_image_post_process(&src, &length);
+#endif
+
+       memcpy((void*)load_addr, src, length);
+
+       if (image_info) {
+               image_info->load_addr = load_addr;
+               image_info->size = length;
+               if (entry == -1UL)
+                       image_info->entry_point = load_addr;
+               else
+                       image_info->entry_point = entry;
+       }
+
+       return 0;
+}
+
 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {
        int sectors;
-       ulong size, load;
+       ulong size;
        unsigned long count;
+       struct spl_image_info image_info;
        int node, images;
-       void *load_ptr;
-       int fdt_offset, fdt_len;
-       int data_offset, data_size;
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
-       int src_sector;
-       void *dst, *src;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
                return -1;
        }

common/spl/spl_fit.c +2 -50

        spl_image->os = IH_OS_U_BOOT;
 
-       /*
-        * Work out where to place the image. We read it so that the first
-        * byte will be at 'load'. This may mean we need to load it starting
-        * before then, since we can only read whole blocks.
-        */
-       data_offset += base_offset;
-       sectors = get_aligned_image_size(info, data_size, data_offset);
-       load_ptr = (void *)load;
-       debug("U-Boot size %x, data %p\n", data_size, load_ptr);
-       dst = load_ptr;
-
-       /* Read the image */
-       src_sector = sector + get_aligned_image_offset(info, data_offset);
-       debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",
-             dst, src_sector, sectors);
-       count = info->read(info, src_sector, sectors, dst);
-       if (count != sectors)
-               return -EIO;
-       debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
-             data_size);
-       src = dst + get_aligned_image_overhead(info, data_offset);
-
-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
-       board_fit_image_post_process((void **)&src, (size_t *)&data_size);
-#endif
-
-       memcpy(dst, src, data_size);
-
        /* Figure out which device tree the board wants to use */
        node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
        if (node < 0) {
                debug("%s: cannot find FDT node\n", __func__);
                return node;
        }
-       fdt_offset = fdt_getprop_u32(fit, node, "data-offset");
-       fdt_len = fdt_getprop_u32(fit, node, "data-size");
-
-       /*
-        * Read the device tree and place it after the image. There may be
-        * some extra data before it since we can only read entire blocks.
-        * And also align the destination address to ARCH_DMA_MINALIGN.
-        */
-       dst = (void *)((load + data_size + align_len) & ~align_len);
-       fdt_offset += base_offset;
-       sectors = get_aligned_image_size(info, fdt_len, fdt_offset);
-       src_sector = sector + get_aligned_image_offset(info, fdt_offset);
-       count = info->read(info, src_sector, sectors, dst);
-       debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",
-             dst, src_sector, sectors);
-       if (count != sectors)
-               return -EIO;
 
        /*
-        * Copy the device tree so that it starts immediately after the image.
-        * After this we will have the U-Boot image and its device tree ready
-        * for us to start.
+        * Read the device tree and place it after the image.
+        * Align the destination address to ARCH_DMA_MINALIGN.
         */
-       debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
-             fdt_len);

common/spl/spl_fit.c +0 -1

        return 0;
 }
--

2.8.2

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by Andre Przywara <[email protected]>

So far we were not using the FIT image format to its full potential:
The SPL FIT loader was just loading the first image from the /images node plus one of the listed DTBs.
Now with the refactored loader code it's easy to load an arbitrary number of images in addition to the two mentioned above.
As described in the FIT image source file format description, iterate over all images listed at the "loadables" property in the configuration node and load every image at its desired location.
This allows to load any kind of images:
- firmware images to execute before U-Boot proper (for instance ARM Trusted Firmware (ATF))
- firmware images for management processors (SCP, arisc, ...)
- firmware images for devices like WiFi controllers
- bit files for FPGAs
- additional configuration data
- kernels and/or ramdisks
The actual usage of this feature would be platform and/or board specific.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

common/spl/spl_fit.c +0 -0

index ad5ba15..5583e09 100644

common/spl/spl_fit.c +6 -0

        }
 
        return 0;

@@ -196,6 +193,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        struct spl_image_info image_info;
        int node, images;
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
+       int index = 0;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -240,6 +238,11 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        if (node < 0) {
                debug("could not find firmware image, trying loadables...\n");
                node = spl_fit_get_image_node(fit, images, "loadables", 0);
+               /*
+                * If we pick the U-Boot image from "loadables", start at
+                * the second image when later loading additional images.
+                */
+               index = 1;
        }
        if (node < 0) {
                debug("%s: Cannot find u-boot image node: %d\n",

@@ -265,5 +268,26 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        image_info.load_addr = spl_image->load_addr + spl_image->size;

common/spl/spl_fit.c +2 -1

+               spl_image->entry_point = spl_image->load_addr;
+
        return 0;
 }
--

2.8.2

Hi Andre,

Quoted 28 lines · Andre Przywara

So far we were not using the FIT image format to its full potential:
The SPL FIT loader was just loading the first image from the /images node plus one of the listed DTBs.
Now with the refactored loader code it's easy to load an arbitrary number of images in addition to the two mentioned above.
As described in the FIT image source file format description, iterate over all images listed at the "loadables" property in the configuration node and load every image at its desired location.
This allows to load any kind of images:
- firmware images to execute before U-Boot proper (for instance ARM Trusted Firmware (ATF))
- firmware images for management processors (SCP, arisc, ...)
- firmware images for devices like WiFi controllers
- bit files for FPGAs
- additional configuration data
- kernels and/or ramdisks
The actual usage of this feature would be platform and/or board specific.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

common/spl/spl_fit.c +0 -0

index ad5ba15..5583e09 100644

common/spl/spl_fit.c static int spl_load_fit_image(struct spl_load_info *info, ulong sector, · line 181

Quoted 8 lines

common/spl/spl_fit.c +1 -4

@@ -178,10 +178,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
        if (image_info) {
                image_info->load_addr = load_addr;
                image_info->size = length;
-               if (entry == -1UL)
-                       image_info->entry_point = load_addr;
-               else
-                       image_info->entry_point = entry;
+               image_info->entry_point = entry;

Need to update function comment to indicate that it can put -1 in here.

Quoted 25 lines

common/spl/spl_fit.c +6 -0

        }
 
        return 0;

@@ -196,6 +193,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        struct spl_image_info image_info;
        int node, images;
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
+       int index = 0;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -240,6 +238,11 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        if (node < 0) {
                debug("could not find firmware image, trying loadables...\n");
                node = spl_fit_get_image_node(fit, images, "loadables", 0);
+               /*
+                * If we pick the U-Boot image from "loadables", start at
+                * the second image when later loading additional images.
+                */
+               index = 1;
        }
        if (node < 0) {
                debug("%s: Cannot find u-boot image node: %d\n",

@@ -265,5 +268,26 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        image_info.load_addr = spl_image->load_addr + spl_image->size;

common/spl/spl_fit.c int spl_load_simple_fit(struct spl_image_info *spl_image, · line 278

Quoted 10 lines

common/spl/spl_fit.c +8 -0

        spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
 
+       /* Now check if there are more images for us to load */
+       for (; ; index++) {
+               node = spl_fit_get_image_node(fit, images, "loadables", index);
+               if (node < 0)
+                       break;
+
+               spl_load_fit_image(info, sector, fit, base_offset, node,
+                                  &image_info);

Error check?

common/spl/spl_fit.c int spl_load_simple_fit(struct spl_image_info *spl_image, · line 289

Quoted 11 lines

common/spl/spl_fit.c +11 -0

+
+               /*
+                * If the "firmware" image did not provide an entry point,
+                * use the first valid entry point from the loadables.
+                */
+               if (spl_image->entry_point == -1UL &&
+                   image_info.entry_point != -1UL)
+                       spl_image->entry_point = image_info.entry_point;
+       }
+
+       if (spl_image->entry_point == -1UL || spl_image->entry_point == 0)

Why does 0 mean there is no entry point? I suppose that is safe, but would anyone use this?

Quoted 7 lines

common/spl/spl_fit.c +2 -1

+               spl_image->entry_point = spl_image->load_addr;
+
        return 0;
 }
--

2.8.2

Regards,
Simon

Quoted 80 lines · Simon Glass

Hi Andre,

Quoted 28 lines · Andre Przywara

So far we were not using the FIT image format to its full potential:
The SPL FIT loader was just loading the first image from the /images node plus one of the listed DTBs.
Now with the refactored loader code it's easy to load an arbitrary number of images in addition to the two mentioned above.
As described in the FIT image source file format description, iterate over all images listed at the "loadables" property in the configuration node and load every image at its desired location.
This allows to load any kind of images:
- firmware images to execute before U-Boot proper (for instance ARM Trusted Firmware (ATF))
- firmware images for management processors (SCP, arisc, ...)
- firmware images for devices like WiFi controllers
- bit files for FPGAs
- additional configuration data
- kernels and/or ramdisks
The actual usage of this feature would be platform and/or board specific.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl_fit.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

common/spl/spl_fit.c +0 -0

index ad5ba15..5583e09 100644

common/spl/spl_fit.c static int spl_load_fit_image(struct spl_load_info *info, ulong sector, · line 181

Quoted 8 lines

common/spl/spl_fit.c +1 -4

@@ -178,10 +178,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
        if (image_info) {
                image_info->load_addr = load_addr;
                image_info->size = length;
-               if (entry == -1UL)
-                       image_info->entry_point = load_addr;
-               else
-                       image_info->entry_point = entry;
+               image_info->entry_point = entry;

Need to update function comment to indicate that it can put -1 in here.

Quoted 25 lines

common/spl/spl_fit.c +6 -0

        }
 
        return 0;

@@ -196,6 +193,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        struct spl_image_info image_info;
        int node, images;
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
+       int index = 0;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -240,6 +238,11 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        if (node < 0) {
                debug("could not find firmware image, trying loadables...\n");
                node = spl_fit_get_image_node(fit, images, "loadables", 0);
+               /*
+                * If we pick the U-Boot image from "loadables", start at
+                * the second image when later loading additional images.
+                */
+               index = 1;
        }
        if (node < 0) {
                debug("%s: Cannot find u-boot image node: %d\n",

@@ -265,5 +268,26 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        image_info.load_addr = spl_image->load_addr + spl_image->size;

common/spl/spl_fit.c int spl_load_simple_fit(struct spl_image_info *spl_image, · line 278

Quoted 10 lines

common/spl/spl_fit.c +8 -0

        spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
 
+       /* Now check if there are more images for us to load */
+       for (; ; index++) {
+               node = spl_fit_get_image_node(fit, images, "loadables", index);
+               if (node < 0)
+                       break;
+
+               spl_load_fit_image(info, sector, fit, base_offset, node,
+                                  &image_info);

Error check?

Quoted 14 lines

common/spl/spl_fit.c int spl_load_simple_fit(struct spl_image_info *spl_image, · line 289

Quoted 11 lines

common/spl/spl_fit.c +11 -0

+
+               /*
+                * If the "firmware" image did not provide an entry point,
+                * use the first valid entry point from the loadables.
+                */
+               if (spl_image->entry_point == -1UL &&
+                   image_info.entry_point != -1UL)
+                       spl_image->entry_point = image_info.entry_point;
+       }
+
+       if (spl_image->entry_point == -1UL || spl_image->entry_point == 0)

Why does 0 mean there is no entry point? I suppose that is safe, but would anyone use this?

So this is due to U-Boot's own Makefile, which sets CONFIG_SYS_UBOOT_START to 0 if it isn't already defined. This then gets passed on to mkimage -f auto as the -e argument.
So today's FIT images have 0 as an entry point - a least on sunxi. As I wanted to retain compatibility with that, I added this check. Maybe sunxi (and other platforms?) should explicitly define the entry point to avoid it being set to 0, or we should default to the load address as the fallback in absence of such an explicit definition.

In any case I wanted to keep existing u-boot.img files booting, so I added this safe guard here to do "the right thing (tm)".

I added a comment there to point this out.

Cheers,
Andre.

So far we were not using the FIT image format to its full potential:
The SPL FIT loader was just loading the first image from the /images node plus one of the listed DTBs.
Now with the refactored loader code it's easy to load an arbitrary number of images in addition to the two mentioned above.
As described in the FIT image source file format description, iterate over all images listed at the "loadables" property in the configuration node and load every image at its desired location.
This allows to load any kind of images:
- firmware images to execute before U-Boot proper (for instance ARM Trusted Firmware (ATF))
- firmware images for management processors (SCP, arisc, ...)
- firmware images for devices like WiFi controllers
- bit files for FPGAs
- additional configuration data
- kernels and/or ramdisks
The actual usage of this feature would be platform and/or board specific.

Signed-off-by Andre Przywara <[email protected]>

mksunxiboot limits the size of the resulting SPL binaries to pretty conservative values to cover all SoCs and all boot media (NAND). It turns out that we have limit checks in place in the build process, so mksunxiboot can be relaxed and allow packaging binaries up to the actual 32KB the mask boot ROM actually imposes.
This allows to have a bigger SPL, which is crucial for AArch64 builds.

Signed-off-by Andre Przywara <[email protected]>

---
 tools/mksunxiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mksunxiboot limits the size of the resulting SPL binaries to pretty conservative values to cover all SoCs and all boot media (NAND). It turns out that we have limit checks in place in the build process, so mksunxiboot can be relaxed and allow packaging binaries up to the actual 32KB the mask boot ROM actually imposes.
This allows to have a bigger SPL, which is crucial for AArch64 builds.

Signed-off-by Andre Przywara <[email protected]>

---
 tools/mksunxiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/mksunxiboot.c +0 -0

index 0f0b003..6bb649c 100644

tools/mksunxiboot.c +0 -0

 #define SRAM_LOAD_MAX_SIZE (SUN4I_SRAM_SIZE - sizeof(struct boot_file_head))
 
 /*

Hey Andre,

Quoted 20 lines · Andre Przywara

mksunxiboot limits the size of the resulting SPL binaries to pretty conservative values to cover all SoCs and all boot media (NAND). It turns out that we have limit checks in place in the build process, so mksunxiboot can be relaxed and allow packaging binaries up to the actual 32KB the mask boot ROM actually imposes.
This allows to have a bigger SPL, which is crucial for AArch64 builds.

Signed-off-by Andre Przywara <[email protected]>

---
 tools/mksunxiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/mksunxiboot.c +0 -0

index 0f0b003..6bb649c 100644
@@ -48,7 +48,7 @@ int gen_check_sum(struct boot_file_head *head_p)
 #define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x))(a)-1)
 #define __ALIGN_MASK(x, mask) (((x)+(mask))&~(mask))
 

Quoted 3 lines

tools/mksunxiboot.c +1 -1

-#define SUN4I_SRAM_SIZE 0x7600    /* 0x7748+ is used by BROM */
+#define SUN4I_SRAM_SIZE 0x8000    /* SoC with smaller size are

limited before */

This seems a little scary,

but besides that, you need to rename the
define to SUNXI imo too then.

SUN4I, SUN5I and SUN7I (I think) all have the 24K limit imposed by the BROM. Only the later gens have a 32k limit. Since it is nice to have a single binary for all, name the define as SUNXI too I think. As a comment, I would put the responsibility elsewhere (instead of making it an assumption).

With that said, why do we have to check it at all then, other then to give a warning? The size is limited before already, right?

Olliver

Quoted 5 lines

#define SRAM_LOAD_MAX_SIZE (SUN4I_SRAM_SIZE - sizeof(struct
boot_file_head))

/*

Not every SoC needs to set up the GIC interrupt controller, so link think code only when the respective config option is set. This shaves off some bytes from the SPL code size.

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/lib/Makefile | 2 ++
 1 file changed, 2 insertions(+)

The generic ARMv8 assembly code contains routines for setting up a CCN interconnect, though the Freescale SoCs are the only user. Link this code only for Freescale targets, this saves some precious bytes in the chronically tight SPL.

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/cpu/armv8/fsl-layerscape/Makefile | 1 +
 arch/arm/lib/Makefile                      | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

arch/arm/cpu/armv8/fsl-layerscape/Makefile +0 -0

index c9ab93e..ca09973 100644

arch/arm/cpu/armv8/fsl-layerscape/Makefile +0 -0

 obj-$(CONFIG_MP) += mp.o
 obj-$(CONFIG_OF_LIBFDT) += fdt.o
 obj-$(CONFIG_SPL) += spl.o

arch/arm/lib/Makefile +0 -2

index 71de1ca..60ffb4a 100644
@@ -43,7 +43,6 @@ obj-y += stack.o
 ifdef CONFIG_CPU_V7M
 obj-y  += interrupts_m.o
 else ifdef CONFIG_ARM64
-obj-y  += ccn504.o
 ifneq ($(CONFIG_GICV2)$(CONFIG_GICV3),)
 obj-y  += gic_64.o
 endif
--

2.8.2

Hi Andre,

Quoted 11 lines · Andre Przywara

The generic ARMv8 assembly code contains routines for setting up a CCN interconnect, though the Freescale SoCs are the only user. Link this code only for Freescale targets, this saves some precious bytes in the chronically tight SPL.

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/cpu/armv8/fsl-layerscape/Makefile | 1 +
 arch/arm/lib/Makefile                      | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

arch/arm/cpu/armv8/fsl-layerscape/Makefile line 10

Quoted 9 lines

arch/arm/cpu/armv8/fsl-layerscape/Makefile +1 -0

index c9ab93e..ca09973 100644
@@ -7,6 +7,7 @@
 obj-y += cpu.o
 obj-y += lowlevel.o
 obj-y += soc.o
+obj-y += ccn504.o

Don't you need to move the file into the same directory?

Quoted 17 lines

arch/arm/cpu/armv8/fsl-layerscape/Makefile +0 -0

 obj-$(CONFIG_MP) += mp.o
 obj-$(CONFIG_OF_LIBFDT) += fdt.o
 obj-$(CONFIG_SPL) += spl.o

arch/arm/lib/Makefile +0 -2

index 71de1ca..60ffb4a 100644
@@ -43,7 +43,6 @@ obj-y += stack.o
 ifdef CONFIG_CPU_V7M
 obj-y  += interrupts_m.o
 else ifdef CONFIG_ARM64
-obj-y  += ccn504.o
 ifneq ($(CONFIG_GICV2)$(CONFIG_GICV3),)
 obj-y  += gic_64.o
 endif
--

Quoted 2 lines

2.8.2

Regards,
Simon

The SPL stack is usually located at the end of SRAM A1, where it grows towards the end of the SPL.
For the really big AArch64 binaries the stack overwrites code pretty soon, so move the SPL stack to the end of SRAM A2, which is unused at this time.

Signed-off-by Andre Przywara <[email protected]>

---
 include/configs/sunxi-common.h | 5 +++++
 1 file changed, 5 insertions(+)

The sunxi SPL was holding the detected RAM size in some local variable only, so it wasn't accessible for other functions.
Store the value in gd->ram_size instead, so it can be used later on.

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/board.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

For a board or platform to support FIT loading in the SPL, it has to provide a board_fit_config_name_match() routine, which helps to select one of possibly multiple DTBs contained in a FIT image. Provide a simple function which chooses the DT name U-Boot was configured with.
If the DT name is one of the two Pine64 versions, determine the exact model by checking the DRAM size.

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/board.c | 23 +++++++++++++++++++++++

 1 file changed, 23 insertions(+)

board/sunxi/board.c +23 -1

index a510422..2ddff28 100644
@@ -725,3 +725,26 @@ int ft_board_setup(void *blob, bd_t *bd)
  #endif
          return 0;
  }
+
+#ifdef CONFIG_SPL_LOAD_FIT
+int board_fit_config_name_match(const char *name)
+{
+ const char *cmp_str;
+
+#ifdef CONFIG_DEFAULT_DEVICE_TREE
+ cmp_str = CONFIG_DEFAULT_DEVICE_TREE;
+#else
+ return 0;
+#endif
+
+/* Differentiate the two Pine64 board DTs by their DRAM size. */
+ if (strstr(name, "-pine64") && strstr(cmp_str, "-pine64")) {
+ if ((gd->ram_size > 512 * 1024 * 1024))
+ return !strstr(name, "plus");
+ else
+ return !!strstr(name, "plus");
+ } else {
+ return strcmp(name, cmp_str);
+ }
+}
+#endif
--

2.8.2

--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/d/optout.

01.03.2017, 10:26, "Andre Przywara" <[email protected]>:

Quoted 8 lines

For a board or platform to support FIT loading in the SPL, it has to provide a board_fit_config_name_match() routine, which helps to select one of possibly multiple DTBs contained in a FIT image. Provide a simple function which chooses the DT name U-Boot was configured with.
If the DT name is one of the two Pine64 versions, determine the exact model by checking the DRAM size.

I think we shouldn't have is specially for Pine64 here, but make a framework for other boards that can be easily checked.

Then make Pine64 series the first user of this framework.

Quoted 40 lines

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/board.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

board/sunxi/board.c +23 -1

index a510422..2ddff28 100644
@@ -725,3 +725,26 @@ int ft_board_setup(void *blob, bd_t *bd)
 #endif
         return 0;
 }
+
+#ifdef CONFIG_SPL_LOAD_FIT
+int board_fit_config_name_match(const char *name)
+{
+ const char *cmp_str;
+
+#ifdef CONFIG_DEFAULT_DEVICE_TREE
+ cmp_str = CONFIG_DEFAULT_DEVICE_TREE;
+#else
+ return 0;
+#endif
+
+/* Differentiate the two Pine64 board DTs by their DRAM size. */
+ if (strstr(name, "-pine64") && strstr(cmp_str, "-pine64")) {
+ if ((gd->ram_size > 512 * 1024 * 1024))
+ return !strstr(name, "plus");
+ else
+ return !!strstr(name, "plus");
+ } else {
+ return strcmp(name, cmp_str);
+ }
+}
+#endif
--

2.8.2

Quoted 4 lines

--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/d/optout.

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/listinfo/u-boot

For a board or platform to support FIT loading in the SPL, it has to provide a board_fit_config_name_match() routine, which helps to select one of possibly multiple DTBs contained in a FIT image. Provide a simple function which chooses the DT name U-Boot was configured with.
If the DT name is one of the two Pine64 versions, determine the exact model by checking the DRAM size.

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/board.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Some platforms require more complex U-Boot images than we can easily generate via the mkimage command line, for instance to load additional image files.
Introduce a CONFIG_SPL_FIT_SOURCE and CONFIG_SPL_FIT_GENERATOR symbol, which can either hold an .its source file describing the image layout, or, in the second case, a generator tool (script) to create such a source file. This script gets passed the list of device tree files from the CONFIG_OF_LIST variable.
A platform or board can define either of those in their defconfig file to allow an easy building of such an image.

Signed-off-by Andre Przywara <[email protected]>

---

 Kconfig  | 17 +++++++++++++++++
 Makefile | 20 ++++++++++++++++++++
 2 files changed, 37 insertions(+)

Kconfig +17 -0

index 81b4226..f3e4243 100644
@@ -238,6 +238,23 @@ config SPL_FIT_IMAGE_POST_PROCESS
 	  injected into the FIT creation (i.e. the blobs would have been pre-
 	  processed before being added to the FIT image).
 
+config SPL_FIT_SOURCE
+	string ".its source file for U-Boot FIT image"
+	depends on SPL_FIT
+	help
+	  Specifies a (platform specific) FIT source file to generate the
+	  U-Boot FIT image. This could specify further image to load and/or
+	  execute.
+
+config SPL_FIT_GENERATOR
+	string ".its file generator script for U-Boot FIT image"
+	depends on SPL_FIT
+	help
+	  Specifies a (platform specific) script file to generate the FIT
+	  source file used to build the U-Boot FIT image file. This gets
+	  passed a list of supported device tree file stub names to
+	  include in the generated image.
+
 endif # FIT
 
 config OF_BOARD_SETUP

Makefile +20 -0

index 38b42da..e09b0d9 100644
@@ -826,6 +826,10 @@ quiet_cmd_mkimage = MKIMAGE $@
 cmd_mkimage = $(objtree)/tools/mkimage $(MKIMAGEFLAGS_$(@F)) -d $< $@ \
 	$(if $(KBUILD_VERBOSE:1=), >$(MKIMAGEOUTPUT))
 
+quiet_cmd_mkfitimage = MKIMAGE $@
+cmd_mkfitimage = $(objtree)/tools/mkimage $(MKIMAGEFLAGS_$(@F)) -f $(U_BOOT_ITS) -E $@ \
+	$(if $(KBUILD_VERBOSE:1=), >$(MKIMAGEOUTPUT))
+
 quiet_cmd_cat = CAT     $@
 cmd_cat = cat $(filter-out $(PHONY), $^) > $@
 

@@ -945,6 +949,19 @@ quiet_cmd_cpp_cfg = CFG     $@
 cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 	-DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $<
 
+# Boards with more complex image requirments can provide an .its source file
+# or a generator script
+ifneq ($(CONFIG_SPL_FIT_SOURCE),"")
+U_BOOT_ITS = $(subst ",,$(CONFIG_SPL_FIT_SOURCE))
+else
+ifneq ($(CONFIG_SPL_FIT_GENERATOR),"")
+U_BOOT_ITS := u-boot.its
+$(U_BOOT_ITS): FORCE
+	$(srctree)/$(CONFIG_SPL_FIT_GENERATOR) \
+	$(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) > $@
+endif
+endif
+
 ifdef CONFIG_SPL_LOAD_FIT
 MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
 	-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \

@@ -977,6 +994,9 @@ u-boot-dtb.img u-boot.img u-boot.kwb u-boot.pbl u-boot-ivt.img: \
 		$(if $(CONFIG_SPL_LOAD_FIT),u-boot-nodtb.bin dts/dt.dtb,u-boot.bin) FORCE
 	$(call if_changed,mkimage)
 
+u-boot.itb: u-boot-nodtb.bin dts/dt.dtb $(U_BOOT_ITS) FORCE
+	$(call if_changed,mkfitimage)
+
 u-boot-spl.kwb: u-boot.img spl/u-boot-spl.bin FORCE
 	$(call if_changed,mkimage)
 

Some platforms require more complex U-Boot images than we can easily generate via the mkimage command line, for instance to load additional image files.
Introduce a CONFIG_SPL_FIT_SOURCE and CONFIG_SPL_FIT_GENERATOR symbol, which can either hold an .its source file describing the image layout, or, in the second case, a generator tool (script) to create such a source file. This script gets passed the list of device tree files from the CONFIG_OF_LIST variable.
A platform or board can define either of those in their defconfig file to allow an easy building of such an image.

Signed-off-by Andre Przywara <[email protected]>

---
 Kconfig  | 17 +++++++++++++++++
 Makefile | 20 ++++++++++++++++++++
 2 files changed, 37 insertions(+)

Now that the Makefile can call a generator script to build a more advanced FIT image, let's use this feature to address the needs of Allwinner A64 boards.
The (DTB stripped) U-Boot binary and the ATF are static, but we allow an arbitrary number of supported device trees to be passed. The script enters both a DT entry in the /images node and the respective subnode in /configurations to support all listed DTBs.

This requires to copy the ARM Trusted Firmware build (bl31.bin) into the U-Boot source directory (or to create a symlink to it).

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/mksunxi_fit_atf.sh | 73 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100755 board/sunxi/mksunxi_fit_atf.sh

The Pine64 (and all other 64-bit Allwinner boards) need to load an ARM Trusted Firmware image beside the actual U-Boot proper. This can now be easily achieved by using the just extended SPL FIT loading support, so enable it in the Pine64 defconfig. Also add the FIT image as a build target to 64-bit sunxi board to trigger the respective Makefile rules.

Signed-off-by Andre Przywara <[email protected]>

---
 configs/pine64_plus_defconfig  | 6 ++++++
 include/configs/sunxi-common.h | 4 ++++
 2 files changed, 10 insertions(+)

configs/pine64_plus_defconfig +0 -0

index 7c7d86f..2b47157 100644
@@ -3,9 +3,14 @@ CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y
 CONFIG_ARCH_SUNXI=y

Enable the SPL FIT support and the FIT generator script for the OrangePi PC2 board, as it also need to load an ATF binary.

Signed-off-by Andre Przywara <[email protected]>

---
 configs/orangepi_pc2_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

From: Siarhei Siamashka <[email protected]>

This patch updates the mksunxiboot tool to optionally add the default device tree name string to the SPL header. This information can be used by the firmware upgrade tools to protect users from harming themselves by trying to upgrade to an incompatible bootloader.

The primary use case here is a non-removable bootable media (such as NAND, eMMC or SPI flash), which already may have a properly working, but a little bit outdated bootloader installed. For example, the user may download or build a new U-Boot image for "Cubieboard", and then attemept to install it on a "Cubieboard2" hardware by mistake as a replacement for the already existing bootloader. If this happens, the flash programming tool can identify this problem and warn the user.

The size of the SPL header is also increased from 64 bytes to 96 bytes to provide enough space for the device tree name string.
[Andre: split patch to remove OF_LIST hash feature]

Signed-off-by Siarhei Siamashka <[email protected]>

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++---
 include/configs/sunxi-common.h        |  8 +++---
 scripts/Makefile.spl                  |  3 ++-
 tools/mksunxiboot.c                   | 49 ++++++++++++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 12 deletions(-)

Quoted 23 lines

From: Siarhei Siamashka <[email protected]>

This patch updates the mksunxiboot tool to optionally add the default device tree name string to the SPL header. This information can be used by the firmware upgrade tools to protect users from harming themselves by trying to upgrade to an incompatible bootloader.

The primary use case here is a non-removable bootable media (such as NAND, eMMC or SPI flash), which already may have a properly working, but a little bit outdated bootloader installed. For example, the user may download or build a new U-Boot image for "Cubieboard", and then attemept to install it on a "Cubieboard2" hardware by mistake as a replacement for the already existing bootloader. If this happens, the flash programming tool can identify this problem and warn the user.

The size of the SPL header is also increased from 64 bytes to 96 bytes to provide enough space for the device tree name string.
[Andre: split patch to remove OF_LIST hash feature]

Quoted 8 lines

Signed-off-by Siarhei Siamashka <[email protected]>

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++---
 include/configs/sunxi-common.h        |  8 +++---
 scripts/Makefile.spl                  |  3 ++-
 tools/mksunxiboot.c                   | 49 ++++++++++++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 12 deletions(-)

Can this code move into mkimage as a new image type? This is what rockchip does. It feels like this tool should be subsumed. If that doesn't work, perhaps binman?

On 08/03/17 21:01, Simon Glass wrote:

Hi Simon,

many thanks for the review, finally found some time to look at this. I have finished the needed rework (including documentation) and will post something after some testing and some sleep ;-)

Quoted 33 lines · Andre Przywara

Quoted 23 lines

From: Siarhei Siamashka <[email protected]>

This patch updates the mksunxiboot tool to optionally add the default device tree name string to the SPL header. This information can be used by the firmware upgrade tools to protect users from harming themselves by trying to upgrade to an incompatible bootloader.

The primary use case here is a non-removable bootable media (such as NAND, eMMC or SPI flash), which already may have a properly working, but a little bit outdated bootloader installed. For example, the user may download or build a new U-Boot image for "Cubieboard", and then attemept to install it on a "Cubieboard2" hardware by mistake as a replacement for the already existing bootloader. If this happens, the flash programming tool can identify this problem and warn the user.

The size of the SPL header is also increased from 64 bytes to 96 bytes to provide enough space for the device tree name string.
[Andre: split patch to remove OF_LIST hash feature]

Quoted 8 lines

Signed-off-by Siarhei Siamashka <[email protected]>

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++---
 include/configs/sunxi-common.h        |  8 +++---
 scripts/Makefile.spl                  |  3 ++-
 tools/mksunxiboot.c                   | 49 ++++++++++++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 12 deletions(-)

Quoted 3 lines

Can this code move into mkimage as a new image type? This is what rockchip does. It feels like this tool should be subsumed. If that doesn't work, perhaps binman?

Interesting, I wasn't aware that mkimage can do more than legacy and FIT. Indeed that sounds useful, especially as mkimage seems to be packaged separately and is available in some distros.

So I hacked something up, but that needs some more love. I am tempted to drop (or split) this patch from this series for now, since this extension here and the move to mkimage could be treated separately from the SPL FIT code.

Now that we can store a DT name in the SPL header, use this string (if available) when finding the right DT blob to load for U-Boot proper. This allows a generic U-Boot (proper) image to be combined with a bunch of supported DTs, with just the SPL (possibly only that string) to be different.
Eventually this string can be written after the build process by some firmware update tool.

Signed-off-by Andre Przywara <[email protected]>

---
 board/sunxi/board.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

This is an updated and slightly extended version of the SPL FIT loading series I posted as an RFC some weeks ago.
I tried to fix all bugs that have been pointed out by the diligent reviewers, also added patches to automatically build the FIT images.

The first patch is a bug fix for a regression introduced with -rc1. I put this in there to allow people testing the series and to provide an actual patch for this fix, which should make it still into 2017.03. The next four patches introduce the core of the extened SPL FIT loading support, see below for a description.
Patches 6-9 make some room in the sunxi 64-bit SPL to allow compiling in the FIT loading bits. Patch 10 and 11 let the SPL choose the proper DT from the FIT image.
The next two patches add the infrastructure and an actual generator script, so the FIT image is automatically created at build time. Patches 14 and 15 enable the SPL FIT support in the Pine64 and the OrangePi PC 2 defconfigs.
The last two patches are new and eventually store a DT file name in the SPL header, so U-Boot can easily pick the proper DT when scanning the FIT image. The idea is that this DT name should stay with the board, ideally on eMMC or SPI flash. So both U-Boot and a firmware update tool could identify a board, updating with compatible firmware while keeping the DT name in place. Ideally a board vendor would once seed this name onto on-board storage like SPI flash.

Let me know what you think!

Cheers,
Andre.

Based on top of sunxi/master (35affe7698e9).

-------
Currently the FIT format is not used to its full potential in the SPL:
It only loads the first image from the /images node and appends the proper FDT.
Some boards and platforms would benefit from loading more images before starting U-Boot proper, notably Allwinner A64 and ARMv8 Rockchip boards, which use an ARM Trusted Firmware (ATF) image to be executed before U-Boot.

This series tries to solve this in a board agnostic and generic way:
We extend the SPL FIT loading scheme to allow loading multiple images. So apart from loading the image which is referenced by the "firmware" property in the respective configuration node and placing the DTB right behind it, we iterate over all strings in the "loadable" property. Each image referenced there will be loaded to its specified load address. The entry point U-Boot eventually branches to will be taken from the first image to explicitly provide the "entry" property, or, if none of them does so, from the load address of the "firmware" image. This keeps the scheme compatible with the FIT images our Makefile creates automatically at the moment.
Apart from the already mentioned ATF scenario this opens up more usage scenarios, of which the commit message of patch 04/11 lists some. The remaining patches prepare ane finally enable this scheme for the 64-bit Allwinner boards.

Andre Przywara (15):
SPL: FIT: refactor FDT loading
SPL: FIT: rework U-Boot image loading
SPL: FIT: factor out spl_load_fit_image()
SPL: FIT: allow loading multiple images
tools: mksunxiboot: allow larger SPL binaries
armv8: SPL: only compile GIC code if needed
armv8: fsl: move ccn504 code into FSL Makefile
sunxi: A64: move SPL stack to end of SRAM A2
sunxi: SPL: store RAM size in gd
sunxi: SPL: add FIT config selector for Pine64 boards
Makefile: add rules to generate SPL FIT images
sunxi: A64: Pine64: introduce FIT generator script
sunxi: Pine64: defconfig: enable SPL FIT support
sunxi: OrangePi-PC2: defconfig: enable SPL FIT support
sunxi: use SPL header DT name for FIT board matching

Philipp Tomsich (1):
armv8: spl: Call spl_relocate_stack_gd for ARMv8

Siarhei Siamashka (1):
sunxi: Store the device tree name in the SPL header

  Kconfig                                    |  17 ++
  Makefile                                   |  20 +++
  arch/arm/cpu/armv8/fsl-layerscape/Makefile |   1 +
  arch/arm/include/asm/arch-sunxi/spl.h      |  19 ++-
  arch/arm/lib/Makefile                      |   3 +-
  arch/arm/lib/crt0_64.S                     |  14 +-
  board/sunxi/board.c                        |  36 ++++-
  board/sunxi/mksunxi_fit_atf.sh             |  73 +++++++++
  common/spl/spl_fit.c                       | 246 +++++++++++++++++------------
  configs/orangepi_pc2_defconfig             |   6 +
  configs/pine64_plus_defconfig              |   6 +
  include/configs/sunxi-common.h             |  17 +-
  scripts/Makefile.spl                       |   3 +-
  tools/mksunxiboot.c                        |  51 +++++-
  14 files changed, 387 insertions(+), 125 deletions(-)

create mode 100755 board/sunxi/mksunxi_fit_atf.sh

This is an updated and slightly extended version of the SPL FIT loading series I posted as an RFC some weeks ago.
I tried to fix all bugs that have been pointed out by the diligent reviewers, also added patches to automatically build the FIT images.

The first patch is a bug fix for a regression introduced with -rc1. I put this in there to allow people testing the series and to provide an actual patch for this fix, which should make it still into 2017.03. The next four patches introduce the core of the extened SPL FIT loading support, see below for a description.
Patches 6-9 make some room in the sunxi 64-bit SPL to allow compiling in the FIT loading bits. Patch 10 and 11 let the SPL choose the proper DT from the FIT image.
The next two patches add the infrastructure and an actual generator script, so the FIT image is automatically created at build time. Patches 14 and 15 enable the SPL FIT support in the Pine64 and the OrangePi PC 2 defconfigs.
The last two patches are new and eventually store a DT file name in the SPL header, so U-Boot can easily pick the proper DT when scanning the FIT image. The idea is that this DT name should stay with the board, ideally on eMMC or SPI flash. So both U-Boot and a firmware update tool could identify a board, updating with compatible firmware while keeping the DT name in place. Ideally a board vendor would once seed this name onto on-board storage like SPI flash.

Let me know what you think!

Cheers,
Andre.

Based on top of sunxi/master (35affe7698e9).

-------
Currently the FIT format is not used to its full potential in the SPL:
It only loads the first image from the /images node and appends the proper FDT.
Some boards and platforms would benefit from loading more images before starting U-Boot proper, notably Allwinner A64 and ARMv8 Rockchip boards, which use an ARM Trusted Firmware (ATF) image to be executed before U-Boot.

This series tries to solve this in a board agnostic and generic way:
We extend the SPL FIT loading scheme to allow loading multiple images. So apart from loading the image which is referenced by the "firmware" property in the respective configuration node and placing the DTB right behind it, we iterate over all strings in the "loadable" property. Each image referenced there will be loaded to its specified load address. The entry point U-Boot eventually branches to will be taken from the first image to explicitly provide the "entry" property, or, if none of them does so, from the load address of the "firmware" image. This keeps the scheme compatible with the FIT images our Makefile creates automatically at the moment.
Apart from the already mentioned ATF scenario this opens up more usage scenarios, of which the commit message of patch 04/11 lists some. The remaining patches prepare ane finally enable this scheme for the 64-bit Allwinner boards.

Andre Przywara (15):
SPL: FIT: refactor FDT loading
SPL: FIT: rework U-Boot image loading
SPL: FIT: factor out spl_load_fit_image()
SPL: FIT: allow loading multiple images
tools: mksunxiboot: allow larger SPL binaries
armv8: SPL: only compile GIC code if needed
armv8: fsl: move ccn504 code into FSL Makefile
sunxi: A64: move SPL stack to end of SRAM A2
sunxi: SPL: store RAM size in gd
sunxi: SPL: add FIT config selector for Pine64 boards
Makefile: add rules to generate SPL FIT images
sunxi: A64: Pine64: introduce FIT generator script
sunxi: Pine64: defconfig: enable SPL FIT support
sunxi: OrangePi-PC2: defconfig: enable SPL FIT support
sunxi: use SPL header DT name for FIT board matching

Philipp Tomsich (1):
armv8: spl: Call spl_relocate_stack_gd for ARMv8

Siarhei Siamashka (1):
sunxi: Store the device tree name in the SPL header

Kconfig                                    |  17 ++
Makefile                                   |  20 +++
arch/arm/cpu/armv8/fsl-layerscape/Makefile |   1 +
arch/arm/include/asm/arch-sunxi/spl.h      |  19 ++-
arch/arm/lib/Makefile                      |   3 +-
arch/arm/lib/crt0_64.S                     |  14 +-
board/sunxi/board.c                        |  36 ++++-
board/sunxi/mksunxi_fit_atf.sh             |  73 +++++++++
common/spl/spl_fit.c                       | 246 +++++++++++++++++------------
configs/orangepi_pc2_defconfig             |   6 +
configs/pine64_plus_defconfig              |   6 +
include/configs/sunxi-common.h             |  17 +-
scripts/Makefile.spl                       |   3 +-
tools/mksunxiboot.c                        |  51 +++++-
14 files changed, 387 insertions(+), 125 deletions(-)

create mode 100755 board/sunxi/mksunxi_fit_atf.sh

--
2.8.2