diff mbox series

[V5,4/5] kbuild: Add support to build overlays (%.dtbo)

Message ID 6e57e9c84429416c628f1f4235c42a5809747c4c.1611124778.git.viresh.kumar@linaro.org (mailing list archive)
State New
Headers show
Series dt: build overlays | expand

Commit Message

Viresh Kumar Jan. 20, 2021, 7:06 a.m. UTC
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(-)

Comments

Masahiro Yamada Jan. 20, 2021, 8:58 a.m. UTC | #1
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
>
Viresh Kumar Jan. 20, 2021, 9:55 a.m. UTC | #2
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
Masahiro Yamada Jan. 20, 2021, 11:04 a.m. UTC | #3
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
Viresh Kumar Jan. 20, 2021, 11:27 a.m. UTC | #4
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.
David Gibson Jan. 21, 2021, 12:49 a.m. UTC | #5
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
Viresh Kumar Jan. 21, 2021, 4:13 a.m. UTC | #6
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.
Frank Rowand Jan. 21, 2021, 5:40 a.m. UTC | #7
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
David Gibson Jan. 21, 2021, 6:25 a.m. UTC | #8
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 mbox series

Patch

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