* [PATCH] firmware: fix missing firmware handling for external firmware
@ 2024-01-10 14:59 Marco Felsch
2024-01-16 14:44 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Marco Felsch @ 2024-01-10 14:59 UTC (permalink / raw)
To: barebox
Currently we depend on the external firmware file since we need it for
the sha256sum. But this dependency break the 'optional missing firmware'
feature.
Fix this by dropping the firmware file prerequisite and instead evaluate
the FWNAME_EXISTS variable. If the firmware file does not exist a dummy
file is created and the build continues till the linking process (like
we do for the other firmware).
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
firmware/Makefile | 6 +++---
images/Makefile | 2 +-
scripts/Makefile.lib | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/firmware/Makefile b/firmware/Makefile
index 51d98d54bf14..a07b31caec73 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -41,7 +41,7 @@ fwobjdir := $(objtree)/firmware
pbl-y := $(addsuffix .gen.o, $(pbl-firmware-y))
obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
-FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
+FWNAME = $(patsubst $(obj)/%.sum,%,$(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)))
FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
@@ -91,8 +91,8 @@ $(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
$(obj)/%.sha.bin: $(obj)/%.sum FORCE
$(call if_changed,sha256bin)
-$(obj)/%.sum: $(obj)/% FORCE
- $(call if_changed,sha256sum)
+$(obj)/%.sum: FORCE
+ $(if $(findstring 1,$(FWNAME_EXISTS)), $(call if_changed,sha256sum,$(obj)/$*), @touch $@)
clean-files += *.sha.bin *.sum
diff --git a/images/Makefile b/images/Makefile
index 7b4e01953185..c0105609eec2 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -132,7 +132,7 @@ $(obj)/barebox.sha.bin: $(obj)/barebox.sum FORCE
$(call if_changed,sha256bin)
$(obj)/barebox.sum: $(obj)/barebox.z FORCE
- $(call if_changed,sha256sum)
+ $(call if_changed,sha256sum,$<)
# barebox.z - compressed barebox binary
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0b236babb275..f205e08afc13 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -301,7 +301,7 @@ quiet_cmd_sha256bin ?= SHA-BIN $@
while read -r byte; do printf '\%o' $$byte; done)" > $@
quiet_cmd_sha256sum ?= SHA $@
- cmd_sha256sum ?= sha256sum $< > $@
+ cmd_sha256sum ?= sha256sum $2 > $@
# Decompressor for barebox proper binary when using PBL
# ---------------------------------------------------------------------------
--
2.39.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: fix missing firmware handling for external firmware
2024-01-10 14:59 [PATCH] firmware: fix missing firmware handling for external firmware Marco Felsch
@ 2024-01-16 14:44 ` Sascha Hauer
2024-01-17 9:50 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2024-01-16 14:44 UTC (permalink / raw)
To: Marco Felsch; +Cc: barebox
Hi Marco,
On Wed, Jan 10, 2024 at 03:59:08PM +0100, Marco Felsch wrote:
> Currently we depend on the external firmware file since we need it for
> the sha256sum. But this dependency break the 'optional missing firmware'
> feature.
>
> Fix this by dropping the firmware file prerequisite and instead evaluate
> the FWNAME_EXISTS variable. If the firmware file does not exist a dummy
> file is created and the build continues till the linking process (like
> we do for the other firmware).
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> firmware/Makefile | 6 +++---
> images/Makefile | 2 +-
> scripts/Makefile.lib | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 51d98d54bf14..a07b31caec73 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -41,7 +41,7 @@ fwobjdir := $(objtree)/firmware
> pbl-y := $(addsuffix .gen.o, $(pbl-firmware-y))
> obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
>
> -FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
> +FWNAME = $(patsubst $(obj)/%.sum,%,$(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)))
The purpose of this change is not clear to me. You replace the pattern
%.sum with %, but I don't see how this ever matches.
Dropping this change has no visible effect for me.
> FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
> FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
>
> @@ -91,8 +91,8 @@ $(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
> $(obj)/%.sha.bin: $(obj)/%.sum FORCE
> $(call if_changed,sha256bin)
>
> -$(obj)/%.sum: $(obj)/% FORCE
> - $(call if_changed,sha256sum)
> +$(obj)/%.sum: FORCE
> + $(if $(findstring 1,$(FWNAME_EXISTS)), $(call if_changed,sha256sum,$(obj)/$*), @touch $@)
^^^
$(obj) should be $(fwdir) as this is the directory where the firmware
files are.
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 |
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] firmware: fix missing firmware handling for external firmware
2024-01-16 14:44 ` Sascha Hauer
@ 2024-01-17 9:50 ` Sascha Hauer
2024-01-17 9:58 ` Marco Felsch
0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2024-01-17 9:50 UTC (permalink / raw)
To: Marco Felsch; +Cc: barebox
On Tue, Jan 16, 2024 at 03:44:16PM +0100, Sascha Hauer wrote:
> Hi Marco,
>
> On Wed, Jan 10, 2024 at 03:59:08PM +0100, Marco Felsch wrote:
> > Currently we depend on the external firmware file since we need it for
> > the sha256sum. But this dependency break the 'optional missing firmware'
> > feature.
> >
> > Fix this by dropping the firmware file prerequisite and instead evaluate
> > the FWNAME_EXISTS variable. If the firmware file does not exist a dummy
> > file is created and the build continues till the linking process (like
> > we do for the other firmware).
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > firmware/Makefile | 6 +++---
> > images/Makefile | 2 +-
> > scripts/Makefile.lib | 2 +-
> > 3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/firmware/Makefile b/firmware/Makefile
> > index 51d98d54bf14..a07b31caec73 100644
> > --- a/firmware/Makefile
> > +++ b/firmware/Makefile
> > @@ -41,7 +41,7 @@ fwobjdir := $(objtree)/firmware
> > pbl-y := $(addsuffix .gen.o, $(pbl-firmware-y))
> > obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
> >
> > -FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
> > +FWNAME = $(patsubst $(obj)/%.sum,%,$(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)))
>
> The purpose of this change is not clear to me. You replace the pattern
> %.sum with %, but I don't see how this ever matches.
Ok, now I see why this is needed. FWNAME works on $@, the filename of
the target of the rule...
>
> Dropping this change has no visible effect for me.
>
> > FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
> > FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
> >
> > @@ -91,8 +91,8 @@ $(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
> > $(obj)/%.sha.bin: $(obj)/%.sum FORCE
> > $(call if_changed,sha256bin)
> >
> > -$(obj)/%.sum: $(obj)/% FORCE
> > - $(call if_changed,sha256sum)
> > +$(obj)/%.sum: FORCE
> > + $(if $(findstring 1,$(FWNAME_EXISTS)), $(call if_changed,sha256sum,$(obj)/$*), @touch $@)
...and the target here indeed has the pattern %.sum
I think we should rather use the target stem which you are using
already. Drop FWNAME_EXISTS so this becomes:
$(if $(wildcard $(fwdir)/$*), $(call if_changed,sha256sum,$(fwdir)/$*),@touch $@)
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 |
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: fix missing firmware handling for external firmware
2024-01-17 9:50 ` Sascha Hauer
@ 2024-01-17 9:58 ` Marco Felsch
0 siblings, 0 replies; 4+ messages in thread
From: Marco Felsch @ 2024-01-17 9:58 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hi Sascha,
sry. for the delayed answer.
On 24-01-17, Sascha Hauer wrote:
> On Tue, Jan 16, 2024 at 03:44:16PM +0100, Sascha Hauer wrote:
> > Hi Marco,
> >
> > On Wed, Jan 10, 2024 at 03:59:08PM +0100, Marco Felsch wrote:
> > > Currently we depend on the external firmware file since we need it for
> > > the sha256sum. But this dependency break the 'optional missing firmware'
> > > feature.
> > >
> > > Fix this by dropping the firmware file prerequisite and instead evaluate
> > > the FWNAME_EXISTS variable. If the firmware file does not exist a dummy
> > > file is created and the build continues till the linking process (like
> > > we do for the other firmware).
> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > > firmware/Makefile | 6 +++---
> > > images/Makefile | 2 +-
> > > scripts/Makefile.lib | 2 +-
> > > 3 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/firmware/Makefile b/firmware/Makefile
> > > index 51d98d54bf14..a07b31caec73 100644
> > > --- a/firmware/Makefile
> > > +++ b/firmware/Makefile
> > > @@ -41,7 +41,7 @@ fwobjdir := $(objtree)/firmware
> > > pbl-y := $(addsuffix .gen.o, $(pbl-firmware-y))
> > > obj-pbl-y := $(addsuffix .gen.o, $(firmware-y))
> > >
> > > -FWNAME = $(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@))
> > > +FWNAME = $(patsubst $(obj)/%.sum,%,$(patsubst $(obj)/%.extgen.S,%,$(patsubst $(obj)/%.gen.S,%,$@)))
> >
> > The purpose of this change is not clear to me. You replace the pattern
> > %.sum with %, but I don't see how this ever matches.
>
> Ok, now I see why this is needed. FWNAME works on $@, the filename of
> the target of the rule...
yes.
> > Dropping this change has no visible effect for me.
> >
> > > FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
> > > FWNAME_EXISTS = $(if $(wildcard $(fwdir)/$(FWNAME)),1,0)
> > >
> > > @@ -91,8 +91,8 @@ $(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
> > > $(obj)/%.sha.bin: $(obj)/%.sum FORCE
> > > $(call if_changed,sha256bin)
> > >
> > > -$(obj)/%.sum: $(obj)/% FORCE
> > > - $(call if_changed,sha256sum)
> > > +$(obj)/%.sum: FORCE
> > > + $(if $(findstring 1,$(FWNAME_EXISTS)), $(call if_changed,sha256sum,$(obj)/$*), @touch $@)
>
> ...and the target here indeed has the pattern %.sum
and yes.
> I think we should rather use the target stem which you are using
> already. Drop FWNAME_EXISTS so this becomes:
>
> $(if $(wildcard $(fwdir)/$*), $(call if_changed,sha256sum,$(fwdir)/$*),@touch $@)
I wasn't considering this but it looks good to me. Now I see the issue
with $(obj)/% as well :/ I used the $(obj)/firmware path for my firmware
binaries and therefore had no issues with out-of-tree building.
Regards,
Marco
>
> 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 |
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-17 9:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-10 14:59 [PATCH] firmware: fix missing firmware handling for external firmware Marco Felsch
2024-01-16 14:44 ` Sascha Hauer
2024-01-17 9:50 ` Sascha Hauer
2024-01-17 9:58 ` Marco Felsch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox