[PATCH] kbuild: fix dependency of dtbs targets

[PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

The target "dtbs" should depend on "scripts" because it needs to
build dtc.  The "prepare" target is unneeded here.

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

 arch/arm/Makefile   | 2 +-
 arch/arm64/Makefile | 2 +-
 arch/nios2/Makefile | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 47d3a1a..cdb5b55 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -342,7 +342,7 @@ $(INSTALL_TARGETS):
 
 PHONY += dtbs dtbs_install
 
-dtbs: prepare scripts
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(boot)/dts
 
 dtbs_install:
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 939b310..fd56fbc 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -133,7 +133,7 @@ zinstall install:
 
 PHONY += dtbs dtbs_install
 
-dtbs: prepare scripts
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(boot)/dts
 
 dtbs_install:
diff --git a/arch/nios2/Makefile b/arch/nios2/Makefile
index 8673a79..abc3f5b 100644
--- a/arch/nios2/Makefile
+++ b/arch/nios2/Makefile
@@ -61,7 +61,7 @@ archclean:
 %.dtb: | scripts
 	$(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@
 
-dtbs:
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@
 
 $(BOOT_TARGETS): vmlinux
-- 
2.7.4

[PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

The target "dtbs" should depend on "scripts" because it needs to
build dtc.  The "prepare" target is unneeded here.

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

 arch/arm/Makefile   | 2 +-
 arch/arm64/Makefile | 2 +-
 arch/nios2/Makefile | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 47d3a1a..cdb5b55 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -342,7 +342,7 @@ $(INSTALL_TARGETS):
 
 PHONY += dtbs dtbs_install
 
-dtbs: prepare scripts
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(boot)/dts
 
 dtbs_install:
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 939b310..fd56fbc 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -133,7 +133,7 @@ zinstall install:
 
 PHONY += dtbs dtbs_install
 
-dtbs: prepare scripts
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(boot)/dts
 
 dtbs_install:
diff --git a/arch/nios2/Makefile b/arch/nios2/Makefile
index 8673a79..abc3f5b 100644
--- a/arch/nios2/Makefile
+++ b/arch/nios2/Makefile
@@ -61,7 +61,7 @@ archclean:
 %.dtb: | scripts
 	$(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@
 
-dtbs:
+dtbs: scripts
 	$(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@
 
 $(BOOT_TARGETS): vmlinux
-- 
2.7.4

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Will Deacon]

On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/arm/Makefile   | 2 +-
>  arch/arm64/Makefile | 2 +-
>  arch/nios2/Makefile | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 47d3a1a..cdb5b55 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -342,7 +342,7 @@ $(INSTALL_TARGETS):
>  
>  PHONY += dtbs dtbs_install
>  
> -dtbs: prepare scripts
> +dtbs: scripts
>  	$(Q)$(MAKE) $(build)=$(boot)/dts
>  
>  dtbs_install:
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 939b310..fd56fbc 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -133,7 +133,7 @@ zinstall install:
>  
>  PHONY += dtbs dtbs_install
>  
> -dtbs: prepare scripts
> +dtbs: scripts
>  	$(Q)$(MAKE) $(build)=$(boot)/dts
>  
>  dtbs_install:

I think this is ok, although I was initially nervous about the possibility
of the archprepare step having interactions with the dt-bindings headers.
That doesn't appear to be the case, so for arm64:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH] kbuild: fix dependency of dtbs targets

[Will Deacon]

On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/arm/Makefile   | 2 +-
>  arch/arm64/Makefile | 2 +-
>  arch/nios2/Makefile | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 47d3a1a..cdb5b55 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -342,7 +342,7 @@ $(INSTALL_TARGETS):
>  
>  PHONY += dtbs dtbs_install
>  
> -dtbs: prepare scripts
> +dtbs: scripts
>  	$(Q)$(MAKE) $(build)=$(boot)/dts
>  
>  dtbs_install:
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 939b310..fd56fbc 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -133,7 +133,7 @@ zinstall install:
>  
>  PHONY += dtbs dtbs_install
>  
> -dtbs: prepare scripts
> +dtbs: scripts
>  	$(Q)$(MAKE) $(build)=$(boot)/dts
>  
>  dtbs_install:

I think this is ok, although I was initially nervous about the possibility
of the archprepare step having interactions with the dt-bindings headers.
That doesn't appear to be the case, so for arm64:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Russell King - ARM Linux]

On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.

Looks fine for ARM, as the only thing the dtbs should depend on is
the kernel configuration (to decide which to build) and DT tooling.

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH] kbuild: fix dependency of dtbs targets

[Russell King - ARM Linux]

On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.

Looks fine for ARM, as the only thing the dtbs should depend on is
the kernel configuration (to decide which to build) and DT tooling.

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

2017-10-04 13:27 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---


Applied to linux-kbuild/kbuild.

-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

2017-10-04 13:27 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> The target "dtbs" should depend on "scripts" because it needs to
> build dtc.  The "prepare" target is unneeded here.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---


Applied to linux-kbuild/kbuild.

-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

Hi.


2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>> The target "dtbs" should depend on "scripts" because it needs to
>> build dtc.  The "prepare" target is unneeded here.
>
> Looks fine for ARM, as the only thing the dtbs should depend on is
> the kernel configuration (to decide which to build) and DT tooling.
>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> --


I found a potential issue on this
because the default DTB install path depends on $(KERNELRELEASE).


In top-level Makefile:
export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)


The include/config/kernel.release is created by "prepare3" target.

If the dependency on "parepare" is removed,
it is possible to run "make dtbs" and "make dtbs_install"
without creating include/config/kernel.release.

So, the $(KERNELRELEASE) could be empty when installing DTB.


Maybe, drop this patch, or reduce the dependency to "parepare3"?







-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

Hi.


2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>> The target "dtbs" should depend on "scripts" because it needs to
>> build dtc.  The "prepare" target is unneeded here.
>
> Looks fine for ARM, as the only thing the dtbs should depend on is
> the kernel configuration (to decide which to build) and DT tooling.
>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> --


I found a potential issue on this
because the default DTB install path depends on $(KERNELRELEASE).


In top-level Makefile:
export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)


The include/config/kernel.release is created by "prepare3" target.

If the dependency on "parepare" is removed,
it is possible to run "make dtbs" and "make dtbs_install"
without creating include/config/kernel.release.

So, the $(KERNELRELEASE) could be empty when installing DTB.


Maybe, drop this patch, or reduce the dependency to "parepare3"?







-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Rob Herring]

On Wed, Oct 25, 2017 at 12:40 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi.
>
>
> 2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
>> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>>> The target "dtbs" should depend on "scripts" because it needs to
>>> build dtc.  The "prepare" target is unneeded here.
>>
>> Looks fine for ARM, as the only thing the dtbs should depend on is
>> the kernel configuration (to decide which to build) and DT tooling.
>>
>> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>>
>> --
>
>
> I found a potential issue on this
> because the default DTB install path depends on $(KERNELRELEASE).
>
>
> In top-level Makefile:
> export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
>
>
> The include/config/kernel.release is created by "prepare3" target.
>
> If the dependency on "parepare" is removed,
> it is possible to run "make dtbs" and "make dtbs_install"
> without creating include/config/kernel.release.
>
> So, the $(KERNELRELEASE) could be empty when installing DTB.
>
>
> Maybe, drop this patch, or reduce the dependency to "parepare3"?

I was doing some work to get dtb builds to work without depending on
$arch cross compiler and this patch fixes some of the issues. The
dtbs_install target has the prepare dependency, so that should be
sufficient and your patch should be fine. BTW, Based on prior
discussion on "ARM: kbuild: Fix forced rebuild after 'make dtbs'"
thread, prepare should not be needed just for $(KERNELRELEASE).

Rob

[PATCH] kbuild: fix dependency of dtbs targets

[Rob Herring]

On Wed, Oct 25, 2017 at 12:40 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi.
>
>
> 2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
>> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>>> The target "dtbs" should depend on "scripts" because it needs to
>>> build dtc.  The "prepare" target is unneeded here.
>>
>> Looks fine for ARM, as the only thing the dtbs should depend on is
>> the kernel configuration (to decide which to build) and DT tooling.
>>
>> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>>
>> --
>
>
> I found a potential issue on this
> because the default DTB install path depends on $(KERNELRELEASE).
>
>
> In top-level Makefile:
> export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
>
>
> The include/config/kernel.release is created by "prepare3" target.
>
> If the dependency on "parepare" is removed,
> it is possible to run "make dtbs" and "make dtbs_install"
> without creating include/config/kernel.release.
>
> So, the $(KERNELRELEASE) could be empty when installing DTB.
>
>
> Maybe, drop this patch, or reduce the dependency to "parepare3"?

I was doing some work to get dtb builds to work without depending on
$arch cross compiler and this patch fixes some of the issues. The
dtbs_install target has the prepare dependency, so that should be
sufficient and your patch should be fine. BTW, Based on prior
discussion on "ARM: kbuild: Fix forced rebuild after 'make dtbs'"
thread, prepare should not be needed just for $(KERNELRELEASE).

Rob

Re: [PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

2017-12-14 6:31 GMT+09:00 Rob Herring <robh@kernel.org>:
> On Wed, Oct 25, 2017 at 12:40 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Hi.
>>
>>
>> 2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
>>> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>>>> The target "dtbs" should depend on "scripts" because it needs to
>>>> build dtc.  The "prepare" target is unneeded here.
>>>
>>> Looks fine for ARM, as the only thing the dtbs should depend on is
>>> the kernel configuration (to decide which to build) and DT tooling.
>>>
>>> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>
>>> --
>>
>>
>> I found a potential issue on this
>> because the default DTB install path depends on $(KERNELRELEASE).
>>
>>
>> In top-level Makefile:
>> export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
>>
>>
>> The include/config/kernel.release is created by "prepare3" target.
>>
>> If the dependency on "parepare" is removed,
>> it is possible to run "make dtbs" and "make dtbs_install"
>> without creating include/config/kernel.release.
>>
>> So, the $(KERNELRELEASE) could be empty when installing DTB.
>>
>>
>> Maybe, drop this patch, or reduce the dependency to "parepare3"?
>
> I was doing some work to get dtb builds to work without depending on
> $arch cross compiler and this patch fixes some of the issues. The
> dtbs_install target has the prepare dependency,

Which line?

I checked arch/{arm,arm64,mips}/Makefile, but I did not see any
dependency.


> so that should be
> sufficient and your patch should be fine.

Generally, adding dependencies to install targets is dangerous
because install targets are often invoked in root privilege.

This is stated by, for example,
commit 19514fc665ffbce624785f76ee7ad0ea6378a527



> BTW, Based on prior
> discussion on "ARM: kbuild: Fix forced rebuild after 'make dtbs'"
> thread, prepare should not be needed just for $(KERNELRELEASE).
>
> Rob

No.
If "prepare" target is dropped, you can run dtbs_install
without include/config/kenrel.release





-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: fix dependency of dtbs targets

[Masahiro Yamada]

2017-12-14 6:31 GMT+09:00 Rob Herring <robh@kernel.org>:
> On Wed, Oct 25, 2017 at 12:40 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Hi.
>>
>>
>> 2017-10-10 0:05 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
>>> On Wed, Oct 04, 2017 at 01:27:20PM +0900, Masahiro Yamada wrote:
>>>> The target "dtbs" should depend on "scripts" because it needs to
>>>> build dtc.  The "prepare" target is unneeded here.
>>>
>>> Looks fine for ARM, as the only thing the dtbs should depend on is
>>> the kernel configuration (to decide which to build) and DT tooling.
>>>
>>> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>
>>> --
>>
>>
>> I found a potential issue on this
>> because the default DTB install path depends on $(KERNELRELEASE).
>>
>>
>> In top-level Makefile:
>> export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
>>
>>
>> The include/config/kernel.release is created by "prepare3" target.
>>
>> If the dependency on "parepare" is removed,
>> it is possible to run "make dtbs" and "make dtbs_install"
>> without creating include/config/kernel.release.
>>
>> So, the $(KERNELRELEASE) could be empty when installing DTB.
>>
>>
>> Maybe, drop this patch, or reduce the dependency to "parepare3"?
>
> I was doing some work to get dtb builds to work without depending on
> $arch cross compiler and this patch fixes some of the issues. The
> dtbs_install target has the prepare dependency,

Which line?

I checked arch/{arm,arm64,mips}/Makefile, but I did not see any
dependency.


> so that should be
> sufficient and your patch should be fine.

Generally, adding dependencies to install targets is dangerous
because install targets are often invoked in root privilege.

This is stated by, for example,
commit 19514fc665ffbce624785f76ee7ad0ea6378a527



> BTW, Based on prior
> discussion on "ARM: kbuild: Fix forced rebuild after 'make dtbs'"
> thread, prepare should not be needed just for $(KERNELRELEASE).
>
> Rob

No.
If "prepare" target is dropped, you can run dtbs_install
without include/config/kenrel.release





-- 
Best Regards
Masahiro Yamada