[PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

All architectures that use cmd_dtc do so in the same way. Move the build
rule to a central location to avoid duplication.

Update Documentation/kbuild to remove the explicit call to cmd_dtc from
the example, now that the rule exists in a centralized location, and in
fact replace the example with a couple that still exist.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v5: Update Documentation/kbuild
v4: No change.
v3: No change.
v2: New patch.
---
 Documentation/kbuild/makefiles.txt |   21 +++++++++++++++------
 arch/arm/boot/Makefile             |    4 ----
 arch/c6x/boot/Makefile             |    3 ---
 arch/openrisc/boot/Makefile        |    3 ---
 arch/powerpc/boot/Makefile         |    4 ----
 scripts/Makefile.lib               |    3 +++
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index ec9ae67..a0b0671 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -1175,15 +1175,24 @@ When kbuild executes, the following steps are followed (roughly):
 	in an init section in the image. Platform code *must* copy the
 	blob to non-init memory prior to calling unflatten_device_tree().
 
+	To use this command, simply add *.dtb into obj-y or targets, or make
+	some other target depend on %.dtb
+
+	A central rule exists to create $(obj)/%.dtb from $(src)/dts/%.dts;
+	architecture Makefiles do no need to explicitly write out that rule.
+
 	Example:
-		#arch/x86/platform/ce4100/Makefile
-		clean-files := *dtb.S
+		#arch/arm/boot/Makefile
+		targets += $(dtb-y)
+		clean-files := *.dtb
+
+		#arch/powerpc/boot/Makefile
+		DTC_FLAGS       ?= -p 1024
 
-		DTC_FLAGS := -p 1024
-		obj-y += foo.dtb.o
+		$(obj)/dtbImage.%: vmlinux $(wrapperbits) $(obj)/%.dtb
+			$(call if_changed,wrap,$*,,$(obj)/$*.dtb)
 
-		$(obj)/%.dtb: $(src)/%.dts
-			$(call cmd,dtc)
+		clean-files += ... *.dtb
 
 --- 6.8 Custom kbuild commands
 
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index f2aa09e..208bb4c 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -61,10 +61,6 @@ endif
 
 targets += $(dtb-y)
 
-# Rule to build device tree blobs
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
 $(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y))
 
 clean-files := *.dtb
diff --git a/arch/c6x/boot/Makefile b/arch/c6x/boot/Makefile
index 6891257..ad605fb 100644
--- a/arch/c6x/boot/Makefile
+++ b/arch/c6x/boot/Makefile
@@ -12,9 +12,6 @@ ifneq ($(DTB),)
 obj-y += linked_dtb.o
 endif
 
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
 quiet_cmd_cp = CP      $< $@$2
 	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
 
diff --git a/arch/openrisc/boot/Makefile b/arch/openrisc/boot/Makefile
index 0995835..fd329bd 100644
--- a/arch/openrisc/boot/Makefile
+++ b/arch/openrisc/boot/Makefile
@@ -10,6 +10,3 @@ obj-y += $(BUILTIN_DTB)
 clean-files := *.dtb.S
 
 #DTC_FLAGS ?= -p 1024
-
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 6a15c96..90206f2 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -356,10 +356,6 @@ $(obj)/treeImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits)
 $(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
 	$(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)
 
-# Rule to build device tree blobs
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
 # If there isn't a platform selected then just strip the vmlinux.
 ifeq (,$(image-y))
 image-y := vmlinux.strip
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0be6f11..425578e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -266,6 +266,9 @@ $(obj)/%.dtb.S: $(obj)/%.dtb
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) -d $(depfile) $<
 
+$(obj)/%.dtb: $(src)/dts/%.dts FORCE
+	$(call if_changed_dep,dtc)
+
 # Bzip2
 # ---------------------------------------------------------------------------
 
-- 
1.7.0.4

[PATCH V5 2/2] kbuild: run the pre-processor on *.dts files

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Modify cmd_dtc to run the C pre-processor on the input .dts file before
passing it to dtc for final compilation. This allows the use of #define
and #include within the .dts file.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v5:
* Update Documentation/kbuild for the new command and rule.
v4:
* Use -x assembler-with-cpp so pre-defined macros are set up so that
  #included header files know to only use cpp syntax, not C syntax.
* Define __DTS__ for similar reasons.
* use $(CPP) not $(CC) -E, and use $(cpp_flags).
* Save the pre-processed results so they can be easily inspected when
  debugging build issues.
* The use of -x assembler-with-cpp causes cpp to recognize directives in
  column 1 only. Hence, there's no need to escape property names that
  begin with #. Hence, there's no need for separate skeleton.dtsi and
  skeleton.dtsip. Maintain a separate file extension and build rule so that
  CPP-usage is opt-in. In particular, when using CPP, #include must be used
  rather than /include/ so that dependencies work.
v3: Pass "-x c" not "-xc" to cpp.
v2: Place make %.dtb: %.dtsp rule into Makefile.lib.
---
 Documentation/kbuild/makefiles.txt |   23 +++++++++++++++++++++++
 scripts/Makefile.lib               |    9 +++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index a0b0671..dfe6838 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -1194,6 +1194,29 @@ When kbuild executes, the following steps are followed (roughly):
 
 		clean-files += ... *.dtb
 
+    dtc_cpp
+	This is just like dtc as describe above, except that the C pre-
+	processor is invoked upon the .dtsp file before compiling the result
+	with dtc.
+
+	In order for build dependencies to work, all files compiled using
+	dtc_cpp must use the C pre-processor's #include functionality and not
+	dtc's /include/ functionality.
+
+	Using the C pre-processor allows use of #define to create named
+	constants. In turn, the #defines will typically appear in a header
+	file, which may be shared with regular C code. Since the dtc language
+	represents a data structure rather than code in C syntax, similar
+	restrictions are placed on a header file included by a device tree
+	file as for a header file included by an assembly language file.
+	In particular, the C pre-processor is passed -x assembler-with-cpp,
+	which sets macro __ASSEMBLY__. __DTS__ is also set. These allow header
+	files to restrict their content to that compatible with device tree
+	source.
+
+	A central rule exists to create $(obj)/%.dtb from $(src)/dts/%.dtsp;
+	architecture Makefiles do no need to explicitly write out that rule.
+
 --- 6.8 Custom kbuild commands
 
 	When kbuild is executing with KBUILD_VERBOSE=0, then only a shorthand
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 425578e..33432f4 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -269,6 +269,15 @@ cmd_dtc = $(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) -d $(depfile
 $(obj)/%.dtb: $(src)/dts/%.dts FORCE
 	$(call if_changed_dep,dtc)
 
+dtc-tmp = $(subst $(comma),_,$(dot-target).dts)
+
+quiet_cmd_dtc_cpp = DTC+CPP $@
+cmd_dtc_cpp = $(CPP) $(cpp_flags) -D__DTS__ -x assembler-with-cpp -o $(dtc-tmp) $< ; \
+	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 $(DTC_FLAGS) $(dtc-tmp)
+
+$(obj)/%.dtb: $(src)/dts/%.dtsp FORCE
+	$(call if_changed_dep,dtc_cpp)
+
 # Bzip2
 # ---------------------------------------------------------------------------
 
-- 
1.7.0.4

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Sam Ravnborg]

On Wed, Oct 31, 2012 at 04:10:30PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> All architectures that use cmd_dtc do so in the same way. Move the build
> rule to a central location to avoid duplication.
> 
> Update Documentation/kbuild to remove the explicit call to cmd_dtc from
> the example, now that the rule exists in a centralized location, and in
> fact replace the example with a couple that still exist.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH V5 2/2] kbuild: run the pre-processor on *.dts files

[Sam Ravnborg]

On Wed, Oct 31, 2012 at 04:10:31PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Modify cmd_dtc to run the C pre-processor on the input .dts file before
> passing it to dtc for final compilation. This allows the use of #define
> and #include within the .dts file.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Ralf Baechle]

On Wed, Oct 31, 2012 at 04:10:30PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> All architectures that use cmd_dtc do so in the same way. Move the build
> rule to a central location to avoid duplication.

Can you fold these MIPS bits into your patch?

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/cavium-octeon/Makefile | 3 ---
 arch/mips/netlogic/dts/Makefile  | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index bc96e29..6e927cf 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -24,9 +24,6 @@ DTB_FILES = $(patsubst %.dts, %.dtb, $(DTS_FILES))
 
 obj-y += $(patsubst %.dts, %.dtb.o, $(DTS_FILES))
 
-$(obj)/%.dtb: $(src)/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
 # Let's keep the .dtb files around in case we want to look at them.
 .SECONDARY:  $(addprefix $(obj)/, $(DTB_FILES))
 
diff --git a/arch/mips/netlogic/dts/Makefile b/arch/mips/netlogic/dts/Makefile
index 67ae3fe2..d117d46 100644
--- a/arch/mips/netlogic/dts/Makefile
+++ b/arch/mips/netlogic/dts/Makefile
@@ -1,4 +1 @@
 obj-$(CONFIG_DT_XLP_EVP) := xlp_evp.dtb.o
-
-$(obj)/%.dtb: $(obj)/%.dts
-	$(call if_changed,dtc)

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Ralf Baechle]

On Fri, Nov 02, 2012 at 10:58:01AM +0100, Ralf Baechle wrote:

> Can you fold these MIPS bits into your patch?

I missed Lantiq.

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/cavium-octeon/Makefile | 3 ---
 arch/mips/lantiq/dts/Makefile    | 3 ---
 arch/mips/netlogic/dts/Makefile  | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index bc96e29..6e927cf 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -24,9 +24,6 @@ DTB_FILES = $(patsubst %.dts, %.dtb, $(DTS_FILES))
 
 obj-y += $(patsubst %.dts, %.dtb.o, $(DTS_FILES))
 
-$(obj)/%.dtb: $(src)/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
 # Let's keep the .dtb files around in case we want to look at them.
 .SECONDARY:  $(addprefix $(obj)/, $(DTB_FILES))
 
diff --git a/arch/mips/lantiq/dts/Makefile b/arch/mips/lantiq/dts/Makefile
index 674fca4..6fa72dd 100644
--- a/arch/mips/lantiq/dts/Makefile
+++ b/arch/mips/lantiq/dts/Makefile
@@ -1,4 +1 @@
 obj-$(CONFIG_DT_EASY50712) := easy50712.dtb.o
-
-$(obj)/%.dtb: $(obj)/%.dts
-	$(call if_changed,dtc)
diff --git a/arch/mips/netlogic/dts/Makefile b/arch/mips/netlogic/dts/Makefile
index 67ae3fe2..d117d46 100644
--- a/arch/mips/netlogic/dts/Makefile
+++ b/arch/mips/netlogic/dts/Makefile
@@ -1,4 +1 @@
 obj-$(CONFIG_DT_XLP_EVP) := xlp_evp.dtb.o
-
-$(obj)/%.dtb: $(obj)/%.dts
-	$(call if_changed,dtc)

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Stephen Warren]

On 11/02/2012 04:23 AM, Ralf Baechle wrote:
> On Fri, Nov 02, 2012 at 10:58:01AM +0100, Ralf Baechle wrote:
> 
>> Can you fold these MIPS bits into your patch?
> 
> I missed Lantiq.

Thanks, I've squashed that in, and with a quick grep noticed that
arch/{arm64,microblaze} also need updating.

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

On Fri, Nov 2, 2012 at 10:23 AM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Fri, Nov 02, 2012 at 10:58:01AM +0100, Ralf Baechle wrote:
>
>> Can you fold these MIPS bits into your patch?
>
> I missed Lantiq.
>
>   Ralf
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>
>  arch/mips/cavium-octeon/Makefile | 3 ---
>  arch/mips/lantiq/dts/Makefile    | 3 ---
>  arch/mips/netlogic/dts/Makefile  | 3 ---
>  3 files changed, 9 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
> index bc96e29..6e927cf 100644
> --- a/arch/mips/cavium-octeon/Makefile
> +++ b/arch/mips/cavium-octeon/Makefile
> @@ -24,9 +24,6 @@ DTB_FILES = $(patsubst %.dts, %.dtb, $(DTS_FILES))
>
>  obj-y += $(patsubst %.dts, %.dtb.o, $(DTS_FILES))
>
> -$(obj)/%.dtb: $(src)/%.dts FORCE
> -       $(call if_changed_dep,dtc)
> -
>  # Let's keep the .dtb files around in case we want to look at them.
>  .SECONDARY:  $(addprefix $(obj)/, $(DTB_FILES))
>
> diff --git a/arch/mips/lantiq/dts/Makefile b/arch/mips/lantiq/dts/Makefile
> index 674fca4..6fa72dd 100644
> --- a/arch/mips/lantiq/dts/Makefile
> +++ b/arch/mips/lantiq/dts/Makefile
> @@ -1,4 +1 @@
>  obj-$(CONFIG_DT_EASY50712) := easy50712.dtb.o
> -
> -$(obj)/%.dtb: $(obj)/%.dts
> -       $(call if_changed,dtc)
> diff --git a/arch/mips/netlogic/dts/Makefile b/arch/mips/netlogic/dts/Makefile
> index 67ae3fe2..d117d46 100644
> --- a/arch/mips/netlogic/dts/Makefile
> +++ b/arch/mips/netlogic/dts/Makefile
> @@ -1,4 +1 @@
>  obj-$(CONFIG_DT_XLP_EVP) := xlp_evp.dtb.o
> -
> -$(obj)/%.dtb: $(obj)/%.dts
> -       $(call if_changed,dtc)

This actually breaks MIPS builds. MIPS builds the .dtbs in the same
directory as the .dts files. Everyone else has a dts/ subdirectory,
which is admittedly a bit insane, but until things are resolved I've
dropped the above MIPS hunks.

MIPS /could/ be changed to also use a dts directory, but I'd actually
rather if someone could make all the other platforms build the dtbs in
the same directory. I've looked at it briefly, but I haven't figured
out all the make magic needed to do it nicely.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

On Thu, Nov 15, 2012 at 10:15 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Fri, Nov 2, 2012 at 10:23 AM, Ralf Baechle <ralf@linux-mips.org> wrote:
>> On Fri, Nov 02, 2012 at 10:58:01AM +0100, Ralf Baechle wrote:
>>
>>> Can you fold these MIPS bits into your patch?
>>
>> I missed Lantiq.
>>
>>   Ralf
>>
>> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>>
>>  arch/mips/cavium-octeon/Makefile | 3 ---
>>  arch/mips/lantiq/dts/Makefile    | 3 ---
>>  arch/mips/netlogic/dts/Makefile  | 3 ---
>>  3 files changed, 9 deletions(-)
>>
>> diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
>> index bc96e29..6e927cf 100644
>> --- a/arch/mips/cavium-octeon/Makefile
>> +++ b/arch/mips/cavium-octeon/Makefile
>> @@ -24,9 +24,6 @@ DTB_FILES = $(patsubst %.dts, %.dtb, $(DTS_FILES))
>>
>>  obj-y += $(patsubst %.dts, %.dtb.o, $(DTS_FILES))
>>
>> -$(obj)/%.dtb: $(src)/%.dts FORCE
>> -       $(call if_changed_dep,dtc)
>> -
>>  # Let's keep the .dtb files around in case we want to look at them.
>>  .SECONDARY:  $(addprefix $(obj)/, $(DTB_FILES))
>>
>> diff --git a/arch/mips/lantiq/dts/Makefile b/arch/mips/lantiq/dts/Makefile
>> index 674fca4..6fa72dd 100644
>> --- a/arch/mips/lantiq/dts/Makefile
>> +++ b/arch/mips/lantiq/dts/Makefile
>> @@ -1,4 +1 @@
>>  obj-$(CONFIG_DT_EASY50712) := easy50712.dtb.o
>> -
>> -$(obj)/%.dtb: $(obj)/%.dts
>> -       $(call if_changed,dtc)
>> diff --git a/arch/mips/netlogic/dts/Makefile b/arch/mips/netlogic/dts/Makefile
>> index 67ae3fe2..d117d46 100644
>> --- a/arch/mips/netlogic/dts/Makefile
>> +++ b/arch/mips/netlogic/dts/Makefile
>> @@ -1,4 +1 @@
>>  obj-$(CONFIG_DT_XLP_EVP) := xlp_evp.dtb.o
>> -
>> -$(obj)/%.dtb: $(obj)/%.dts
>> -       $(call if_changed,dtc)
>
> This actually breaks MIPS builds. MIPS builds the .dtbs in the same
> directory as the .dts files. Everyone else has a dts/ subdirectory,
> which is admittedly a bit insane, but until things are resolved I've
> dropped the above MIPS hunks.
>
> MIPS /could/ be changed to also use a dts directory, but I'd actually
> rather if someone could make all the other platforms build the dtbs in
> the same directory. I've looked at it briefly, but I haven't figured
> out all the make magic needed to do it nicely.

On second thought. I'm dropping this patch entirely for now. I don't
want to put in a generic rule that expects files in weird locations.
If we can fix up all the architectures to build dtb in the dts
directory, then it is better for each architecture to have a separate
fixup patch instead of changing everyone at once.

Or how about: I could pick up the patch with only the MIPS hunk and
every other user can be fixed up independently to use the new rule.

g.

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

Grant Likely wrote:
> Or how about: I could pick up the patch with only the MIPS hunk and
> every other user can be fixed up independently to use the new rule.

Here's a trial patch to fix up ARM. Does this look correct? This patch
depends on the generic dtb build rule already being applied.

g.

---

arm/of: Change .dtb build rules to build in dts directory

The current rules have the .dtb files build in a different directory
from the .dts files. The only reason for this is that it was what
PowerPC has done historically. This patch changes ARM to use the generic
dtb rule which builds .dtb files in the same directory as the source .dts.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 arch/arm/Makefile          |    4 ++--
 arch/arm/boot/Makefile     |   12 ------------
 arch/arm/boot/dts/Makefile |    4 ++++
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 5f914fc..c35baf1 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -292,10 +292,10 @@ zinstall uinstall install: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@
 
 %.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@
 
 dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs
 
 # We use MRPROPER_FILES and CLEAN_FILES now
 archclean:
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index f2aa09e..801b92c 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -15,8 +15,6 @@ ifneq ($(MACHINE),)
 include $(srctree)/$(MACHINE)/Makefile.boot
 endif
 
-include $(srctree)/arch/arm/boot/dts/Makefile
-
 # Note: the following conditions must always be true:
 #   ZRELADDR == virt_to_phys(PAGE_OFFSET + TEXT_OFFSET)
 #   PARAMS_PHYS must be within 4MB of ZRELADDR
@@ -59,16 +57,6 @@ $(obj)/zImage:	$(obj)/compressed/vmlinux FORCE
 
 endif
 
-targets += $(dtb-y)
-
-# Rule to build device tree blobs
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
-$(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y))
-
-clean-files := *.dtb
-
 ifneq ($(LOADADDR),)
   UIMAGE_LOADADDR=$(LOADADDR)
 else
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index f37cf9f..2aef042 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -104,4 +104,8 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \
 	wm8505-ref.dtb \
 	wm8650-mid.dtb
 
+targets += dtbs
 endif
+
+dtbs: $(addprefix $(obj)/, $(dtb-y))
+clean-files := *.dtb
-- 
1.7.10.4

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

Grant Likely wrote:
> Or how about: I could pick up the patch with only the MIPS hunk and
> every other user can be fixed up independently to use the new rule.

Here's a trial patch to fix up ARM. Does this look correct? This patch
depends on the generic dtb build rule already being applied.

g.

---

arm/of: Change .dtb build rules to build in dts directory

The current rules have the .dtb files build in a different directory
from the .dts files. The only reason for this is that it was what
PowerPC has done historically. This patch changes ARM to use the generic
dtb rule which builds .dtb files in the same directory as the source .dts.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 arch/arm/Makefile          |    4 ++--
 arch/arm/boot/Makefile     |   12 ------------
 arch/arm/boot/dts/Makefile |    4 ++++
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 5f914fc..c35baf1 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -292,10 +292,10 @@ zinstall uinstall install: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@
 
 %.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@
 
 dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs
 
 # We use MRPROPER_FILES and CLEAN_FILES now
 archclean:
diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index f2aa09e..801b92c 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -15,8 +15,6 @@ ifneq ($(MACHINE),)
 include $(srctree)/$(MACHINE)/Makefile.boot
 endif
 
-include $(srctree)/arch/arm/boot/dts/Makefile
-
 # Note: the following conditions must always be true:
 #   ZRELADDR == virt_to_phys(PAGE_OFFSET + TEXT_OFFSET)
 #   PARAMS_PHYS must be within 4MB of ZRELADDR
@@ -59,16 +57,6 @@ $(obj)/zImage:	$(obj)/compressed/vmlinux FORCE
 
 endif
 
-targets += $(dtb-y)
-
-# Rule to build device tree blobs
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
-$(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y))
-
-clean-files := *.dtb
-
 ifneq ($(LOADADDR),)
   UIMAGE_LOADADDR=$(LOADADDR)
 else
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index f37cf9f..2aef042 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -104,4 +104,8 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \
 	wm8505-ref.dtb \
 	wm8650-mid.dtb
 
+targets += dtbs
 endif
+
+dtbs: $(addprefix $(obj)/, $(dtb-y))
+clean-files := *.dtb
-- 
1.7.10.4

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Stephen Warren]

On 11/15/2012 04:51 AM, Grant Likely wrote:
> Grant Likely wrote:
>> Or how about: I could pick up the patch with only the MIPS hunk and
>> every other user can be fixed up independently to use the new rule.
> 
> Here's a trial patch to fix up ARM. Does this look correct? This patch
> depends on the generic dtb build rule already being applied.

I think the patch looks OK technically, except for one minor comment below.

One issue with this patch is that it moves *.dts from arch/arm/boot to
arch/arm/boot/dts, which means everyone has to adjust their scripts/...
that package/install/... the kernel. I guess it's an easy change for
people to make, but could easily catch people unawares if they do
incremental builds so that arch/arm/boot/*.dtb still exists but is stale.

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index f37cf9f..2aef042 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -104,4 +104,8 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \
>  	wm8505-ref.dtb \
>  	wm8650-mid.dtb
>  
> +targets += dtbs

Doesn't that make the "dtbs" target always run by default? Perhaps
that's reasonable though, and doesn't actually affect anything since the
make command for this directory always specifies an explicit target?

Or, was that meant to be the following that got removed from ../Makefile?

targets += $(dtb-y)

>  endif
> +
> +dtbs: $(addprefix $(obj)/, $(dtb-y))
> +clean-files := *.dtb

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

On Thu, Nov 15, 2012 at 6:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 11/15/2012 04:51 AM, Grant Likely wrote:
>> Grant Likely wrote:
>>> Or how about: I could pick up the patch with only the MIPS hunk and
>>> every other user can be fixed up independently to use the new rule.
>>
>> Here's a trial patch to fix up ARM. Does this look correct? This patch
>> depends on the generic dtb build rule already being applied.
>
> I think the patch looks OK technically, except for one minor comment below.
>
> One issue with this patch is that it moves *.dts from arch/arm/boot to
> arch/arm/boot/dts, which means everyone has to adjust their scripts/...
> that package/install/... the kernel. I guess it's an easy change for
> people to make, but could easily catch people unawares if they do
> incremental builds so that arch/arm/boot/*.dtb still exists but is stale.

True. We could temporarily remove or rename if the same file exists in
the directory below to help people catch that problem. I really would
like to clean up that build rule to be consistent though.

The other option is to move all the .dts files into the boot
directory, but I don't think that is a good idea at all.

>> +targets += dtbs
>
> Doesn't that make the "dtbs" target always run by default? Perhaps
> that's reasonable though, and doesn't actually affect anything since the
> make command for this directory always specifies an explicit target?
>
> Or, was that meant to be the following that got removed from ../Makefile?
>
> targets += $(dtb-y)

Yes it is supposed to be the same thing. Doesn't it effectively do the
same since dtbs depends on $(dtb-y)?

g.

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Stephen Warren]

On 11/15/2012 11:20 AM, Grant Likely wrote:
> On Thu, Nov 15, 2012 at 6:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 11/15/2012 04:51 AM, Grant Likely wrote:
>>> Grant Likely wrote:
>>>> Or how about: I could pick up the patch with only the MIPS hunk and
>>>> every other user can be fixed up independently to use the new rule.
>>>
>>> Here's a trial patch to fix up ARM. Does this look correct? This patch
>>> depends on the generic dtb build rule already being applied.
>>
>> I think the patch looks OK technically, except for one minor comment below.
>>
>> One issue with this patch is that it moves *.dts from arch/arm/boot to
>> arch/arm/boot/dts, which means everyone has to adjust their scripts/...
>> that package/install/... the kernel. I guess it's an easy change for
>> people to make, but could easily catch people unawares if they do
>> incremental builds so that arch/arm/boot/*.dtb still exists but is stale.
> 
> True. We could temporarily remove or rename if the same file exists in
> the directory below to help people catch that problem. I really would
> like to clean up that build rule to be consistent though.
> 
> The other option is to move all the .dts files into the boot
> directory, but I don't think that is a good idea at all.

Maybe we can just add "rm *.dtb" to the following rules in
boot/Makefile, before calling the child make?

%.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@

 dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs

>>> +targets += dtbs
>>
>> Doesn't that make the "dtbs" target always run by default? Perhaps
>> that's reasonable though, and doesn't actually affect anything since the
>> make command for this directory always specifies an explicit target?
>>
>> Or, was that meant to be the following that got removed from ../Makefile?
>>
>> targets += $(dtb-y)
> 
> Yes it is supposed to be the same thing. Doesn't it effectively do the
> same since dtbs depends on $(dtb-y)?

Ah, I think so yes.

I guess anyway that $(targets) is presumably ignored if an explicit
build target is requested from make.

Re: [PATCH V5 1/2] kbuild: centralize .dts->.dtb rule

[Grant Likely]

On Thu, 15 Nov 2012 11:31:38 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 11/15/2012 11:20 AM, Grant Likely wrote:
> > On Thu, Nov 15, 2012 at 6:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> On 11/15/2012 04:51 AM, Grant Likely wrote:
> >>> Grant Likely wrote:
> >>>> Or how about: I could pick up the patch with only the MIPS hunk and
> >>>> every other user can be fixed up independently to use the new rule.
> >>>
> >>> Here's a trial patch to fix up ARM. Does this look correct? This patch
> >>> depends on the generic dtb build rule already being applied.
> >>
> >> I think the patch looks OK technically, except for one minor comment below.
> >>
> >> One issue with this patch is that it moves *.dts from arch/arm/boot to
> >> arch/arm/boot/dts, which means everyone has to adjust their scripts/...
> >> that package/install/... the kernel. I guess it's an easy change for
> >> people to make, but could easily catch people unawares if they do
> >> incremental builds so that arch/arm/boot/*.dtb still exists but is stale.
> > 
> > True. We could temporarily remove or rename if the same file exists in
> > the directory below to help people catch that problem. I really would
> > like to clean up that build rule to be consistent though.
> > 
> > The other option is to move all the .dts files into the boot
> > directory, but I don't think that is a good idea at all.
> 
> Maybe we can just add "rm *.dtb" to the following rules in
> boot/Makefile, before calling the child make?
> 
> %.dtb: scripts
> -	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
> +	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@
> 
>  dtbs: scripts
> -	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/$@
> +	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) dtbs
> 
> >>> +targets += dtbs
> >>
> >> Doesn't that make the "dtbs" target always run by default? Perhaps
> >> that's reasonable though, and doesn't actually affect anything since the
> >> make command for this directory always specifies an explicit target?
> >>
> >> Or, was that meant to be the following that got removed from ../Makefile?
> >>
> >> targets += $(dtb-y)
> > 
> > Yes it is supposed to be the same thing. Doesn't it effectively do the
> > same since dtbs depends on $(dtb-y)?
> 
> Ah, I think so yes.
> 
> I guess anyway that $(targets) is presumably ignored if an explicit
> build target is requested from make.

Can you pull into your series and get it working. The merge window is
going to open this week, so I have to push it back to v3.9 anyway.

g.

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.