Skip to contentSkip to conversation

uboot: install multiple spl images

PATCH New Jason Abele 4 participants 5 messages Mar 10, 2016 – Jul 5, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 uboot: install multiple spl images

From: Jason Abele <[email protected]>

For some platforms, there are multiple generated spl images. Extend BR2_TARGET_UBOOT_SPL_NAME to allow these multiple images to be installed after uboot build completes.

---
For example, the NextThingCo C.H.I.P. uses two binaries from uboot, spl/sunxi-spl.bin and spl/sunxi-spl-with-ecc.bin.

This patch allows them to be listed explicitly.

 boot/uboot/Config.in |  2 +-
 boot/uboot/uboot.mk  | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)
boot/uboot/Config.in +1 -1
index 4a6dc56..d4f9445 100644
@@ -322,7 +322,7 @@ config BR2_TARGET_UBOOT_SPL_NAME
 	default "spl/u-boot-spl.bin"
 	depends on BR2_TARGET_UBOOT_SPL
 	help
-	  This is the name of the SPL binary, generated during
+	  A space-separated list of SPL binaries, generated during
 	  u-boot build. For most platform it is spl/u-boot-spl.bin
 	  but not always. It is MLO on OMAP for example.
 
boot/uboot/uboot.mk +8 -4
index d539b31..aae99f8 100644
@@ -169,7 +169,9 @@ define UBOOT_INSTALL_IMAGES_CMDS
 	$(if $(BR2_TARGET_UBOOT_FORMAT_NAND),
 		cp -dpf $(@D)/$(UBOOT_MAKE_TARGET) $(BINARIES_DIR))
 	$(if $(BR2_TARGET_UBOOT_SPL),
-		cp -dpf $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)) $(BINARIES_DIR)/)
+		for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+			cp -dpf $(@D)/$$p $(BINARIES_DIR)/; \
+		done)
 	$(if $(BR2_TARGET_UBOOT_ENVIMAGE),
 		$(HOST_DIR)/usr/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
 		$(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
@@ -196,9 +198,11 @@ endif
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQ_IMAGE),y)
 define UBOOT_GENERATE_ZYNQ_IMAGE
-	$(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
-		-u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))     \
-		-o $(BINARIES_DIR)/BOOT.BIN
+	for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+		$(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
+			-u $(@D)/$$p -o $(BINARIES_DIR)/BOOT.BIN; \
+		fi; \
+	done
 endef
 UBOOT_DEPENDENCIES += host-zynq-boot-bin
 UBOOT_POST_INSTALL_IMAGES_HOOKS += UBOOT_GENERATE_ZYNQ_IMAGE
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/1 uboot: install multiple spl images

Jason,

Quoted 7 lines · Jason Abele

From: Jason Abele <[email protected]>

For some platforms, there are multiple generated spl images. Extend BR2_TARGET_UBOOT_SPL_NAME to allow these multiple images to be installed after uboot build completes.

For some reason, I was expecting this patch to arrive :-)

Quoted 3 lines

---
For example, the NextThingCo C.H.I.P. uses two binaries from uboot, spl/sunxi-spl.bin and spl/sunxi-spl-with-ecc.bin.

This should be part of the commit log itself as it is a very useful example of why this patch was needed.

Quoted 18 lines
boot/uboot/Config.in +1 -1
index 4a6dc56..d4f9445 100644
@@ -322,7 +322,7 @@ config BR2_TARGET_UBOOT_SPL_NAME
 	default "spl/u-boot-spl.bin"
 	depends on BR2_TARGET_UBOOT_SPL
 	help
-	  This is the name of the SPL binary, generated during
+	  A space-separated list of SPL binaries, generated during
 	  u-boot build. For most platform it is spl/u-boot-spl.bin
 	  but not always. It is MLO on OMAP for example.
 
boot/uboot/uboot.mk +0 -0
index d539b31..aae99f8 100644

boot/uboot/uboot.mk define UBOOT_INSTALL_IMAGES_CMDS · line 174

Quoted 7 lines
boot/uboot/uboot.mk +3 -1
@@ -169,7 +169,9 @@ define UBOOT_INSTALL_IMAGES_CMDS
 	$(if $(BR2_TARGET_UBOOT_FORMAT_NAND),
 		cp -dpf $(@D)/$(UBOOT_MAKE_TARGET) $(BINARIES_DIR))
 	$(if $(BR2_TARGET_UBOOT_SPL),
-		cp -dpf $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)) $(BINARIES_DIR)/)
+		for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+			cp -dpf $(@D)/$$p $(BINARIES_DIR)/; \
+		done)

Ideally, using the make $(foreach ...) function here would be better.

boot/uboot/uboot.mk endif · line 205

Quoted 10 lines
boot/uboot/uboot.mk +5 -3
 ifeq ($(BR2_TARGET_UBOOT_ZYNQ_IMAGE),y)
 define UBOOT_GENERATE_ZYNQ_IMAGE
-	$(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
-		-u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))     \
-		-o $(BINARIES_DIR)/BOOT.BIN
+	for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+		$(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
+			-u $(@D)/$$p -o $(BINARIES_DIR)/BOOT.BIN; \
+		fi; \
+	done

Having a for loop here is really useless, since the output file is BOOT.BIN at each iteration of the loop. Since for the specific Zynq case it doesn't make sense to have multiple values in BR2_TARGET_UBOOT_SPL_NAME, I would suggest to just do:

- -u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))
+ -u $(@D)/$(firstword $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)))

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 uboot: install multiple spl images

Thomas,

Quoted 33 lines · Thomas Petazzoni

Jason,

Quoted 7 lines · Jason Abele

From: Jason Abele <[email protected]>

For some platforms, there are multiple generated spl images. Extend BR2_TARGET_UBOOT_SPL_NAME to allow these multiple images to be installed after uboot build completes.

For some reason, I was expecting this patch to arrive :-)

Quoted 3 lines

---
For example, the NextThingCo C.H.I.P. uses two binaries from uboot, spl/sunxi-spl.bin and spl/sunxi-spl-with-ecc.bin.

This should be part of the commit log itself as it is a very useful example of why this patch was needed.

Quoted 13 lines
boot/uboot/Config.in +1 -1
index 4a6dc56..d4f9445 100644
@@ -322,7 +322,7 @@ config BR2_TARGET_UBOOT_SPL_NAME
      default "spl/u-boot-spl.bin"
      depends on BR2_TARGET_UBOOT_SPL
      help
-       This is the name of the SPL binary, generated during
+       A space-separated list of SPL binaries, generated during
        u-boot build. For most platform it is spl/u-boot-spl.bin
        but not always. It is MLO on OMAP for example.
 
Quoted 14 lines

boot/uboot/uboot.mk define UBOOT_INSTALL_IMAGES_CMDS · line 174

Quoted 12 lines
boot/uboot/uboot.mk +3 -1
index d539b31..aae99f8 100644
@@ -169,7 +169,9 @@ define UBOOT_INSTALL_IMAGES_CMDS
      $(if $(BR2_TARGET_UBOOT_FORMAT_NAND),
              cp -dpf $(@D)/$(UBOOT_MAKE_TARGET) $(BINARIES_DIR))
      $(if $(BR2_TARGET_UBOOT_SPL),
-             cp -dpf $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)) $(BINARIES_DIR)/)
+             for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+                     cp -dpf $(@D)/$$p $(BINARIES_DIR)/; \
+             done)

Ideally, using the make $(foreach ...) function here would be better.

Ok, should I stack a patch ahead of this fixing the usage of the shell 'for' loop with UBOOT_PATCH variable above here? Seems like maintaining consistency within the file is probably a good idea

Quoted 18 lines

boot/uboot/uboot.mk endif · line 205

Quoted 11 lines
boot/uboot/uboot.mk +5 -3
 ifeq ($(BR2_TARGET_UBOOT_ZYNQ_IMAGE),y)
 define UBOOT_GENERATE_ZYNQ_IMAGE
-     $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
-             -u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))     \
-             -o $(BINARIES_DIR)/BOOT.BIN
+     for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+             $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
+                     -u $(@D)/$$p -o $(BINARIES_DIR)/BOOT.BIN; \
+             fi; \
+     done

Having a for loop here is really useless, since the output file is BOOT.BIN at each iteration of the loop. Since for the specific Zynq case it doesn't make sense to have multiple values in BR2_TARGET_UBOOT_SPL_NAME, I would suggest to just do:

Review

Quoted 2 lines
(quoted diff) +1 -1
-       -u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))
+       -u $(@D)/$(firstword $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)))

So, I feel like the whole SPL_NAME can be MLO or BOOT.BIN or what not is a horrible mess and that it would be bad form for BR2_TARGET_UBOOT_SPL_NAME to take a list of files in some cases and only a single file in other cases. I am no uboot expert, but this seems like it would be hard to document in a clear way. If there is better language for the documentation which will make the difference in usage clear, then I am happy to amend the documentation and use make's firstword function for the xynq platform.

It feels like the cleaner solution would just be to use BR2_TARGET_UBOOT_SPL_NAME="spl/sunxi-spl*.bin" in the defconfig for my platform than add yet another variant usage in uboot.mk, any thoughts on this option?

Thanks for the review,
Jason

#3 lore raw

PATCH 1/1 uboot: install multiple spl images

In reply to Jason Abele

Hi,

(late reply, just noticed this patch)

Quoted 50 lines · Jason Abele

From: Jason Abele <[email protected]>

For some platforms, there are multiple generated spl images. Extend BR2_TARGET_UBOOT_SPL_NAME to allow these multiple images to be installed after uboot build completes.

---
For example, the NextThingCo C.H.I.P. uses two binaries from uboot, spl/sunxi-spl.bin and spl/sunxi-spl-with-ecc.bin.

This patch allows them to be listed explicitly.

 boot/uboot/Config.in |  2 +-
 boot/uboot/uboot.mk  | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)
boot/uboot/Config.in +1 -1
index 4a6dc56..d4f9445 100644
@@ -322,7 +322,7 @@ config BR2_TARGET_UBOOT_SPL_NAME
        default "spl/u-boot-spl.bin"
        depends on BR2_TARGET_UBOOT_SPL
        help
-         This is the name of the SPL binary, generated during
+         A space-separated list of SPL binaries, generated during
          u-boot build. For most platform it is spl/u-boot-spl.bin
          but not always. It is MLO on OMAP for example.
 
boot/uboot/uboot.mk +3 -2
index d539b31..aae99f8 100644
@@ -169,7 +169,9 @@ define UBOOT_INSTALL_IMAGES_CMDS
        $(if $(BR2_TARGET_UBOOT_FORMAT_NAND),
                cp -dpf $(@D)/$(UBOOT_MAKE_TARGET) $(BINARIES_DIR))
        $(if $(BR2_TARGET_UBOOT_SPL),
-               cp -dpf $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)) $(BINARIES_DIR)/)
+               for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+                       cp -dpf $(@D)/$$p $(BINARIES_DIR)/; \
+               done)
        $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
                $(HOST_DIR)/usr/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
                $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
@@ -196,9 +198,11 @@ endif
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQ_IMAGE),y)
 define UBOOT_GENERATE_ZYNQ_IMAGE
-       $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \

boot/uboot/uboot.mk endif · line 208

Quoted 10 lines
boot/uboot/uboot.mk +5 -2
-               -u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))     \
-               -o $(BINARIES_DIR)/BOOT.BIN
+       for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+               $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
+                       -u $(@D)/$$p -o $(BINARIES_DIR)/BOOT.BIN; \
+               fi; \
+       done
 endef
 UBOOT_DEPENDENCIES += host-zynq-boot-bin
 UBOOT_POST_INSTALL_IMAGES_HOOKS += UBOOT_GENERATE_ZYNQ_IMAGE

We have a similar modification in our tree but for the real u-boot image and not the SPL image. In this case, the UBOOT_BOARD_NAME can take a list of board names, and we use a similar for loop to iterate over the configure, build, install step for each board.

Use case is to use a single defconfig for different hardware boards that are very much alike but need some different u-boot (different device tree, among other subtleties).

I thought I once shared this patch with the list but I cannot find it back.

If the path is now more open for inclusion of such change, I would be more than happy to send it.

Let me know,
Thomas

#4 lore raw

PATCH 1/1 uboot: install multiple spl images

Hi Jason, all,

I have respinned your patch, you can find it here:
https://patchwork.ozlabs.org/patch/644708/

Your patch has been marked as "Changes Requested" in our patchwork.

Thomas: I haven't taken into account your demand since you have a patch, it's better that you send yours it will match your needs better that what I could propose :).

Thanks for your patch

Quoted 86 lines · Thomas De Schampheleire

Hi,

(late reply, just noticed this patch)

Quoted 50 lines · Jason Abele

From: Jason Abele <[email protected]>

For some platforms, there are multiple generated spl images. Extend BR2_TARGET_UBOOT_SPL_NAME to allow these multiple images to be installed after uboot build completes.

---
For example, the NextThingCo C.H.I.P. uses two binaries from uboot, spl/sunxi-spl.bin and spl/sunxi-spl-with-ecc.bin.

This patch allows them to be listed explicitly.

 boot/uboot/Config.in |  2 +-
 boot/uboot/uboot.mk  | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)
boot/uboot/Config.in +1 -1
index 4a6dc56..d4f9445 100644
@@ -322,7 +322,7 @@ config BR2_TARGET_UBOOT_SPL_NAME
        default "spl/u-boot-spl.bin"
        depends on BR2_TARGET_UBOOT_SPL
        help
-         This is the name of the SPL binary, generated during
+         A space-separated list of SPL binaries, generated during
          u-boot build. For most platform it is spl/u-boot-spl.bin
          but not always. It is MLO on OMAP for example.
 
boot/uboot/uboot.mk +3 -2
index d539b31..aae99f8 100644
@@ -169,7 +169,9 @@ define UBOOT_INSTALL_IMAGES_CMDS
        $(if $(BR2_TARGET_UBOOT_FORMAT_NAND),
                cp -dpf $(@D)/$(UBOOT_MAKE_TARGET) $(BINARIES_DIR))
        $(if $(BR2_TARGET_UBOOT_SPL),
-               cp -dpf $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)) $(BINARIES_DIR)/)
+               for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+                       cp -dpf $(@D)/$$p $(BINARIES_DIR)/; \
+               done)
        $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
                $(HOST_DIR)/usr/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
                $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
@@ -196,9 +198,11 @@ endif
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQ_IMAGE),y)
 define UBOOT_GENERATE_ZYNQ_IMAGE
-       $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \

boot/uboot/uboot.mk endif · line 208

Quoted 10 lines
boot/uboot/uboot.mk +5 -2
-               -u $(@D)/$(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME))     \
-               -o $(BINARIES_DIR)/BOOT.BIN
+       for p in $(call qstrip,$(BR2_TARGET_UBOOT_SPL_NAME)); do \
+               $(HOST_DIR)/usr/bin/python2 $(HOST_DIR)/usr/bin/zynq-boot-bin.py \
+                       -u $(@D)/$$p -o $(BINARIES_DIR)/BOOT.BIN; \
+               fi; \
+       done
 endef
 UBOOT_DEPENDENCIES += host-zynq-boot-bin
 UBOOT_POST_INSTALL_IMAGES_HOOKS += UBOOT_GENERATE_ZYNQ_IMAGE

We have a similar modification in our tree but for the real u-boot image and not the SPL image. In this case, the UBOOT_BOARD_NAME can take a list of board names, and we use a similar for loop to iterate over the configure, build, install step for each board.

Use case is to use a single defconfig for different hardware boards that are very much alike but need some different u-boot (different device tree, among other subtleties).

I thought I once shared this patch with the list but I cannot find it back.

If the path is now more open for inclusion of such change, I would be more than happy to send it.

Let me know,
Thomas
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot