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