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>
In-Reply-To: <44dad578df8a848fc378cd358f03b071f44c9a5b.1612955268.git.viresh.kumar@linaro.org>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Fri, 12 Feb 2021 03:27:23 +0900
Message-ID: <CAK7LNARa8GzhhvZWV_KAW=MC0DRcSsfPsQ-KTBRRpbBgBqY=ig@mail.gmail.com>
Subject: Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Frank Rowand <frowand.list@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        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>,
        Rob Herring <robh@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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


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


>  DT_CHECKER ?= dt-validate
>  DT_BINDING_DIR := Documentation/devicetree/bindings
>  # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
> --
> 2.25.0.rc1.19.g042ed3e048af
>


--
Best Regards
Masahiro Yamada