* [PATCH] kbuild: add dtbs_prepare target
@ 2022-07-16 9:31 Masahiro Yamada
2022-07-22 10:13 ` Nicolas Schier
2022-07-27 9:58 ` Dmitry Baryshkov
0 siblings, 2 replies; 5+ messages in thread
From: Masahiro Yamada @ 2022-07-16 9:31 UTC (permalink / raw)
To: linux-kbuild, devicetree, Rob Herring
Cc: Dmitry Baryshkov, Masahiro Yamada, Michal Marek,
Nick Desaulniers, linux-kernel
Factor out the common prerequisites for DT compilation into the new
target, dtbs_prepare.
Add comments in case you wonder why include/config/kernel.release is
the prerequisite. Our policy is that installation targets must not
(re)compile any build artifacts in the tree. If we make modules_install
depend on include/config/kernel.release and it is executed under the
root privilege, it may be owned by root.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
Makefile | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index a9bd55edb75e..8aa4dbb8f878 100644
--- a/Makefile
+++ b/Makefile
@@ -1367,16 +1367,22 @@ endif
ifneq ($(dtstree),)
-%.dtb: include/config/kernel.release scripts_dtc
+%.dtb: dtbs_prepare
$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
-%.dtbo: include/config/kernel.release scripts_dtc
+%.dtbo: dtbs_prepare
$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
-PHONY += dtbs dtbs_install dtbs_check
-dtbs: include/config/kernel.release scripts_dtc
+PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
+dtbs: dtbs_prepare
$(Q)$(MAKE) $(build)=$(dtstree)
+# include/config/kernel.release is not actually required for building DTBs,
+# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE).
+# We do not want to move it to dtbs_install. The policy is installation
+# targets (, which may run as root) must not modify the tree.
+dtbs_prepare: include/config/kernel.release scripts_dtc
+
ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
export CHECK_DTBS=y
dtbs: dt_binding_check
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: add dtbs_prepare target
2022-07-16 9:31 [PATCH] kbuild: add dtbs_prepare target Masahiro Yamada
@ 2022-07-22 10:13 ` Nicolas Schier
2022-07-23 0:33 ` Masahiro Yamada
2022-07-27 9:58 ` Dmitry Baryshkov
1 sibling, 1 reply; 5+ messages in thread
From: Nicolas Schier @ 2022-07-22 10:13 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, devicetree, Rob Herring, Dmitry Baryshkov,
Michal Marek, Nick Desaulniers, linux-kernel
On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote:
> Factor out the common prerequisites for DT compilation into the new
> target, dtbs_prepare.
>
> Add comments in case you wonder why include/config/kernel.release is
> the prerequisite. Our policy is that installation targets must not
> (re)compile any build artifacts in the tree. If we make modules_install
> depend on include/config/kernel.release and it is executed under the
> root privilege, it may be owned by root.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Makefile | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a9bd55edb75e..8aa4dbb8f878 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1367,16 +1367,22 @@ endif
>
> ifneq ($(dtstree),)
>
> -%.dtb: include/config/kernel.release scripts_dtc
> +%.dtb: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>
> -%.dtbo: include/config/kernel.release scripts_dtc
> +%.dtbo: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
Is there a reason, why both rules are not unified? I guess it is, but
I can't see it.
>
> -PHONY += dtbs dtbs_install dtbs_check
> -dtbs: include/config/kernel.release scripts_dtc
> +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
> +dtbs: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree)
>
> +# include/config/kernel.release is not actually required for building DTBs,
> +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE).
> +# We do not want to move it to dtbs_install. The policy is installation
> +# targets (, which may run as root) must not modify the tree.
Is the comma after the opening parenthesis intended?
Kind regards,
Nicolas
> +dtbs_prepare: include/config/kernel.release scripts_dtc
> +
> ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
> export CHECK_DTBS=y
> dtbs: dt_binding_check
> --
> 2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: add dtbs_prepare target
2022-07-22 10:13 ` Nicolas Schier
@ 2022-07-23 0:33 ` Masahiro Yamada
2022-07-23 19:27 ` Nicolas Schier
0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2022-07-23 0:33 UTC (permalink / raw)
To: Nicolas Schier
Cc: Linux Kbuild mailing list, DTML, Rob Herring, Dmitry Baryshkov,
Michal Marek, Nick Desaulniers, Linux Kernel Mailing List
On Fri, Jul 22, 2022 at 7:14 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote:
> > Factor out the common prerequisites for DT compilation into the new
> > target, dtbs_prepare.
> >
> > Add comments in case you wonder why include/config/kernel.release is
> > the prerequisite. Our policy is that installation targets must not
> > (re)compile any build artifacts in the tree. If we make modules_install
> > depend on include/config/kernel.release and it is executed under the
> > root privilege, it may be owned by root.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > Makefile | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index a9bd55edb75e..8aa4dbb8f878 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1367,16 +1367,22 @@ endif
> >
> > ifneq ($(dtstree),)
> >
> > -%.dtb: include/config/kernel.release scripts_dtc
> > +%.dtb: dtbs_prepare
> > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
> >
> > -%.dtbo: include/config/kernel.release scripts_dtc
> > +%.dtbo: dtbs_prepare
> > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>
> Is there a reason, why both rules are not unified? I guess it is, but
> I can't see it.
See the GNU Make manual:
https://www.gnu.org/software/make/manual/html_node/Pattern-Examples.html
The last paragraph, "This pattern rule has two targets ..."
%.dtb %.dtbo: dtbs_prepare
...
means foo.dtb and foo.dtbo are generated at the same
time by the rule. This is strange.
>
> >
> > -PHONY += dtbs dtbs_install dtbs_check
> > -dtbs: include/config/kernel.release scripts_dtc
> > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
> > +dtbs: dtbs_prepare
> > $(Q)$(MAKE) $(build)=$(dtstree)
> >
> > +# include/config/kernel.release is not actually required for building DTBs,
> > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE).
> > +# We do not want to move it to dtbs_install. The policy is installation
> > +# targets (, which may run as root) must not modify the tree.
>
> Is the comma after the opening parenthesis intended?
I will rephrase the comment in v2.
>
> Kind regards,
> Nicolas
>
> > +dtbs_prepare: include/config/kernel.release scripts_dtc
> > +
> > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
> > export CHECK_DTBS=y
> > dtbs: dt_binding_check
> > --
> > 2.34.1
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: add dtbs_prepare target
2022-07-23 0:33 ` Masahiro Yamada
@ 2022-07-23 19:27 ` Nicolas Schier
0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Schier @ 2022-07-23 19:27 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, DTML, Rob Herring, Dmitry Baryshkov,
Michal Marek, Nick Desaulniers, Linux Kernel Mailing List
On Sat, Jul 23, 2022 at 09:33:03AM +0900, Masahiro Yamada wrote:
> On Fri, Jul 22, 2022 at 7:14 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote:
> > > Factor out the common prerequisites for DT compilation into the new
> > > target, dtbs_prepare.
> > >
> > > Add comments in case you wonder why include/config/kernel.release is
> > > the prerequisite. Our policy is that installation targets must not
> > > (re)compile any build artifacts in the tree. If we make modules_install
> > > depend on include/config/kernel.release and it is executed under the
> > > root privilege, it may be owned by root.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > > Makefile | 14 ++++++++++----
> > > 1 file changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index a9bd55edb75e..8aa4dbb8f878 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1367,16 +1367,22 @@ endif
> > >
> > > ifneq ($(dtstree),)
> > >
> > > -%.dtb: include/config/kernel.release scripts_dtc
> > > +%.dtb: dtbs_prepare
> > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
> > >
> > > -%.dtbo: include/config/kernel.release scripts_dtc
> > > +%.dtbo: dtbs_prepare
> > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
> >
> > Is there a reason, why both rules are not unified? I guess it is, but
> > I can't see it.
>
>
>
> See the GNU Make manual:
> https://www.gnu.org/software/make/manual/html_node/Pattern-Examples.html
>
> The last paragraph, "This pattern rule has two targets ..."
>
>
>
> %.dtb %.dtbo: dtbs_prepare
> ...
>
> means foo.dtb and foo.dtbo are generated at the same
> time by the rule. This is strange.
uh, I'm working with Makefile since years but I wasn't aware of that. Thanks a
lot for the pointer and explanation!
Kind regards,
Nicolas
>
> >
> > >
> > > -PHONY += dtbs dtbs_install dtbs_check
> > > -dtbs: include/config/kernel.release scripts_dtc
> > > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
> > > +dtbs: dtbs_prepare
> > > $(Q)$(MAKE) $(build)=$(dtstree)
> > >
> > > +# include/config/kernel.release is not actually required for building DTBs,
> > > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE).
> > > +# We do not want to move it to dtbs_install. The policy is installation
> > > +# targets (, which may run as root) must not modify the tree.
> >
> > Is the comma after the opening parenthesis intended?
>
>
> I will rephrase the comment in v2.
>
>
>
>
> >
> > Kind regards,
> > Nicolas
> >
> > > +dtbs_prepare: include/config/kernel.release scripts_dtc
> > > +
> > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
> > > export CHECK_DTBS=y
> > > dtbs: dt_binding_check
> > > --
> > > 2.34.1
>
>
>
> --
> Best Regards
> Masahiro Yamada
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: add dtbs_prepare target
2022-07-16 9:31 [PATCH] kbuild: add dtbs_prepare target Masahiro Yamada
2022-07-22 10:13 ` Nicolas Schier
@ 2022-07-27 9:58 ` Dmitry Baryshkov
1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Baryshkov @ 2022-07-27 9:58 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild, devicetree, Rob Herring
Cc: Michal Marek, Nick Desaulniers, linux-kernel
On 16/07/2022 12:31, Masahiro Yamada wrote:
> Factor out the common prerequisites for DT compilation into the new
> target, dtbs_prepare.
>
> Add comments in case you wonder why include/config/kernel.release is
> the prerequisite. Our policy is that installation targets must not
> (re)compile any build artifacts in the tree. If we make modules_install
> depend on include/config/kernel.release and it is executed under the
> root privilege, it may be owned by root.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
With the comment fixed:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>
> Makefile | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a9bd55edb75e..8aa4dbb8f878 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1367,16 +1367,22 @@ endif
>
> ifneq ($(dtstree),)
>
> -%.dtb: include/config/kernel.release scripts_dtc
> +%.dtb: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>
> -%.dtbo: include/config/kernel.release scripts_dtc
> +%.dtbo: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>
> -PHONY += dtbs dtbs_install dtbs_check
> -dtbs: include/config/kernel.release scripts_dtc
> +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check
> +dtbs: dtbs_prepare
> $(Q)$(MAKE) $(build)=$(dtstree)
>
> +# include/config/kernel.release is not actually required for building DTBs,
> +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE).
> +# We do not want to move it to dtbs_install. The policy is installation
> +# targets (, which may run as root) must not modify the tree.
> +dtbs_prepare: include/config/kernel.release scripts_dtc
> +
> ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
> export CHECK_DTBS=y
> dtbs: dt_binding_check
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-27 9:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-16 9:31 [PATCH] kbuild: add dtbs_prepare target Masahiro Yamada
2022-07-22 10:13 ` Nicolas Schier
2022-07-23 0:33 ` Masahiro Yamada
2022-07-23 19:27 ` Nicolas Schier
2022-07-27 9:58 ` Dmitry Baryshkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.