[PATCH 1/3] firmware: support optional firmware in barebox proper

[PATCH 1/3] firmware: support optional firmware in barebox proper

[Ahmad Fatoum]

For firmware used in the prebootloader, missing firmware is handled
according to the CONFIG_MISSING_FIRMWARE_ERROR symbol:

  - If set, barebox will throw an error when assmebling the final image
    with a list of missing firmware

  - If unset, a warning will be printed for each barebox image that
    can't be built due to missing firmware and all

This replaced the previous behavior of make throwing an error due to a
missing dependency, which broke the ability to use a single config for
multiple platforms, where only some have missing firmware.

Nothing of the sort was done for firmware in barebox proper. We only
have that on Layerscape for PPA (EL3 Secure Monitor) and FMan (NIC).

With the addition of Layerscape to multi_v8_defconfig, building that
config now also throws aforementioned make errors.

This is now also resolved depending on CONFIG_MISSING_FIRMWARE_ERROR:

  - If set, an immediate make error occurs as before as we can't
    pinpoint which of the enabled barebox images will at runtime
    instantiate the driver that requires the firmware.

  - If unset, we continue build as normal and the size of the firmware
    as determined at runtime will be zero bytes. Code in barebox
    proper is expected to check for that and report an error.

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

diff --git a/firmware/Makefile b/firmware/Makefile
index 83ce77f510ba..d5e4cee8594b 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -61,7 +61,7 @@ filechk_fwbin = { \
 	echo "_fw_$(FWSTR)_start:"				;\
 	echo "\#if $(FWNAME_EXISTS)"				;\
 	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
-	echo "\#else"						;\
+	echo "\#elif defined(__PBL__)"				;\
 	echo "ASM_PTR _fwname_$(FWSTR)"				;\
 	echo "\#endif"						;\
 	echo ".global _fw_$(FWSTR)_end"				;\
@@ -102,13 +102,25 @@ $(obj)/%.sum: FORCE
 
 clean-files += *.sha.bin *.sum
 
-# The .o files depend on the binaries directly; the .S files don't.
-$(patsubst %.gen.o,$(obj)/%.gen.o, $(obj-pbl-y)): $(obj)/%.gen.o: $(fwdir)/%
+# This dependency is used if missing firmware should fail the build immediately
+fwdep-required-y = $(fwdir)/%
+# This dependency expands to nothing if the file doesn't exist. This allows
+# delaying the firmware check:
+#
+#   - to final assembly of the PBL image for pbl-firmware
+#   - to runtime for firmware in barebox proper
+#
+# This way, we allow users to build defconfigs with multiple images without requiring
+# them to install all firmware for all platforms if only few are of interest.
+fwdep-required-n = $$(wildcard $(fwdir)/%)
 
-# The same for pbl:
 .SECONDEXPANSION:
-$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y) $(pbl-y)): $(obj)/%.gen.pbl.o: $$(wildcard $(fwdir)/%)
-$(patsubst %.extgen.o,$(obj)/%.extgen.pbl.o, $(pbl-fwext-y)): $(obj)/%.extgen.pbl.o: $$(wildcard $(fwdir)/%)
+# The .o files depend on the binaries directly if available; the .S files don't.
+$(patsubst %.gen.o,$(obj)/%.gen.pbl.o, $(obj-pbl-y) $(pbl-y)): $(obj)/%.gen.pbl.o: $(fwdep-required-n)
+$(patsubst %.extgen.o,$(obj)/%.extgen.pbl.o, $(pbl-fwext-y)): $(obj)/%.extgen.pbl.o: $(fwdep-required-n)
+# For barebox proper, firmware existance is either checked here
+# or in driver code by checking whether size != 0
+$(patsubst %.gen.o,$(obj)/%.gen.o, $(obj-pbl-y)): $(obj)/%.gen.o: $(fwdep-required-$(CONFIG_MISSING_FIRMWARE_ERROR))
 
 pbl-y += $(pbl-fwext-y)
 
-- 
2.39.2

[PATCH 2/3] ARM: layerscape: add helpful runtime warning when firmware is missing

[Ahmad Fatoum]

Firmware compiled into barebox PBL is rarely optional, so even if
CONFIG_MISSING_FIRMWARE_ERROR is disabled, barebox will not generate
images with missing PBL firmware.

We can relax this for firmware compiled into barebox proper though: If
the user disables CONFIG_MISSING_FIRMWARE_ERROR, just have the driver
fail at runtime as not to break other boards in the same build that
don't require the firmware.

Missing barebox proper firmware will be detectable as a 0-byte firmware
blob in the follow-up commit, so prepare for that by printing an
informative error message.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-layerscape/ppa.c | 4 ++++
 drivers/net/fsl-fman.c         | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
index 21efaae3ab32..99b76b4c30d7 100644
--- a/arch/arm/mach-layerscape/ppa.c
+++ b/arch/arm/mach-layerscape/ppa.c
@@ -121,6 +121,10 @@ int ls1046a_ppa_init(resource_size_t ppa_start, resource_size_t ppa_size)
 	}
 
 	get_builtin_firmware(ppa_ls1046a_bin, &ppa_fw, &ppa_fw_size);
+	if (!ppa_fw_size) {
+		pr_err("PPA Firmware was not included in build\n");
+		return -ENOSYS;
+	}
 
 	if (el == 3) {
 		unsigned long cr;
diff --git a/drivers/net/fsl-fman.c b/drivers/net/fsl-fman.c
index 5528ecccc950..98af3dafb44a 100644
--- a/drivers/net/fsl-fman.c
+++ b/drivers/net/fsl-fman.c
@@ -213,6 +213,10 @@ static int fman_upload_firmware(struct device *dev, struct fm_imem *fm_imem)
 	const struct qe_firmware *firmware;
 
 	get_builtin_firmware(fsl_fman_ucode_ls1046_r1_0_106_4_18_bin, &firmware, &size);
+	if (!size) {
+		dev_err(dev, "FMan Firmware was not included in build\n");
+		return -ENOSYS;
+	}
 
 	ret = qe_validate_firmware(firmware, size);
 	if (ret)
-- 
2.39.2

Re: [PATCH 2/3] ARM: layerscape: add helpful runtime warning when firmware is missing

[Sascha Hauer]

On Fri, May 03, 2024 at 12:32:29PM +0200, Ahmad Fatoum wrote:
> Firmware compiled into barebox PBL is rarely optional, so even if
> CONFIG_MISSING_FIRMWARE_ERROR is disabled, barebox will not generate
> images with missing PBL firmware.
> 
> We can relax this for firmware compiled into barebox proper though: If
> the user disables CONFIG_MISSING_FIRMWARE_ERROR, just have the driver
> fail at runtime as not to break other boards in the same build that
> don't require the firmware.
> 
> Missing barebox proper firmware will be detectable as a 0-byte firmware
> blob in the follow-up commit, so prepare for that by printing an
> informative error message.

I changed the order of the first two patches to what this sentence
suggests.

Sascha

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

[PATCH 3/3] ci: test: remove generation of dummy firmware

[Ahmad Fatoum]

Generating dummy firmware for images that are not only built, but also
runtime tested doesn't make sense. Either the firmware is required for
proper operation (which isn't the case for any platform we currently
test) or it isn't, in which case enabling it could hide errors during
build.

The added benefit here is that we now test e.g. multi_v8_defconfig the
same way that a user with a new checkout would: Using the defconfig and
without any generated firmware.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .github/workflows/test-labgrid-pytest.yml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/.github/workflows/test-labgrid-pytest.yml b/.github/workflows/test-labgrid-pytest.yml
index 514122ebf886..6eb38cc03e6b 100644
--- a/.github/workflows/test-labgrid-pytest.yml
+++ b/.github/workflows/test-labgrid-pytest.yml
@@ -55,8 +55,6 @@ jobs:
       run: |
         export ARCH=${{matrix.arch}}
 
-        ./test/generate-dummy-fw.sh
-
         ./MAKEALL -O build-${{matrix.arch}} -k test/kconfig/enable_self_test.kconf \
                 -k test/kconfig/disable_target_tools.kconf ${{matrix.defconfig}}
 
-- 
2.39.2

Re: [PATCH 1/3] firmware: support optional firmware in barebox proper

[Sascha Hauer]

On Fri, 03 May 2024 12:32:28 +0200, Ahmad Fatoum wrote:
> For firmware used in the prebootloader, missing firmware is handled
> according to the CONFIG_MISSING_FIRMWARE_ERROR symbol:
> 
>   - If set, barebox will throw an error when assmebling the final image
>     with a list of missing firmware
> 
>   - If unset, a warning will be printed for each barebox image that
>     can't be built due to missing firmware and all
> 
> [...]

Applied, thanks!

[1/3] firmware: support optional firmware in barebox proper
      https://git.pengutronix.de/cgit/barebox/commit/?id=bd29a3aa7e2a (link may not be stable)
[2/3] ARM: layerscape: add helpful runtime warning when firmware is missing
      https://git.pengutronix.de/cgit/barebox/commit/?id=9e436059695a (link may not be stable)
[3/3] ci: test: remove generation of dummy firmware
      https://git.pengutronix.de/cgit/barebox/commit/?id=8dc8635d90cb (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>