Skip to contentSkip to conversation

kbuild: Add support for DT binding schema checks

PATCH In review Rob Herring 3 participants 9 messages Dec 11, 2018 – Jan 28, 2019

Subsystems (4)
Device tree bindings Documentation Kbuild Device tree
lore
Share & tools
lore

Rob Herring

author maintainer
#0 lore raw

PATCH v4 1/1 kbuild: Add support for DT binding schema checks

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Cc Jonathan Corbet <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>

---
v4:
- Rework libyaml check and error message with Masahiro's version
- Simplify build rules and dependencies

 .gitignore                                   |  1 +
 Documentation/Makefile                       |  2 +-
 Documentation/devicetree/bindings/.gitignore |  2 ++
 Documentation/devicetree/bindings/Makefile   | 27 ++++++++++++++++++++
 Makefile                                     | 13 +++++++---
 scripts/Makefile.lib                         | 24 +++++++++++++++--
 scripts/dtc/Makefile                         |  4 +++
 7 files changed, 67 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/.gitignore
 create mode 100644 Documentation/devicetree/bindings/Makefile
.gitignore +1 -0
index 97ba6b79834c..a20ac26aa2f5 100644
@@ -15,6 +15,7 @@
 *.bin
 *.bz2
 *.c.[012]*.*
+*.dt.yaml
 *.dtb
 *.dtb.S
 *.dwo
Documentation/Makefile +1 -1
index 2ca77ad0f238..9786957c6a35 100644
@@ -2,7 +2,7 @@
 # Makefile for Sphinx documentation
 #
 
-subdir-y :=
+subdir-y := devicetree/bindings/
 
 # You can set these variables from the command line.
 SPHINXBUILD   = sphinx-build
Documentation/devicetree/bindings/.gitignore new file +2 -0
new file mode 100644
index 000000000000..ef82fcfcccab
@@ -0,0 +1,2 @@
+*.example.dts
+processed-schema.yaml
Documentation/devicetree/bindings/Makefile new file +27 -0
new file mode 100644
index 000000000000..6e5cef0ed6fb
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0
+DT_DOC_CHECKER ?= dt-doc-validate
+DT_EXTRACT_EX ?= dt-extract-example
+DT_MK_SCHEMA ?= dt-mk-schema
+DT_MK_SCHEMA_FLAGS := $(if $(DT_SCHEMA_FILES), -u)
+
+quiet_cmd_chk_binding = CHKDT   $(patsubst $(srctree)/%,%,$<)
+      cmd_chk_binding = $(DT_DOC_CHECKER) $< ; \
+                        $(DT_EXTRACT_EX) $< > $@
+
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+	$(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := processed-schema.yaml
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA  $@
+      cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
+
+DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml')
+DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS))
+
+extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES))
+extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))
+
+$(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE
+	$(call if_changed,mk_schema)
Makefile +10 -3
index 2f36db897895..a3e2db2a3119 100644
@@ -1232,10 +1232,13 @@ ifneq ($(dtstree),)
 %.dtb: prepare3 scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
-PHONY += dtbs dtbs_install
-dtbs: prepare3 scripts_dtc
+PHONY += dtbs dtbs_install dt_binding_check
+dtbs dtbs_check: prepare3 scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)
 
+dtbs_check: export CHECK_DTBS=1
+dtbs_check: dt_binding_check
+
 dtbs_install:
 	$(Q)$(MAKE) $(dtbinst)=$(dtstree)
 

@@ -1249,6 +1252,9 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
 	$(Q)$(MAKE) $(build)=scripts/dtc
 
+dt_binding_check: scripts_dtc
+	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
+
 # ---------------------------------------------------------------------------
 # Modules
 

@@ -1611,7 +1617,8 @@ clean: $(clean-dirs)
 	$(call cmd,rmfiles)
 	@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 '*.ko.*' \
+		-o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
 		-o -name '*.dwo' -o -name '*.lst' \
 		-o -name '*.su'  \
 		-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
scripts/Makefile.lib +22 -2
index 8fe4468f9bda..723b33c942df 100644
@@ -61,6 +61,11 @@ real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y))
 extra-y				+= $(dtb-y)
 extra-$(CONFIG_OF_ALL_DTBS)	+= $(dtb-)
 
+ifneq ($(CHECK_DTBS),)
+extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
+extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
+endif
+
 # Add subdir path
 
 extra-y		:= $(addprefix $(obj)/,$(extra-y))

@@ -284,13 +289,28 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
 	$(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(DTC) -O dtb -o $@ -b 0 \
+	$(DTC) -O $(2) -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
-	$(call if_changed_dep,dtc)
+	$(call if_changed_dep,dtc,dtb)
+
+DT_CHECKER ?= dt-validate
+DT_BINDING_DIR := Documentation/devicetree/bindings
+DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
+
+quiet_cmd_dtb_check =	CHECK   $@
+      cmd_dtb_check =	$(DT_CHECKER) -p $(DT_TMP_SCHEMA) $@ ;
+
+define rule_dtc_dt_yaml
+	$(call cmd_and_fixdep,dtc,yaml)		\
+	$(call echo-cmd,dtb_check) $(cmd_dtb_check)
+endef
+
+$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
+	$(call if_changed_rule,dtc_dt_yaml)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
scripts/dtc/Makefile +4 -0
index 056d5da6c477..5f227d8d39d8 100644
@@ -12,6 +12,10 @@ dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
 HOST_EXTRACFLAGS := -I$(src)/libfdt
 
 ifeq ($(wildcard /usr/include/yaml.h),)
+ifneq ($(CHECK_DTBS),)
+$(error dtc needs libyaml for DT schema validation support. \
+	Install the necessary libyaml development package.)
+endif
 HOST_EXTRACFLAGS += -DNO_YAML
 else
 dtc-objs	+= yamltree.o
Signature · 1 line
2.19.1
Acked-by on v4 1/1
#1 lore raw
Quoted 26 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Cc Jonathan Corbet <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
Quoted 5 lines

---
v4:
- Rework libyaml check and error message with Masahiro's version
- Simplify build rules and dependencies

Acked-by Masahiro Yamada <[email protected]>

Thanks.

Signature · 2 lines
Best Regards
Masahiro Yamada
#2 lore raw
In reply to Rob Herring

Hi Rob,

Quoted 9 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Quoted 7 lines

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Thanks, I'm giving this a try, and get errors like:

DTC arch/arm/boot/dts/emev2-kzm9d.dt.yaml
FATAL ERROR: No markers present in property 'cpu0' value

and

DTC arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
FATAL ERROR: No markers present in property 'audio_clk_a' value

Do you have a clue?
Thanks!

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#3 lore raw
Quoted 26 lines · Geert Uytterhoeven

Hi Rob,

Quoted 9 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Quoted 7 lines

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Thanks, I'm giving this a try, and get errors like:

DTC arch/arm/boot/dts/emev2-kzm9d.dt.yaml
FATAL ERROR: No markers present in property 'cpu0' value

Quoted 6 lines

and

DTC arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
FATAL ERROR: No markers present in property 'audio_clk_a' value

Do you have a clue?

That's really strange because those aren't even properties. Are other dts files okay? This is the in tree dtc?

The only time you should be missing markers is if you did a dts -> dts -> dt.yaml.

Rob

#4 lore raw

Hi Rob,

Quoted 29 lines · Rob Herring
Quoted 28 lines · Geert Uytterhoeven
Quoted 9 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Quoted 7 lines

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Thanks, I'm giving this a try, and get errors like:

DTC arch/arm/boot/dts/emev2-kzm9d.dt.yaml
FATAL ERROR: No markers present in property 'cpu0' value

and

DTC arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
FATAL ERROR: No markers present in property 'audio_clk_a' value

Quoted 7 lines
Quoted 1 line

Do you have a clue?

That's really strange because those aren't even properties. Are other dts files okay? This is the in tree dtc?

The only time you should be missing markers is if you did a dts -> dts -> dt.yaml.

Found it: make dtbs_check doesn't play well with my local change to add symbols for DT overlays:

    --- a/scripts/Makefile.lib
    +++ b/scripts/Makefile.lib
    @@ -285,6 +285,10 @@ cmd_dt_S_dtb=
             \
     $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
            $(call if_changed,dt_S_dtb)

    +ifeq ($(CONFIG_OF_OVERLAY),y)
    +DTC_FLAGS += -@
    +endif
    +
     quiet_cmd_dtc = DTC     $@
     cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
            $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o

$(dtc-tmp) $< ; \

Do you see a way to handle that better?

Apart from a few expected issues, I'm seeing one other strange message:

    arch/arm/boot/dts/sh73a0-kzm9g.dt.yaml: interrupts: [[2, 4], [3,

4]] is too long

This is the interrupts property in the adi,adxl345 node in arch/arm/boot/dts/sh73a0-kzm9g.dts.
Apparently the check complains if more than one interrupt is listed here. Is this a known issue?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

author maintainer
#5 lore raw
Quoted 35 lines · Geert Uytterhoeven

Hi Rob,

Quoted 31 lines · Rob Herring
Quoted 29 lines · Geert Uytterhoeven
Quoted 9 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Quoted 7 lines

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Thanks, I'm giving this a try, and get errors like:

DTC arch/arm/boot/dts/emev2-kzm9d.dt.yaml
FATAL ERROR: No markers present in property 'cpu0' value

and

DTC arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
FATAL ERROR: No markers present in property 'audio_clk_a' value

Do you have a clue?

Quoted 8 lines
Quoted 5 lines

That's really strange because those aren't even properties. Are other dts files okay? This is the in tree dtc?

The only time you should be missing markers is if you did a dts -> dts -> dt.yaml.

Found it: make dtbs_check doesn't play well with my local change to add symbols for DT overlays:

Now that makes sense.

Quoted 14 lines
    --- a/scripts/Makefile.lib
    +++ b/scripts/Makefile.lib
    @@ -285,6 +285,10 @@ cmd_dt_S_dtb=
             \
     $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
            $(call if_changed,dt_S_dtb)

    +ifeq ($(CONFIG_OF_OVERLAY),y)
    +DTC_FLAGS += -@
    +endif
    +
     quiet_cmd_dtc = DTC     $@
     cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
            $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o
Quoted 3 lines

$(dtc-tmp) $< ; \

Do you see a way to handle that better?

We need to have the code that generates these properties to also add markers. Or we could drop the __symbols__ nodes on YAML output. Or ignore the option when doing YAML output.

Quoted 9 lines

Apart from a few expected issues, I'm seeing one other strange message:

    arch/arm/boot/dts/sh73a0-kzm9g.dt.yaml: interrupts: [[2, 4], [3,

4]] is too long

This is the interrupts property in the adi,adxl345 node in arch/arm/boot/dts/sh73a0-kzm9g.dts.
Apparently the check complains if more than one interrupt is listed here. Is this a known issue?

There are lots of warnings... I've gone thru and checked some to make sure they are valid, but certainly not all. There's probably some cases that are too strict too.

This one is because this device is listed in trivial-devices.yaml and you can't have 2 interrupts for a trivial device (because you need to define the interrupt order). Looks like we have a binding doc for it too, so we should just remove it from trivial-devices.yaml.

There are lots of '... is too (long|short)' messages because of how the dts file property values are bracketed. This used to not matter, but is significant in the YAML output. I have a dtc patch to give warnings on all these (and dtc will give source location).

Rob

#6 lore raw
In reply to Rob Herring

Hi Rob,

Quoted 17 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Quoted 8 lines
Cc Jonathan Corbet <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>

BTW, what are the CONFIG dependencies for this to work? E.g. defconfig on x86_64 fails, even after enabling CONFIG_OF:

$ make dt_binding_check
  SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
  CHKDT   Documentation/devicetree/bindings/arm/primecell.yaml
  ...
  CHKDT   Documentation/devicetree/bindings/trivial-devices.yaml
make[1]: *** No rule to make target
'Documentation/devicetree/bindings/arm/primecell.example.dtb', needed
by '__build'.  Stop.

Obviously it does work for arm/arm64.

Thanks!

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#7 lore raw
Quoted 30 lines · Geert Uytterhoeven

Hi Rob,

Quoted 17 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Quoted 8 lines
Cc Jonathan Corbet <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
Quoted 2 lines

BTW, what are the CONFIG dependencies for this to work? E.g. defconfig on x86_64 fails, even after enabling CONFIG_OF:

I generally use allmodconfig which enables building all DTs.

Yes, there's a dependency on CONFIG_DTC which isn't always enabled with CONFIG_OF. Maybe it should be. The only other solutions I've thought of are either always build dtc or make the targets conditional on CONFIG_DTC. The latter would only change the error message.

Rob

#8 lore raw

Hi Rob,

Quoted 19 lines · Rob Herring
Quoted 17 lines · Geert Uytterhoeven
Quoted 16 lines · Rob Herring

This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema.

Check DT binding schema documents:
make dt_binding_check

Build dts files and check using DT binding schema:
make dtbs_check

Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema.

Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated.

Quoted 7 lines
Quoted 2 lines

BTW, what are the CONFIG dependencies for this to work? E.g. defconfig on x86_64 fails, even after enabling CONFIG_OF:

I generally use allmodconfig which enables building all DTs.

Yes, there's a dependency on CONFIG_DTC which isn't always enabled with CONFIG_OF. Maybe it should be. The only other solutions I've

Oh, didn't think of CONFIG_DTC.

Quoted 2 lines

thought of are either always build dtc or make the targets conditional on CONFIG_DTC. The latter would only change the error message.

Making the target conditional may make it more obvious to the user what's going on. Cfr. "make modules_install" giving a nice explanation when CONFIG_MODULES=n.

Thanks.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds