linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: gregkh@linuxfoundation.org
Cc: bp@suse.de, akpm@linux-foundation.org, josh@joshtriplett.org,
	rishabhb@codeaurora.org, kubakici@wp.pl, maco@android.com,
	david.brown@linaro.org, bjorn.andersson@linaro.org,
	linux-wireless@vger.kernel.org, keescook@chromium.org,
	shuah@kernel.org, mfuzzey@parkeon.com, zohar@linux.vnet.ibm.com,
	dhowells@redhat.com, pali.rohar@gmail.com, tiwai@suse.de,
	arend.vanspriel@broadcom.com, zajec5@gmail.com, nbroeking@me.com,
	broonie@kernel.org, dmitry.torokhov@gmail.com,
	dwmw2@infradead.org, torvalds@linux-foundation.org,
	Abhay_Salunke@dell.com, jewalt@lgsinnovations.com,
	cantabile.desu@gmail.com, ast@fb.com, andresx7@gmail.com,
	brendanhiggins@google.com, yzaikin@google.com,
	sfr@canb.auug.org.au, rdunlap@infradead.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Luis Chamberlain <mcgrof@kernel.org>
Subject: [PATCH v4 2/4] firmware_loader: move builtin build helper to shared library
Date: Mon, 25 Oct 2021 12:50:29 -0700	[thread overview]
Message-ID: <20211025195031.4169165-3-mcgrof@kernel.org> (raw)
In-Reply-To: <20211025195031.4169165-1-mcgrof@kernel.org>

If we wanted to use a different directory for building target
builtin firmware it is easier if we just have a shared library
Makefile, and each target directory can then just include it and
populate the respective needed variables. This reduces clutter,
makes things easier to read, and also most importantly allows
us to not have to try to magically adjust only one target
kconfig symbol for built-in firmware files. Trying to do this
can easily end up causing odd build issues if the user is not
careful.

As an example issue, if we are going to try to extend the
FW_LOADER_BUILTIN_FILES list and FW_LOADER_BUILTIN_DIR in case
of a new test firmware builtin support currently our only option
would be modify the defaults of each of these in case test firmware
builtin support was enabled. Defaults however won't augment a prior
setting, and so if FW_LOADER_BUILTIN_DIR="/lib/firmware" and you
and want this to be changed to something like
FW_LOADER_BUILTIN_DIR="drivers/base/firmware_loader/test-builtin"
the change will not take effect as a prior build already had it
set, and the build would fail. Trying to augment / append the
variables in the Makefile just makes this very difficult to
read.

Using a library let's us split up possible built-in targets so
that the user does not have to be involved. This will be used
in a subsequent patch which will add another user to this
built-in firmware library Makefile and demo how to use it outside
of the default FW_LOADER_BUILTIN_DIR and FW_LOADER_BUILTIN_FILES.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/builtin/Makefile | 34 ++-----------------
 .../base/firmware_loader/builtin/lib.Makefile | 32 +++++++++++++++++
 2 files changed, 34 insertions(+), 32 deletions(-)
 create mode 100644 drivers/base/firmware_loader/builtin/lib.Makefile

diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 7cdd0b5c7384..9b0dc193f6c7 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
+include $(srctree)/drivers/base/firmware_loader/builtin/lib.Makefile
+
 obj-y  += main.o
 
 # Create $(fwdir) from $(CONFIG_FW_LOADER_BUILTIN_DIR) -- if it doesn't have a
@@ -8,35 +10,3 @@ fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
 
 firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_FW_LOADER_BUILTIN_FILES)))
 obj-y += $(firmware)
-
-FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
-FWSTR     = $(subst $(comma),_,$(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))))
-ASM_WORD  = $(if $(CONFIG_64BIT),.quad,.long)
-ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
-PROGBITS  = $(if $(CONFIG_ARM),%,@)progbits
-
-filechk_fwbin = \
-	echo "/* Generated by $(src)/Makefile */"		;\
-	echo "    .section .rodata"				;\
-	echo "    .p2align 4"					;\
-	echo "_fw_$(FWSTR)_bin:"				;\
-	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
-	echo "_fw_end:"						;\
-	echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"	;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
-	echo "_fw_$(FWSTR)_name:"				;\
-	echo "    .string \"$(FWNAME)\""			;\
-	echo "    .section .builtin_fw,\"a\",$(PROGBITS)"	;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
-	echo "    $(ASM_WORD) _fw_$(FWSTR)_name"		;\
-	echo "    $(ASM_WORD) _fw_$(FWSTR)_bin"			;\
-	echo "    $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin"
-
-$(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)/%
-
-targets := $(patsubst $(obj)/%,%, \
-                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
diff --git a/drivers/base/firmware_loader/builtin/lib.Makefile b/drivers/base/firmware_loader/builtin/lib.Makefile
new file mode 100644
index 000000000000..e979a67acfa7
--- /dev/null
+++ b/drivers/base/firmware_loader/builtin/lib.Makefile
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: GPL-2.0
+FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
+FWSTR     = $(subst $(comma),_,$(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))))
+ASM_WORD  = $(if $(CONFIG_64BIT),.quad,.long)
+ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
+PROGBITS  = $(if $(CONFIG_ARM),%,@)progbits
+
+filechk_fwbin = \
+	echo "/* Generated by $(src)/Makefile */"		;\
+	echo "    .section .rodata"				;\
+	echo "    .p2align 4"					;\
+	echo "_fw_$(FWSTR)_bin:"				;\
+	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
+	echo "_fw_end:"						;\
+	echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"	;\
+	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "_fw_$(FWSTR)_name:"				;\
+	echo "    .string \"$(FWNAME)\""			;\
+	echo "    .section .builtin_fw,\"a\",$(PROGBITS)"	;\
+	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "    $(ASM_WORD) _fw_$(FWSTR)_name"		;\
+	echo "    $(ASM_WORD) _fw_$(FWSTR)_bin"			;\
+	echo "    $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin"
+
+$(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)/%
+
+targets := $(patsubst $(obj)/%,%, \
+                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
-- 
2.30.2


  parent reply	other threads:[~2021-10-25 20:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-25 19:50 [PATCH v4 0/4] firmware_loader: built-in API and make x86 use it Luis Chamberlain
2021-10-25 19:50 ` [PATCH v4 1/4] firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR Luis Chamberlain
2021-10-25 19:50 ` Luis Chamberlain [this message]
2021-10-25 19:50 ` [PATCH v4 3/4] test_firmware: move a few test knobs out to its library Luis Chamberlain
2021-10-25 19:50 ` [PATCH v4 4/4] test_firmware: add support for testing built-in firmware Luis Chamberlain
2021-11-17  0:15 ` [PATCH v4 0/4] firmware_loader: built-in API and make x86 use it Luis Chamberlain
2021-11-26 15:54   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211025195031.4169165-3-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=Abhay_Salunke@dell.com \
    --cc=akpm@linux-foundation.org \
    --cc=andresx7@gmail.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=ast@fb.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=bp@suse.de \
    --cc=brendanhiggins@google.com \
    --cc=broonie@kernel.org \
    --cc=cantabile.desu@gmail.com \
    --cc=david.brown@linaro.org \
    --cc=dhowells@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jewalt@lgsinnovations.com \
    --cc=josh@joshtriplett.org \
    --cc=keescook@chromium.org \
    --cc=kubakici@wp.pl \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=maco@android.com \
    --cc=mfuzzey@parkeon.com \
    --cc=nbroeking@me.com \
    --cc=pali.rohar@gmail.com \
    --cc=rdunlap@infradead.org \
    --cc=rishabhb@codeaurora.org \
    --cc=sfr@canb.auug.org.au \
    --cc=shuah@kernel.org \
    --cc=tiwai@suse.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yzaikin@google.com \
    --cc=zajec5@gmail.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).