Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1612955268.git.viresh.kumar@linaro.org> <44dad578df8a848fc378cd358f03b071f44c9a5b.1612955268.git.viresh.kumar@linaro.org>
 <CAK7LNARa8GzhhvZWV_KAW=MC0DRcSsfPsQ-KTBRRpbBgBqY=ig@mail.gmail.com>
In-Reply-To: <CAK7LNARa8GzhhvZWV_KAW=MC0DRcSsfPsQ-KTBRRpbBgBqY=ig@mail.gmail.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Thu, 11 Feb 2021 14:05:35 -0600
Message-ID: <CAL_JsqKHUG6VvvpQ18YdzsOA_gZ59gFsc2tUzt1SxKHsO2A03g@mail.gmail.com>
Subject: Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay
To:     Masahiro Yamada <masahiroy@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Frank Rowand <frowand.list@gmail.com>,
        Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
        Michal Marek <michal.lkml@markovi.net>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Anmar Oueja <anmar.oueja@linaro.org>,
        Bill Mills <bill.mills@linaro.org>,
        David Gibson <david@gibson.dropbear.id.au>,
        DTML <devicetree@vger.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Michal Simek <michal.simek@xilinx.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 11, 2021 at 12:28 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Feb 10, 2021 at 8:13 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > From: Rob Herring <robh@kernel.org>
> >
> > Add a generic rule to apply fdtoverlay in Makefile.lib, so every
> > platform doesn't need to carry the complex rule.
> >
> > The platform's Makefile only needs to have this now:
> >
> >  DTC_FLAGS_foo_base += -@
> >  foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> >  overlay-y := foo.dtb
>
>
> Please reuse dtb-y instead of introducing the new
> overlay-y syntax, that is,
>
> foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> dtb-y := foo.dtb

That's what I had, but I believe Viresh changed this because we don't
want to run schema checks on foo.dtb (as foo.dts doesn't exist).
However, we should be able to filter those out using something similar
to technique used for multi-used-y and real-obj-y. We just need to
drop composite entries when creating the .dt.yaml list.

> This resembles to composite modules.
>
> foo-objs := foo1.o foo2.o foo3.o
> obj-m := foo.o
>
>
>
>
>
> > Rearrange Makefile.lib to keep DT specific stuff together.

Would be better if you do this in a separate patch or not at all. It
was sorted more by the type of operation, than the target. Also, we
also already have one conflict in linux-next with changing extra- to
always-, let's not make it worse or this will be 5.13 for sure.

> > The files from overlay-y (i.e. files generated by fdtoverlay) aren't
> > added to dtb-y here, as dtb-y is later used to generate .dt.yaml files
> > and the files in overlay-y don't have a corresponding dts file and make
> > dtbs_check fails for them.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > [ Viresh: Add commit log and replace dtb-y with overlay-y, handle
> >           CONFIG_OF_ALL_DTBS case, rearrange Makefile, don't add
> >           overlay-y to dtb-y to skip dtbs_check for them. ]
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  scripts/Makefile.lib | 39 +++++++++++++++++++++++++++------------
> >  1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index b00855b247e0..a6e79e3be527 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -66,23 +66,16 @@ multi-used   := $(multi-used-y) $(multi-used-m)
> >  real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m)))
> >  real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m)))
> >
> > -always-y += $(always-m)
> > -
> > -# hostprogs-always-y += foo
> > -# ... is a shorthand for
> > -# hostprogs += foo
> > -# always-y  += foo
> > -hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> > -always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> > -
> > -# userprogs-always-y is likewise.
> > -userprogs += $(userprogs-always-y) $(userprogs-always-m)
> > -always-y += $(userprogs-always-y) $(userprogs-always-m)
> > +# Add base dtb and overlay dtbo
> > +dtb-y += $(foreach m,$(overlay-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
> > +dtb-$(CONFIG_OF_ALL_DTBS) += $(foreach m,$(overlay-), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
> >
> >  # DTB
> >  # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
> >  extra-y                                += $(dtb-y)
> > +extra-y                                += $(overlay-y)
> >  extra-$(CONFIG_OF_ALL_DTBS)    += $(dtb-)
> > +extra-$(CONFIG_OF_ALL_DTBS)    += $(overlay-)
> >
> >  ifneq ($(CHECK_DTBS),)
> >  extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
> > @@ -91,6 +84,19 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
> >  extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
> >  endif
> >
> > +always-y += $(always-m)
> > +
> > +# hostprogs-always-y += foo
> > +# ... is a shorthand for
> > +# hostprogs += foo
> > +# always-y  += foo
> > +hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> > +always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> > +
> > +# userprogs-always-y is likewise.
> > +userprogs += $(userprogs-always-y) $(userprogs-always-m)
> > +always-y += $(userprogs-always-y) $(userprogs-always-m)
> > +
> >  # Add subdir path
> >
> >  extra-y                := $(addprefix $(obj)/,$(extra-y))
> > @@ -332,6 +338,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> >  $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> >         $(call if_changed_dep,dtc)
> >
> > +
> > +quiet_cmd_fdtoverlay = DTOVL   $@
> > +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
> > +
> > +.SECONDEXPANSION:
> > +
> > +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE
> > +       $(call if_changed,fdtoverlay)
> > +
>
>
>
> Please do not use .SECONDEXPANSION.
>
> This will parse the Makefile twice
> in _all_ directories, while only a few
> directories use the overlay-y syntax.
>
>
> Use the multi_depend macro.

I knew you'd have a better way.

Rob