| Message ID | 1458513351-6556-4-git-send-email-pieter@boesman.nl |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Mar 20, 2016 at 11:35 PM, Pieter Smith <pieter@boesman.nl> wrote: > In preparation for building a 2nd barebox config, a configuration option is > added to allow customization of the image filename when the built image is > copied to the output/images directory. > > Signed-off-by: Pieter Smith <pieter@boesman.nl> Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com> > --- > boot/barebox/Config.in | 7 +++++++ > boot/barebox/barebox.mk | 9 +++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/boot/barebox/Config.in b/boot/barebox/Config.in > index 4f6872c..7769866 100644 > --- a/boot/barebox/Config.in > +++ b/boot/barebox/Config.in > @@ -106,6 +106,13 @@ config BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > > Set to barebox.bin for barebox versions older than 2012.10. > > +config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > + string "Output image filename" > + default "barebox.bin" > + help > + Name to use when copying the barebox image to the output/images > + directory. > + > config BR2_TARGET_BAREBOX_BAREBOXENV > bool "bareboxenv tool in target" > help > diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk > index d0f28cf..4eea470 100644 > --- a/boot/barebox/barebox.mk > +++ b/boot/barebox/barebox.mk > @@ -93,9 +93,11 @@ endef > > define BAREBOX_INSTALL_IMAGES_CMDS > if test -e $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > elif test -e $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > else \ > echo "error: Specified built image file not found: $(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE))" >&2 ; \ > echo " in: $(@D)/" >&2 ; \ > @@ -124,6 +126,9 @@ endif > ifndef BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > $(error No barebox built image filename specified. Check your BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE setting) > endif > +ifndef BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > +$(error No barebox output image filename specified. Check your BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE setting) > +endif > endif > > $(eval $(kconfig-package)) > -- > 2.5.0 >
On 03/20/16 23:35, Pieter Smith wrote: > In preparation for building a 2nd barebox config, a configuration option is > added to allow customization of the image filename when the built image is > copied to the output/images directory. > > Signed-off-by: Pieter Smith <pieter@boesman.nl> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Regards, Arnout > --- > boot/barebox/Config.in | 7 +++++++ > boot/barebox/barebox.mk | 9 +++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/boot/barebox/Config.in b/boot/barebox/Config.in > index 4f6872c..7769866 100644 > --- a/boot/barebox/Config.in > +++ b/boot/barebox/Config.in > @@ -106,6 +106,13 @@ config BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > > Set to barebox.bin for barebox versions older than 2012.10. > > +config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > + string "Output image filename" > + default "barebox.bin" > + help > + Name to use when copying the barebox image to the output/images > + directory. > + > config BR2_TARGET_BAREBOX_BAREBOXENV > bool "bareboxenv tool in target" > help > diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk > index d0f28cf..4eea470 100644 > --- a/boot/barebox/barebox.mk > +++ b/boot/barebox/barebox.mk > @@ -93,9 +93,11 @@ endef > > define BAREBOX_INSTALL_IMAGES_CMDS > if test -e $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > elif test -e $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > else \ > echo "error: Specified built image file not found: $(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE))" >&2 ; \ > echo " in: $(@D)/" >&2 ; \ > @@ -124,6 +126,9 @@ endif > ifndef BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > $(error No barebox built image filename specified. Check your BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE setting) > endif > +ifndef BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > +$(error No barebox output image filename specified. Check your BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE setting) > +endif > endif > > $(eval $(kconfig-package)) >
On 03/20/16 23:35, Pieter Smith wrote: > In preparation for building a 2nd barebox config, a configuration option is > added to allow customization of the image filename when the built image is > copied to the output/images directory. > > Signed-off-by: Pieter Smith <pieter@boesman.nl> > --- > boot/barebox/Config.in | 7 +++++++ > boot/barebox/barebox.mk | 9 +++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/boot/barebox/Config.in b/boot/barebox/Config.in > index 4f6872c..7769866 100644 > --- a/boot/barebox/Config.in > +++ b/boot/barebox/Config.in > @@ -106,6 +106,13 @@ config BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > > Set to barebox.bin for barebox versions older than 2012.10. > > +config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > + string "Output image filename" With my suggestion of BR2_TARGET_BAREBOX_IMAGE_NAME, this should be changed to e.g. BR2_TARGET_BAREBOX_OUTPUT_IMAGE_NAME. The prompt looks OK though. However, on second thought I'm wondering if it is really needed. For u-boot or the kernel, we always copy the images with the same name as they were built. If that is not what the ROM boot loader expects, the name can be changed in the image generation script. For example, genimage.cfg could contain: image boot.vfat { vfat { file MLO { "barebox-am33xx-beaglebone-mlo.img" } file barebox.img { "barebox-am33xx-beaglebone.img" } } size = 8M } What do you think? Regards, Arnout > + default "barebox.bin" > + help > + Name to use when copying the barebox image to the output/images > + directory. > + > config BR2_TARGET_BAREBOX_BAREBOXENV > bool "bareboxenv tool in target" > help > diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk > index d0f28cf..4eea470 100644 > --- a/boot/barebox/barebox.mk > +++ b/boot/barebox/barebox.mk > @@ -93,9 +93,11 @@ endef > > define BAREBOX_INSTALL_IMAGES_CMDS > if test -e $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > elif test -e $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ > - cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ > + cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ > + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ > else \ > echo "error: Specified built image file not found: $(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE))" >&2 ; \ > echo " in: $(@D)/" >&2 ; \ > @@ -124,6 +126,9 @@ endif > ifndef BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > $(error No barebox built image filename specified. Check your BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE setting) > endif > +ifndef BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > +$(error No barebox output image filename specified. Check your BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE setting) > +endif > endif > > $(eval $(kconfig-package)) >
Hello, On Tue, 5 Apr 2016 01:20:13 +0200, Arnout Vandecappelle wrote: > > +config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > > + string "Output image filename" > > With my suggestion of BR2_TARGET_BAREBOX_IMAGE_NAME, this should be changed > to e.g. BR2_TARGET_BAREBOX_OUTPUT_IMAGE_NAME. The prompt looks OK though. > > > However, on second thought I'm wondering if it is really needed. For u-boot or > the kernel, we always copy the images with the same name as they were built. If > that is not what the ROM boot loader expects, the name can be changed in the > image generation script. For example, genimage.cfg could contain: > > image boot.vfat { > vfat { > file MLO { > "barebox-am33xx-beaglebone-mlo.img" > } > file barebox.img { > "barebox-am33xx-beaglebone.img" > } > } > size = 8M > } > > > What do you think? I agree, we don't do this for U-Boot/kernel or any other thing, so I don't think we should be doing it for Barebox. Best regards, Thomas
Hi Arnout / Thomas, On Tue, Apr 05, 2016 at 01:20:13AM +0200, Arnout Vandecappelle wrote: > On 03/20/16 23:35, Pieter Smith wrote: > >In preparation for building a 2nd barebox config, a configuration option is > >added to allow customization of the image filename when the built image is > >copied to the output/images directory. > > > >Signed-off-by: Pieter Smith <pieter@boesman.nl> > >--- > > boot/barebox/Config.in | 7 +++++++ > > boot/barebox/barebox.mk | 9 +++++++-- > > 2 files changed, 14 insertions(+), 2 deletions(-) > > > >diff --git a/boot/barebox/Config.in b/boot/barebox/Config.in > >index 4f6872c..7769866 100644 > >--- a/boot/barebox/Config.in > >+++ b/boot/barebox/Config.in > >@@ -106,6 +106,13 @@ config BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE > > > > Set to barebox.bin for barebox versions older than 2012.10. > > > >+config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE > >+ string "Output image filename" > > With my suggestion of BR2_TARGET_BAREBOX_IMAGE_NAME, this should be > changed to e.g. BR2_TARGET_BAREBOX_OUTPUT_IMAGE_NAME. The prompt looks OK > though. > > > However, on second thought I'm wondering if it is really needed. For u-boot > or the kernel, we always copy the images with the same name as they were > built. If that is not what the ROM boot loader expects, the name can be > changed in the image generation script. For example, genimage.cfg could > contain: > > image boot.vfat { > vfat { > file MLO { > "barebox-am33xx-beaglebone-mlo.img" > } > file barebox.img { > "barebox-am33xx-beaglebone.img" > } > } > size = 8M > } > > > What do you think? I agree that this is cleaner from a buildroot perspective. But this would require carrying a genimage.cfg patch for barebox as part of beaglebone_barebox_defconfig. Is this what you are suggesting? > Regards, > Arnout [snip] - Pieter
On 04/06/16 22:14, Pieter Smith wrote: > Hi Arnout / Thomas, > > On Tue, Apr 05, 2016 at 01:20:13AM +0200, Arnout Vandecappelle wrote: [snip] >> However, on second thought I'm wondering if it is really needed. For u-boot >> or the kernel, we always copy the images with the same name as they were >> built. If that is not what the ROM boot loader expects, the name can be >> changed in the image generation script. For example, genimage.cfg could >> contain: >> >> image boot.vfat { >> vfat { >> file MLO { >> "barebox-am33xx-beaglebone-mlo.img" >> } >> file barebox.img { >> "barebox-am33xx-beaglebone.img" >> } >> } >> size = 8M >> } >> >> >> What do you think? > > I agree that this is cleaner from a buildroot perspective. But this would > require carrying a genimage.cfg patch for barebox as part of > beaglebone_barebox_defconfig. Is this what you are suggesting? I don't really understand what you mean. As I suggested in that patch, it would be nice to add a genimage config as well. When you do that, you can use a fragment like above to make sure the files will have the correct name in the filesystem. (Like above, except I screwed up the indentation :-) Regards, Arnout
On Thu, Apr 07, 2016 at 01:06:49AM +0200, Arnout Vandecappelle wrote: > On 04/06/16 22:14, Pieter Smith wrote: > >Hi Arnout / Thomas, > > > >On Tue, Apr 05, 2016 at 01:20:13AM +0200, Arnout Vandecappelle wrote: > [snip] > >> However, on second thought I'm wondering if it is really needed. For u-boot > >>or the kernel, we always copy the images with the same name as they were > >>built. If that is not what the ROM boot loader expects, the name can be > >>changed in the image generation script. For example, genimage.cfg could > >>contain: > >> > >>image boot.vfat { > >> vfat { > >> file MLO { > >> "barebox-am33xx-beaglebone-mlo.img" > >> } > >> file barebox.img { > >> "barebox-am33xx-beaglebone.img" > >> } > >> } > >> size = 8M > >>} > >> > >> > >> What do you think? > > > >I agree that this is cleaner from a buildroot perspective. But this would > >require carrying a genimage.cfg patch for barebox as part of > >beaglebone_barebox_defconfig. Is this what you are suggesting? > > I don't really understand what you mean. As I suggested in that patch, it > would be nice to add a genimage config as well. When you do that, you can > use a fragment like above to make sure the files will have the correct name > in the filesystem. (Like above, except I screwed up the indentation :-) The vfat file attribute also expects an "image=" key, not just the image name. Thanks! This will be in v5 of the patch-set. [snip] - Pieter Smith
diff --git a/boot/barebox/Config.in b/boot/barebox/Config.in index 4f6872c..7769866 100644 --- a/boot/barebox/Config.in +++ b/boot/barebox/Config.in @@ -106,6 +106,13 @@ config BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE Set to barebox.bin for barebox versions older than 2012.10. +config BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE + string "Output image filename" + default "barebox.bin" + help + Name to use when copying the barebox image to the output/images + directory. + config BR2_TARGET_BAREBOX_BAREBOXENV bool "bareboxenv tool in target" help diff --git a/boot/barebox/barebox.mk b/boot/barebox/barebox.mk index d0f28cf..4eea470 100644 --- a/boot/barebox/barebox.mk +++ b/boot/barebox/barebox.mk @@ -93,9 +93,11 @@ endef define BAREBOX_INSTALL_IMAGES_CMDS if test -e $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ - cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ + cp -L $(@D)/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ elif test -e $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)); then \ - cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) $(BINARIES_DIR)/barebox.bin ; \ + cp $(@D)/images/$(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE)) \ + $(BINARIES_DIR)/$(call qstrip,$(BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE)) ; \ else \ echo "error: Specified built image file not found: $(call qstrip,$(BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE))" >&2 ; \ echo " in: $(@D)/" >&2 ; \ @@ -124,6 +126,9 @@ endif ifndef BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE $(error No barebox built image filename specified. Check your BR2_TARGET_BAREBOX_BUILT_IMAGE_FILE setting) endif +ifndef BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE +$(error No barebox output image filename specified. Check your BR2_TARGET_BAREBOX_OUTPUT_IMAGE_FILE setting) +endif endif $(eval $(kconfig-package))
In preparation for building a 2nd barebox config, a configuration option is added to allow customization of the image filename when the built image is copied to the output/images directory. Signed-off-by: Pieter Smith <pieter@boesman.nl> --- boot/barebox/Config.in | 7 +++++++ boot/barebox/barebox.mk | 9 +++++++-- 2 files changed, 14 insertions(+), 2 deletions(-)