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 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 <corbet@lwn.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masahiro Yamada <redacted>
Cc: Michal Marek <redacted>
Cc: linux-doc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
- Fix error causing only 1st schema file to get used.
- Add a more useful error message when dtc is missing YAML support
telling the user they need to install libyaml devel package.
.gitignore | 1 +
Documentation/Makefile | 2 +-
Documentation/devicetree/bindings/.gitignore | 1 +
Documentation/devicetree/bindings/Makefile | 33 +++++++++++++++++
Makefile | 11 ++++--
scripts/Makefile.lib | 37 ++++++++++++++++++--
6 files changed, 80 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/.gitignore
create mode 100644 Documentation/devicetree/bindings/Makefile
diff --git a/.gitignore b/.gitignore
index 97ba6b79834c..a20ac26aa2f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@
*.bin
*.bz2
*.c.[012]*.*
+*.dt.yaml
*.dtb
*.dtb.S
*.dwo
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2ca77ad0f238..9786957c6a35 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -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
diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
new file mode 100644
index 000000000000..d9194c02dd08
--- /dev/null
+++ b/Documentation/devicetree/bindings/.gitignore
@@ -0,0 +1 @@
+*.example.dts
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
new file mode 100644
index 000000000000..43f8657ab070
--- /dev/null
+++ b/Documentation/devicetree/bindings/Makefile
@@ -0,0 +1,33 @@
+# 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 $<
+ cmd_chk_binding = (set -e; \
+ $(DT_DOC_CHECKER) $< ; \
+ mkdir -p $(dir $@) ; \
+ $(DT_EXTRACT_EX) $< > $@ )
+
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(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): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
+
+$(obj)/$(DT_TMP_SCHEMA): $(addprefix $(srctree)/, $(DT_SCHEMA_FILES)) FORCE
+ $(call if_changed,mk_schema)
diff --git a/Makefile b/Makefile
index 2f36db897895..ff59adf43300 100644
--- a/Makefile
+++ b/Makefile
@@ -1232,10 +1232,13 @@ ifneq ($(dtstree),)
%.dtb: prepare3 scripts_dtc
$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
-PHONY += dtbs dtbs_install
+PHONY += dtbs dtbs_install dt_binding_check
dtbs: prepare3 scripts_dtc
$(Q)$(MAKE) $(build)=$(dtstree)
+dtbs_check: prepare3 dt_binding_check
+ $(Q)$(MAKE) $(build)=$(dtstree) CHECK_DTBS=1
+
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' \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8fe4468f9bda..6a3450b20041 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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,41 @@ $(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)/.schema.yaml.tmp
+
+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
+
+cmd_dtc_yaml_chk = \
+ if ! echo "/dts-v1/; /{};" | $(DTC) -I dts -O yaml > /dev/null ; then \
+ echo "*"; \
+ echo "* dtc needs libyaml for DT schema validation support."; \
+ echo "* Install the necessary libyaml development package from your distro."; \
+ echo "*"; \
+ false; \
+ fi; \
+ touch $@
+
+$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp: $(DTC) FORCE
+ $(call if_changed,dtc_yaml_chk)
+
+$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE | $(objtree)/scripts/dtc/.dtc-yaml-chk.tmp
+ $(call if_changed_rule,dtc_dt_yaml)
dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
--
2.19.1
Hi Rob,
On Tue, Dec 11, 2018 at 5:50 AM Rob Herring [off-list ref] wrote:
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 passed in with a schema file(s) to use
Perhaps, "can be passed" ?
quoted hunk
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 <corbet@lwn.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masahiro Yamada <redacted>
Cc: Michal Marek <redacted>
Cc: linux-doc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
- Fix error causing only 1st schema file to get used.
- Add a more useful error message when dtc is missing YAML support
telling the user they need to install libyaml devel package.
.gitignore | 1 +
Documentation/Makefile | 2 +-
Documentation/devicetree/bindings/.gitignore | 1 +
Documentation/devicetree/bindings/Makefile | 33 +++++++++++++++++
Makefile | 11 ++++--
scripts/Makefile.lib | 37 ++++++++++++++++++--
6 files changed, 80 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/.gitignore
create mode 100644 Documentation/devicetree/bindings/Makefile
diff --git a/.gitignore b/.gitignore
index 97ba6b79834c..a20ac26aa2f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@
*.bin
*.bz2
*.c.[012]*.*
+*.dt.yaml
*.dtb
*.dtb.S
*.dwo
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2ca77ad0f238..9786957c6a35 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -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
diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
new file mode 100644
index 000000000000..d9194c02dd08
--- /dev/null
+++ b/Documentation/devicetree/bindings/.gitignore
@@ -0,0 +1 @@
+*.example.dts
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
new file mode 100644
index 000000000000..43f8657ab070
--- /dev/null
+++ b/Documentation/devicetree/bindings/Makefile
@@ -0,0 +1,33 @@
+# 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 $<
In convention, the short log displays the target name
instead of the prerequisite.
If O=foo/bar is given, $< shows the full path,
then log will look like follows:
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/calxeda.yaml
DTC Documentation/devicetree/bindings/arm/calxeda.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/qcom.yaml
DTC Documentation/devicetree/bindings/arm/qcom.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/xilinx.yaml
DTC Documentation/devicetree/bindings/arm/xilinx.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/nspire.yaml
DTC Documentation/devicetree/bindings/arm/ti/nspire.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/ti,davinci.yaml
DTC Documentation/devicetree/bindings/arm/ti/ti,davinci.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/altera.yaml
You might think it is ugly.
+ cmd_chk_binding = (set -e; \
+ $(DT_DOC_CHECKER) $< ; \
+ mkdir -p $(dir $@) ; \
+ $(DT_EXTRACT_EX) $< > $@ )
- 'set -e' is redundant because if_changed already has it.
- 'mkdir mkdir -p $(dir $@)' is also redundant because
scripts/Makefile.build automatically creates it.
- You do not need to execute this in a sub-shell
You can simplify like this:
cmd_chk_binding = $(DT_DOC_CHECKER) $< ; \
$(DT_EXTRACT_EX) $< > $@
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
BTW, why does this file start with a period?
What is the meaning of '.tmp' extension?
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
"mkdir -p $(obj)" is redundant.
Why is 'rm -f $@' necessary ?
Can't dt-mk-schema overwrite the output file?
+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))
I assume you intentionally did not do like this:
extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
From the commit description, DT_SCHEMA_FILES might be overridden by a user.
So, I think this is OK.
+$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
I do not understand this line.
Why is it necessary?
*.example.dtb files are generated anyway
since they are listed in extra-y.
+$(obj)/$(DT_TMP_SCHEMA): $(addprefix $(srctree)/, $(DT_SCHEMA_FILES)) FORCE
+ $(call if_changed,mk_schema)
You do not need to prefix $(srctree)/ because Kbuild uses VPATH.
$(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE
$(call if_changed,mk_schema)
is fine.
+cmd_dtc_yaml_chk = \
+ if ! echo "/dts-v1/; /{};" | $(DTC) -I dts -O yaml > /dev/null ; then \
+ echo "*"; \
+ echo "* dtc needs libyaml for DT schema validation support."; \
+ echo "* Install the necessary libyaml development package from your distro."; \
+ echo "*"; \
+ false; \
+ fi; \
+ touch $@
+
+$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp: $(DTC) FORCE
+ $(call if_changed,dtc_yaml_chk)
Hmm, this is kind of ugly.
I'd rather check this in scripts/dtc/Makefile
How about something like below?
diff --git a/Makefile b/Makefile
index ff59adf..a3e2db2 100644
--- a/Makefile
+++ b/Makefile
@@ -1233,11 +1233,11 @@ ifneq ($(dtstree),)
$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
PHONY += dtbs dtbs_install dt_binding_check
-dtbs: prepare3 scripts_dtc
+dtbs dtbs_check: prepare3 scripts_dtc
$(Q)$(MAKE) $(build)=$(dtstree)
-dtbs_check: prepare3 dt_binding_check
- $(Q)$(MAKE) $(build)=$(dtstree) CHECK_DTBS=1
+dtbs_check: export CHECK_DTBS=1
+dtbs_check: dt_binding_check
dtbs_install:
$(Q)$(MAKE) $(dtbinst)=$(dtstree)diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8a7d622..5017175 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -309,20 +309,7 @@ define rule_dtc_dt_yaml
$(call echo-cmd,dtb_check) $(cmd_dtb_check)
endef
-cmd_dtc_yaml_chk = \
- if ! echo "/dts-v1/; /{};" | $(DTC) -I dts -O yaml > /dev/null ; then \
- echo "*"; \
- echo "* dtc needs libyaml for DT schema validation support."; \
- echo "* Install the necessary libyaml developmentpackage from your distro."; \
- echo "*"; \
- false; \
- fi; \
- touch $@
-
-$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp: $(DTC) FORCE
- $(call if_changed,dtc_yaml_chk)
-
-$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE |
$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp
+$(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)diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 056d5da..3e497f1 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -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),)
+ifeq ($(CHECK_DTBS),1)
+$(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
One drawback of this approach is,
this is not checked if you are using out-of-tree DTC.
Probably, Rob is the only person that overrides DTC.
--
Best Regards
Masahiro Yamada
On Mon, Dec 10, 2018 at 10:39 PM Masahiro Yamada
[off-list ref] wrote:
Hi Rob,
On Tue, Dec 11, 2018 at 5:50 AM Rob Herring [off-list ref] wrote:
quoted
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 passed in with a schema file(s) to use
Perhaps, "can be passed" ?
Yes.
[...]
quoted
+quiet_cmd_chk_binding = CHKDT $<
In convention, the short log displays the target name
instead of the prerequisite.
Yes, but here there is no target. We're just running a check on the source.
If O=foo/bar is given, $< shows the full path,
then log will look like follows:
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/calxeda.yaml
DTC Documentation/devicetree/bindings/arm/calxeda.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/qcom.yaml
DTC Documentation/devicetree/bindings/arm/qcom.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/xilinx.yaml
DTC Documentation/devicetree/bindings/arm/xilinx.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/nspire.yaml
DTC Documentation/devicetree/bindings/arm/ti/nspire.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/ti,davinci.yaml
DTC Documentation/devicetree/bindings/arm/ti/ti,davinci.example.dtb
CHKDT /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/altera.yaml
You might think it is ugly.
I've changed it to this:
quiet_cmd_chk_binding = CHKDT $(patsubst $(srctree)/%,%,$<)
[...]
quoted
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
BTW, why does this file start with a period?
What is the meaning of '.tmp' extension?
Nothing really. Just named it something so it gets cleaned and ignored by git.
quoted
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
"mkdir -p $(obj)" is redundant.
Why is 'rm -f $@' necessary ?
Can't dt-mk-schema overwrite the output file?
It is for error case when the output file is not generated. I can
handle this within dt-mk-schema instead.
quoted
+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))
I assume you intentionally did not do like this:
extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
From the commit description, DT_SCHEMA_FILES might be overridden by a user.
So, I think this is OK.
quoted
+$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
I do not understand this line.
Why is it necessary?
*.example.dtb files are generated anyway
since they are listed in extra-y.
It is enforcing the ordering. Without it, the binding checks and
building .schema.yaml.tmp happen in parallel because both only have
the source files as dependencies. The '|' keeps the dependencies out
of the dependency list($^).
[...]
quoted
+cmd_dtc_yaml_chk = \
+ if ! echo "/dts-v1/; /{};" | $(DTC) -I dts -O yaml > /dev/null ; then \
+ echo "*"; \
+ echo "* dtc needs libyaml for DT schema validation support."; \
+ echo "* Install the necessary libyaml development package from your distro."; \
+ echo "*"; \
+ false; \
+ fi; \
+ touch $@
+
+$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp: $(DTC) FORCE
+ $(call if_changed,dtc_yaml_chk)
Hmm, this is kind of ugly.
I'd rather check this in scripts/dtc/Makefile
How about something like below?
Looks good.
ifeq ($(wildcard /usr/include/yaml.h),)
+ifeq ($(CHECK_DTBS),1)
I'm going to change this to "ifneq ($CHECK_DTBS),)" in case we start
supporting more than 1 value such as warning levels.
+$(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
One drawback of this approach is,
this is not checked if you are using out-of-tree DTC.
Probably, Rob is the only person that overrides DTC.
Sadly, that's probably true.
Thanks for your thorough review.
Rob
On Wed, Dec 12, 2018 at 12:13 AM Rob Herring [off-list ref] wrote:
quoted
quoted
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
BTW, why does this file start with a period?
What is the meaning of '.tmp' extension?
Nothing really. Just named it something so it gets cleaned and ignored by git.
It is cleaned whatever file name you use.
See scripts/Makefile.clean
__clean-files := $(extra-y) $(extra-m) $(extra-) \
$(always) $(targets) $(clean-files) \
$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \
$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \
$(hostcxxlibs-y) $(hostcxxlibs-m)
$(extra-y) is cleaned.
You are adding *.example.dts to .gitignore
Why not "schema.yaml" ?
quoted
quoted
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
"mkdir -p $(obj)" is redundant.
Why is 'rm -f $@' necessary ?
Can't dt-mk-schema overwrite the output file?
It is for error case when the output file is not generated. I can
handle this within dt-mk-schema instead.
quoted
quoted
+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))
I assume you intentionally did not do like this:
extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
From the commit description, DT_SCHEMA_FILES might be overridden by a user.
So, I think this is OK.
quoted
+$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
I do not understand this line.
Why is it necessary?
*.example.dtb files are generated anyway
since they are listed in extra-y.
It is enforcing the ordering. Without it, the binding checks and
building .schema.yaml.tmp happen in parallel because both only have
the source files as dependencies. The '|' keeps the dependencies out
of the dependency list($^).
What kind problem would you see if
the binding checks and building .schema.yaml.tmp
happen in parallel?
--
Best Regards
Masahiro Yamada
On Tue, Dec 11, 2018 at 10:03 AM Masahiro Yamada
[off-list ref] wrote:
On Wed, Dec 12, 2018 at 12:13 AM Rob Herring [off-list ref] wrote:
quoted
quoted
quoted
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
BTW, why does this file start with a period?
What is the meaning of '.tmp' extension?
Nothing really. Just named it something so it gets cleaned and ignored by git.
It is cleaned whatever file name you use.
See scripts/Makefile.clean
__clean-files := $(extra-y) $(extra-m) $(extra-) \
$(always) $(targets) $(clean-files) \
$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \
$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \
$(hostcxxlibs-y) $(hostcxxlibs-m)
$(extra-y) is cleaned.
True.
You are adding *.example.dts to .gitignore
Why not "schema.yaml" ?
Okay. I'll do "processed-schema.yaml" to give a bit better name of
what it contains.
quoted
quoted
quoted
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
"mkdir -p $(obj)" is redundant.
Why is 'rm -f $@' necessary ?
Can't dt-mk-schema overwrite the output file?
It is for error case when the output file is not generated. I can
handle this within dt-mk-schema instead.
quoted
quoted
+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))
I assume you intentionally did not do like this:
extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
From the commit description, DT_SCHEMA_FILES might be overridden by a user.
So, I think this is OK.
quoted
+$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
I do not understand this line.
Why is it necessary?
*.example.dtb files are generated anyway
since they are listed in extra-y.
It is enforcing the ordering. Without it, the binding checks and
building .schema.yaml.tmp happen in parallel because both only have
the source files as dependencies. The '|' keeps the dependencies out
of the dependency list($^).
What kind problem would you see if
the binding checks and building .schema.yaml.tmp
happen in parallel?
In case of no errors in the binding docs, it doesn't matter. If there
are errors, I don't want the dtbs validation to run if any schema
doesn't validate. However, I played around with this a bit more and it
seems like having the examples' dts/dtb in extra-y prevents that from
happening. Does that match your expections? If so, then I think we can
remove the dependency.
Here's an example.
SCHEMA Documentation/devicetree/bindings/.schema.yaml.tmp
CHKDT Documentation/devicetree/bindings/arm/vt8500.yaml
CHKDT Documentation/devicetree/bindings/arm/zte.yaml
CHKDT Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.yaml
CHKDT Documentation/devicetree/bindings/arm/ti/ti,davinci.yaml
CHKDT Documentation/devicetree/bindings/arm/ti/nspire.yaml
CHKDT Documentation/devicetree/bindings/arm/spear.yaml
CHKDT Documentation/devicetree/bindings/arm/altera.yaml
warning: no schema found in file:
../Documentation/devicetree/bindings/arm/xilinx.yaml
/home/rob/proj/git/linux-2.6/Documentation/devicetree/bindings/arm/xilinx.yaml:
ignoring, error in schema 'onOf'
CHKDT Documentation/devicetree/bindings/arm/sti.yaml
CHKDT Documentation/devicetree/bindings/arm/qcom.yaml
CHKDT Documentation/devicetree/bindings/arm/primecell.yaml
CHKDT Documentation/devicetree/bindings/arm/cpus.yaml
CHKDT Documentation/devicetree/bindings/arm/sirf.yaml
CHKDT Documentation/devicetree/bindings/arm/calxeda.yaml
CHKDT Documentation/devicetree/bindings/arm/xilinx.yaml
CHKDT Documentation/devicetree/bindings/example-schema.yaml
/home/rob/proj/git/linux-2.6/Documentation/devicetree/bindings/arm/xilinx.yaml:
properties:compatible:onOf: 'onOf' is not one of ['$ref',
'additionalItems', 'allOf', 'const', 'contains', 'default',
'dependencies', 'description', 'enum', 'items', 'minItems', 'minimum',
'maxItems', 'maximum', 'not', 'oneOf', 'pattern', 'patternProperties',
'properties', 'required', 'type', 'typeSize']
make[2]: *** [../Documentation/devicetree/bindings/Makefile:12:
Documentation/devicetree/bindings/arm/xilinx.example.dts] Error 1
Rob
On Wed, Dec 12, 2018 at 3:36 AM Rob Herring [off-list ref] wrote:
On Tue, Dec 11, 2018 at 10:03 AM Masahiro Yamada
[off-list ref] wrote:
quoted
On Wed, Dec 12, 2018 at 12:13 AM Rob Herring [off-list ref] wrote:
quoted
quoted
quoted
+$(obj)/%.example.dts: $(src)/%.yaml FORCE
+ $(call if_changed,chk_binding)
+
+DT_TMP_SCHEMA := .schema.yaml.tmp
BTW, why does this file start with a period?
What is the meaning of '.tmp' extension?
Nothing really. Just named it something so it gets cleaned and ignored by git.
It is cleaned whatever file name you use.
See scripts/Makefile.clean
__clean-files := $(extra-y) $(extra-m) $(extra-) \
$(always) $(targets) $(clean-files) \
$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \
$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \
$(hostcxxlibs-y) $(hostcxxlibs-m)
$(extra-y) is cleaned.
True.
quoted
You are adding *.example.dts to .gitignore
Why not "schema.yaml" ?
Okay. I'll do "processed-schema.yaml" to give a bit better name of
what it contains.
quoted
quoted
quoted
quoted
+extra-y += $(DT_TMP_SCHEMA)
+
+quiet_cmd_mk_schema = SCHEMA $@
+ cmd_mk_schema = mkdir -p $(obj); \
+ rm -f $@; \
+ $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^)
"mkdir -p $(obj)" is redundant.
Why is 'rm -f $@' necessary ?
Can't dt-mk-schema overwrite the output file?
It is for error case when the output file is not generated. I can
handle this within dt-mk-schema instead.
quoted
quoted
+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))
I assume you intentionally did not do like this:
extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
From the commit description, DT_SCHEMA_FILES might be overridden by a user.
So, I think this is OK.
quoted
+$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
I do not understand this line.
Why is it necessary?
*.example.dtb files are generated anyway
since they are listed in extra-y.
It is enforcing the ordering. Without it, the binding checks and
building .schema.yaml.tmp happen in parallel because both only have
the source files as dependencies. The '|' keeps the dependencies out
of the dependency list($^).
What kind problem would you see if
the binding checks and building .schema.yaml.tmp
happen in parallel?
In case of no errors in the binding docs, it doesn't matter. If there
are errors, I don't want the dtbs validation to run if any schema
doesn't validate. However, I played around with this a bit more and it
seems like having the examples' dts/dtb in extra-y prevents that from
happening. Does that match your expections?
Exactly.
If any error occurs in Documentation/devicetree/bindings/Makefile,
Make terminates before proceeding to the dtbs_check stage.
--
Best Regards
Masahiro Yamada