[PATCH master v1 0/6] firmware: optionally turn missing firmware errors into warnings

[PATCH master v1 0/6] firmware: optionally turn missing firmware errors into warnings

[Ahmad Fatoum]

Many of the ARMv8 defconfigs can require firmware external to barebox
for proper operation: TF-A as BL31, DDR PHY firmware, SDRAM setup
or OP-TEE. This led to defconfigs that fail to build by default
unless the user installs the firmware files into the firmware directory.

While this worked for the SoC-specific defconfigs, it's really annoying
for the multi_v8_defconfig:

  - The user will need to install firmware even for unrelated
    platforms

  - Some platforms like e.g. qemu-virt64, which need no firmware to
    be compiled into barebox, can't be built just by using the
    defconfig

For v2023.06.0, this was resolved by disabling boards that lack firmware
through hiding their Kconfig symbol. This has a few shortcomings:

  - Firmware is looked up relative to source tree and can't take
    CONFIG_EXTRA_FIRMWARE_DIR into account that early

  - Firmware needs to exist at configure stage, which breaks
    build system recipes that install firmware after configure
    stage

  - menuconfig e.g. in Yocto's cml1.bbclass doesn't take make options
    into account that are otherwise used for olddefconfig and build

This series resolves the underlying issue differently: We record
whether firmware is missing and just continue the build till the end
at which time the build would succeed (with warnings) if new option
CONFIG_MISSING_FIRMWARE_ERROR was unset.

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, e.g.  ** firmware missing for barebox-rk3568-bpi-r2pro.img**

  * 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

Ahmad Fatoum (6):
  firmware: reference pointer alignment defined in
    <asm-generic/pointer.h>
  firmware: turn missing firmware into linker error
  firmware: optionally turn missing firmware errors into warnings
  ARM: Rockchip: gracefully handle missing firmware
  ARM64: unset CONFIG_MISSING_FIRMWARE_ERROR for Rockchip/i.MX
  firmware: don't hardcode firmware paths in srctree for existence check

 arch/arm/configs/imx_v8_defconfig      |  1 +
 arch/arm/configs/multi_v8_defconfig    |  1 +
 arch/arm/configs/rockchip_v8_defconfig |  1 +
 arch/arm/mach-imx/Kconfig              | 49 -------------------
 arch/arm/mach-rockchip/Kconfig         |  6 ---
 firmware/Kconfig                       | 49 +++++--------------
 firmware/Makefile                      | 18 +++++--
 images/Makefile                        | 23 ++++++---
 images/Makefile.rockchip               | 68 +++++++++-----------------
 scripts/Makefile.lib                   |  3 ++
 10 files changed, 73 insertions(+), 146 deletions(-)

-- 
2.39.2

[PATCH master v1 1/6] firmware: reference pointer alignment defined in <asm-generic/pointer.h>

[Ahmad Fatoum]

We already define pointer size constants for use in assembly. Make use
of this header. No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 firmware/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index efdd5c0da541..8050d0418d33 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -41,13 +41,13 @@ obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
 
 FWNAME    = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
 FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
-ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
 
 filechk_fwbin = { \
 	echo "/* Generated by $(src)/Makefile */"		;\
+	echo "\#include <asm-generic/pointer.h>"		;\
 	echo ".section .note.GNU-stack,\"\",%progbits"		;\
 	echo "    .section $2,\"$3\""				;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "    .p2align $(ASM_LGPTR)"			;\
 	echo ".global _fw_$(FWSTR)_start"			;\
 	echo "_fw_$(FWSTR)_start:"				;\
 	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
@@ -57,7 +57,7 @@ filechk_fwbin = { \
 
 __fwbin_sha = { \
 	echo "    .section .rodata.$(FWSTR).sha"		;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "    .p2align $(ASM_LGPTR)"			;\
 	echo ".global _fw_$(FWSTR)_sha_start"			;\
 	echo "_fw_$(FWSTR)_sha_start:"				;\
 	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
-- 
2.39.2

[PATCH master v1 2/6] firmware: turn missing firmware into linker error

[Ahmad Fatoum]

Appending to firmware-y results in an assembly file being generated that
defines a section embedding the firmware in question verbatim via .incbin.
To update the assembly file as required, the target has a dependency on
the firmware. When the firmware is missing, the build will abort there.

In preparation for deferring missing PBL firmware errors till the very
end, let's remove the firmware dependency if the missing firmware file
is built into PBL and replace the .incbin with a reference to an ultimately
undefined variable.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 firmware/Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index 8050d0418d33..c66d19c677e8 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -41,6 +41,7 @@ obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
 
 FWNAME    = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
 FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
+FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
 
 filechk_fwbin = { \
 	echo "/* Generated by $(src)/Makefile */"		;\
@@ -50,7 +51,11 @@ filechk_fwbin = { \
 	echo "    .p2align $(ASM_LGPTR)"			;\
 	echo ".global _fw_$(FWSTR)_start"			;\
 	echo "_fw_$(FWSTR)_start:"				;\
+	echo "\#if $(FWNAME_EXISTS)"				;\
 	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
+	echo "\#else"						;\
+	echo "ASM_PTR _fwname_$(FWSTR)"				;\
+	echo "\#endif"						;\
 	echo ".global _fw_$(FWSTR)_end"				;\
 	echo "_fw_$(FWSTR)_end:"				;\
 }
@@ -88,7 +93,7 @@ clean-files += *.sha.bin *.sum
 $(patsubst %.gen.o,$(obj)/%.gen.o, $(obj-pbl-y)): $(obj)/%.gen.o: $(fwdir)/%
 
 # The same for pbl:
-$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y)): $(obj)/%.gen.pbl.o: $(fwdir)/%
+$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y)): $(obj)/%.gen.pbl.o: $(wildcard $(fwdir)/%)
 $(patsubst %.gen.o,$(obj)/%.extgen.pbl.o, $(pbl-y)): $(obj)/%.extgen.pbl.o: $(fwdir)/%
 
 pbl-y := $(addsuffix .extgen.o, $(fw-external-y))
-- 
2.39.2

[PATCH master v1 3/6] firmware: optionally turn missing firmware errors into warnings

[Ahmad Fatoum]

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@pengutronix.de>
---
 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
+	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" 						;\
 }
 
 __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)
+	$(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 **"; \
+	  $(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

[PATCH master v1 4/6] ARM: Rockchip: gracefully handle missing firmware

[Ahmad Fatoum]

Unlike the Rockchip BL31 images, the sdram-init.bin for Rockchip SoCs isn't
included via the regular firmware mechanism, but instead is passed as an
argument to the rkimg tool. Like done for regular firmware, have the Rockchip
image build rules handle missing firmware gracefully and update the
.missing-firmware file to list what firmware was missing.

Example of a build with CONFIG_MACH_RADXA_ROCK3, but with both firmware
files missing:

  $ find . -name *rock3a*.missing-firmware -exec grep -Hr . {} \;

  start_rock3a.pblb.missing-firmware:firmware/rk3568-bl31.bin
  start_rock3a.pblb.rkimg.missing-firmware:arch/arm/boards/radxa-rock3/sdram-init.bin
  barebox-rock3a.img.missing-firmware:arch/arm/boards/radxa-rock3/sdram-init.bin
  barebox-rock3a.img.missing-firmware:firmware/rk3568-bl31.bin

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 images/Makefile          |  2 +-
 images/Makefile.rockchip | 68 ++++++++++++++--------------------------
 2 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/images/Makefile b/images/Makefile
index 16d86a5a5da4..c66f3764aa3e 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -220,6 +220,6 @@ $(flash-list): $(image-y-path)
 
 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 \
+	*.t30img.cfg *.t124img *.t124img.cfg *.mlospi *.mlo *.mxsbs *.mxssd *.rkimg \
 	start_*.simximg start_*.usimximg *.zynqimg *.image *.swapped *.missing-firmware
 clean-files += pbl.lds
diff --git a/images/Makefile.rockchip b/images/Makefile.rockchip
index 47779a7d3585..ea32af42414b 100644
--- a/images/Makefile.rockchip
+++ b/images/Makefile.rockchip
@@ -3,6 +3,24 @@
 # barebox image generation Makefile for Rockchip images
 #
 
+quiet_cmd_rkimg_image = RK-IMG $@
+      cmd_rkimg_image = $(objtree)/scripts/rkimage -o $@ $(word 2,$^) $(word 1,$^)
+
+# params: CONFIG symbol, entry point, sdram-init.bin, board identifier string
+define build_rockchip_image =
+$(eval
+ifeq ($($(strip $(1))), y)
+	pblb-y += $(strip $(2))
+	FILE_barebox-$(strip $(4)).img = $(strip $(2)).pblb.rkimg
+	image-y += barebox-$(strip $(4)).img
+
+$$(obj)/$(strip $(2)).pblb.rkimg: $$(obj)/$(strip $(2)).pblb $$(wildcard $(board)/$(strip $(3))) FORCE
+	$$(Q)$$(if $$(word 3,$$^),rm -f $$@.missing-firmware,echo arch/$(SRCARCH)/boards/$(strip $(3)) >$$@.missing-firmware)
+	$$(call if_changed,$$(if $$(word 3,$$^),rkimg_image,0size))
+endif
+)
+endef
+
 pblb-$(CONFIG_MACH_RADXA_ROCK) += start_radxa_rock
 FILE_barebox-radxa-rock.img = start_radxa_rock.pblb
 image-$(CONFIG_MACH_RADXA_ROCK) += barebox-radxa-rock.img
@@ -11,47 +29,9 @@ pblb-$(CONFIG_MACH_PHYTEC_SOM_RK3288) += start_rk3288_phycore_som
 FILE_barebox-rk3288-phycore-som.img = start_rk3288_phycore_som.pblb
 image-$(CONFIG_MACH_PHYTEC_SOM_RK3288) += barebox-rk3288-phycore-som.img
 
-pblb-$(CONFIG_MACH_RK3568_EVB) += start_rk3568_evb
-image-$(CONFIG_MACH_RK3568_EVB) += barebox-rk3568-evb.img
-
-pblb-$(CONFIG_MACH_RK3568_BPI_R2PRO) += start_rk3568_bpi_r2pro
-image-$(CONFIG_MACH_RK3568_BPI_R2PRO) += barebox-rk3568-bpi-r2pro.img
-
-pblb-$(CONFIG_MACH_PINE64_QUARTZ64) += start_quartz64a
-image-$(CONFIG_MACH_PINE64_QUARTZ64) += barebox-quartz64a.img
-
-pblb-$(CONFIG_MACH_RADXA_ROCK3) += start_rock3a
-image-$(CONFIG_MACH_RADXA_ROCK3) += barebox-rock3a.img
-
-pblb-$(CONFIG_MACH_RADXA_ROCK5) += start_rock5b
-image-$(CONFIG_MACH_RADXA_ROCK5) += barebox-rock5b.img
-
-pblb-$(CONFIG_MACH_RADXA_CM3) += start_radxa-cm3-io.img
-image-$(CONFIG_MACH_RADXA_CM3) += barebox-radxa-cm3-io.img
-
-quiet_cmd_rkimg_image = RK-IMG $@
-      cmd_rkimg_image = $(objtree)/scripts/rkimage -o $@ $(word 2,$^) $(word 1,$^)
-
-$(obj)/barebox-rk3568-evb.img: $(obj)/start_rk3568_evb.pblb \
-                $(board)/rockchip-rk3568-evb/sdram-init.bin
-	$(call if_changed,rkimg_image)
-
-$(obj)/barebox-rk3568-bpi-r2pro.img: $(obj)/start_rk3568_bpi_r2pro.pblb \
-                $(board)/rockchip-rk3568-bpi-r2pro/sdram-init.bin
-	$(call if_changed,rkimg_image)
-
-$(obj)/barebox-quartz64a.img: $(obj)/start_quartz64a.pblb \
-                $(board)/pine64-quartz64/sdram-init.bin
-	$(call if_changed,rkimg_image)
-
-$(obj)/barebox-rock3a.img: $(obj)/start_rock3a.pblb \
-                $(board)/radxa-rock3/sdram-init.bin
-	$(call if_changed,rkimg_image)
-
-$(obj)/barebox-rock5b.img: $(obj)/start_rock5b.pblb \
-                $(board)/radxa-rock5/sdram-init.bin
-	$(call if_changed,rkimg_image)
-
-$(obj)/barebox-radxa-cm3-io.img: $(obj)/start_radxa_cm3_io.pblb \
-                $(board)/radxa-cm3/sdram-init.bin
-	$(call if_changed,rkimg_image)
+$(call build_rockchip_image, CONFIG_MACH_RK3568_EVB, start_rk3568_evb, rockchip-rk3568-evb/sdram-init.bin, rk3568-evb)
+$(call build_rockchip_image, CONFIG_MACH_RK3568_BPI_R2PRO, start_rk3568_bpi_r2pro, rockchip-rk3568-bpi-r2pro/sdram-init.bin, rk3568-bpi-r2pro)
+$(call build_rockchip_image, CONFIG_MACH_PINE64_QUARTZ64, start_quartz64a, pine64-quartz64/sdram-init.bin, quartz64a)
+$(call build_rockchip_image, CONFIG_MACH_RADXA_ROCK3, start_rock3a, radxa-rock3/sdram-init.bin, rock3a)
+$(call build_rockchip_image, CONFIG_MACH_RADXA_ROCK5, start_rock5b, radxa-rock5/sdram-init.bin, rock5b)
+$(call build_rockchip_image, CONFIG_MACH_RADXA_CM3, start_radxa_cm3_io, radxa-cm3/sdram-init.bin, radxa-cm3-io)
-- 
2.39.2

[PATCH master v1 5/6] ARM64: unset CONFIG_MISSING_FIRMWARE_ERROR for Rockchip/i.MX

[Ahmad Fatoum]

barebox ARMv8 images for Rockchip and i.MX depend on external firmware
for RAM setup (e.g. DDR PHY firmware) and on a BL31 binary to service
secure monitor calls of the operating system (usually TF-A).

Images without this firmware are not functional and the build warns
about it, yet for development, it's more convenient to have the build
not fail. This is especially useful for multi_v8_defconfig, which also
builds the drivers Qemu Virt64 image and which requires no extra
firmware.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/configs/imx_v8_defconfig      | 1 +
 arch/arm/configs/multi_v8_defconfig    | 1 +
 arch/arm/configs/rockchip_v8_defconfig | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm/configs/imx_v8_defconfig b/arch/arm/configs/imx_v8_defconfig
index 93b85ff5ea40..99d871bfb968 100644
--- a/arch/arm/configs/imx_v8_defconfig
+++ b/arch/arm/configs/imx_v8_defconfig
@@ -147,3 +147,4 @@ CONFIG_FS_FAT_WRITE=y
 CONFIG_FS_FAT_LFN=y
 CONFIG_FS_RATP=y
 CONFIG_ZLIB=y
+# CONFIG_MISSING_FIRMWARE_ERROR is not set
diff --git a/arch/arm/configs/multi_v8_defconfig b/arch/arm/configs/multi_v8_defconfig
index e0ff21641a01..76b80975887d 100644
--- a/arch/arm/configs/multi_v8_defconfig
+++ b/arch/arm/configs/multi_v8_defconfig
@@ -241,3 +241,4 @@ CONFIG_FS_PSTORE=y
 CONFIG_FS_PSTORE_CONSOLE=y
 CONFIG_FS_RATP=y
 CONFIG_LZO_DECOMPRESS=y
+# CONFIG_MISSING_FIRMWARE_ERROR is not set
diff --git a/arch/arm/configs/rockchip_v8_defconfig b/arch/arm/configs/rockchip_v8_defconfig
index cc3c3c6d489b..7d1d41965ece 100644
--- a/arch/arm/configs/rockchip_v8_defconfig
+++ b/arch/arm/configs/rockchip_v8_defconfig
@@ -149,3 +149,4 @@ CONFIG_FS_FAT_LFN=y
 CONFIG_FS_BPKFS=y
 CONFIG_FS_UIMAGEFS=y
 CONFIG_LZO_DECOMPRESS=y
+# CONFIG_MISSING_FIRMWARE_ERROR is not set
-- 
2.39.2

[PATCH master v1 6/6] firmware: don't hardcode firmware paths in srctree for existence check

[Ahmad Fatoum]

Now that PBL firmware can be made optional, we can drop the existence
checks in the Kconfig. These were flawed anyway, because they didn't
take CONFIG_EXTRA_FIRMWARE_DIR into account and they led to issues with
build system recipes that didn't expect firmware to be required before
compilation step at oldconfig/menuconfig time.

This effectively reverts commits:

  624962fb45c6 ("ARM: i.MX: make boards selectable only when firmware files are present")
  4ff2e5510ec9 ("ARM: Rockchip: make boards only selectable when firmware is present")
  41a89611774a ("ARM: rockchip: don't attempt building MACH_RADXA_ROCK5 without firmware")

Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-imx/Kconfig      | 49 ----------------------------------
 arch/arm/mach-rockchip/Kconfig |  6 -----
 firmware/Kconfig               | 39 ---------------------------
 3 files changed, 94 deletions(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 5cd1428f9a6b..819c23bae88a 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -561,35 +561,9 @@ if 64BIT
 
 comment "i.MX8M boards"
 
-if !HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-comment "LPDDR4 firmware files missing, some boards are not selectable"
-endif
-
-if !HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
-comment "DDR4 firmware files missing, some boards are not selectable"
-endif
-
-if !HAVE_FIRMWARE_IMX8MM_ATF
-comment "i.MX8MM TF-A files missing, i.MX8MM boards are disabled"
-endif
-
-if !HAVE_FIRMWARE_IMX8MQ_ATF
-comment "i.MX8MQ TF-A files missing, i.MX8MQ boards are disabled"
-endif
-
-if !HAVE_FIRMWARE_IMX8MN_ATF
-comment "i.MX8MN TF-A files missing, i.MX8MN boards are disabled"
-endif
-
-if !HAVE_FIRMWARE_IMX8MP_ATF
-comment "i.MX8MP TF-A files missing, i.MX8MP boards are disabled"
-endif
-
 config MACH_INNOCOMM_WB15
 	bool "InnoComm WB15 (i.MX8MM) EVK"
 	select ARCH_IMX8MM
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MM_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MM_ATF
 	select ARM_SMCCC
@@ -622,8 +596,6 @@ config MACH_BEO_MOZART2
 config MACH_MNT_REFORM
 	bool "MNT Reform"
 	select ARCH_IMX8MQ
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MQ_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MQ_ATF
 	select ARM_SMCCC
@@ -633,8 +605,6 @@ config MACH_MNT_REFORM
 config MACH_NXP_IMX8MM_EVK
 	bool "NXP i.MX8MM EVK Board"
 	select ARCH_IMX8MM
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MM_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MM_ATF
 	select ARM_SMCCC
@@ -646,9 +616,6 @@ config MACH_NXP_IMX8MM_EVK
 config MACH_NXP_IMX8MN_EVK
 	bool "NXP i.MX8MN EVK Board"
 	select ARCH_IMX8MN
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MN_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX_DDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MN_ATF
@@ -660,8 +627,6 @@ config MACH_NXP_IMX8MN_EVK
 config MACH_NXP_IMX8MP_EVK
 	bool "NXP i.MX8MP EVK Board"
 	select ARCH_IMX8MP
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MP_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MP_ATF
 	select ARM_SMCCC
@@ -672,8 +637,6 @@ config MACH_NXP_IMX8MP_EVK
 config MACH_NXP_IMX8MQ_EVK
 	bool "NXP i.MX8MQ EVK Board"
 	select ARCH_IMX8MQ
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MQ_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MQ_ATF
 	select ARM_SMCCC
@@ -682,8 +645,6 @@ config MACH_NXP_IMX8MQ_EVK
 config MACH_PHYTEC_SOM_IMX8MQ
 	bool "Phytec i.MX8M SOM"
 	select ARCH_IMX8MQ
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MQ_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MQ_ATF
 	select ARM_SMCCC
@@ -692,8 +653,6 @@ config MACH_PHYTEC_SOM_IMX8MQ
 config MACH_POLYHEX_DEBIX
 	bool "Polyhex DEBIX Model-A/B (i.MX8MP) Board"
 	select ARCH_IMX8MP
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MP_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MP_ATF
 	select ARM_SMCCC
@@ -704,8 +663,6 @@ config MACH_POLYHEX_DEBIX
 config MACH_PROTONIC_IMX8M
 	bool "Protonic-Holland i.MX8Mx based boards"
 	select ARCH_IMX8MM
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MM_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MM_ATF
 	select ARM_SMCCC
@@ -716,8 +673,6 @@ config MACH_PROTONIC_IMX8M
 config MACH_TQ_MBA8MPXL
 	bool "TQ i.MX8MP Dual/Quad on MBa8MPxL Board"
 	select ARCH_IMX8MP
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MP_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MP_ATF
 	select ARM_SMCCC
@@ -728,8 +683,6 @@ config MACH_TQ_MBA8MPXL
 config MACH_VARISCITE_DT8MCUSTOMBOARD_IMX8MP
 	bool "Variscite DT8MCustomBoard with DART-MX8M-PLUS"
 	select ARCH_IMX8MP
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MP_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MP_ATF
 	select ARM_SMCCC
@@ -740,8 +693,6 @@ config MACH_VARISCITE_DT8MCUSTOMBOARD_IMX8MP
 config MACH_ZII_IMX8MQ_DEV
 	bool "ZII i.MX8MQ based devices"
 	select ARCH_IMX8MQ
-	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
-	depends on HAVE_FIRMWARE_IMX8MQ_ATF
 	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	select FIRMWARE_IMX8MQ_ATF
 	select ARM_SMCCC
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index a71c4ae35dd9..8cdf2c28a912 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -75,42 +75,36 @@ if 64BIT
 
 config MACH_RK3568_EVB
 	select ARCH_RK3568
-	depends on $(success,test -e $(srctree)/arch/arm/boards/rockchip-rk3568-evb/sdram-init.bin)
 	bool "RK3568 EVB"
 	help
 	  Say Y here if you are using a RK3568 EVB
 
 config MACH_RK3568_BPI_R2PRO
 	select ARCH_RK3568
-	depends on $(success,test -e $(srctree)/arch/arm/boards/rockchip-rk3568-bpi-r2pro/sdram-init.bin)
 	bool "RK3568 BPI R2PRO"
 	help
 	  Say Y here if you are using a RK3568 Bananpi R2 Pro
 
 config MACH_PINE64_QUARTZ64
 	select ARCH_RK3568
-	depends on $(success,test -e $(srctree)/arch/arm/boards/pine64-quartz64/sdram-init.bin)
 	bool "Pine64 Quartz64"
 	help
 	  Say Y here if you are using a Pine64 Quartz64
 
 config MACH_RADXA_ROCK3
 	select ARCH_RK3568
-	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-rock3/sdram-init.bin)
 	bool "Radxa ROCK3"
 	help
 	  Say Y here if you are using a Radxa ROCK3
 
 config MACH_RADXA_ROCK5
 	select ARCH_RK3588
-	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-rock5/sdram-init.bin)
 	bool "Radxa ROCK5"
 	help
 	  Say Y here if you are using a Radxa ROCK5
 
 config MACH_RADXA_CM3
 	select ARCH_RK3568
-	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-cm3/sdram-init.bin)
 	bool "Radxa CM3"
 	help
 	  Say Y here if you are using a Radxa CM3
diff --git a/firmware/Kconfig b/firmware/Kconfig
index 56d6d0d6c030..3328dbc0b111 100644
--- a/firmware/Kconfig
+++ b/firmware/Kconfig
@@ -20,74 +20,35 @@ config MISSING_FIRMWARE_ERROR
 	  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
-	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_1d_dmem.bin)
-	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_1d_imem.bin)
-	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_2d_dmem.bin)
-	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_2d_imem.bin)
-
 config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 	bool
 
-config HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
-	bool
-	default y
-	depends on $(success,test -e $(srctree)/firmware/ddr4_dmem_1d.bin)
-	depends on $(success,test -e $(srctree)/firmware/ddr4_dmem_2d.bin)
-	depends on $(success,test -e $(srctree)/firmware/ddr4_imem_1d.bin)
-	depends on $(success,test -e $(srctree)/firmware/ddr4_imem_2d.bin)
-
 config FIRMWARE_IMX_DDR4_PMU_TRAIN
 	bool
 
-config HAVE_FIRMWARE_IMX8MM_ATF
-	bool
-	default y
-	depends on $(success,test -e $(srctree)/firmware/imx8mm-bl31.bin)
-
 config FIRMWARE_IMX8MM_ATF
 	bool
 
-config HAVE_FIRMWARE_IMX8MN_ATF
-	bool
-	default y
-	depends on $(success,test -e $(srctree)/firmware/imx8mn-bl31.bin)
-
 config FIRMWARE_IMX8MN_ATF
 	bool
 
-config HAVE_FIRMWARE_IMX8MP_ATF
-	bool
-	default y
-	depends on $(success,test -e $(srctree)/firmware/imx8mp-bl31.bin)
-
 config FIRMWARE_IMX8MP_ATF
 	bool
 
-config HAVE_FIRMWARE_IMX8MQ_ATF
-	bool
-	default y
-	depends on $(success,test -e $(srctree)/firmware/imx8mq-bl31.bin)
-
 config FIRMWARE_IMX8MQ_ATF
 	bool
 
 config FIRMWARE_IMX8MM_OPTEE
 	bool "install OP-TEE on i.MX8MM boards"
 	depends on FIRMWARE_IMX8MM_ATF && PBL_OPTEE
-	depends on $(success,test -e $(srctree)/firmware/imx8mm-bl32.bin)
 
 config FIRMWARE_IMX8MN_OPTEE
 	bool "install OP-TEE on i.MX8MN boards"
 	depends on FIRMWARE_IMX8MN_ATF && PBL_OPTEE
-	depends on $(success,test -e $(srctree)/firmware/imx8mn-bl32.bin)
 
 config FIRMWARE_IMX8MP_OPTEE
 	bool "install OP-TEE on i.MX8MP boards"
 	depends on FIRMWARE_IMX8MP_ATF && PBL_OPTEE
-	depends on $(success,test -e $(srctree)/firmware/imx8mp-bl32.bin)
 
 config FIRMWARE_CCBV2_OPTEE
 	bool
-- 
2.39.2

Re: [PATCH master v1 1/6] firmware: reference pointer alignment defined in <asm-generic/pointer.h>

[Marco Felsch]

On 23-06-26, Ahmad Fatoum wrote:
> We already define pointer size constants for use in assembly. Make use
> of this header. No functional change.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  firmware/Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index efdd5c0da541..8050d0418d33 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -41,13 +41,13 @@ obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
>  
>  FWNAME    = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
>  FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
> -ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
>  
>  filechk_fwbin = { \
>  	echo "/* Generated by $(src)/Makefile */"		;\
> +	echo "\#include <asm-generic/pointer.h>"		;\
>  	echo ".section .note.GNU-stack,\"\",%progbits"		;\
>  	echo "    .section $2,\"$3\""				;\
> -	echo "    .p2align $(ASM_ALIGN)"			;\
> +	echo "    .p2align $(ASM_LGPTR)"			;\
>  	echo ".global _fw_$(FWSTR)_start"			;\
>  	echo "_fw_$(FWSTR)_start:"				;\
>  	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
> @@ -57,7 +57,7 @@ filechk_fwbin = { \
>  
>  __fwbin_sha = { \
>  	echo "    .section .rodata.$(FWSTR).sha"		;\
> -	echo "    .p2align $(ASM_ALIGN)"			;\
> +	echo "    .p2align $(ASM_LGPTR)"			;\
>  	echo ".global _fw_$(FWSTR)_sha_start"			;\
>  	echo "_fw_$(FWSTR)_sha_start:"				;\
>  	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH master v1 2/6] firmware: turn missing firmware into linker error

[Marco Felsch]

On 23-06-26, Ahmad Fatoum wrote:
> Appending to firmware-y results in an assembly file being generated that
> defines a section embedding the firmware in question verbatim via .incbin.
> To update the assembly file as required, the target has a dependency on
> the firmware. When the firmware is missing, the build will abort there.
> 
> In preparation for deferring missing PBL firmware errors till the very
> end, let's remove the firmware dependency if the missing firmware file
> is built into PBL and replace the .incbin with a reference to an ultimately
> undefined variable.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  firmware/Makefile | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 8050d0418d33..c66d19c677e8 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -41,6 +41,7 @@ obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
>  
>  FWNAME    = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
>  FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
> +FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
>  
>  filechk_fwbin = { \
>  	echo "/* Generated by $(src)/Makefile */"		;\
> @@ -50,7 +51,11 @@ filechk_fwbin = { \
>  	echo "    .p2align $(ASM_LGPTR)"			;\
>  	echo ".global _fw_$(FWSTR)_start"			;\
>  	echo "_fw_$(FWSTR)_start:"				;\
> +	echo "\#if $(FWNAME_EXISTS)"				;\
>  	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
> +	echo "\#else"						;\
> +	echo "ASM_PTR _fwname_$(FWSTR)"				;\
> +	echo "\#endif"						;\
>  	echo ".global _fw_$(FWSTR)_end"				;\
>  	echo "_fw_$(FWSTR)_end:"				;\
>  }
> @@ -88,7 +93,7 @@ clean-files += *.sha.bin *.sum
>  $(patsubst %.gen.o,$(obj)/%.gen.o, $(obj-pbl-y)): $(obj)/%.gen.o: $(fwdir)/%
>  
>  # The same for pbl:
> -$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y)): $(obj)/%.gen.pbl.o: $(fwdir)/%
> +$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y)): $(obj)/%.gen.pbl.o: $(wildcard $(fwdir)/%)
>  $(patsubst %.gen.o,$(obj)/%.extgen.pbl.o, $(pbl-y)): $(obj)/%.extgen.pbl.o: $(fwdir)/%
>  
>  pbl-y := $(addsuffix .extgen.o, $(fw-external-y))
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH master v1 3/6] firmware: optionally turn missing firmware errors into warnings

[Marco Felsch]

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@pengutronix.de>
> ---
>  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
> 
> 
> 

Re: [PATCH master v1 5/6] ARM64: unset CONFIG_MISSING_FIRMWARE_ERROR for Rockchip/i.MX

[Marco Felsch]

On 23-06-26, Ahmad Fatoum wrote:
> barebox ARMv8 images for Rockchip and i.MX depend on external firmware
> for RAM setup (e.g. DDR PHY firmware) and on a BL31 binary to service
> secure monitor calls of the operating system (usually TF-A).
> 
> Images without this firmware are not functional and the build warns
> about it, yet for development, it's more convenient to have the build
> not fail. This is especially useful for multi_v8_defconfig, which also
> builds the drivers Qemu Virt64 image and which requires no extra
> firmware.

IMHO most user just use the defconfig as base and so they never notice
the new CONFIG_MISSING_FIRMWARE_ERROR switch. Since you have added a
great mechanism to inform the user that a particular barebox-bin is
missing firmware-files I would tend to set this config to 'n'.

Regards,
  Marco

> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/configs/imx_v8_defconfig      | 1 +
>  arch/arm/configs/multi_v8_defconfig    | 1 +
>  arch/arm/configs/rockchip_v8_defconfig | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm/configs/imx_v8_defconfig b/arch/arm/configs/imx_v8_defconfig
> index 93b85ff5ea40..99d871bfb968 100644
> --- a/arch/arm/configs/imx_v8_defconfig
> +++ b/arch/arm/configs/imx_v8_defconfig
> @@ -147,3 +147,4 @@ CONFIG_FS_FAT_WRITE=y
>  CONFIG_FS_FAT_LFN=y
>  CONFIG_FS_RATP=y
>  CONFIG_ZLIB=y
> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
> diff --git a/arch/arm/configs/multi_v8_defconfig b/arch/arm/configs/multi_v8_defconfig
> index e0ff21641a01..76b80975887d 100644
> --- a/arch/arm/configs/multi_v8_defconfig
> +++ b/arch/arm/configs/multi_v8_defconfig
> @@ -241,3 +241,4 @@ CONFIG_FS_PSTORE=y
>  CONFIG_FS_PSTORE_CONSOLE=y
>  CONFIG_FS_RATP=y
>  CONFIG_LZO_DECOMPRESS=y
> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
> diff --git a/arch/arm/configs/rockchip_v8_defconfig b/arch/arm/configs/rockchip_v8_defconfig
> index cc3c3c6d489b..7d1d41965ece 100644
> --- a/arch/arm/configs/rockchip_v8_defconfig
> +++ b/arch/arm/configs/rockchip_v8_defconfig
> @@ -149,3 +149,4 @@ CONFIG_FS_FAT_LFN=y
>  CONFIG_FS_BPKFS=y
>  CONFIG_FS_UIMAGEFS=y
>  CONFIG_LZO_DECOMPRESS=y
> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH master v1 6/6] firmware: don't hardcode firmware paths in srctree for existence check

[Marco Felsch]

On 23-06-26, Ahmad Fatoum wrote:
> Now that PBL firmware can be made optional, we can drop the existence
> checks in the Kconfig. These were flawed anyway, because they didn't
> take CONFIG_EXTRA_FIRMWARE_DIR into account and they led to issues with
> build system recipes that didn't expect firmware to be required before
> compilation step at oldconfig/menuconfig time.
> 
> This effectively reverts commits:
> 
>   624962fb45c6 ("ARM: i.MX: make boards selectable only when firmware files are present")
>   4ff2e5510ec9 ("ARM: Rockchip: make boards only selectable when firmware is present")
>   41a89611774a ("ARM: rockchip: don't attempt building MACH_RADXA_ROCK5 without firmware")
> 
> Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  arch/arm/mach-imx/Kconfig      | 49 ----------------------------------
>  arch/arm/mach-rockchip/Kconfig |  6 -----
>  firmware/Kconfig               | 39 ---------------------------
>  3 files changed, 94 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 5cd1428f9a6b..819c23bae88a 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -561,35 +561,9 @@ if 64BIT
>  
>  comment "i.MX8M boards"
>  
> -if !HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -comment "LPDDR4 firmware files missing, some boards are not selectable"
> -endif
> -
> -if !HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
> -comment "DDR4 firmware files missing, some boards are not selectable"
> -endif
> -
> -if !HAVE_FIRMWARE_IMX8MM_ATF
> -comment "i.MX8MM TF-A files missing, i.MX8MM boards are disabled"
> -endif
> -
> -if !HAVE_FIRMWARE_IMX8MQ_ATF
> -comment "i.MX8MQ TF-A files missing, i.MX8MQ boards are disabled"
> -endif
> -
> -if !HAVE_FIRMWARE_IMX8MN_ATF
> -comment "i.MX8MN TF-A files missing, i.MX8MN boards are disabled"
> -endif
> -
> -if !HAVE_FIRMWARE_IMX8MP_ATF
> -comment "i.MX8MP TF-A files missing, i.MX8MP boards are disabled"
> -endif
> -
>  config MACH_INNOCOMM_WB15
>  	bool "InnoComm WB15 (i.MX8MM) EVK"
>  	select ARCH_IMX8MM
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MM_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MM_ATF
>  	select ARM_SMCCC
> @@ -622,8 +596,6 @@ config MACH_BEO_MOZART2
>  config MACH_MNT_REFORM
>  	bool "MNT Reform"
>  	select ARCH_IMX8MQ
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MQ_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MQ_ATF
>  	select ARM_SMCCC
> @@ -633,8 +605,6 @@ config MACH_MNT_REFORM
>  config MACH_NXP_IMX8MM_EVK
>  	bool "NXP i.MX8MM EVK Board"
>  	select ARCH_IMX8MM
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MM_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MM_ATF
>  	select ARM_SMCCC
> @@ -646,9 +616,6 @@ config MACH_NXP_IMX8MM_EVK
>  config MACH_NXP_IMX8MN_EVK
>  	bool "NXP i.MX8MN EVK Board"
>  	select ARCH_IMX8MN
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MN_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX_DDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MN_ATF
> @@ -660,8 +627,6 @@ config MACH_NXP_IMX8MN_EVK
>  config MACH_NXP_IMX8MP_EVK
>  	bool "NXP i.MX8MP EVK Board"
>  	select ARCH_IMX8MP
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MP_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MP_ATF
>  	select ARM_SMCCC
> @@ -672,8 +637,6 @@ config MACH_NXP_IMX8MP_EVK
>  config MACH_NXP_IMX8MQ_EVK
>  	bool "NXP i.MX8MQ EVK Board"
>  	select ARCH_IMX8MQ
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MQ_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MQ_ATF
>  	select ARM_SMCCC
> @@ -682,8 +645,6 @@ config MACH_NXP_IMX8MQ_EVK
>  config MACH_PHYTEC_SOM_IMX8MQ
>  	bool "Phytec i.MX8M SOM"
>  	select ARCH_IMX8MQ
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MQ_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MQ_ATF
>  	select ARM_SMCCC
> @@ -692,8 +653,6 @@ config MACH_PHYTEC_SOM_IMX8MQ
>  config MACH_POLYHEX_DEBIX
>  	bool "Polyhex DEBIX Model-A/B (i.MX8MP) Board"
>  	select ARCH_IMX8MP
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MP_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MP_ATF
>  	select ARM_SMCCC
> @@ -704,8 +663,6 @@ config MACH_POLYHEX_DEBIX
>  config MACH_PROTONIC_IMX8M
>  	bool "Protonic-Holland i.MX8Mx based boards"
>  	select ARCH_IMX8MM
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MM_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MM_ATF
>  	select ARM_SMCCC
> @@ -716,8 +673,6 @@ config MACH_PROTONIC_IMX8M
>  config MACH_TQ_MBA8MPXL
>  	bool "TQ i.MX8MP Dual/Quad on MBa8MPxL Board"
>  	select ARCH_IMX8MP
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MP_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MP_ATF
>  	select ARM_SMCCC
> @@ -728,8 +683,6 @@ config MACH_TQ_MBA8MPXL
>  config MACH_VARISCITE_DT8MCUSTOMBOARD_IMX8MP
>  	bool "Variscite DT8MCustomBoard with DART-MX8M-PLUS"
>  	select ARCH_IMX8MP
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MP_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MP_ATF
>  	select ARM_SMCCC
> @@ -740,8 +693,6 @@ config MACH_VARISCITE_DT8MCUSTOMBOARD_IMX8MP
>  config MACH_ZII_IMX8MQ_DEV
>  	bool "ZII i.MX8MQ based devices"
>  	select ARCH_IMX8MQ
> -	depends on HAVE_FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> -	depends on HAVE_FIRMWARE_IMX8MQ_ATF
>  	select FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	select FIRMWARE_IMX8MQ_ATF
>  	select ARM_SMCCC
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index a71c4ae35dd9..8cdf2c28a912 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -75,42 +75,36 @@ if 64BIT
>  
>  config MACH_RK3568_EVB
>  	select ARCH_RK3568
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/rockchip-rk3568-evb/sdram-init.bin)
>  	bool "RK3568 EVB"
>  	help
>  	  Say Y here if you are using a RK3568 EVB
>  
>  config MACH_RK3568_BPI_R2PRO
>  	select ARCH_RK3568
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/rockchip-rk3568-bpi-r2pro/sdram-init.bin)
>  	bool "RK3568 BPI R2PRO"
>  	help
>  	  Say Y here if you are using a RK3568 Bananpi R2 Pro
>  
>  config MACH_PINE64_QUARTZ64
>  	select ARCH_RK3568
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/pine64-quartz64/sdram-init.bin)
>  	bool "Pine64 Quartz64"
>  	help
>  	  Say Y here if you are using a Pine64 Quartz64
>  
>  config MACH_RADXA_ROCK3
>  	select ARCH_RK3568
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-rock3/sdram-init.bin)
>  	bool "Radxa ROCK3"
>  	help
>  	  Say Y here if you are using a Radxa ROCK3
>  
>  config MACH_RADXA_ROCK5
>  	select ARCH_RK3588
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-rock5/sdram-init.bin)
>  	bool "Radxa ROCK5"
>  	help
>  	  Say Y here if you are using a Radxa ROCK5
>  
>  config MACH_RADXA_CM3
>  	select ARCH_RK3568
> -	depends on $(success,test -e $(srctree)/arch/arm/boards/radxa-cm3/sdram-init.bin)
>  	bool "Radxa CM3"
>  	help
>  	  Say Y here if you are using a Radxa CM3
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index 56d6d0d6c030..3328dbc0b111 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -20,74 +20,35 @@ config MISSING_FIRMWARE_ERROR
>  	  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
> -	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_1d_dmem.bin)
> -	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_1d_imem.bin)
> -	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_2d_dmem.bin)
> -	depends on $(success,test -e $(srctree)/firmware/lpddr4_pmu_train_2d_imem.bin)
> -
>  config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  	bool
>  
> -config HAVE_FIRMWARE_IMX_DDR4_PMU_TRAIN
> -	bool
> -	default y
> -	depends on $(success,test -e $(srctree)/firmware/ddr4_dmem_1d.bin)
> -	depends on $(success,test -e $(srctree)/firmware/ddr4_dmem_2d.bin)
> -	depends on $(success,test -e $(srctree)/firmware/ddr4_imem_1d.bin)
> -	depends on $(success,test -e $(srctree)/firmware/ddr4_imem_2d.bin)
> -
>  config FIRMWARE_IMX_DDR4_PMU_TRAIN
>  	bool
>  
> -config HAVE_FIRMWARE_IMX8MM_ATF
> -	bool
> -	default y
> -	depends on $(success,test -e $(srctree)/firmware/imx8mm-bl31.bin)
> -
>  config FIRMWARE_IMX8MM_ATF
>  	bool
>  
> -config HAVE_FIRMWARE_IMX8MN_ATF
> -	bool
> -	default y
> -	depends on $(success,test -e $(srctree)/firmware/imx8mn-bl31.bin)
> -
>  config FIRMWARE_IMX8MN_ATF
>  	bool
>  
> -config HAVE_FIRMWARE_IMX8MP_ATF
> -	bool
> -	default y
> -	depends on $(success,test -e $(srctree)/firmware/imx8mp-bl31.bin)
> -
>  config FIRMWARE_IMX8MP_ATF
>  	bool
>  
> -config HAVE_FIRMWARE_IMX8MQ_ATF
> -	bool
> -	default y
> -	depends on $(success,test -e $(srctree)/firmware/imx8mq-bl31.bin)
> -
>  config FIRMWARE_IMX8MQ_ATF
>  	bool
>  
>  config FIRMWARE_IMX8MM_OPTEE
>  	bool "install OP-TEE on i.MX8MM boards"
>  	depends on FIRMWARE_IMX8MM_ATF && PBL_OPTEE
> -	depends on $(success,test -e $(srctree)/firmware/imx8mm-bl32.bin)
>  
>  config FIRMWARE_IMX8MN_OPTEE
>  	bool "install OP-TEE on i.MX8MN boards"
>  	depends on FIRMWARE_IMX8MN_ATF && PBL_OPTEE
> -	depends on $(success,test -e $(srctree)/firmware/imx8mn-bl32.bin)
>  
>  config FIRMWARE_IMX8MP_OPTEE
>  	bool "install OP-TEE on i.MX8MP boards"
>  	depends on FIRMWARE_IMX8MP_ATF && PBL_OPTEE
> -	depends on $(success,test -e $(srctree)/firmware/imx8mp-bl32.bin)
>  
>  config FIRMWARE_CCBV2_OPTEE
>  	bool
> -- 
> 2.39.2
> 
> 
> 

Re: [PATCH master v1 3/6] firmware: optionally turn missing firmware errors into warnings

[Ahmad Fatoum]

Hello Marco,

On 03.07.23 08:25, Marco Felsch wrote:
> 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@pengutronix.de>
>> ---
>>  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.

Existing users will get the new default. Users that build in a BSP
will want y too as they may not see warnings. It doesn't help people
copying the multi defconfig and not changing it, but that's not a reason
for having existing configs to not fail when they lack firmware.

> 
>> +	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?

It doesn't matter much. Linker will garbage collect the unused section.

> 
>>  }
>>  
>>  __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.

It's possible and I don't feel strong one way or the other.

> 
>> +	$(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.

We need to alert the user somehow that the image is missing/non-functional.
It's just a warning message.

> 
> 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
>>
>>
>>
> 

-- 
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 |

Re: [PATCH master v1 5/6] ARM64: unset CONFIG_MISSING_FIRMWARE_ERROR for Rockchip/i.MX

[Ahmad Fatoum]

On 03.07.23 08:33, Marco Felsch wrote:
> On 23-06-26, Ahmad Fatoum wrote:
>> barebox ARMv8 images for Rockchip and i.MX depend on external firmware
>> for RAM setup (e.g. DDR PHY firmware) and on a BL31 binary to service
>> secure monitor calls of the operating system (usually TF-A).
>>
>> Images without this firmware are not functional and the build warns
>> about it, yet for development, it's more convenient to have the build
>> not fail. This is especially useful for multi_v8_defconfig, which also
>> builds the drivers Qemu Virt64 image and which requires no extra
>> firmware.
> 
> IMHO most user just use the defconfig as base and so they never notice
> the new CONFIG_MISSING_FIRMWARE_ERROR switch. Since you have added a
> great mechanism to inform the user that a particular barebox-bin is
> missing firmware-files I would tend to set this config to 'n'.

I can't follow the reasoning. Some people will not benefit from the
default, so just remove the default?

> 
> Regards,
>   Marco
> 
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  arch/arm/configs/imx_v8_defconfig      | 1 +
>>  arch/arm/configs/multi_v8_defconfig    | 1 +
>>  arch/arm/configs/rockchip_v8_defconfig | 1 +
>>  3 files changed, 3 insertions(+)
>>
>> diff --git a/arch/arm/configs/imx_v8_defconfig b/arch/arm/configs/imx_v8_defconfig
>> index 93b85ff5ea40..99d871bfb968 100644
>> --- a/arch/arm/configs/imx_v8_defconfig
>> +++ b/arch/arm/configs/imx_v8_defconfig
>> @@ -147,3 +147,4 @@ CONFIG_FS_FAT_WRITE=y
>>  CONFIG_FS_FAT_LFN=y
>>  CONFIG_FS_RATP=y
>>  CONFIG_ZLIB=y
>> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
>> diff --git a/arch/arm/configs/multi_v8_defconfig b/arch/arm/configs/multi_v8_defconfig
>> index e0ff21641a01..76b80975887d 100644
>> --- a/arch/arm/configs/multi_v8_defconfig
>> +++ b/arch/arm/configs/multi_v8_defconfig
>> @@ -241,3 +241,4 @@ CONFIG_FS_PSTORE=y
>>  CONFIG_FS_PSTORE_CONSOLE=y
>>  CONFIG_FS_RATP=y
>>  CONFIG_LZO_DECOMPRESS=y
>> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
>> diff --git a/arch/arm/configs/rockchip_v8_defconfig b/arch/arm/configs/rockchip_v8_defconfig
>> index cc3c3c6d489b..7d1d41965ece 100644
>> --- a/arch/arm/configs/rockchip_v8_defconfig
>> +++ b/arch/arm/configs/rockchip_v8_defconfig
>> @@ -149,3 +149,4 @@ CONFIG_FS_FAT_LFN=y
>>  CONFIG_FS_BPKFS=y
>>  CONFIG_FS_UIMAGEFS=y
>>  CONFIG_LZO_DECOMPRESS=y
>> +# CONFIG_MISSING_FIRMWARE_ERROR is not set
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
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 |