kbuild: Always validate DT binding examples against all schemas

PATCH Superseded Rob Herring 2 participants 4 messages Feb 21, 2020 – Feb 22, 2020

Subsystems (2)

Device tree bindings Kbuild

lore

Share & tools

lore

Rob Herring

author maintainer

2020-02-21 22:29 UTC #0 lore raw

PATCH 1/2 kbuild: Always validate DT binding examples against all schemas

Most folks only run dt_binding_check on the single schema they care about by setting DT_SCHEMA_FILES. That means example is only checked against that one schema which is not always sufficient.

Let's address this by splitting processed-schema.yaml into 2 files: one that's always all schemas for the examples and one that's just the schema in DT_SCHEMA_FILES for dtbs.

Cc Masahiro Yamada <[email protected]>

Cc Michal Marek <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
 Documentation/devicetree/bindings/Makefile | 20 +++++++++++---------
 scripts/Makefile.lib                       | 12 ++++++++----
 2 files changed, 19 insertions(+), 13 deletions(-)

Documentation/devicetree/bindings/Makefile +11 -9

index 646cb3525373..16498eb19f5f 100644
@@ -11,26 +11,28 @@ quiet_cmd_chk_binding = CHKDT   $(patsubst $(srctree)/%,%,$<)
 $(obj)/%.example.dts: $(src)/%.yaml FORCE
 	$(call if_changed,chk_binding)
 
-DT_TMP_SCHEMA := processed-schema.yaml
-
 quiet_cmd_mk_schema = SCHEMA  $@
-      cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs)
+      cmd_mk_schema = $(DT_MK_SCHEMA) $(2) -o $@ $(real-prereqs)
 
-DT_DOCS = $(shell \
+DT_DOCS = $(addprefix $(src)/, \
+	$(shell \
 	cd $(srctree)/$(src) && \
 	find * \( -name '*.yaml' ! \
-		-name $(DT_TMP_SCHEMA) ! \
+		-name 'processed-schema-*' ! \
 		-name '*.example.dt.yaml' \) \
-	)
+	))
 
-DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS))
+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))
 endif
 
-$(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE
+$(obj)/processed-schema-examples.yaml: $(DT_DOCS) FORCE
 	$(call if_changed,mk_schema)
 
-extra-y += $(DT_TMP_SCHEMA)
+$(obj)/processed-schema-dtbs.yaml: $(DT_SCHEMA_FILES) FORCE
+	$(call if_changed,mk_schema, $(DT_MK_SCHEMA_FLAGS))
+
+extra-y += processed-schema-examples.yaml processed-schema-dtbs.yaml

scripts/Makefile.lib +8 -4

index bae62549e3d2..78fa1a3d983a 100644
@@ -297,17 +297,21 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
-DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
+DT_DTBS_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema-dtbs.yaml
+DT_EXAMPLES_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema-examples.yaml
 
 quiet_cmd_dtb_check =	CHECK   $@
-      cmd_dtb_check =	$(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ ;
+      cmd_dtb_check =	$(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(2) $@ ;
 
 define rule_dtc_dt_yaml
 	$(call cmd_and_fixdep,dtc,yaml)
-	$(call cmd,dtb_check)
+	$(call cmd,dtb_check,$(word 2, $(real-prereqs)))
 endef
 
-$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
+$(obj)/%.example.dt.yaml: $(src)/%.example.dts $(DT_EXAMPLES_SCHEMA) $(DTC) FORCE
+	$(call if_changed_rule,dtc_dt_yaml)
+
+$(obj)/%.dt.yaml: $(src)/%.dts $(DT_DTBS_SCHEMA) $(DTC) FORCE
 	$(call if_changed_rule,dtc_dt_yaml)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

Signature · 1 line

2.20.1

Rob Herring

author maintainer

2020-02-21 22:29 UTC #1 lore raw

PATCH 2/2 kbuild: Build DT binding examples with dtc warnings enabled

Now that we have a separate rule for DT binding examples, we can customize the dtc options. Let's adjust the dtc warnings to me more strict by default so the examples get cleaned up as they get converted to schema.

Leaving 'avoid_unnecessary_addr_size' and 'graph_child_address' warnings disabled as examples tend to be incomplete and they generates a lot of warnings.

Cc Masahiro Yamada <[email protected]>

Cc Michal Marek <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
 scripts/Makefile.lib | 4 ++++
 1 file changed, 4 insertions(+)

scripts/Makefile.lib +4 -0

index 78fa1a3d983a..1a149e680308 100644
@@ -308,6 +308,10 @@ define rule_dtc_dt_yaml
 	$(call cmd,dtb_check,$(word 2, $(real-prereqs)))
 endef
 
+$(obj)/%.example.dt.yaml: DTC_FLAGS = \
+	-Wno-avoid_unnecessary_addr_size \
+	-Wno-graph_child_address
+
 $(obj)/%.example.dt.yaml: $(src)/%.example.dts $(DT_EXAMPLES_SCHEMA) $(DTC) FORCE
 	$(call if_changed_rule,dtc_dt_yaml)

Signature · 1 line

2.20.1

Masahiro Yamada

2020-02-22 16:50 UTC #2 lore raw

Re: [PATCH 2/2] kbuild: Build DT binding examples with dtc warnings enabled

Hi Rob,

Quoted 22 lines · Rob Herring

Now that we have a separate rule for DT binding examples, we can customize the dtc options. Let's adjust the dtc warnings to me more strict by default so the examples get cleaned up as they get converted to schema.
Leaving 'avoid_unnecessary_addr_size' and 'graph_child_address' warnings disabled as examples tend to be incomplete and they generates a lot of warnings.
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
---
 scripts/Makefile.lib | 4 ++++
 1 file changed, 4 insertions(+)
scripts/Makefile.lib +0 -0
index 78fa1a3d983a..1a149e680308 100644

scripts/Makefile.lib define rule_dtc_dt_yaml · line 314

Quoted 7 lines

scripts/Makefile.lib +4 -0

@@ -308,6 +308,10 @@ define rule_dtc_dt_yaml
        $(call cmd,dtb_check,$(word 2, $(real-prereqs)))
 endef
 
+$(obj)/%.example.dt.yaml: DTC_FLAGS = \
+       -Wno-avoid_unnecessary_addr_size \
+       -Wno-graph_child_address
+

This does not work since I suggested to not add this pattern rule in 1/2.

Instead, you can override DTC_FLAGS
from Documentation/devicetree/bindings/Makefile

So, alternative solution is like follows (on top of my suggestion in 1/2)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/Makefile index 7c40d5ba1b51..e44c972849e6 100644

Documentation/devicetree/bindings/Makefile +2 -0

@@ -12,6 +12,8 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE
 
 # Use full schemas when checking %.example.dts
 DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml
+# More strict checks for examples
+override DTC_FLAGS := -Wno-avoid_unnecessary_addr_size -Wno-graph_child_address
 
 quiet_cmd_mk_schema = SCHEMA  $@
       cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@

$(real-prereqs)

One limitation for this way is that you cannot use W=1, W=2, W=3 for example DT.

Quoted 2 lines

scripts/Makefile.lib +0 -0

 $(obj)/%.example.dt.yaml: $(src)/%.example.dts $(DT_EXAMPLES_SCHEMA) $(DTC) FORCE
        $(call if_changed_rule,dtc_dt_yaml)

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

2020-02-22 15:52 UTC #3 lore raw

In reply to Rob Herring

Hi Rob,

Quoted 5 lines · Rob Herring

Most folks only run dt_binding_check on the single schema they care about by setting DT_SCHEMA_FILES. That means example is only checked against that one schema which is not always sufficient.

Quoted 8 lines

Let's address this by splitting processed-schema.yaml into 2 files: one that's always all schemas for the examples and one that's just the schema in DT_SCHEMA_FILES for dtbs.
Cc Masahiro Yamada <[email protected]>
Cc Michal Marek <[email protected]>
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>

I understood what you want to do, but
you are adding too complicated code
into scripts/Malefile.lib

Also, I do not like fragile workaround like
$(word 2, $(real-prereqs)

I attached a simpler implementation below.
I also changed .gitignore

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/.gitignore
index ef82fcfcccab..57afa1533a5f 100644

Documentation/devicetree/bindings/.gitignore +1 -1

@@ -1,2 +1,2 @@
 *.example.dts
-processed-schema.yaml
+processed-schema*.yaml

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/Makefile
index 646cb3525373..7c40d5ba1b51 100644

Documentation/devicetree/bindings/Makefile +0 -1

@@ -2,7 +2,6 @@
 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) -u $(srctree)/$(src) $< ; \

@@ -11,26 +10,33 @@ quiet_cmd_chk_binding = CHKDT   $(patsubst

$(srctree)/%,%,$<)
$(obj)/%.example.dts: $(src)/%.yaml FORCE

        $(call if_changed,chk_binding)

-DT_TMP_SCHEMA := processed-schema.yaml
+# Use full schemas when checking %.example.dts +DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml

quiet_cmd_mk_schema = SCHEMA $@

       cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@

$(real-prereqs)

-DT_DOCS = $(shell \
+DT_DOCS = $(addprefix $(src)/, \

+       $(shell \
        cd $(srctree)/$(src) && \
        find * \( -name '*.yaml' ! \
-               -name $(DT_TMP_SCHEMA) ! \
+               -name 'processed-schema*' ! \
                -name '*.example.dt.yaml' \) \

- )
+ ))

-DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) +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
+
+$(obj)/processed-schema-examples.yaml: $(DT_DOCS) FORCE
+ $(call if_changed,mk_schema)
endif

-$(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE +$(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := -u +$(obj)/processed-schema.yaml: $(DT_SCHEMA_FILES) FORCE

        $(call if_changed,mk_schema)

-extra-y += $(DT_TMP_SCHEMA)
+extra-y += processed-schema.yaml

scripts/Makefile.lib +1 -1

index bae62549e3d2..7ddeb4f718b6 100644
@@ -297,7 +297,8 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
-DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml
+# DT_TMP_SCHEMA may be overridden from

Documentation/devicetree/bindings/Makefile
+DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.yaml

quiet_cmd_dtb_check = CHECK $@

       cmd_dtb_check =  $(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR)

-p $(DT_TMP_SCHEMA) $@ ;

Signature · 2 lines

Best Regards
Masahiro Yamada