[PATCH 0/3] kbuild: improve DT build rules

[PATCH 0/3] kbuild: improve DT build rules

[Masahiro Yamada]

This series is applicable on
Linus' tree + the following two patches:
https://patchwork.kernel.org/patch/11413625/
https://patchwork.kernel.org/patch/11413623/



Masahiro Yamada (3):
  kbuild: avoid concurrency issue in parallel building dtbs and
    dtbs_check
  kbuild: allow to run dt_binding_check and dtbs_check in a single
    command
  kbuild: allow to run dt_binding_check without kernel configuration

 Documentation/devicetree/bindings/Makefile  |  8 +++-----
 Documentation/devicetree/writing-schema.rst |  4 ++++
 Makefile                                    | 16 ++++++++++++----
 scripts/dtc/Makefile                        |  5 +++--
 4 files changed, 22 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH 1/3] kbuild: avoid concurrency issue in parallel building dtbs and dtbs_check

[Masahiro Yamada]

'make dtbs_check' checks the shecma in addition to building *.dtb files,
in other words, 'make dtbs_check' is a super-set of 'make dtbs'.
So, you do not have to do 'make dtbs dtbs_check', but I want to keep
the build system robust in any use.

Currently, 'dtbs' and 'dtbs_check' are independent of each other.
In parallel building, two threads descend into arch/*/boot/dts/,
one for dtbs and the other for dtbs_check, then end up with building
the same DTB simultaneously.

This commit fixes the concurrency issue. Otherwise, I see build errors
like follows:

$ make ARCH=arm64 defconfig
$ make -j16 ARCH=arm64 DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/psci.yaml dtbs dtbs_check
  <snip>
  DTC     arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dtb
  DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtb
  DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb
  DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb
  DTC     arch/arm64/boot/dts/freescale/imx8mn-evk.dtb
  DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb
  DTC     arch/arm64/boot/dts/zte/zx296718-pcbox.dtb
  DTC     arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dt.yaml
  DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dtb
  DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1254-revA.dtb
  DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dtb
  DTC     arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet-inx.dtb
  DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb
  CHECK   arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dt.yaml
fixdep: error opening file: arch/arm64/boot/dts/allwinner/.sun50i-h6-orangepi-lite2.dtb.d: No such file or directory
make[2]: *** [scripts/Makefile.lib:296: arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb] Error 2
make[2]: *** Deleting file 'arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb'
make[2]: *** Waiting for unfinished jobs....
  DTC     arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet-kd.dtb
  DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p231.dtb
  DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1275-revA.dtb
  DTC     arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dtb
fixdep: parse error; no targets found
make[2]: *** [scripts/Makefile.lib:296: arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb] Error 1
make[2]: *** Deleting file 'arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb'
make[1]: *** [scripts/Makefile.build:505: arch/arm64/boot/dts/allwinner] Error 2
make[1]: *** Waiting for unfinished jobs....
  DTC     arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dtb

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1a1a0d271697..e94d4fe3ef77 100644
--- a/Makefile
+++ b/Makefile
@@ -1240,11 +1240,15 @@ ifneq ($(dtstree),)
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
 PHONY += dtbs dtbs_install dtbs_check
-dtbs dtbs_check: include/config/kernel.release scripts_dtc
+dtbs: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)
 
+ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
+dtbs: dt_binding_check
+endif
+
 dtbs_check: export CHECK_DTBS=1
-dtbs_check: dt_binding_check
+dtbs_check: dtbs
 
 dtbs_install:
 	$(Q)$(MAKE) $(dtbinst)=$(dtstree)
-- 
2.17.1

Re: [PATCH 1/3] kbuild: avoid concurrency issue in parallel building dtbs and dtbs_check

[Rob Herring]

On Tue, Mar 3, 2020 at 9:20 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 'make dtbs_check' checks the shecma in addition to building *.dtb files,

typo

> in other words, 'make dtbs_check' is a super-set of 'make dtbs'.
> So, you do not have to do 'make dtbs dtbs_check', but I want to keep
> the build system robust in any use.
>
> Currently, 'dtbs' and 'dtbs_check' are independent of each other.
> In parallel building, two threads descend into arch/*/boot/dts/,
> one for dtbs and the other for dtbs_check, then end up with building
> the same DTB simultaneously.
>
> This commit fixes the concurrency issue. Otherwise, I see build errors
> like follows:
>
> $ make ARCH=arm64 defconfig
> $ make -j16 ARCH=arm64 DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/psci.yaml dtbs dtbs_check
>   <snip>
>   DTC     arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dtb
>   DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtb
>   DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb
>   DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb
>   DTC     arch/arm64/boot/dts/freescale/imx8mn-evk.dtb
>   DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb
>   DTC     arch/arm64/boot/dts/zte/zx296718-pcbox.dtb
>   DTC     arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dt.yaml
>   DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dtb
>   DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1254-revA.dtb
>   DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dtb
>   DTC     arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet-inx.dtb
>   DTC     arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb
>   CHECK   arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dt.yaml
> fixdep: error opening file: arch/arm64/boot/dts/allwinner/.sun50i-h6-orangepi-lite2.dtb.d: No such file or directory
> make[2]: *** [scripts/Makefile.lib:296: arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb] Error 2
> make[2]: *** Deleting file 'arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb'
> make[2]: *** Waiting for unfinished jobs....
>   DTC     arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet-kd.dtb
>   DTC     arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p231.dtb
>   DTC     arch/arm64/boot/dts/xilinx/zynqmp-zc1275-revA.dtb
>   DTC     arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dtb
> fixdep: parse error; no targets found
> make[2]: *** [scripts/Makefile.lib:296: arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb] Error 1
> make[2]: *** Deleting file 'arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb'
> make[1]: *** [scripts/Makefile.build:505: arch/arm64/boot/dts/allwinner] Error 2
> make[1]: *** Waiting for unfinished jobs....
>   DTC     arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dtb
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Makefile | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/3] kbuild: allow to run dt_binding_check and dtbs_check in a single command

[Masahiro Yamada]

Since commit 93512dad334d ("dt-bindings: Improve validation build error
handling"), 'make dtbs_check' does not validate the schema fully.

If you want to check everything, you need to run two commands.

  $ make ARCH=arm dt_binding_check
  $ make ARCH=arm dtbs_check

They are exclusive each other, so you cannot do like this:

  $ make ARCH=arm dt_binding_check dtbs_check

In this case, dt-doc-validate and dt-extract-example are skipped
because CHECK_DTBS is set.

Let's make it possible to run those two targets simultaneously.
It will be useful for schema writers.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Documentation/devicetree/bindings/Makefile  | 8 +++-----
 Documentation/devicetree/writing-schema.rst | 4 ++++
 Makefile                                    | 6 +++++-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index b62c0470f122..1df680d07461 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -26,10 +26,9 @@ DT_DOCS = $(addprefix $(src)/, \
 
 DT_SCHEMA_FILES ?= $(DT_DOCS)
 
-ifeq ($(CHECK_DTBS),)
-extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES))
-extra-y += $(patsubst $(src)/%.yaml,%.example.dt.yaml, $(DT_SCHEMA_FILES))
-extra-y += processed-schema-examples.yaml
+extra-$(CHECK_DT_BINDING) += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES))
+extra-$(CHECK_DT_BINDING) += $(patsubst $(src)/%.yaml,%.example.dt.yaml, $(DT_SCHEMA_FILES))
+extra-$(CHECK_DT_BINDING) += processed-schema-examples.yaml
 
 override DTC_FLAGS := \
 	-Wno-avoid_unnecessary_addr_size \
@@ -37,7 +36,6 @@ override DTC_FLAGS := \
 
 $(obj)/processed-schema-examples.yaml: $(DT_DOCS) FORCE
 	$(call if_changed,mk_schema)
-endif
 
 $(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := -u
 $(obj)/processed-schema.yaml: $(DT_SCHEMA_FILES) FORCE
diff --git a/Documentation/devicetree/writing-schema.rst b/Documentation/devicetree/writing-schema.rst
index 7635ab230456..220cf464ed77 100644
--- a/Documentation/devicetree/writing-schema.rst
+++ b/Documentation/devicetree/writing-schema.rst
@@ -147,6 +147,10 @@ Note that ``dtbs_check`` will skip any binding schema files with errors. It is
 necessary to use ``dt_binding_check`` to get all the validation errors in the
 binding schema files.
 
+It is possible to run both in a single command::
+
+    make dt_binding_check dtbs_check
+
 It is also possible to run checks with a single schema file by setting the
 ``DT_SCHEMA_FILES`` variable to a specific schema file.
 
diff --git a/Makefile b/Makefile
index e94d4fe3ef77..7dec7b343842 100644
--- a/Makefile
+++ b/Makefile
@@ -1244,10 +1244,10 @@ dtbs: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)
 
 ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
+export CHECK_DTBS=y
 dtbs: dt_binding_check
 endif
 
-dtbs_check: export CHECK_DTBS=1
 dtbs_check: dtbs
 
 dtbs_install:
@@ -1263,6 +1263,10 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
 	$(Q)$(MAKE) $(build)=scripts/dtc
 
+ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),)
+export CHECK_DT_BINDING=y
+endif
+
 PHONY += dt_binding_check
 dt_binding_check: scripts_dtc
 	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
-- 
2.17.1

Re: [PATCH 2/3] kbuild: allow to run dt_binding_check and dtbs_check in a single command

[Sam Ravnborg]

Hi Masahiro

Thanks for the nice improvements to the dt infrastructure.

Stealing a thread here..

>  It is also possible to run checks with a single schema file by setting the
>  ``DT_SCHEMA_FILES`` variable to a specific schema file.
Would it be simple to enable the use of dirs for DT_SCHEMA_FILES?

So I for example could do:

make dt_bindings_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/

I did a very quick look add it but failed to dechiper all the
makefile logic.

It is a corner case when one wants to check a full dir,
so unless it is very simple the current logic should not
be complicated by this (if you take the bait and look at it).

	Sam

Re: [PATCH 2/3] kbuild: allow to run dt_binding_check and dtbs_check in a single command

[Rob Herring]

On Tue, Mar 3, 2020 at 11:55 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Masahiro
>
> Thanks for the nice improvements to the dt infrastructure.
>
> Stealing a thread here..
>
> >  It is also possible to run checks with a single schema file by setting the
> >  ``DT_SCHEMA_FILES`` variable to a specific schema file.
> Would it be simple to enable the use of dirs for DT_SCHEMA_FILES?

I did name that with the intent of supporting more than one file.

> So I for example could do:
>
> make dt_bindings_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/

Does this work?:

make dt_bindings_check DT_SCHEMA_FILES="$(find
Documentation/devicetree/bindings/display/panel/ -name '*.yaml' |
xargs)"

Rob

Re: [PATCH 2/3] kbuild: allow to run dt_binding_check and dtbs_check in a single command

[Masahiro Yamada]

Hi Sam, Rob,

On Thu, Mar 5, 2020 at 12:19 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Tue, Mar 3, 2020 at 11:55 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Masahiro
> >
> > Thanks for the nice improvements to the dt infrastructure.
> >
> > Stealing a thread here..
> >
> > >  It is also possible to run checks with a single schema file by setting the
> > >  ``DT_SCHEMA_FILES`` variable to a specific schema file.
> > Would it be simple to enable the use of dirs for DT_SCHEMA_FILES?
>
> I did name that with the intent of supporting more than one file.
>
> > So I for example could do:
> >
> > make dt_bindings_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/
>
> Does this work?:
>
> make dt_bindings_check DT_SCHEMA_FILES="$(find
> Documentation/devicetree/bindings/display/panel/ -name '*.yaml' |
> xargs)"
>
> Rob


Rob proposed a solution, so
I do not think we should extend this too much.


BTW, there is a limitation that
DT_SCHEMA_FILES must point to file(s)
in Documentation/devicetree/bindings/.


$ cp  Documentation/devicetree/bindings/arm/psci.yaml   ./
$ make  dt_binding_check  DT_SCHEMA_FILES=psci.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
make[1]: *** No rule to make target
'Documentation/devicetree/bindings/psci.yaml', needed by '__build'.
Stop.
make: *** [Makefile:1278: dt_binding_check] Error 2



$(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES))
does not work if DT_SCHEMA_FILES is outside of
Documentation/devicetree/bindings/, but I have no
solution for this.

--
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] kbuild: allow to run dt_binding_check and dtbs_check in a single command

[Rob Herring]

On Tue, Mar 3, 2020 at 9:20 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Since commit 93512dad334d ("dt-bindings: Improve validation build error
> handling"), 'make dtbs_check' does not validate the schema fully.
>
> If you want to check everything, you need to run two commands.
>
>   $ make ARCH=arm dt_binding_check
>   $ make ARCH=arm dtbs_check
>
> They are exclusive each other, so you cannot do like this:
>
>   $ make ARCH=arm dt_binding_check dtbs_check
>
> In this case, dt-doc-validate and dt-extract-example are skipped
> because CHECK_DTBS is set.
>
> Let's make it possible to run those two targets simultaneously.
> It will be useful for schema writers.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Documentation/devicetree/bindings/Makefile  | 8 +++-----
>  Documentation/devicetree/writing-schema.rst | 4 ++++
>  Makefile                                    | 6 +++++-
>  3 files changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 3/3] kbuild: allow to run dt_binding_check without kernel configuration

[Masahiro Yamada]

The dt_binding_check target is located outside of the
'ifneq ($(dtstree),) ... endif' block.

Hence, you can run 'make dt_binding_check' on any architecture.
This makes a perfect sense because the dt-schema is arch-agnostic.

The only one problem I see is that scripts/dtc/dtc is not always built.
For example, ARCH=x86 defconfig does not define CONFIG_DTC. Kbuild
descends into scripts/dtc/, but does nothing. Then, it fails to build
*.example.dt.yaml files.

Let's build scripts/dtc/dtc forcibly when running dt_binding_check.

The dt-schema does not depend on any CONFIG option either, so you
should be able to run dt_binding_check without the .config file.

Going forward, you can directly run 'make dt_binding_check' in a
pristine source tree.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile             | 2 +-
 scripts/dtc/Makefile | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7dec7b343842..190f595c7bfc 100644
--- a/Makefile
+++ b/Makefile
@@ -255,7 +255,7 @@ clean-targets := %clean mrproper cleandocs
 no-dot-config-targets := $(clean-targets) \
 			 cscope gtags TAGS tags help% %docs check% coccicheck \
 			 $(version_h) headers headers_% archheaders archscripts \
-			 %asm-generic kernelversion %src-pkg
+			 %asm-generic kernelversion %src-pkg dt_binding_check
 no-sync-config-targets := $(no-dot-config-targets) install %install \
 			   kernelrelease
 single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.s %.symtypes %/
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 3acbb410904c..2f3c3a7e1620 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 # scripts/dtc makefile
 
-hostprogs		:= dtc
-always-$(CONFIG_DTC)	:= $(hostprogs)
+hostprogs			:= dtc
+always-$(CONFIG_DTC)		+= $(hostprogs)
+always-$(CHECK_DT_BINDING)	+= $(hostprogs)
 
 dtc-objs	:= dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
 		   srcpos.o checks.o util.o
-- 
2.17.1

Re: [PATCH 3/3] kbuild: allow to run dt_binding_check without kernel configuration

[Rob Herring]

On Tue, Mar 3, 2020 at 9:21 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The dt_binding_check target is located outside of the
> 'ifneq ($(dtstree),) ... endif' block.
>
> Hence, you can run 'make dt_binding_check' on any architecture.
> This makes a perfect sense because the dt-schema is arch-agnostic.
>
> The only one problem I see is that scripts/dtc/dtc is not always built.
> For example, ARCH=x86 defconfig does not define CONFIG_DTC. Kbuild
> descends into scripts/dtc/, but does nothing. Then, it fails to build
> *.example.dt.yaml files.

Yeah, I've just worked around this by doing 'make CONFIG_DTC=y
dt_binding_check'.

The only thing I'd come up with was just always building dtc, but I
didn't want to do that.

> Let's build scripts/dtc/dtc forcibly when running dt_binding_check.
>
> The dt-schema does not depend on any CONFIG option either, so you
> should be able to run dt_binding_check without the .config file.
>
> Going forward, you can directly run 'make dt_binding_check' in a
> pristine source tree.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Makefile             | 2 +-
>  scripts/dtc/Makefile | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 7dec7b343842..190f595c7bfc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -255,7 +255,7 @@ clean-targets := %clean mrproper cleandocs
>  no-dot-config-targets := $(clean-targets) \
>                          cscope gtags TAGS tags help% %docs check% coccicheck \
>                          $(version_h) headers headers_% archheaders archscripts \
> -                        %asm-generic kernelversion %src-pkg
> +                        %asm-generic kernelversion %src-pkg dt_binding_check
>  no-sync-config-targets := $(no-dot-config-targets) install %install \
>                            kernelrelease
>  single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.s %.symtypes %/
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 3acbb410904c..2f3c3a7e1620 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -1,8 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # scripts/dtc makefile
>
> -hostprogs              := dtc
> -always-$(CONFIG_DTC)   := $(hostprogs)
> +hostprogs                      := dtc
> +always-$(CONFIG_DTC)           += $(hostprogs)
> +always-$(CHECK_DT_BINDING)     += $(hostprogs)

This looks like a good solution.

Reviewed-by: Rob Herring <robh@kernel.org>