Re: [PATCH] firmware: handle firmware files being links correctly

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 9/17/25 11:58 AM, Sascha Hauer wrote:
> Sometimes firmware files can be links. When rebuilding barebox correctly
> rebuilds the firmware when file the link points to is updated, but the
> firmware is not rebuilt when the link itself is changed to link to
> another file.

This is surprising. I though GNU make looks at the mtime of the destination.

> Fix this by including the sha256sum directly in the generated assembly
> file by using .byte rather than generating a file containing the
> binary sha256sum and include that using .incbin. This way the generated
> assembly file for sure changes when the firmware file changes and it's
> rebuilt when necessary (and not when it's not).

The if_changed in just one of the $(if branches looks a bit strange, so
it's good to drop it IMO..

> 
> Reproducer:
> 
> export ARCH=arm
> make imx_v8_defconfig
> echo foo > firmware/foo
> echo bar > firmware/bar
> ln -sf foo firmware/imx8mm-bl31.bin
> make
> 
> The following should rebuild the barebox image including the updated
> firmware, but doesn't:
> 
> ln -sf bar firmware/imx8mm-bl31.bin
> make
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

But see suggestion below.

> ---
>  firmware/Makefile | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 0c9da2996f..df9f277475 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -57,6 +57,8 @@ FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
>  FWNAME_EXISTS = $(if $(wildcard $(FIRMWARE_DIR)/$(FWNAME)),1,0)
>  
>  filechk_fwbin = { \
> +	SHA=$$(sha256sum $(FIRMWARE_DIR)/$(FWNAME) | sed 's/ .*$$//;s/../0x&, /g;s/, $$//') ;\
> +	\
>  	echo "/* Generated by $(src)/Makefile */"		;\
>  	echo "\#include <asm-generic/pointer.h>"		;\
>  	echo ".section .note.GNU-stack,\"\",%progbits"		;\
> @@ -80,7 +82,7 @@ filechk_fwbin = { \
>  	echo "    .p2align ASM_LGPTR"				;\
>  	echo ".global _fw_$(FWSTR)_sha_start"			;\
>  	echo "_fw_$(FWSTR)_sha_start:"				;\
> -	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
> +	echo "    .byte $${SHA}"				;\
>  	echo ".global _fw_$(FWSTR)_sha_end"			;\

How about we add here an

.if _fw_$(FWSTR)_sha_start - _fw_$(FWSTR)_sha_end
	.err
	.warnig "sha256sum empty"
.endif

As .byte doesn't require an argument, this would ensure breakage to be
noticed at compile time.

Cheers,
Ahmad

>  }
> @@ -89,20 +91,12 @@ filechk_fwbin_ext = { \
>  	$(filechk_fwbin)					;\
>  }
>  
> -$(obj)/%.gen.S: $(obj)/%.sha.bin FORCE
> +$(obj)/%.gen.S: FORCE
>  	$(call filechk,fwbin,.rodata.$(FWSTR),)
>  
> -$(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
> +$(obj)/%.extgen.S: FORCE
>  	$(call filechk,fwbin_ext,.pblext.$(FWSTR),a)
>  
> -$(obj)/%.sha.bin: $(obj)/%.sum FORCE
> -	$(call if_changed,sha256bin)
> -
> -$(obj)/%.sum: FORCE
> -	$(if $(wildcard $(FIRMWARE_DIR)/$*), $(call if_changed,sha256sum,$(FIRMWARE_DIR)/$*), @touch $@)
> -
> -clean-files += *.sha.bin *.sum
> -
>  # This dependency is used if missing firmware should fail the build immediately
>  fwdep-required-y = $(FIRMWARE_DIR)/%
>  # This dependency expands to nothing if the file doesn't exist. This allows

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |