[PATCH v2 2/2] firmware_loader: Enable compressed files with EXTRA_FIRMWARE

Kevin Martin posted 2 patches 1 year, 11 months ago
[PATCH v2 2/2] firmware_loader: Enable compressed files with EXTRA_FIRMWARE
Posted by Kevin Martin 1 year, 11 months ago
The linux-firmware packages on Gentoo, Fedora, Arch, and others
compress the firmware files. This works well with
CONFIG_FW_LOADER_COMPRESS but does not work with CONFIG_EXTRA_FIRMWARE.
This patch allows the build system to decompress firmware files
specified by CONFIG_EXTRA_FIRMWARE. Uncompressed files are used first,
then the compressed files are used.

The patch works by copying or decompressing the specified firmware files
into the build directory, then compiling them in from there. I would
prefer to not copy any uncompressed files, but I have not found a clean
way to do that.

Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
---
Changes in v2:
- Changed .gitignore to ignore all firmware files copied into the
directory.
- Fixed a tab.

 drivers/base/firmware_loader/Kconfig            |  5 ++++-
 drivers/base/firmware_loader/builtin/.gitignore |  5 ++++-
 drivers/base/firmware_loader/builtin/Makefile   | 16 ++++++++++++----
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 5ca00e02f..b7a908bff 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -76,7 +76,10 @@ config EXTRA_FIRMWARE
 	  image since it combines both GPL and non-GPL work. You should
 	  consult a lawyer of your own before distributing such an image.
 
-	  NOTE: Compressed files are not supported in EXTRA_FIRMWARE.
+	  NOTE: Compressed files are supported by EXTRA_FIRMWARE. The build
+	  system will look for uncompressed files first then fall back to
+	  searching for compressed files in a similar way to
+	  CONFIG_FW_LOADER_COMPRESS.
 
 config EXTRA_FIRMWARE_DIR
 	string "Firmware blobs root directory"
diff --git a/drivers/base/firmware_loader/builtin/.gitignore b/drivers/base/firmware_loader/builtin/.gitignore
index 166f76b43..b3124ee78 100644
--- a/drivers/base/firmware_loader/builtin/.gitignore
+++ b/drivers/base/firmware_loader/builtin/.gitignore
@@ -1,2 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-*.gen.S
+*
+!.gitignore
+!Makefile
+!main.c
diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 6c067dedc..cc60eb441 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -20,7 +20,7 @@ filechk_fwbin = \
 	echo "    .section .rodata"				;\
 	echo "    .p2align 4"					;\
 	echo "_fw_$(FWSTR)_bin:"				;\
-	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
+	echo "    .incbin \"$(obj)/$(FWNAME)\""			;\
 	echo "_fw_end:"						;\
 	echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"	;\
 	echo "    .p2align $(ASM_ALIGN)"			;\
@@ -36,7 +36,15 @@ $(obj)/%.gen.S: FORCE
 	$(call filechk,fwbin)
 
 # The .o files depend on the binaries directly; the .S files don't.
-$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
+$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(obj)/%
 
-targets := $(patsubst $(obj)/%,%, \
-                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
+$(obj)/% : $(fwdir)/% FORCE
+	$(call if_changed,copy)
+
+$(obj)/% : $(fwdir)/%.xz FORCE
+	$(call if_changed,xzdec)
+
+$(obj)/% : $(fwdir)/%.zst FORCE
+	$(call if_changed,zstddec)
+
+targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)
-- 
2.41.0
Re: [PATCH v2 2/2] firmware_loader: Enable compressed files with EXTRA_FIRMWARE
Posted by Masahiro Yamada 1 year, 11 months ago
On Fri, Jan 5, 2024 at 3:11 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
>
> The linux-firmware packages on Gentoo, Fedora, Arch, and others
> compress the firmware files. This works well with
> CONFIG_FW_LOADER_COMPRESS but does not work with CONFIG_EXTRA_FIRMWARE.
> This patch allows the build system to decompress firmware files
> specified by CONFIG_EXTRA_FIRMWARE. Uncompressed files are used first,
> then the compressed files are used.
>
> The patch works by copying or decompressing the specified firmware files
> into the build directory, then compiling them in from there. I would
> prefer to not copy any uncompressed files, but I have not found a clean
> way to do that.
>
> Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> ---
> Changes in v2:
> - Changed .gitignore to ignore all firmware files copied into the
> directory.
> - Fixed a tab.
>
>  drivers/base/firmware_loader/Kconfig            |  5 ++++-
>  drivers/base/firmware_loader/builtin/.gitignore |  5 ++++-
>  drivers/base/firmware_loader/builtin/Makefile   | 16 ++++++++++++----
>  3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 5ca00e02f..b7a908bff 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -76,7 +76,10 @@ config EXTRA_FIRMWARE
>           image since it combines both GPL and non-GPL work. You should
>           consult a lawyer of your own before distributing such an image.
>
> -         NOTE: Compressed files are not supported in EXTRA_FIRMWARE.
> +         NOTE: Compressed files are supported by EXTRA_FIRMWARE. The build
> +         system will look for uncompressed files first then fall back to
> +         searching for compressed files in a similar way to
> +         CONFIG_FW_LOADER_COMPRESS.
>
>  config EXTRA_FIRMWARE_DIR
>         string "Firmware blobs root directory"
> diff --git a/drivers/base/firmware_loader/builtin/.gitignore b/drivers/base/firmware_loader/builtin/.gitignore
> index 166f76b43..b3124ee78 100644
> --- a/drivers/base/firmware_loader/builtin/.gitignore
> +++ b/drivers/base/firmware_loader/builtin/.gitignore
> @@ -1,2 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -*.gen.S
> +*
> +!.gitignore
> +!Makefile
> +!main.c
> diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
> index 6c067dedc..cc60eb441 100644
> --- a/drivers/base/firmware_loader/builtin/Makefile
> +++ b/drivers/base/firmware_loader/builtin/Makefile
> @@ -20,7 +20,7 @@ filechk_fwbin = \
>         echo "    .section .rodata"                             ;\
>         echo "    .p2align 4"                                   ;\
>         echo "_fw_$(FWSTR)_bin:"                                ;\
> -       echo "    .incbin \"$(fwdir)/$(FWNAME)\""               ;\
> +       echo "    .incbin \"$(obj)/$(FWNAME)\""                 ;\
>         echo "_fw_end:"                                         ;\
>         echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"   ;\
>         echo "    .p2align $(ASM_ALIGN)"                        ;\
> @@ -36,7 +36,15 @@ $(obj)/%.gen.S: FORCE
>         $(call filechk,fwbin)
>
>  # The .o files depend on the binaries directly; the .S files don't.
> -$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
> +$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(obj)/%
>
> -targets := $(patsubst $(obj)/%,%, \
> -                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
> +$(obj)/% : $(fwdir)/% FORCE
> +       $(call if_changed,copy)
> +
> +$(obj)/% : $(fwdir)/%.xz FORCE
> +       $(call if_changed,xzdec)
> +
> +$(obj)/% : $(fwdir)/%.zst FORCE
> +       $(call if_changed,zstddec)
> +
> +targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)


I noticed that "make clean" leaves copied firmware files
in drivers/base/firmware_loader/builtin/.


You need to clean up all files in
drivers/base/firmware_loader/builtin/
except Makefile, main.c.

The following worked for me.


diff --git a/drivers/base/firmware_loader/builtin/Makefile
b/drivers/base/firmware_loader/builtin/Makefile
index bcac1723dc32..4d62ee9f06f6 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -48,3 +48,5 @@ $(obj)/% : $(fwdir)/%.zst FORCE
        $(call if_changed,zstddec)

 targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)
+
+clean-files := $(filter-out Makefile main.c, $(patsubst $(obj)/%,%,
$(wildcard $(obj)/*)))













-- 
Best Regards
Masahiro Yamada
Re: [PATCH v2 2/2] firmware_loader: Enable compressed files with EXTRA_FIRMWARE
Posted by Kevin Martin 1 year, 11 months ago
On Sun, 7 Jan 2024, Masahiro Yamada wrote:

> On Fri, Jan 5, 2024 at 3:11 PM Kevin Martin <kevinmbecause@gmail.com> wrote:
> >
> > The linux-firmware packages on Gentoo, Fedora, Arch, and others
> > compress the firmware files. This works well with
> > CONFIG_FW_LOADER_COMPRESS but does not work with CONFIG_EXTRA_FIRMWARE.
> > This patch allows the build system to decompress firmware files
> > specified by CONFIG_EXTRA_FIRMWARE. Uncompressed files are used first,
> > then the compressed files are used.
> >
> > The patch works by copying or decompressing the specified firmware files
> > into the build directory, then compiling them in from there. I would
> > prefer to not copy any uncompressed files, but I have not found a clean
> > way to do that.
> >
> > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com>
> > ---
> > Changes in v2:
> > - Changed .gitignore to ignore all firmware files copied into the
> > directory.
> > - Fixed a tab.
> >
> >  drivers/base/firmware_loader/Kconfig            |  5 ++++-
> >  drivers/base/firmware_loader/builtin/.gitignore |  5 ++++-
> >  drivers/base/firmware_loader/builtin/Makefile   | 16 ++++++++++++----
> >  3 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> > index 5ca00e02f..b7a908bff 100644
> > --- a/drivers/base/firmware_loader/Kconfig
> > +++ b/drivers/base/firmware_loader/Kconfig
> > @@ -76,7 +76,10 @@ config EXTRA_FIRMWARE
> >           image since it combines both GPL and non-GPL work. You should
> >           consult a lawyer of your own before distributing such an image.
> >
> > -         NOTE: Compressed files are not supported in EXTRA_FIRMWARE.
> > +         NOTE: Compressed files are supported by EXTRA_FIRMWARE. The build
> > +         system will look for uncompressed files first then fall back to
> > +         searching for compressed files in a similar way to
> > +         CONFIG_FW_LOADER_COMPRESS.
> >
> >  config EXTRA_FIRMWARE_DIR
> >         string "Firmware blobs root directory"
> > diff --git a/drivers/base/firmware_loader/builtin/.gitignore b/drivers/base/firmware_loader/builtin/.gitignore
> > index 166f76b43..b3124ee78 100644
> > --- a/drivers/base/firmware_loader/builtin/.gitignore
> > +++ b/drivers/base/firmware_loader/builtin/.gitignore
> > @@ -1,2 +1,5 @@
> >  # SPDX-License-Identifier: GPL-2.0
> > -*.gen.S
> > +*
> > +!.gitignore
> > +!Makefile
> > +!main.c
> > diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
> > index 6c067dedc..cc60eb441 100644
> > --- a/drivers/base/firmware_loader/builtin/Makefile
> > +++ b/drivers/base/firmware_loader/builtin/Makefile
> > @@ -20,7 +20,7 @@ filechk_fwbin = \
> >         echo "    .section .rodata"                             ;\
> >         echo "    .p2align 4"                                   ;\
> >         echo "_fw_$(FWSTR)_bin:"                                ;\
> > -       echo "    .incbin \"$(fwdir)/$(FWNAME)\""               ;\
> > +       echo "    .incbin \"$(obj)/$(FWNAME)\""                 ;\
> >         echo "_fw_end:"                                         ;\
> >         echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"   ;\
> >         echo "    .p2align $(ASM_ALIGN)"                        ;\
> > @@ -36,7 +36,15 @@ $(obj)/%.gen.S: FORCE
> >         $(call filechk,fwbin)
> >
> >  # The .o files depend on the binaries directly; the .S files don't.
> > -$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
> > +$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(obj)/%
> >
> > -targets := $(patsubst $(obj)/%,%, \
> > -                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
> > +$(obj)/% : $(fwdir)/% FORCE
> > +       $(call if_changed,copy)
> > +
> > +$(obj)/% : $(fwdir)/%.xz FORCE
> > +       $(call if_changed,xzdec)
> > +
> > +$(obj)/% : $(fwdir)/%.zst FORCE
> > +       $(call if_changed,zstddec)
> > +
> > +targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)
> 
> 
> I noticed that "make clean" leaves copied firmware files
> in drivers/base/firmware_loader/builtin/.
> 
> 
> You need to clean up all files in
> drivers/base/firmware_loader/builtin/
> except Makefile, main.c.
> 
> The following worked for me.
> 
> 
> diff --git a/drivers/base/firmware_loader/builtin/Makefile
> b/drivers/base/firmware_loader/builtin/Makefile
> index bcac1723dc32..4d62ee9f06f6 100644
> --- a/drivers/base/firmware_loader/builtin/Makefile
> +++ b/drivers/base/firmware_loader/builtin/Makefile
> @@ -48,3 +48,5 @@ $(obj)/% : $(fwdir)/%.zst FORCE
>         $(call if_changed,zstddec)
> 
>  targets := $(patsubst %.gen.o, %.gen.S, $(firmware)) $(CONFIG_EXTRA_FIRMWARE)
> +
> +clean-files := $(filter-out Makefile main.c, $(patsubst $(obj)/%,%,
> $(wildcard $(obj)/*)))
> 

This explains why the "shell find" command was used. I was attempting to 
generate the list from $(CONFIG_EXTRA_FIRMWARE), but that is not defined 
during "make clean" as I am just now learning. While I would prefer to use 
a whitelist instead of a blacklist, I do not know a way to accomplish 
that. If everyone is ok with wiping out all files other than Makefile and 
main.c, then I will add the wildcard to "targets" and submit a new 
patch revision.

> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
>