| Message ID | 6e57e9c84429416c628f1f4235c42a5809747c4c.1611124778.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt: build overlays | expand |
On Wed, Jan 20, 2021 at 4:07 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Add support for building DT overlays (%.dtbo). The overlay's source file > will have the usual extension, i.e. .dts, though the blob will have > .dtbo extension to distinguish it from normal blobs. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > .gitignore | 3 +-- > Makefile | 4 ++-- > scripts/Makefile.dtbinst | 3 +++ > scripts/Makefile.lib | 4 +++- > 4 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/.gitignore b/.gitignore > index d01cda8e1177..0458c36f3cb2 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -17,8 +17,7 @@ > *.bz2 > *.c.[012]*.* > *.dt.yaml > -*.dtb > -*.dtb.S > +*.dtb* Personally, I prefer adding .dtbo explicitly > *.dwo > *.elf > *.gcno > diff --git a/Makefile b/Makefile > index 9e73f82e0d86..b84f5e0b46ab 100644 > --- a/Makefile > +++ b/Makefile > @@ -1334,7 +1334,7 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +%.dtb %.dtbo: include/config/kernel.release scripts_dtc > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ No, this is wrong because it does not work as grouped targets. You need to separate them. %.dtb: include/config/kernel.release scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ %.dtbo: include/config/kernel.release scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ See GNU make manual. "Pattern rules may have more than one target; however, every target must contain a % character. Pattern rules are always treated as grouped targets" https://www.gnu.org/software/make/manual/html_node/Pattern-Intro.html > PHONY += dtbs dtbs_install dtbs_check > @@ -1816,7 +1816,7 @@ clean: $(clean-dirs) > @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ > \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ > -o -name '*.ko.*' \ > - -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ > + -o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ > -o -name '*.dwo' -o -name '*.lst' \ > -o -name '*.su' -o -name '*.mod' \ > -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ > diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst > index 50d580d77ae9..ba01f5ba2517 100644 > --- a/scripts/Makefile.dtbinst > +++ b/scripts/Makefile.dtbinst > @@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@ > $(dst)/%.dtb: $(obj)/%.dtb > $(call cmd,dtb_install) > > +$(dst)/%.dtbo: $(obj)/%.dtbo > + $(call cmd,dtb_install) > + > PHONY += $(subdirs) > $(subdirs): > $(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@) > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 213677a5ed33..30bc0a8e0087 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(dtb-) > > ifneq ($(CHECK_DTBS),) > extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) > +extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y)) > extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) > +extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-)) > endif > > # Add subdir path > @@ -324,7 +326,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; > -d $(depfile).dtc.tmp $(dtc-tmp) ; \ > cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > +$(obj)/%.dtb $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) Same here. You need to duplicate the rules everywhere, unfortunately. > DT_CHECKER ?= dt-validate > -- > 2.25.0.rc1.19.g042ed3e048af >
On 20-01-21, 17:58, Masahiro Yamada wrote: > > +%.dtb %.dtbo: include/config/kernel.release scripts_dtc > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > No, this is wrong because it does not work > as grouped targets. > > You need to separate them. > > > > %.dtb: include/config/kernel.release scripts_dtc > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > %.dtbo: include/config/kernel.release scripts_dtc > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > > See GNU make manual. > > > "Pattern rules may have more than one target; however, every target > must contain a % character. > Pattern rules are always treated as grouped targets" > > https://www.gnu.org/software/make/manual/html_node/Pattern-Intro.html Hmm, okay I will do that. I did it this way because I saw similar stuff at some other places. I am not a regular Makefile hacker, there is every chance I am reading it wrong. $ git grep "%.*%.*:" | grep Makefile Makefile:%/config/auto.conf %/config/auto.conf.cmd %/generated/autoconf.h: $(KCONFIG_CONFIG) scripts/Makefile.build:$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler scripts/Makefile.host:$(obj)/%.tab.c $(obj)/%.tab.h: $(src)/%.y FORCE scripts/genksyms/Makefile:$(obj)/pars%.tab.c $(obj)/pars%.tab.h: $(src)/pars%.y FORCE tools/perf/Documentation/Makefile:$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : %.txt tools/perf/Documentation/Makefile:$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
On Wed, Jan 20, 2021 at 6:55 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 20-01-21, 17:58, Masahiro Yamada wrote: > > > +%.dtb %.dtbo: include/config/kernel.release scripts_dtc > > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > > > No, this is wrong because it does not work > > as grouped targets. > > > > You need to separate them. > > > > > > > > %.dtb: include/config/kernel.release scripts_dtc > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > %.dtbo: include/config/kernel.release scripts_dtc > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > > > > > > > See GNU make manual. > > > > > > "Pattern rules may have more than one target; however, every target > > must contain a % character. > > Pattern rules are always treated as grouped targets" > > > > https://www.gnu.org/software/make/manual/html_node/Pattern-Intro.html > > Hmm, okay I will do that. > > I did it this way because I saw similar stuff at some other places. I > am not a regular Makefile hacker, there is every chance I am reading > it wrong. > In grouped targets, a recipe must be able to create all the targets in a single invocation. > $ git grep "%.*%.*:" | grep Makefile > Makefile:%/config/auto.conf %/config/auto.conf.cmd %/generated/autoconf.h: $(KCONFIG_CONFIG) One single invocation of Kconfig generates three files, auto.conf, auto.conf.cmd, autoconf.h So, this is correct. > scripts/Makefile.build:$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler asn1_compiler takes one source file, then outputs two files %.asn1.c and %.asn1.h So, this is correct. > scripts/Makefile.host:$(obj)/%.tab.c $(obj)/%.tab.h: $(src)/%.y FORCE bison takes one source file, then outputs two files %.tab.c and %.tab.h So, this is correct. > scripts/genksyms/Makefile:$(obj)/pars%.tab.c $(obj)/pars%.tab.h: $(src)/pars%.y FORCE This is also a bison rule. Correct. > tools/perf/Documentation/Makefile:$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : %.txt > tools/perf/Documentation/Makefile:$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml These are out of scope of my maintenance coverage. I do not know whether they are correct or not. The DTC rule takes one source input, and one output file. It cannot generate .dtb and .dtbo at the same time. That is why a grouped target does not fit here. > -- > viresh
On 20-01-21, 20:04, Masahiro Yamada wrote: > The DTC rule takes one source input, and one output file. > > It cannot generate .dtb and .dtbo at the same time. > > That is why a grouped target does not fit here. Okay, thanks for taking time to explain this. Will fix this in the next version.
On Wed, Jan 20, 2021 at 12:36:46PM +0530, Viresh Kumar wrote: > Add support for building DT overlays (%.dtbo). The overlay's source file > will have the usual extension, i.e. .dts, though the blob will have > .dtbo extension to distinguish it from normal blobs. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > .gitignore | 3 +-- > Makefile | 4 ++-- > scripts/Makefile.dtbinst | 3 +++ > scripts/Makefile.lib | 4 +++- > 4 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/.gitignore b/.gitignore > index d01cda8e1177..0458c36f3cb2 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -17,8 +17,7 @@ > *.bz2 > *.c.[012]*.* > *.dt.yaml > -*.dtb > -*.dtb.S > +*.dtb* > *.dwo > *.elf > *.gcno > diff --git a/Makefile b/Makefile > index 9e73f82e0d86..b84f5e0b46ab 100644 > --- a/Makefile > +++ b/Makefile > @@ -1334,7 +1334,7 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +%.dtb %.dtbo: include/config/kernel.release scripts_dtc > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > PHONY += dtbs dtbs_install dtbs_check > @@ -1816,7 +1816,7 @@ clean: $(clean-dirs) > @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ > \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ > -o -name '*.ko.*' \ > - -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ > + -o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ > -o -name '*.dwo' -o -name '*.lst' \ > -o -name '*.su' -o -name '*.mod' \ > -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ > diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst > index 50d580d77ae9..ba01f5ba2517 100644 > --- a/scripts/Makefile.dtbinst > +++ b/scripts/Makefile.dtbinst > @@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@ > $(dst)/%.dtb: $(obj)/%.dtb > $(call cmd,dtb_install) > > +$(dst)/%.dtbo: $(obj)/%.dtbo > + $(call cmd,dtb_install) > + > PHONY += $(subdirs) > $(subdirs): > $(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@) > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 213677a5ed33..30bc0a8e0087 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(dtb-) > > ifneq ($(CHECK_DTBS),) > extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) > +extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y)) > extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) > +extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-)) > endif > > # Add subdir path > @@ -324,7 +326,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; > -d $(depfile).dtc.tmp $(dtc-tmp) ; \ > cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > +$(obj)/%.dtb $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) If you're using overlays, you probably need the -@ flag, for both the base file and the overlays, which AFAICT is not already the case. > > DT_CHECKER ?= dt-validate
On 21-01-21, 11:49, David Gibson wrote: > If you're using overlays, you probably need the -@ flag, for both the > base file and the overlays, which AFAICT is not already the case. I think the idea was to do that in the platform specific Makefiles, unless I have misunderstood that from earlier discussions. So a platform may want to do that per-file or just enable it for the entire platform.
On 1/20/21 10:13 PM, Viresh Kumar wrote: > On 21-01-21, 11:49, David Gibson wrote: >> If you're using overlays, you probably need the -@ flag, for both the >> base file and the overlays, which AFAICT is not already the case. > > I think the idea was to do that in the platform specific Makefiles, > unless I have misunderstood that from earlier discussions. So a > platform may want to do that per-file or just enable it for the entire > platform. > Yes, that is correct. For drivers/of/unitest-data/Makefile, we have entries like: DTC_FLAGS_overlay += -@ DTC_FLAGS_overlay_bad_phandle += -@ DTC_FLAGS_overlay_bad_symbol += -@ DTC_FLAGS_overlay_base += -@ DTC_FLAGS_testcases += -@ -Frank
On Thu, Jan 21, 2021 at 09:43:00AM +0530, Viresh Kumar wrote: > On 21-01-21, 11:49, David Gibson wrote: > > If you're using overlays, you probably need the -@ flag, for both the > > base file and the overlays, which AFAICT is not already the case. > > I think the idea was to do that in the platform specific Makefiles, > unless I have misunderstood that from earlier discussions. So a > platform may want to do that per-file or just enable it for the entire > platform. Hm, ok. Any platform that does anything with dtbo files is likely to want it, though.
diff --git a/.gitignore b/.gitignore index d01cda8e1177..0458c36f3cb2 100644 --- a/.gitignore +++ b/.gitignore @@ -17,8 +17,7 @@ *.bz2 *.c.[012]*.* *.dt.yaml -*.dtb -*.dtb.S +*.dtb* *.dwo *.elf *.gcno diff --git a/Makefile b/Makefile index 9e73f82e0d86..b84f5e0b46ab 100644 --- a/Makefile +++ b/Makefile @@ -1334,7 +1334,7 @@ endif ifneq ($(dtstree),) -%.dtb: include/config/kernel.release scripts_dtc +%.dtb %.dtbo: include/config/kernel.release scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ PHONY += dtbs dtbs_install dtbs_check @@ -1816,7 +1816,7 @@ clean: $(clean-dirs) @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ -o -name '*.ko.*' \ - -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ + -o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ -o -name '*.su' -o -name '*.mod' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst index 50d580d77ae9..ba01f5ba2517 100644 --- a/scripts/Makefile.dtbinst +++ b/scripts/Makefile.dtbinst @@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@ $(dst)/%.dtb: $(obj)/%.dtb $(call cmd,dtb_install) +$(dst)/%.dtbo: $(obj)/%.dtbo + $(call cmd,dtb_install) + PHONY += $(subdirs) $(subdirs): $(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 213677a5ed33..30bc0a8e0087 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(dtb-) ifneq ($(CHECK_DTBS),) extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) +extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y)) extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) +extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-)) endif # Add subdir path @@ -324,7 +326,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; -d $(depfile).dtc.tmp $(dtc-tmp) ; \ cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE +$(obj)/%.dtb $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) DT_CHECKER ?= dt-validate
Add support for building DT overlays (%.dtbo). The overlay's source file will have the usual extension, i.e. .dts, though the blob will have .dtbo extension to distinguish it from normal blobs. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- .gitignore | 3 +-- Makefile | 4 ++-- scripts/Makefile.dtbinst | 3 +++ scripts/Makefile.lib | 4 +++- 4 files changed, 9 insertions(+), 5 deletions(-)