diff mbox series

[V7,1/3] kbuild: Add generic rule to apply fdtoverlay

Message ID 44dad578df8a848fc378cd358f03b071f44c9a5b.1612955268.git.viresh.kumar@linaro.org (mailing list archive)
State New, archived
Headers show
Series dt: Build unittests statically with fdtoverlay | expand

Commit Message

Viresh Kumar Feb. 10, 2021, 11:13 a.m. UTC
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

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

Comments

Masahiro Yamada Feb. 11, 2021, 6:27 p.m. UTC | #1
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
Rob Herring Feb. 11, 2021, 8:05 p.m. UTC | #2
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
Masahiro Yamada Feb. 12, 2021, 3:07 a.m. UTC | #3
On Fri, Feb 12, 2021 at 5:05 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> 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.



Yes, I think this will work.



BTW, I do not know how to use overlay.
Do we apply overlay in the build time?
If so, I do not know what the benefit of overlay is.
Or is this just for build testing?


I just thought this was done in the boot time,
for example, in U-Boot or something.
Viresh Kumar Feb. 12, 2021, 4:41 a.m. UTC | #4
On 12-02-21, 12:07, Masahiro Yamada wrote:
> BTW, I do not know how to use overlay.
> Do we apply overlay in the build time?

Ideally it can be applied at both build time and runtime, but we
haven't allowed the runtime way until now in kernel. This patchset is
all about applying it at build time.

> If so, I do not know what the benefit of overlay is.
> Or is this just for build testing?

For now the main benefit of using them is that we can keep stuff in
separate files without including each other. For example a primary
board may or may not have an extension board connected to it.

Without overlays we will have this many dtbs for this simple case:
1. primary.dtb
2. extension.dtb
3. primary-includes-extension.dtb

With overlays we will have the first two. Now the same extension can
be applied to lots of boards and multiple extensions can be applied to
the same primary board. This just complicates the process of managing
dtbs.

> I just thought this was done in the boot time,
> for example, in U-Boot or something.

Yes, bootloader can do it as well.
diff mbox series

Patch

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)
+
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile