kbuild: add dtbs_prepare target

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(-)

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

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