[RFC PATCH 6/9] mkimage_fit_atf.sh: produce working binaries by default

[Michal Simek]

On 21. 03. 20 16:19, Petr Štetiar wrote:
> Michal Simek <michal.simek at xilinx.com> [2020-03-20 11:20:17]:
> 
>> I think instead of fixing it on several places we should merge things
>> together and fix this issue there.
> 
> What do you mean exactly? Checking for the deps one layer up, like this for
> example in sunxi?
> 
>   diff --git a/Makefile b/Makefile
>   index 44776b8efcc4..1f4bff4374cf 100644
>   --- a/Makefile
>   +++ b/Makefile
>   @@ -1276,6 +1276,19 @@ ifndef CONFIG_SYS_UBOOT_START
>    CONFIG_SYS_UBOOT_START := $(CONFIG_SYS_TEXT_BASE)
>    endif
>    
>   +define check_its_dep
>   +	@if ! test -f "$(1)"; then \
>   +		if test "$(CONFIG_BUILDBOT_BROKEN_BINARIES)" = "y"; then \
>   +			touch "$(1)"; \
>   +		else \
>   +			echo "ERROR: $(2) file $(1) NOT found. If you want to build without " >&2; \
>   +			echo "a $(2) file (creating a NON-FUNCTIONAL binary), then enable" >&2; \
>   +			echo "config option CONFIG_BUILDBOT_BROKEN_BINARIES." >&2; \
>   +			false; \
>   +		fi \
>   +	fi
>   +endef
>   +
>    # Boards with more complex image requirements can provide an .its source file
>    # or a generator script
>    ifneq ($(CONFIG_SPL_FIT_SOURCE),"")
>   @@ -1289,6 +1302,13 @@ endif
>    ifeq ($(CONFIG_SPL_FIT_GENERATOR),"arch/arm/mach-rockchip/make_fit_atf.py")
>    U_BOOT_ITS_DEPS += u-boot
>    endif
>   +ifeq ($(CONFIG_SPL_FIT_GENERATOR),"board/sunxi/mksunxi_fit_atf.sh")
>   +BL31 := $(CURDIR)/bl31.bin

here should be at least ?=

>   +export BL31
>   +$(BL31):
>   +	$(call check_its_dep,$@,BL31)
>   +U_BOOT_ITS_DEPS += $(BL31)
>   +endif
>    $(U_BOOT_ITS): $(U_BOOT_ITS_DEPS) FORCE
> 	  $(srctree)/$(CONFIG_SPL_FIT_GENERATOR) \
> 	  $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) > $@
>   diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh
>   index 88ad71974706..1b0e5ee7a77a 100755
>   --- a/board/sunxi/mksunxi_fit_atf.sh
>   +++ b/board/sunxi/mksunxi_fit_atf.sh
>   @@ -5,14 +5,6 @@
>    #
>    # usage: $0 <dt_name> [<dt_name> [<dt_name] ...]
>    
>   -[ -z "$BL31" ] && BL31="bl31.bin"
>   -
>   -if [ ! -f $BL31 ]; then
>   -	echo "WARNING: BL31 file $BL31 NOT found, resulting binary is non-functional" >&2
>   -	echo "Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64" >&2
>   -	BL31=/dev/null
>   -fi
>   -
>    if grep -q "^CONFIG_MACH_SUN50I_H6=y" .config; then
> 	  BL31_ADDR=0x104000
>    else
> 
>> Take a look at thread where we discussed it with Tom.
>> https://lists.denx.de/pipermail/u-boot/2019-December/393556.html
> 
> Ok, but that N'th variation was merged anyway. So whats the plan? It's not
> clear from that discussion or I don't get it.

My script was merged mostly because of fedora support and I expect it
was the last one. Plan is to merge all of these to one shell script
because most of the content in that script is very similar.

The issue which I see is that patch above is changing current behavior
which means when this is applied all travis/gitlab/azure jobs will fail
because none has setup this config option.

And truth is that even regular user doesn't know if error out is not
check that final binary won't work because nothing fails.

Anyway I have sent series which enables to run U-Boot to run EL3 that's
why at least zynqmp should be ok to build and run u-boot.itb. But better
to run it with ATF.

Thanks,
Michal