[7/7] MIPS: boot: rebuild ITB when contained DTB is updated

Message ID 1523890067-13641-8-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series
  • MIPS: boot: fix various problems in arch/mips/boot/Makefile
Related show

Commit Message

Masahiro Yamada April 16, 2018, 2:47 p.m.
Since now, the unnecessary rebuild of ITB has been fixed.  Another
problem to be taken care of is, missed rebuild of ITB.

For example, board-boston.its.S includes boston.dtb by the /incbin/
directive.  If boston.dtb is updated, vmlinux.*.dtb must be rebuilt.
Currently, the dependency between ITB and contained DTB files is not
described anywhere.  Previously, this problem was hidden since
vmlinux.*.itb was always rebuilt even if nothing is updated.  By
fixing the spurious rebuild, this is a real problem now.

Use the same strategy for automatic generation of the header file
dependency.  DTC works as a backend of mkimage, and DTC supports -d
option.  It outputs the dependencies, including binary files pulled
by the /incbin/ directive.

The implementation is simpler than cmd_dtc in scripts/Makefile.lib
since we do not need CPP here.  Just pass -d $(depfile) to DTC, and
let the resulted $(depfile) processed by fixdep.

It might be unclear why "$(obj)/dts/%.dtb: ;" is needed.  With this
commit, *.cmd files will contain dependency on DTB files.  In the
next invocation of build, the *.cmd files will be included, then
Make will try to find a rule to update *.dtb files.  Unfortunately,
it is found in scripts/Makefile.lib.  The build rule of $(obj)/%.dtb
is invoked by if_changed_dep, so it needs to include *.cmd files
of DTB, but they are not included because we are in arch/mips/boot,
but those *.cmd files reside in arch/mips/boot/dts/*/.  Cancel the
pattern rule in scripts/Makefile.lib to suppress unneeded rebuilding
of DTB.

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

---

 arch/mips/boot/Makefile | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.7.4

Comments

Paul Burton June 20, 2018, 12:26 a.m. | #1
Hi Masahiro,

On Mon, Apr 16, 2018 at 07:47:47AM -0700, Masahiro Yamada wrote:
> Since now, the unnecessary rebuild of ITB has been fixed.  Another

> problem to be taken care of is, missed rebuild of ITB.

> 

> For example, board-boston.its.S includes boston.dtb by the /incbin/

> directive.  If boston.dtb is updated, vmlinux.*.dtb must be rebuilt.

> Currently, the dependency between ITB and contained DTB files is not

> described anywhere.  Previously, this problem was hidden since

> vmlinux.*.itb was always rebuilt even if nothing is updated.  By

> fixing the spurious rebuild, this is a real problem now.

> 

> Use the same strategy for automatic generation of the header file

> dependency.  DTC works as a backend of mkimage, and DTC supports -d

> option.  It outputs the dependencies, including binary files pulled

> by the /incbin/ directive.

> 

> The implementation is simpler than cmd_dtc in scripts/Makefile.lib

> since we do not need CPP here.  Just pass -d $(depfile) to DTC, and

> let the resulted $(depfile) processed by fixdep.

> 

> It might be unclear why "$(obj)/dts/%.dtb: ;" is needed.  With this

> commit, *.cmd files will contain dependency on DTB files.  In the

> next invocation of build, the *.cmd files will be included, then

> Make will try to find a rule to update *.dtb files.  Unfortunately,

> it is found in scripts/Makefile.lib.  The build rule of $(obj)/%.dtb

> is invoked by if_changed_dep, so it needs to include *.cmd files

> of DTB, but they are not included because we are in arch/mips/boot,

> but those *.cmd files reside in arch/mips/boot/dts/*/.  Cancel the

> pattern rule in scripts/Makefile.lib to suppress unneeded rebuilding

> of DTB.

> 

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

> ---

> 

>  arch/mips/boot/Makefile | 13 ++++++++++---

>  1 file changed, 10 insertions(+), 3 deletions(-)


Thanks - this looks good to me except that it starts outputting a "Don't
know how to preprocess itb-image" message after building the .itb.

I presume we just need an extra case adding to ksym_dep_filter. Do you
want to add that in & resubmit this one?

Thanks,
    Paul

Patch

diff --git a/arch/mips/boot/Makefile b/arch/mips/boot/Makefile
index d102d53..f8dce5b 100644
--- a/arch/mips/boot/Makefile
+++ b/arch/mips/boot/Makefile
@@ -163,11 +163,18 @@  quiet_cmd_itb-image = ITB     $@
 		$(CONFIG_SHELL) $(MKIMAGE) \
 		-D "-I dts -O dtb -p 500 \
 			--include $(objtree)/arch/mips \
-			--warning no-unit_address_vs_reg" \
+			--warning no-unit_address_vs_reg \
+			-d $(depfile)" \
 		-f $(2) $@
 
 $(obj)/vmlinux.itb: $(obj)/vmlinux.its $(obj)/vmlinux.bin FORCE
-	$(call if_changed,itb-image,$<)
+	$(call if_changed_dep,itb-image,$<)
 
 $(obj)/vmlinux.%.itb: $(obj)/vmlinux.%.its $(obj)/vmlinux.bin.% FORCE
-	$(call if_changed,itb-image,$<)
+	$(call if_changed_dep,itb-image,$<)
+
+# The -d option of DTC outputs dependencies of binaries included by the
+# /incbin/ directive.  When .*.cmd files are included, Kbuild tries to
+# update *.dtb because it sees a pattern rule defined in scripts/Makefile.lib.
+# The rule must be cancelled by a more specific rule.
+$(obj)/dts/%.dtb: ;