Re: [PATCH] kbuild: add dtbs_prepare target

[Nicolas Schier <nicolas@fjasle.eu>]

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