Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

forgot to CC linux-arch

On 04/03/2013 12:42 PM, Vineet Gupta wrote:
> Currently, for every ARC kernel build I see the following:
>
> --------------->8-----------------
>   DTB    arch/arc/boot/dts/angel4.dtb.S
>   AS      arch/arc/boot/dts/angel4.dtb.o
>   LD      arch/arc/boot/dts/built-in.o
> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>   CHK     kernel/config_data.h
> --------------->8-----------------
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Cc: Michal Marek <mmarek@suse.cz>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  scripts/Makefile.lib |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 07125e6..17f96f4 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -265,6 +265,8 @@ cmd_dt_S_dtb=						\
>  	echo '.balign STRUCT_ALIGNMENT'; 		\
>  ) > $@
>  
> +.PRECIOUS: $(obj)/%.dtb.S
> +
>  $(obj)/%.dtb.S: $(obj)/%.dtb
>  	$(call cmd,dt_S_dtb)
>  

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

forgot to CC linux-arch

On 04/03/2013 12:42 PM, Vineet Gupta wrote:
> Currently, for every ARC kernel build I see the following:
>
> --------------->8-----------------
>   DTB    arch/arc/boot/dts/angel4.dtb.S
>   AS      arch/arc/boot/dts/angel4.dtb.o
>   LD      arch/arc/boot/dts/built-in.o
> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>   CHK     kernel/config_data.h
> --------------->8-----------------
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Cc: Michal Marek <mmarek@suse.cz>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  scripts/Makefile.lib |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 07125e6..17f96f4 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -265,6 +265,8 @@ cmd_dt_S_dtb=						\
>  	echo '.balign STRUCT_ALIGNMENT'; 		\
>  ) > $@
>  
> +.PRECIOUS: $(obj)/%.dtb.S
> +
>  $(obj)/%.dtb.S: $(obj)/%.dtb
>  	$(call cmd,dt_S_dtb)
>  

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/03/2013 01:14 AM, Vineet Gupta wrote:
> forgot to CC linux-arch
> 
> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>> Currently, for every ARC kernel build I see the following:
>>
>> --------------->8-----------------
>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>   LD      arch/arc/boot/dts/built-in.o
>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>   CHK     kernel/config_data.h
>> --------------->8-----------------

I assume that's because the file is an intermediate file, and only built
due to a chain of build rules, and hence make clean it up itself after
the build?

>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib

>> +.PRECIOUS: $(obj)/%.dtb.S
>> +
>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>  	$(call cmd,dt_S_dtb)

I'm not sure if .PRECIOUS is correct here. That prevents make from
deleting the file if make is CTRL-C'd in the middle of generating it.
Couldn't that leave a stale/corrupt file around that'd break the build.
Judging by:

http://www.gnu.org/software/make/manual/html_node/Special-Targets.html

I think .SECONDARY might be a better choice? Does that solve the problem
you're seeing?

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/03/2013 09:48 PM, Stephen Warren wrote:
> On 04/03/2013 01:14 AM, Vineet Gupta wrote:
>> forgot to CC linux-arch
>>
>> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>>> Currently, for every ARC kernel build I see the following:
>>>
>>> --------------->8-----------------
>>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>>   LD      arch/arc/boot/dts/built-in.o
>>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>>   CHK     kernel/config_data.h
>>> --------------->8-----------------
> I assume that's because the file is an intermediate file, and only built
> due to a chain of build rules, and hence make clean it up itself after
> the build?

Indeed - I should have made that explicit in the Changelog.

>>> +.PRECIOUS: $(obj)/%.dtb.S
>>> +
>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>  	$(call cmd,dt_S_dtb)
> I'm not sure if .PRECIOUS is correct here. That prevents make from
> deleting the file if make is CTRL-C'd in the middle of generating it.
> Couldn't that leave a stale/corrupt file around that'd break the build.
> Judging by:
>
> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>
> I think .SECONDARY might be a better choice? Does that solve the problem
> you're seeing?

Technically .SECONDARY is better - however it doesn't seem to work.
Running make with various debug toggles doesn't seem to be helping with why
.PRECIOUS works but not this.
That is also likely reason for a bunch of other .PRECIOUS entries in the same file
but no .SECONDARY.

I presume this is something not specific to ARC kernel or my host.
Can you try the .SECONDARY locally to see if it works for you.

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/03/2013 09:48 PM, Stephen Warren wrote:
> On 04/03/2013 01:14 AM, Vineet Gupta wrote:
>> forgot to CC linux-arch
>>
>> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>>> Currently, for every ARC kernel build I see the following:
>>>
>>> --------------->8-----------------
>>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>>   LD      arch/arc/boot/dts/built-in.o
>>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>>   CHK     kernel/config_data.h
>>> --------------->8-----------------
> I assume that's because the file is an intermediate file, and only built
> due to a chain of build rules, and hence make clean it up itself after
> the build?

Indeed - I should have made that explicit in the Changelog.

>>> +.PRECIOUS: $(obj)/%.dtb.S
>>> +
>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>  	$(call cmd,dt_S_dtb)
> I'm not sure if .PRECIOUS is correct here. That prevents make from
> deleting the file if make is CTRL-C'd in the middle of generating it.
> Couldn't that leave a stale/corrupt file around that'd break the build.
> Judging by:
>
> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>
> I think .SECONDARY might be a better choice? Does that solve the problem
> you're seeing?

Technically .SECONDARY is better - however it doesn't seem to work.
Running make with various debug toggles doesn't seem to be helping with why
.PRECIOUS works but not this.
That is also likely reason for a bunch of other .PRECIOUS entries in the same file
but no .SECONDARY.

I presume this is something not specific to ARC kernel or my host.
Can you try the .SECONDARY locally to see if it works for you.

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/03/2013 11:30 PM, Vineet Gupta wrote:
> On 04/03/2013 09:48 PM, Stephen Warren wrote:
>> On 04/03/2013 01:14 AM, Vineet Gupta wrote:
>>> forgot to CC linux-arch
>>>
>>> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>>>> Currently, for every ARC kernel build I see the following:
>>>>
>>>> --------------->8-----------------
>>>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>>>   LD      arch/arc/boot/dts/built-in.o
>>>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>>>   CHK     kernel/config_data.h
>>>> --------------->8-----------------
>> I assume that's because the file is an intermediate file, and only built
>> due to a chain of build rules, and hence make clean it up itself after
>> the build?
> 
> Indeed - I should have made that explicit in the Changelog.
> 
>>>> +.PRECIOUS: $(obj)/%.dtb.S
>>>> +
>>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>>  	$(call cmd,dt_S_dtb)
>> I'm not sure if .PRECIOUS is correct here. That prevents make from
>> deleting the file if make is CTRL-C'd in the middle of generating it.
>> Couldn't that leave a stale/corrupt file around that'd break the build.
>> Judging by:
>>
>> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>>
>> I think .SECONDARY might be a better choice? Does that solve the problem
>> you're seeing?
> 
> Technically .SECONDARY is better - however it doesn't seem to work.

Hmmm. It does for me.

$ make --version
GNU Make 3.81

I hacked the ARM makefiles as follows:

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 4737408..70247c6 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -256,6 +256,7 @@ core-y                              +=
arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
 core-y                         += arch/arm/net/
 core-y                         += arch/arm/crypto/
 core-y                         += $(machdirs) $(platdirs)
+core-y                         += arch/arm/boot/dts/

 drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index dedca49..af2202e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -197,3 +197,5 @@ dtbs: $(addprefix $(obj)/, $(dtb-y))
        $(Q)rm -f $(obj)/../*.dtb

 clean-files := *.dtb
+
+obj-y += tegra20-test.dtb.o

and manually created a tegra20-test.dts. For reasons I didn't bother
investigating, the .dtb.S -> .dtb.o conversion failed with syntax
errors, and because the .dtb.S file was an intermediate file, make rm's
it in this case. That's exactly the issue you're seeing.

I then added the following to Makefile.lib:

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a0ab6d7..fc11a67 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -258,6 +258,8 @@ cmd_dt_S_dtb=
        \
        echo '.balign STRUCT_ALIGNMENT';                \
 ) > $@

+.SECONDARY: $(obj)/%.dtb.S
+
 $(obj)/%.dtb.S: $(obj)/%.dtb
        $(call cmd,dt_S_dtb)

and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
behaving as expected, and should fix your problem.

One other problem this highlighted: Unless you also mark the generated
.dtb file as PRECIOUS/SECONDARY (or unless that DTB is included in
targets for some reason) then make also rm's that, since it considers it
intermediate:

  DTC     arch/arm/boot/dts/tegra20-test.dtb
  DTB    arch/arm/boot/dts/tegra20-test.dtb.S
  AS      arch/arm/boot/dts/tegra20-test.dtb.o
[error]
rm arch/arm/boot/dts/tegra20-test.dtb

So, your patch would probably need modification to apply the fix to
*.dtb too, since they might also be intermediate?

Perhaps add something like the following to arch/arc/boot/dts/Makefile?

.SECONDARY: $(obj)/$(builtindtb-y).dtb

> Running make with various debug toggles doesn't seem to be helping with why
> .PRECIOUS works but not this.
> That is also likely reason for a bunch of other .PRECIOUS entries in the same file
> but no .SECONDARY.

The existing .PRECIOUS exist to support lexer/yacc files, for which both
the source .l/.y files are checked in, and the generated
%.lex.c_shipped/%.tab.[cl]_shipped are also checked in. In this case,
the "_shipped" files must be marked PRECIOUS, since they're under source
control and hence should never be deleted. I don't believ this same
situation applies to the .dtb.S files you're having problems with, so I
don't think .PRECIOUS is correct for them.

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/03/2013 11:30 PM, Vineet Gupta wrote:
> On 04/03/2013 09:48 PM, Stephen Warren wrote:
>> On 04/03/2013 01:14 AM, Vineet Gupta wrote:
>>> forgot to CC linux-arch
>>>
>>> On 04/03/2013 12:42 PM, Vineet Gupta wrote:
>>>> Currently, for every ARC kernel build I see the following:
>>>>
>>>> --------------->8-----------------
>>>>   DTB    arch/arc/boot/dts/angel4.dtb.S
>>>>   AS      arch/arc/boot/dts/angel4.dtb.o
>>>>   LD      arch/arc/boot/dts/built-in.o
>>>> rm arch/arc/boot/dts/angel4.dtb.S        <-- forces rebuild next iter
>>>>   CHK     kernel/config_data.h
>>>> --------------->8-----------------
>> I assume that's because the file is an intermediate file, and only built
>> due to a chain of build rules, and hence make clean it up itself after
>> the build?
> 
> Indeed - I should have made that explicit in the Changelog.
> 
>>>> +.PRECIOUS: $(obj)/%.dtb.S
>>>> +
>>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>>  	$(call cmd,dt_S_dtb)
>> I'm not sure if .PRECIOUS is correct here. That prevents make from
>> deleting the file if make is CTRL-C'd in the middle of generating it.
>> Couldn't that leave a stale/corrupt file around that'd break the build.
>> Judging by:
>>
>> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>>
>> I think .SECONDARY might be a better choice? Does that solve the problem
>> you're seeing?
> 
> Technically .SECONDARY is better - however it doesn't seem to work.

Hmmm. It does for me.

$ make --version
GNU Make 3.81

I hacked the ARM makefiles as follows:

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 4737408..70247c6 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -256,6 +256,7 @@ core-y                              +=
arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
 core-y                         += arch/arm/net/
 core-y                         += arch/arm/crypto/
 core-y                         += $(machdirs) $(platdirs)
+core-y                         += arch/arm/boot/dts/

 drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index dedca49..af2202e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -197,3 +197,5 @@ dtbs: $(addprefix $(obj)/, $(dtb-y))
        $(Q)rm -f $(obj)/../*.dtb

 clean-files := *.dtb
+
+obj-y += tegra20-test.dtb.o

and manually created a tegra20-test.dts. For reasons I didn't bother
investigating, the .dtb.S -> .dtb.o conversion failed with syntax
errors, and because the .dtb.S file was an intermediate file, make rm's
it in this case. That's exactly the issue you're seeing.

I then added the following to Makefile.lib:

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a0ab6d7..fc11a67 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -258,6 +258,8 @@ cmd_dt_S_dtb=
        \
        echo '.balign STRUCT_ALIGNMENT';                \
 ) > $@

+.SECONDARY: $(obj)/%.dtb.S
+
 $(obj)/%.dtb.S: $(obj)/%.dtb
        $(call cmd,dt_S_dtb)

and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
behaving as expected, and should fix your problem.

One other problem this highlighted: Unless you also mark the generated
.dtb file as PRECIOUS/SECONDARY (or unless that DTB is included in
targets for some reason) then make also rm's that, since it considers it
intermediate:

  DTC     arch/arm/boot/dts/tegra20-test.dtb
  DTB    arch/arm/boot/dts/tegra20-test.dtb.S
  AS      arch/arm/boot/dts/tegra20-test.dtb.o
[error]
rm arch/arm/boot/dts/tegra20-test.dtb

So, your patch would probably need modification to apply the fix to
*.dtb too, since they might also be intermediate?

Perhaps add something like the following to arch/arc/boot/dts/Makefile?

.SECONDARY: $(obj)/$(builtindtb-y).dtb

> Running make with various debug toggles doesn't seem to be helping with why
> .PRECIOUS works but not this.
> That is also likely reason for a bunch of other .PRECIOUS entries in the same file
> but no .SECONDARY.

The existing .PRECIOUS exist to support lexer/yacc files, for which both
the source .l/.y files are checked in, and the generated
%.lex.c_shipped/%.tab.[cl]_shipped are also checked in. In this case,
the "_shipped" files must be marked PRECIOUS, since they're under source
control and hence should never be deleted. I don't believ this same
situation applies to the .dtb.S files you're having problems with, so I
don't think .PRECIOUS is correct for them.

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/04/2013 11:06 PM, Stephen Warren wrote:
>
>> Technically .SECONDARY is better - however it doesn't seem to work.
> Hmmm. It does for me.
>
> $ make --version
> GNU Make 3.81

Same tools here !

$ make -v
GNU Make 3.81


> I hacked the ARM makefiles as follows:
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 4737408..70247c6 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -256,6 +256,7 @@ core-y                              +=
> arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
>  core-y                         += arch/arm/net/
>  core-y                         += arch/arm/crypto/
>  core-y                         += $(machdirs) $(platdirs)
> +core-y                         += arch/arm/boot/dts/
>
>  drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index dedca49..af2202e 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -197,3 +197,5 @@ dtbs: $(addprefix $(obj)/, $(dtb-y))
>         $(Q)rm -f $(obj)/../*.dtb
>
>  clean-files := *.dtb
> +
> +obj-y += tegra20-test.dtb.o
>
> and manually created a tegra20-test.dts. For reasons I didn't bother
> investigating, the .dtb.S -> .dtb.o conversion failed with syntax
> errors, and because the .dtb.S file was an intermediate file, make rm's
> it in this case. That's exactly the issue you're seeing.
>
> I then added the following to Makefile.lib:
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index a0ab6d7..fc11a67 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -258,6 +258,8 @@ cmd_dt_S_dtb=
>         \
>         echo '.balign STRUCT_ALIGNMENT';                \
>  ) > $@
>
> +.SECONDARY: $(obj)/%.dtb.S
> +
>  $(obj)/%.dtb.S: $(obj)/%.dtb
>         $(call cmd,dt_S_dtb)
>
> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
> behaving as expected, and should fix your problem.

Nope, doesn't work for me. The slight difference in build system could be that ARC
as of now only supports vmlinux embedded dtbs (so that likely changes what is
intermediate from make's perspective and what is not).

James, I remeber u once mentioned the rm of *.dtb.S file for metag as well - can
you please try adding the .secondary/.precious to makefile.lib to confirm what
fixes deletion of intermediate .S files for you.

> One other problem this highlighted: Unless you also mark the generated
> .dtb file as PRECIOUS/SECONDARY (or unless that DTB is included in
> targets for some reason) then make also rm's that, since it considers it
> intermediate:
>
>   DTC     arch/arm/boot/dts/tegra20-test.dtb
>   DTB    arch/arm/boot/dts/tegra20-test.dtb.S
>   AS      arch/arm/boot/dts/tegra20-test.dtb.o
> [error]
> rm arch/arm/boot/dts/tegra20-test.dtb
>
> So, your patch would probably need modification to apply the fix to
> *.dtb too, since they might also be intermediate?
>
> Perhaps add something like the following to arch/arc/boot/dts/Makefile?
>
> .SECONDARY: $(obj)/$(builtindtb-y).dtb

angel4.dtb is not deleted in my case.
If you add tegra20-test.dtb to targets (just as I do in dts/Makefile) what do u see !

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/04/2013 11:06 PM, Stephen Warren wrote:
>
>> Technically .SECONDARY is better - however it doesn't seem to work.
> Hmmm. It does for me.
>
> $ make --version
> GNU Make 3.81

Same tools here !

$ make -v
GNU Make 3.81


> I hacked the ARM makefiles as follows:
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 4737408..70247c6 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -256,6 +256,7 @@ core-y                              +=
> arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
>  core-y                         += arch/arm/net/
>  core-y                         += arch/arm/crypto/
>  core-y                         += $(machdirs) $(platdirs)
> +core-y                         += arch/arm/boot/dts/
>
>  drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index dedca49..af2202e 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -197,3 +197,5 @@ dtbs: $(addprefix $(obj)/, $(dtb-y))
>         $(Q)rm -f $(obj)/../*.dtb
>
>  clean-files := *.dtb
> +
> +obj-y += tegra20-test.dtb.o
>
> and manually created a tegra20-test.dts. For reasons I didn't bother
> investigating, the .dtb.S -> .dtb.o conversion failed with syntax
> errors, and because the .dtb.S file was an intermediate file, make rm's
> it in this case. That's exactly the issue you're seeing.
>
> I then added the following to Makefile.lib:
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index a0ab6d7..fc11a67 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -258,6 +258,8 @@ cmd_dt_S_dtb=
>         \
>         echo '.balign STRUCT_ALIGNMENT';                \
>  ) > $@
>
> +.SECONDARY: $(obj)/%.dtb.S
> +
>  $(obj)/%.dtb.S: $(obj)/%.dtb
>         $(call cmd,dt_S_dtb)
>
> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
> behaving as expected, and should fix your problem.

Nope, doesn't work for me. The slight difference in build system could be that ARC
as of now only supports vmlinux embedded dtbs (so that likely changes what is
intermediate from make's perspective and what is not).

James, I remeber u once mentioned the rm of *.dtb.S file for metag as well - can
you please try adding the .secondary/.precious to makefile.lib to confirm what
fixes deletion of intermediate .S files for you.

> One other problem this highlighted: Unless you also mark the generated
> .dtb file as PRECIOUS/SECONDARY (or unless that DTB is included in
> targets for some reason) then make also rm's that, since it considers it
> intermediate:
>
>   DTC     arch/arm/boot/dts/tegra20-test.dtb
>   DTB    arch/arm/boot/dts/tegra20-test.dtb.S
>   AS      arch/arm/boot/dts/tegra20-test.dtb.o
> [error]
> rm arch/arm/boot/dts/tegra20-test.dtb
>
> So, your patch would probably need modification to apply the fix to
> *.dtb too, since they might also be intermediate?
>
> Perhaps add something like the following to arch/arc/boot/dts/Makefile?
>
> .SECONDARY: $(obj)/$(builtindtb-y).dtb

angel4.dtb is not deleted in my case.
If you add tegra20-test.dtb to targets (just as I do in dts/Makefile) what do u see !

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/04/2013 11:06 PM, Stephen Warren wrote:

> +.SECONDARY: $(obj)/%.dtb.S
> +
>  $(obj)/%.dtb.S: $(obj)/%.dtb
>         $(call cmd,dt_S_dtb)
> 
> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
> behaving as expected, and should fix your problem.

Interestingly, if I make the file name explicit, .SECONDARY works for me too.

.SECONDARY: $(obj)/angel4.dtb.S

http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal

Is this a make bug or is it related to when the rule is parsed by make !

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/09/2013 07:40 PM, Vineet Gupta wrote:
> On 04/04/2013 11:06 PM, Stephen Warren wrote:
>
>> +.SECONDARY: $(obj)/%.dtb.S
>> +
>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>         $(call cmd,dt_S_dtb)
>>
>> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
>> behaving as expected, and should fix your problem.
> Interestingly, if I make the file name explicit, .SECONDARY works for me too.
>
> .SECONDARY: $(obj)/angel4.dtb.S
>
> http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal
>
> Is this a make bug or is it related to when the rule is parsed by make !
>
> -Vineet

Ping ? Anyone know how to resolve this

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/09/2013 07:40 PM, Vineet Gupta wrote:
> On 04/04/2013 11:06 PM, Stephen Warren wrote:
>
>> +.SECONDARY: $(obj)/%.dtb.S
>> +
>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>         $(call cmd,dt_S_dtb)
>>
>> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
>> behaving as expected, and should fix your problem.
> Interestingly, if I make the file name explicit, .SECONDARY works for me too.
>
> .SECONDARY: $(obj)/angel4.dtb.S
>
> http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal
>
> Is this a make bug or is it related to when the rule is parsed by make !
>
> -Vineet

Ping ? Anyone know how to resolve this

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/12/2013 01:40 AM, Vineet Gupta wrote:
> On 04/09/2013 07:40 PM, Vineet Gupta wrote:
>> On 04/04/2013 11:06 PM, Stephen Warren wrote:
>>
>>> +.SECONDARY: $(obj)/%.dtb.S
>>> +
>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>         $(call cmd,dt_S_dtb)
>>>
>>> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
>>> behaving as expected, and should fix your problem.
>> Interestingly, if I make the file name explicit, .SECONDARY works for me too.
>>
>> .SECONDARY: $(obj)/angel4.dtb.S
>>
>> http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal
>>
>> Is this a make bug or is it related to when the rule is parsed by make !
>>
>> -Vineet
> 
> Ping ? Anyone know how to resolve this

After installing the ARC toolchain (which was a bit painful to track
down and install...) I reproduced your exact problem. I believe the
patch below fixes it:

diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
index 5776835..2f2cf23 100644
--- a/arch/arc/boot/dts/Makefile
+++ b/arch/arc/boot/dts/Makefile
@@ -8,6 +8,8 @@ endif
 obj-y   += $(builtindtb-y).dtb.o
 targets += $(builtindtb-y).dtb

+.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
+
 dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

 clean-files := *.dtb

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/12/2013 01:40 AM, Vineet Gupta wrote:
> On 04/09/2013 07:40 PM, Vineet Gupta wrote:
>> On 04/04/2013 11:06 PM, Stephen Warren wrote:
>>
>>> +.SECONDARY: $(obj)/%.dtb.S
>>> +
>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>         $(call cmd,dt_S_dtb)
>>>
>>> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
>>> behaving as expected, and should fix your problem.
>> Interestingly, if I make the file name explicit, .SECONDARY works for me too.
>>
>> .SECONDARY: $(obj)/angel4.dtb.S
>>
>> http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal
>>
>> Is this a make bug or is it related to when the rule is parsed by make !
>>
>> -Vineet
> 
> Ping ? Anyone know how to resolve this

After installing the ARC toolchain (which was a bit painful to track
down and install...) I reproduced your exact problem. I believe the
patch below fixes it:

diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
index 5776835..2f2cf23 100644
--- a/arch/arc/boot/dts/Makefile
+++ b/arch/arc/boot/dts/Makefile
@@ -8,6 +8,8 @@ endif
 obj-y   += $(builtindtb-y).dtb.o
 targets += $(builtindtb-y).dtb

+.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
+
 dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

 clean-files := *.dtb

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/13/2013 03:22 AM, Stephen Warren wrote:
>
> After installing the ARC toolchain (which was a bit painful to track
> down and install...) 

Sorry about that - we have a buildroot based distro about to be pushed upstream -
that would make it easier.

> I reproduced your exact problem. I believe the
> patch below fixes it:
>
> diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
> index 5776835..2f2cf23 100644
> --- a/arch/arc/boot/dts/Makefile
> +++ b/arch/arc/boot/dts/Makefile
> @@ -8,6 +8,8 @@ endif
>  obj-y   += $(builtindtb-y).dtb.o
>  targets += $(builtindtb-y).dtb
>
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>
>  clean-files := *.dtb

Indeed it does - I fell stupid why this didn't occur to me. But given that you
have dealt with the dtb Makefile stuff alot more than I have :-) do you know why
it was not working when put in Makefile.lib because I feel a few other arches also
suffer from the same issue and would need similar fixes.

Anyhow, for the patch, I can manually add --author="you" but it'll still lack your
SOB - you OK with that or do you want to send a formal patch.

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/13/2013 03:22 AM, Stephen Warren wrote:
>
> After installing the ARC toolchain (which was a bit painful to track
> down and install...) 

Sorry about that - we have a buildroot based distro about to be pushed upstream -
that would make it easier.

> I reproduced your exact problem. I believe the
> patch below fixes it:
>
> diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
> index 5776835..2f2cf23 100644
> --- a/arch/arc/boot/dts/Makefile
> +++ b/arch/arc/boot/dts/Makefile
> @@ -8,6 +8,8 @@ endif
>  obj-y   += $(builtindtb-y).dtb.o
>  targets += $(builtindtb-y).dtb
>
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>
>  clean-files := *.dtb

Indeed it does - I fell stupid why this didn't occur to me. But given that you
have dealt with the dtb Makefile stuff alot more than I have :-) do you know why
it was not working when put in Makefile.lib because I feel a few other arches also
suffer from the same issue and would need similar fixes.

Anyhow, for the patch, I can manually add --author="you" but it'll still lack your
SOB - you OK with that or do you want to send a formal patch.

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Stephen Warren]

On 04/15/2013 07:59 AM, Vineet Gupta wrote:
> On 04/13/2013 03:22 AM, Stephen Warren wrote:
>>
>> After installing the ARC toolchain (which was a bit painful to track
>> down and install...) 
> 
> Sorry about that - we have a buildroot based distro about to be pushed upstream -
> that would make it easier.
> 
>> I reproduced your exact problem. I believe the
>> patch below fixes it:
>>
>> diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
>> index 5776835..2f2cf23 100644
>> --- a/arch/arc/boot/dts/Makefile
>> +++ b/arch/arc/boot/dts/Makefile
>> @@ -8,6 +8,8 @@ endif
>>  obj-y   += $(builtindtb-y).dtb.o
>>  targets += $(builtindtb-y).dtb
>>
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>>
>>  clean-files := *.dtb
> 
> Indeed it does - I fell stupid why this didn't occur to me. But given that you
> have dealt with the dtb Makefile stuff alot more than I have :-) do you know why
> it was not working when put in Makefile.lib because I feel a few other arches also
> suffer from the same issue and would need similar fixes.

Searching in Google implies that .SECONDARY doesn't work with wildcards
(%.dtb.S for example), whereas .PRECIOUS does. Seems like perhaps a bug
in make to me, but who knows. Perhaps that's why?

> Anyhow, for the patch, I can manually add --author="you" but it'll still lack your
> SOB - you OK with that or do you want to send a formal patch.

Sure, you can either just say Suggested-by: for me, or apply my s-o-b below:

Signed-off-by: Stephen Warren <swarren@nvidia.com>

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 12/04/13 22:52, Stephen Warren wrote:
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S

Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
since it'll have quotes around it, so you may instead need:
.SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S

(at least that's what's required for the metag equivalent)

> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

You might find the same thing here too.

Cheers
James

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 12/04/13 22:52, Stephen Warren wrote:
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S

Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
since it'll have quotes around it, so you may instead need:
.SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S

(at least that's what's required for the metag equivalent)

> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

You might find the same thing here too.

Cheers
James

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 16/04/13 16:53, James Hogan wrote:
> On 12/04/13 22:52, Stephen Warren wrote:
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> 
> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
> since it'll have quotes around it, so you may instead need:
> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
> 
> (at least that's what's required for the metag equivalent)
> 
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
> 
> You might find the same thing here too.

Also, I think you probably now want *.dtb.S added to clean-files,
otherwise they won't get removed by make clean.

Cheers
James

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 16/04/13 16:53, James Hogan wrote:
> On 12/04/13 22:52, Stephen Warren wrote:
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> 
> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
> since it'll have quotes around it, so you may instead need:
> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
> 
> (at least that's what's required for the metag equivalent)
> 
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
> 
> You might find the same thing here too.

Also, I think you probably now want *.dtb.S added to clean-files,
otherwise they won't get removed by make clean.

Cheers
James

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/16/2013 09:32 PM, James Hogan wrote:
>
> Also, I think you probably now want *.dtb.S added to clean-files,
> otherwise they won't get removed by make clean.

Good catch !

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

On 04/16/2013 09:32 PM, James Hogan wrote:
>
> Also, I think you probably now want *.dtb.S added to clean-files,
> otherwise they won't get removed by make clean.

Good catch !

Thx,
-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[Vineet Gupta]

Hi James,

On 04/16/2013 09:23 PM, James Hogan wrote:
> On 12/04/13 22:52, Stephen Warren wrote:
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
> since it'll have quotes around it, so you may instead need:
> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
>
> (at least that's what's required for the metag equivalent)
>
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
> You might find the same thing here too.

Actually in my Makefile, the quotes are stripped off in the very beginning to
avoid duplicating it in every place.

ifneq ($(CONFIG_ARC_BUILTIN_DTB_NAME),"")
    builtindtb-y    := $(patsubst "%",%,$(CONFIG_ARC_BUILTIN_DTB_NAME))
endif
....

Thus both the above are not required - redundant if at all.

-Vineet

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 17/04/13 05:13, Vineet Gupta wrote:
> Hi James,
> 
> On 04/16/2013 09:23 PM, James Hogan wrote:
>> On 12/04/13 22:52, Stephen Warren wrote:
>>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
>> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
>> since it'll have quotes around it, so you may instead need:
>> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
>>
>> (at least that's what's required for the metag equivalent)
>>
>>> +
>>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>> You might find the same thing here too.
> 
> Actually in my Makefile, the quotes are stripped off in the very beginning to
> avoid duplicating it in every place.
> 
> ifneq ($(CONFIG_ARC_BUILTIN_DTB_NAME),"")
>     builtindtb-y    := $(patsubst "%",%,$(CONFIG_ARC_BUILTIN_DTB_NAME))
> endif

Ah yes, I didn't notice that difference. I'll do the same thing for
metag (it only had 1 reference to it before).

Thanks
James

Re: [PATCH] Kbuild: Avoid DTB rebuilds if source files are untouched

[James Hogan]

On 17/04/13 05:13, Vineet Gupta wrote:
> Hi James,
> 
> On 04/16/2013 09:23 PM, James Hogan wrote:
>> On 12/04/13 22:52, Stephen Warren wrote:
>>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
>> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
>> since it'll have quotes around it, so you may instead need:
>> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
>>
>> (at least that's what's required for the metag equivalent)
>>
>>> +
>>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>> You might find the same thing here too.
> 
> Actually in my Makefile, the quotes are stripped off in the very beginning to
> avoid duplicating it in every place.
> 
> ifneq ($(CONFIG_ARC_BUILTIN_DTB_NAME),"")
>     builtindtb-y    := $(patsubst "%",%,$(CONFIG_ARC_BUILTIN_DTB_NAME))
> endif

Ah yes, I didn't notice that difference. I'll do the same thing for
metag (it only had 1 reference to it before).

Thanks
James