[U-Boot] [PATCH 0/5] kbuild fixes

[U-Boot] [PATCH 0/5] kbuild fixes

[Masahiro Yamada]

Masahiro Yamada (5):
  kbuild: remove unneeded ifdef conditionals around build rules
  kbuild: sunxi: fix build rule of sunxi-spl.bin
  kbuild: use $(call cmd,) rather than $(call if_changed,) where
    possible
  kbuild: add missing FORCE where $(call if_changed,) is used
  kbuild: fix build rule of u-boot-spl.dtb

 Makefile             | 36 ++++++++++++++++++------------------
 scripts/Makefile.spl | 24 ++++++++++--------------
 2 files changed, 28 insertions(+), 32 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/5] kbuild: remove unneeded ifdef conditionals around build rules

[Masahiro Yamada]

These rules are only used for SOCFPGA, SUNXI, but no need to hide
them from other SoCs.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/Makefile.spl | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index f486feb..adabfcf 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -228,18 +228,14 @@ ifneq ($(CONFIG_SPL_TEXT_BASE),)
 LDFLAGS_$(SPL_BIN) += -Ttext $(CONFIG_SPL_TEXT_BASE)
 endif
 
-ifdef CONFIG_ARCH_SOCFPGA
 MKIMAGEFLAGS_$(SPL_BIN).sfp = -T socfpgaimage
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
 	$(call if_changed,mkimage)
-endif
 
-ifdef CONFIG_SUNXI
 quiet_cmd_mksunxiboot = MKSUNXI $@
 cmd_mksunxiboot = $(objtree)/tools/mksunxiboot $< $@
 $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin
 	$(call if_changed,mksunxiboot)
-endif
 
 quiet_cmd_u-boot-spl = LD      $@
       cmd_u-boot-spl = (cd $(obj) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) \
-- 
1.9.1

[U-Boot] [PATCH 2/5] kbuild: sunxi: fix build rule of sunxi-spl.bin

[Masahiro Yamada]

The build command for sunxi-spl.bin is constant, so it is pointless
to use $(call if_changed,...).  $(call cmd,...) is enough.

On the other hand, if the tools/mksunxiboot is updated, the resulted
output may be different.  Make the output image dependent on
tools/mksunxiboot.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/Makefile.spl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index adabfcf..d8b3947 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -234,8 +234,8 @@ $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
 
 quiet_cmd_mksunxiboot = MKSUNXI $@
 cmd_mksunxiboot = $(objtree)/tools/mksunxiboot $< $@
-$(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin
-	$(call if_changed,mksunxiboot)
+$(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin $(objtree)/tools/mksunxiboot
+	$(call cmd,mksunxiboot)
 
 quiet_cmd_u-boot-spl = LD      $@
       cmd_u-boot-spl = (cd $(obj) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) \
-- 
1.9.1

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Masahiro Yamada]

These build commands are constant (mostly, just concatenating images,
or just copying).  No need to use $(call if_changed,...) for them.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile             | 26 +++++++++++++-------------
 scripts/Makefile.spl |  4 ++--
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 430dd4f..aaa5b44 100644
--- a/Makefile
+++ b/Makefile
@@ -827,8 +827,8 @@ quiet_cmd_copy = COPY    $@
       cmd_copy = cp $< $@
 
 ifeq ($(CONFIG_OF_SEPARATE),y)
-u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
-	$(call if_changed,cat)
+u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb
+	$(call cmd,cat)
 
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
@@ -847,8 +847,8 @@ OBJCOPYFLAGS_u-boot.hex := -O ihex
 
 OBJCOPYFLAGS_u-boot.srec := -O srec
 
-u-boot.hex u-boot.srec: u-boot FORCE
-	$(call if_changed,objcopy)
+u-boot.hex u-boot.srec: u-boot
+	$(call cmd,objcopy)
 
 OBJCOPYFLAGS_u-boot-nodtb.bin := -O binary \
 		$(if $(CONFIG_X86_RESET_VECTOR),-R .start16 -R .resetvec)
@@ -881,8 +881,8 @@ OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
 
 OBJCOPYFLAGS_u-boot.ldr.srec := -I binary -O srec
 
-u-boot.ldr.hex u-boot.ldr.srec: u-boot.ldr FORCE
-	$(call if_changed,objcopy)
+u-boot.ldr.hex u-boot.ldr.srec: u-boot.ldr
+	$(call cmd,objcopy)
 
 #
 # U-Boot entry point, needed for booting of full-blown U-Boot
@@ -954,7 +954,7 @@ lpc32xx-boot-1.bin: lpc32xx-spl.img
 	$(call if_changed,objcopy)
 
 lpc32xx-full.bin: lpc32xx-boot-0.bin lpc32xx-boot-1.bin u-boot.img
-	$(call if_changed,cat)
+	$(call cmd,cat)
 
 CLEAN_FILES += lpc32xx-*
 
@@ -1060,8 +1060,8 @@ u-boot.rom: u-boot-x86-16bit.bin u-boot.bin
 	$(call if_changed,ifdtool)
 
 OBJCOPYFLAGS_u-boot-x86-16bit.bin := -O binary -j .start16 -j .resetvec
-u-boot-x86-16bit.bin: u-boot FORCE
-	$(call if_changed,objcopy)
+u-boot-x86-16bit.bin: u-boot
+	$(call cmd,objcopy)
 endif
 
 ifneq ($(CONFIG_SUNXI),)
@@ -1080,8 +1080,8 @@ OBJCOPYFLAGS_u-boot-tegra.bin = -O binary --pad-to=$(CONFIG_SYS_TEXT_BASE)
 u-boot-tegra.bin: spl/u-boot-spl u-boot.bin FORCE
 	$(call if_changed,pad_cat)
 
-u-boot-dtb-tegra.bin: u-boot-tegra.bin FORCE
-	$(call if_changed,copy)
+u-boot-dtb-tegra.bin: u-boot-tegra.bin
+	$(call cmd,copy)
 endif
 
 OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI)
@@ -1108,8 +1108,8 @@ OBJCOPYFLAGS_u-boot-payload.efi := $(OBJCOPYFLAGS_EFI)
 u-boot-payload.efi: u-boot-payload FORCE
 	$(call if_changed,zobjcopy)
 
-u-boot-img.bin: spl/u-boot-spl.bin u-boot.img FORCE
-	$(call if_changed,cat)
+u-boot-img.bin: spl/u-boot-spl.bin u-boot.img
+	$(call cmd,cat)
 
 #Add a target to create boot binary having SPL binary in PBI format
 #concatenated with u-boot binary. It is need by PowerPC SoC having
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index d8b3947..2668ef3 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -167,8 +167,8 @@ quiet_cmd_copy = COPY    $@
 
 ifeq ($(CONFIG_SPL_OF_CONTROL),y)
 $(obj)/$(SPL_BIN)-dtb.bin: $(obj)/$(SPL_BIN)-nodtb.bin $(obj)/$(SPL_BIN)-pad.bin \
-		$(obj)/$(SPL_BIN).dtb FORCE
-	$(call if_changed,cat)
+		$(obj)/$(SPL_BIN).dtb
+	$(call cmd,cat)
 
 $(obj)/$(SPL_BIN).bin: $(obj)/$(SPL_BIN)-dtb.bin FORCE
 	$(call if_changed,copy)
-- 
1.9.1

[U-Boot] [PATCH 4/5] kbuild: add missing FORCE where $(call if_changed, ) is used

[Masahiro Yamada]

FORCE is needed for $(call if_changed,...) to be evaluated every time.
Otherwise, the command is not executed when the command line has
changed but any prerequisite has not been updated.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile             | 10 +++++-----
 scripts/Makefile.spl |  8 ++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index aaa5b44..ee29c1c 100644
--- a/Makefile
+++ b/Makefile
@@ -924,7 +924,7 @@ u-boot.sha1:	u-boot.bin
 u-boot.dis:	u-boot
 		$(OBJDUMP) -d $< > $@
 
-u-boot.cfg:	include/config.h
+u-boot.cfg:	include/config.h FORCE
 	$(call if_changed,cpp_cfg)
 
 ifdef CONFIG_TPL
@@ -945,12 +945,12 @@ lpc32xx-spl.img: spl/u-boot-spl.bin FORCE
 
 OBJCOPYFLAGS_lpc32xx-boot-0.bin = -I binary -O binary --pad-to=$(CONFIG_SPL_PAD_TO)
 
-lpc32xx-boot-0.bin: lpc32xx-spl.img
+lpc32xx-boot-0.bin: lpc32xx-spl.img FORCE
 	$(call if_changed,objcopy)
 
 OBJCOPYFLAGS_lpc32xx-boot-1.bin = -I binary -O binary --pad-to=$(CONFIG_SPL_PAD_TO)
 
-lpc32xx-boot-1.bin: lpc32xx-spl.img
+lpc32xx-boot-1.bin: lpc32xx-spl.img FORCE
 	$(call if_changed,objcopy)
 
 lpc32xx-full.bin: lpc32xx-boot-0.bin lpc32xx-boot-1.bin u-boot.img
@@ -1056,7 +1056,7 @@ endif
 cmd_ifdtool += $(IFDTOOL) $(IFDTOOL_FLAGS) u-boot.tmp;
 cmd_ifdtool += mv u-boot.tmp $@
 
-u-boot.rom: u-boot-x86-16bit.bin u-boot.bin
+u-boot.rom: u-boot-x86-16bit.bin u-boot.bin FORCE
 	$(call if_changed,ifdtool)
 
 OBJCOPYFLAGS_u-boot-x86-16bit.bin := -O binary -j .start16 -j .resetvec
@@ -1171,7 +1171,7 @@ cmd_smap = \
 	$(CC) $(c_flags) -DSYSTEM_MAP="\"$${smap}\"" \
 		-c $(srctree)/common/system_map.c -o common/system_map.o
 
-u-boot:	$(u-boot-init) $(u-boot-main) u-boot.lds
+u-boot:	$(u-boot-init) $(u-boot-main) u-boot.lds FORCE
 	$(call if_changed,u-boot__)
 ifeq ($(CONFIG_KALLSYMS),y)
 	$(call cmd,smap)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 2668ef3..e1af6c8 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -114,7 +114,7 @@ MKIMAGEFLAGS_MLO = -T omapimage -a $(CONFIG_SPL_TEXT_BASE)
 
 MKIMAGEFLAGS_MLO.byteswap = -T omapimage -n byteswap -a $(CONFIG_SPL_TEXT_BASE)
 
-MLO MLO.byteswap: $(obj)/u-boot-spl.bin
+MLO MLO.byteswap: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkimage)
 
 ifeq ($(CONFIG_SYS_SOC),"at91")
@@ -126,12 +126,12 @@ MKIMAGEFLAGS_boot.bin += -n $(shell $(obj)/../tools/atmel_pmecc_params)
 boot.bin: $(obj)/../tools/atmel_pmecc_params
 endif
 
-boot.bin: $(obj)/u-boot-spl.bin
+boot.bin: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkimage)
 else
 MKIMAGEFLAGS_boot.bin = -T zynqimage
 
-spl/boot.bin: $(obj)/u-boot-spl.bin
+spl/boot.bin: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkimage)
 endif
 
@@ -200,7 +200,7 @@ quiet_cmd_cpp_cfg = CFG     $@
 cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 	-DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $<
 
-$(obj)/$(SPL_BIN).cfg:	include/config.h
+$(obj)/$(SPL_BIN).cfg:	include/config.h FORCE
 	$(call if_changed,cpp_cfg)
 
 ifdef CONFIG_SAMSUNG
-- 
1.9.1

[U-Boot] [PATCH 5/5] kbuild: fix build rule of u-boot-spl.dtb

[Masahiro Yamada]

The build command of u-boot-spl.dtb is not constant, but dependent
on CONFIG_OF_SPL_REMOVE_PROPS.  Use $(call if_changed,...) so that
the change of CONFIG_OF_SPL_REMOVE_PROPS is detected.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/Makefile.spl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index e1af6c8..8942ffb 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -193,8 +193,8 @@ quiet_cmd_fdtgrep = FDTGREP $@
 	$(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
 		$(addprefix -P ,$(subst $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
 
-$(obj)/$(SPL_BIN).dtb: dts/dt.dtb
-	$(call cmd,fdtgrep)
+$(obj)/$(SPL_BIN).dtb: dts/dt.dtb FORCE
+	$(call if_changed,fdtgrep)
 
 quiet_cmd_cpp_cfg = CFG     $@
 cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
-- 
1.9.1

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Albert ARIBAUD]

Hello Masahiro,

On Wed,  3 Feb 2016 21:05:12 +0900, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> These build commands are constant (mostly, just concatenating images,
> or just copying).  No need to use $(call if_changed,...) for them.

I am a total kbuild ignorant, so I'm probably asking a stupid question,
but hey.

What are the exact semantics of $(call if_changed,)? I thought it meant
"call the function if any of the dependencies has changed. If it does,
then won't this patch make the corresponding action systematic, causing
unneeded concatenations or copies?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Albert ARIBAUD]

Hello Albert,

On Wed, 3 Feb 2016 13:18:35 +0100, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hello Masahiro,
> 
> On Wed,  3 Feb 2016 21:05:12 +0900, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > These build commands are constant (mostly, just concatenating images,
> > or just copying).  No need to use $(call if_changed,...) for them.
> 
> I am a total kbuild ignorant, so I'm probably asking a stupid question,
> but hey.
> 
> What are the exact semantics of $(call if_changed,)? I thought it meant
> "call the function if any of the dependencies has changed. If it does,
> then won't this patch make the corresponding action systematic, causing
> unneeded concatenations or copies?

NVM, the answer could be found in later patches in the series.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Stephen Warren]

On 02/03/2016 05:05 AM, Masahiro Yamada wrote:
> These build commands are constant (mostly, just concatenating images,
> or just copying).  No need to use $(call if_changed,...) for them.

I disagree, since I believe this change means that if someone /does/ 
change the command in the future (e.g. to replace it with more complex 
processing, or add additional dependencies), then the Makefile will/may 
not automatically rebuild those targets, which is the entire point of 
using if_changed, and is a huge benefit of using Kbuild.

In my opinion, every rule should use if_changed, and contain a single 
command at the Makefile level; i.e. I noticed the following somewhere, 
which also doesn't rebuild the target in all cases if the commands are 
changed, which is bad:

target: sources FORCE
     $(call if_changed,xxx)
     something_else
     yet_more_cmds

If "something_else" or "yet_more_cmds" are edited, the target won't get 
rebuilt unless some other modification causes it to be.

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Masahiro Yamada]

Hi Stephen,


2016-02-04 0:57 GMT+09:00 Stephen Warren <swarren@wwwdotorg.org>:
> On 02/03/2016 05:05 AM, Masahiro Yamada wrote:
>>
>> These build commands are constant (mostly, just concatenating images,
>> or just copying).  No need to use $(call if_changed,...) for them.
>
>
> I disagree, since I believe this change means that if someone /does/ change
> the command in the future (e.g. to replace it with more complex processing,
> or add additional dependencies), then the Makefile will/may not
> automatically rebuild those targets, which is the entire point of using
> if_changed, and is a huge benefit of using Kbuild.

I do not a strong opinion about this, so
I will drop 3/5 and submit v2.



> In my opinion, every rule should use if_changed, and contain a single
> command at the Makefile level; i.e. I noticed the following somewhere, which
> also doesn't rebuild the target in all cases if the commands are changed,
> which is bad:
>
> target: sources FORCE
>     $(call if_changed,xxx)
>     something_else
>     yet_more_cmds
>
> If "something_else" or "yet_more_cmds" are edited, the target won't get
> rebuilt unless some other modification causes it to be.

Patch are welcome to clean up them.


-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 3/5] kbuild: use $(call cmd, ) rather than $(call if_changed, ) where possible

[Ian Campbell]

On Fri, 2016-02-05 at 14:33 +0900, Masahiro Yamada wrote:
> >> These build commands are constant (mostly, just concatenating
> images,
> >> or just copying).  No need to use $(call if_changed,...) for them.
> >
> >
> > I disagree, since I believe this change means that if someone /does/ change
> > the command in the future (e.g. to replace it with more complex processing,
> > or add additional dependencies), then the Makefile will/may not
> > automatically rebuild those targets, which is the entire point of using
> > if_changed, and is a huge benefit of using Kbuild.
> 
> I do not a strong opinion about this, so
> I will drop 3/5 and submit v2.

I think the same logic applies to that part of the change in patch 2 as
well.

Ian.