| Message ID | 20201104143344.2279-1-pali@kernel.org |
|---|---|
| State | Accepted |
| Commit | e9ebc17e59217eed457ae83b3c1eec0f7a86daa7 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Makefile: Do not call useless command 'true' | expand |
Hello! Could you please review this patch? On Wednesday 04 November 2020 15:33:44 Pali Rohár wrote: > Macro 'cmd_objcopy_uboot' currently does not work with passed empty command > expanded from 'cmd_static_rela' and therefore dummy command 'true' is set > in 'cmd_static_rela' to workaround this issue. > > Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with > empty 'cmd_static_rela' macro and remove useless invocation of command > 'true'. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > Makefile | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index b90fe8b865..aeb2c17a9b 100644 > --- a/Makefile > +++ b/Makefile > @@ -885,7 +885,7 @@ cmd_static_rela = \ > tools/relocate-rela $(3) $(4) $$start $$end > else > quiet_cmd_static_rela = > -cmd_static_rela = true > +cmd_static_rela = > endif > > # Always append INPUTS so that arch config.mk's can add custom ones > @@ -1312,7 +1312,11 @@ endif > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } > > quiet_cmd_objcopy_uboot = OBJCOPY $@ > +ifdef cmd_static_rela > cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } > +else > +cmd_objcopy_uboot = $(cmd_objcopy) > +endif > > u-boot-nodtb.bin: u-boot FORCE > $(call if_changed,objcopy_uboot) > -- > 2.20.1 >
Hi Pali, On Fri, 15 Jan 2021 at 17:21, Pali Rohár <pali@kernel.org> wrote: > > Hello! Could you please review this patch? > > On Wednesday 04 November 2020 15:33:44 Pali Rohár wrote: > > Macro 'cmd_objcopy_uboot' currently does not work with passed empty command > > expanded from 'cmd_static_rela' and therefore dummy command 'true' is set > > in 'cmd_static_rela' to workaround this issue. > > > > Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with > > empty 'cmd_static_rela' macro and remove useless invocation of command > > 'true'. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > Makefile | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index b90fe8b865..aeb2c17a9b 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -885,7 +885,7 @@ cmd_static_rela = \ > > tools/relocate-rela $(3) $(4) $$start $$end > > else > > quiet_cmd_static_rela = > > -cmd_static_rela = true > > +cmd_static_rela = > > endif > > > > # Always append INPUTS so that arch config.mk's can add custom ones > > @@ -1312,7 +1312,11 @@ endif > > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } > > > > quiet_cmd_objcopy_uboot = OBJCOPY $@ > > +ifdef cmd_static_rela > > cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } > > +else > > +cmd_objcopy_uboot = $(cmd_objcopy) > > +endif > > > > u-boot-nodtb.bin: u-boot FORCE > > $(call if_changed,objcopy_uboot) > > -- > > 2.20.1 > > I don't know much about that and your patch looks reasonable, but what is the motivation? Is it a code clean-up or a bug? Regards, Simon
On Tuesday 19 January 2021 11:06:12 Simon Glass wrote: > Hi Pali, > > On Fri, 15 Jan 2021 at 17:21, Pali Rohár <pali@kernel.org> wrote: > > > > Hello! Could you please review this patch? > > > > On Wednesday 04 November 2020 15:33:44 Pali Rohár wrote: > > > Macro 'cmd_objcopy_uboot' currently does not work with passed empty command > > > expanded from 'cmd_static_rela' and therefore dummy command 'true' is set > > > in 'cmd_static_rela' to workaround this issue. > > > > > > Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with > > > empty 'cmd_static_rela' macro and remove useless invocation of command > > > 'true'. > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > --- > > > Makefile | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index b90fe8b865..aeb2c17a9b 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -885,7 +885,7 @@ cmd_static_rela = \ > > > tools/relocate-rela $(3) $(4) $$start $$end > > > else > > > quiet_cmd_static_rela = > > > -cmd_static_rela = true > > > +cmd_static_rela = > > > endif > > > > > > # Always append INPUTS so that arch config.mk's can add custom ones > > > @@ -1312,7 +1312,11 @@ endif > > > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } > > > > > > quiet_cmd_objcopy_uboot = OBJCOPY $@ > > > +ifdef cmd_static_rela > > > cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } > > > +else > > > +cmd_objcopy_uboot = $(cmd_objcopy) > > > +endif > > > > > > u-boot-nodtb.bin: u-boot FORCE > > > $(call if_changed,objcopy_uboot) > > > -- > > > 2.20.1 > > > > > I don't know much about that and your patch looks reasonable, but what > is the motivation? Is it a code clean-up or a bug? Hello! I do not know about any bugs here, so it is just code clean-up.
On Tue, 19 Jan 2021 at 11:39, Pali Rohár <pali@kernel.org> wrote: > > On Tuesday 19 January 2021 11:06:12 Simon Glass wrote: > > Hi Pali, > > > > On Fri, 15 Jan 2021 at 17:21, Pali Rohár <pali@kernel.org> wrote: > > > > > > Hello! Could you please review this patch? > > > > > > On Wednesday 04 November 2020 15:33:44 Pali Rohár wrote: > > > > Macro 'cmd_objcopy_uboot' currently does not work with passed empty command > > > > expanded from 'cmd_static_rela' and therefore dummy command 'true' is set > > > > in 'cmd_static_rela' to workaround this issue. > > > > > > > > Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with > > > > empty 'cmd_static_rela' macro and remove useless invocation of command > > > > 'true'. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > > > Makefile | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Makefile b/Makefile > > > > index b90fe8b865..aeb2c17a9b 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -885,7 +885,7 @@ cmd_static_rela = \ > > > > tools/relocate-rela $(3) $(4) $$start $$end > > > > else > > > > quiet_cmd_static_rela = > > > > -cmd_static_rela = true > > > > +cmd_static_rela = > > > > endif > > > > > > > > # Always append INPUTS so that arch config.mk's can add custom ones > > > > @@ -1312,7 +1312,11 @@ endif > > > > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } > > > > > > > > quiet_cmd_objcopy_uboot = OBJCOPY $@ > > > > +ifdef cmd_static_rela > > > > cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } > > > > +else > > > > +cmd_objcopy_uboot = $(cmd_objcopy) > > > > +endif > > > > > > > > u-boot-nodtb.bin: u-boot FORCE > > > > $(call if_changed,objcopy_uboot) > > > > -- > > > > 2.20.1 > > > > > > > > I don't know much about that and your patch looks reasonable, but what > > is the motivation? Is it a code clean-up or a bug? > > Hello! I do not know about any bugs here, so it is just code clean-up. Reviewed-by: Simon Glass <sjg@chromium.org>
On Wed, Nov 04, 2020 at 03:33:44PM +0100, Pali Rohár wrote: > Macro 'cmd_objcopy_uboot' currently does not work with passed empty command > expanded from 'cmd_static_rela' and therefore dummy command 'true' is set > in 'cmd_static_rela' to workaround this issue. > > Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with > empty 'cmd_static_rela' macro and remove useless invocation of command > 'true'. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > Makefile | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index b90fe8b865..aeb2c17a9b 100644 > --- a/Makefile > +++ b/Makefile > @@ -885,7 +885,7 @@ cmd_static_rela = \ > tools/relocate-rela $(3) $(4) $$start $$end > else > quiet_cmd_static_rela = > -cmd_static_rela = true > +cmd_static_rela = > endif > > # Always append INPUTS so that arch config.mk's can add custom ones > @@ -1312,7 +1312,11 @@ endif > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } > > quiet_cmd_objcopy_uboot = OBJCOPY $@ > +ifdef cmd_static_rela > cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } > +else > +cmd_objcopy_uboot = $(cmd_objcopy) > +endif > > u-boot-nodtb.bin: u-boot FORCE > $(call if_changed,objcopy_uboot) Applied to u-boot/master, thanks!
diff --git a/Makefile b/Makefile index b90fe8b865..aeb2c17a9b 100644 --- a/Makefile +++ b/Makefile @@ -885,7 +885,7 @@ cmd_static_rela = \ tools/relocate-rela $(3) $(4) $$start $$end else quiet_cmd_static_rela = -cmd_static_rela = true +cmd_static_rela = endif # Always append INPUTS so that arch config.mk's can add custom ones @@ -1312,7 +1312,11 @@ endif shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } quiet_cmd_objcopy_uboot = OBJCOPY $@ +ifdef cmd_static_rela cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } +else +cmd_objcopy_uboot = $(cmd_objcopy) +endif u-boot-nodtb.bin: u-boot FORCE $(call if_changed,objcopy_uboot)
Macro 'cmd_objcopy_uboot' currently does not work with passed empty command expanded from 'cmd_static_rela' and therefore dummy command 'true' is set in 'cmd_static_rela' to workaround this issue. Eliminate it now by fixing 'cmd_objcopy_uboot' macro to work also with empty 'cmd_static_rela' macro and remove useless invocation of command 'true'. Signed-off-by: Pali Rohár <pali@kernel.org> --- Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)