Hi Ahmad,
On 23-06-26, Ahmad Fatoum wrote:
> Previous commit turned compile-time errors into link-time errors.
> This commit goes a step further and allows the link to succeed
> unconditionally for build coverage and then dependent on the newly
> introduced CONFIG_MISSING_FIRMWARE_ERROR option abort the build with an
> error code and a listing of missing firmware printed to stderr.
>
> In any case, barebox images which contain firmware in their PBL
> that's not available will be marked specially to reduce the risk
> of accidentally putting them to use:
>
> * They're truncated to zero size
>
> * The final "images built:" section marks them as having firmware
> missing
>
> * They are omitted from the listing in the barebox-flash-images file
>
> * Each barebox-broken.img is accompanied with a
> barebox-broken.img.missing-firmware containing a newline delimited
> list of missing firmware images
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> firmware/Kconfig | 14 ++++++++++++++
> firmware/Makefile | 5 +++++
> images/Makefile | 21 ++++++++++++++++-----
> scripts/Makefile.lib | 3 +++
> 4 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index 56ced00bc430..56d6d0d6c030 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -6,6 +6,20 @@ config EXTRA_FIRMWARE_DIR
> string "Firmware blobs root directory"
> default "firmware"
>
> +config MISSING_FIRMWARE_ERROR
> + bool "Fail the build when required firmware is missing"
> + default y
Do we need to make this default y? IMHO multi_v8_defconfig will set this
to 'n' anyway and so the 'n' will become the de-facto default.
> + help
> + In-tree Defconfigs that enable multiple boards with different firmware
> + binary requirements would say y here, so you don't need unrelated firmware
> + for the build to succeed.
> +
> + Defconfigs custom-tailored to products would say n here as all boards
> + being built should be functional and have their firmware available.
> +
> + If in doubt, say Y and refer to the documentation on where to acquire the
> + needed firmware.
> +
> config HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> bool
> default y
> diff --git a/firmware/Makefile b/firmware/Makefile
> index c66d19c677e8..f9490a8e7a15 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -58,6 +58,11 @@ filechk_fwbin = { \
> echo "\#endif" ;\
> echo ".global _fw_$(FWSTR)_end" ;\
> echo "_fw_$(FWSTR)_end:" ;\
> + echo "\#ifdef __PBL__" ;\
> + echo " .section .missing_fw,\"a\"" ;\
> + echo "_fwname_$(FWSTR):" ;\
> + echo ".ascii \"firmware/$(FWNAME)\\n\"" ;\
> + echo "\#endif" ;\
Shouldn't this be part of the #ifdef no-firmware? This way we do add the
.missing_fw section always which shouldn't be the case, right?
> }
>
> __fwbin_sha = { \
> diff --git a/images/Makefile b/images/Makefile
> index c93f9e268978..16d86a5a5da4 100644
> --- a/images/Makefile
> +++ b/images/Makefile
> @@ -72,6 +72,8 @@ $(obj)/%.pbl: $(pbl-lds) $(BAREBOX_PBL_OBJS) $(obj)/piggy.o $(obj)/sha_sum.o FOR
>
> $(obj)/%.pblb: $(obj)/%.pbl FORCE
> $(call if_changed,objcopy_bin,$(*F))
> + $(Q)$(OBJCOPY) -O binary --only-section=.missing_fw $< $@.missing-firmware
> + $(Q)[ -s $@.missing-firmware ] || rm -f $@.missing-firmware
> $(call cmd,check_file_size,$@,$(CONFIG_BAREBOX_MAX_IMAGE_SIZE))
>
> #
> @@ -127,10 +129,14 @@ $(obj)/barebox.z: $(obj)/../barebox.bin FORCE
>
> # %.img - create a copy from another file
> # ----------------------------------------------------------------
> +
> +missing_fw = $(strip $(wildcard $(obj)/$(FILE_$(@F)).missing-firmware $(basename $(obj)/$(FILE_$(@F))).missing-firmware))
> +
> .SECONDEXPANSION:
> $(obj)/%.img: $(obj)/$$(FILE_$$(@F))
> $(Q)if [ -z $(FILE_$(@F)) ]; then echo "FILE_$(@F) empty!"; false; fi
> - $(call if_changed,shipped)
> + $(Q)$(if $(missing_fw),cat $(missing_fw) >$@.missing-firmware,rm -f $@.missing-firmware)
Okay, since we always add the missing-firmware section we need to clean
it up here, right? Isn't it easier to add the missing-firmware section
only if detected? If this is possible.
> + $(call if_changed,$(if $(missing_fw),0size,shipped))
>
> board = $(srctree)/arch/$(SRCARCH)/boards
> objboard = $(objtree)/arch/$(SRCARCH)/boards
> @@ -194,10 +200,15 @@ multi-image-build:
>
> images: $(image-y-path) $(flash-link) $(flash-list) FORCE
> @echo "images built:"
> - @for i in $(image-y); do echo $$i; done
> + @for i in $(image-y); do \
> + if [ -s $(obj)/$$i ]; then echo $$i; \
> + else >&2 echo "** firmware missing for $$i **"; \
Not sure if we should print an error if the user set
CONFIG_MISSING_FIRMWARE_ERROR to n.
Regards,
Marco
> + $(if $(CONFIG_MISSING_FIRMWARE_ERROR), >&2 sed 's/^/\t/' <$(obj)/$${i}.missing-firmware; missing=1;) \
> + fi; done; test -n "$$missing" && \
> + echo >&2 "Firmware missing in CONFIG_MISSING_FIRMWARE_ERROR=y build" && false
>
> __images_install: images
> - @for i in $(image-y-path); do install -t "$(INSTALL_PATH)" $$i; done
> + @for i in $(image-y-path); do if [ -s $$i ]; then install -t "$(INSTALL_PATH)" $$i; fi; done
>
> PHONY += __images_install
>
> @@ -205,10 +216,10 @@ $(flash-link): $(link-dest) FORCE
> $(call if_changed,ln)
>
> $(flash-list): $(image-y-path)
> - @for i in $^; do echo $$i; done > $@
> + @for i in $^; do if [ -s $$i ]; then echo $$i; fi; done > $@
>
> clean-files := *.pbl *.pblb *.map start_*.imximg *.img barebox.z start_*.kwbimg \
> start_*.kwbuartimg *.socfpgaimg *.mlo *.t20img *.t20img.cfg *.t30img \
> *.t30img.cfg *.t124img *.t124img.cfg *.mlospi *.mlo *.mxsbs *.mxssd \
> - start_*.simximg start_*.usimximg *.zynqimg *.image *.swapped
> + start_*.simximg start_*.usimximg *.zynqimg *.image *.swapped *.missing-firmware
> clean-files += pbl.lds
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 42ee27499561..b8fb2684421e 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -236,6 +236,9 @@ endef
> # Shipped files
> # ===========================================================================
>
> +quiet_cmd_0size = 0SIZE $@
> +cmd_0size = : > $@
> +
> quiet_cmd_shipped = SHIPPED $@
> cmd_shipped = cat $< > $@
>
> --
> 2.39.2
>
>
>
