kbuild: fix DT binding schema rule to detect command line changes

PATCH In review Masahiro Yamada 2 participants 12 messages Feb 22, 2020 – Feb 26, 2020

Subsystems (2)

Device tree bindings Kbuild

lore

Share & tools

lore

Masahiro Yamada

author

2020-02-22 19:04 UTC #0 lore raw

PATCH 1/5 kbuild: fix DT binding schema rule to detect command line changes

This if_change_rule is not working; it cannot detect any command line changes.

The reason is because cmd-check in scripts/Kbuild.include compares $(cmd_$@) and $(cmd_$1), but cmd_dtc_dt_yaml does not exist here.

For if_change_rule to work properly, the stem part of cmd_* and rule_* must match. Because this cmd_and_fixdep invokes cmd_dtc, this rule must be named rule_dtc.

Fixes 4f0e3a57d6eb ("kbuild: Add support for DT binding schema checks")

Signed-off-by Masahiro Yamada <[email protected]>

---

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

scripts/Makefile.lib +2 -2

index bae62549e3d2..64b938c10039 100644
@@ -302,13 +302,13 @@ 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) $@ ;
 
-define rule_dtc_dt_yaml
+define rule_dtc
 	$(call cmd_and_fixdep,dtc,yaml)
 	$(call cmd,dtb_check)
 endef
 
 $(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
-	$(call if_changed_rule,dtc_dt_yaml)
+	$(call if_changed_rule,dtc)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

Signature · 1 line

2.17.1

Masahiro Yamada

author

2020-02-22 19:04 UTC #1 lore raw

PATCH 2/5 kbuild: remove unneeded semicolon at the end of cmd_dtb_check

This trailing semicolon is unneeded.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/Makefile.lib +1 -1

index 64b938c10039..752ff0a225a9 100644
@@ -300,7 +300,7 @@ 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) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ ;
+      cmd_dtb_check =	$(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
 
 define rule_dtc
 	$(call cmd_and_fixdep,dtc,yaml)

Signature · 1 line

2.17.1

Masahiro Yamada

author

2020-02-26 17:48 UTC #2 lore raw

Re: [PATCH 2/5] kbuild: remove unneeded semicolon at the end of cmd_dtb_check

Quoted 5 lines · Masahiro Yamada

This trailing semicolon is unneeded.
Signed-off-by Masahiro Yamada <[email protected]>
---

Applied with Rob's Ack.

Quoted 19 lines

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/Makefile.lib +1 -2

index 64b938c10039..752ff0a225a9 100644
@@ -300,7 +300,7 @@ 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) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ ;
+      cmd_dtb_check =  $(DT_CHECKER) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
 
 define rule_dtc
        $(call cmd_and_fixdep,dtc,yaml)
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2020-02-22 19:04 UTC #3 lore raw

PATCH 3/5 kbuild: add dtbs_check to PHONY

In reply to Masahiro Yamada

The dtbs_check should be a phony target, but currently it is not specified so.

'make dtbs_check' works even if a file named 'dtbs_check' exists because it depends on another phony target, scripts_dtc, but we should not rely on it.

Add dtbs_check to PHONY.

Signed-off-by Masahiro Yamada <[email protected]>

---

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index aab38cb02b24..102710a9228c 100644
@@ -1238,7 +1238,7 @@ ifneq ($(dtstree),)
 %.dtb: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
-PHONY += dtbs dtbs_install dt_binding_check
+PHONY += dtbs dtbs_install dtbs_check dt_binding_check
 dtbs dtbs_check: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)

Signature · 1 line

2.17.1

Masahiro Yamada

author

2020-02-26 17:48 UTC #4 lore raw

Re: [PATCH 3/5] kbuild: add dtbs_check to PHONY

Quoted 4 lines · Masahiro Yamada

The dtbs_check should be a phony target, but currently it is not specified so.

Quoted 8 lines

'make dtbs_check' works even if a file named 'dtbs_check' exists because it depends on another phony target, scripts_dtc, but we should not rely on it.
Add dtbs_check to PHONY.
Signed-off-by Masahiro Yamada <[email protected]>
---

Applied with Rob's Ack.

Quoted 20 lines

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -2

index aab38cb02b24..102710a9228c 100644
@@ -1238,7 +1238,7 @@ ifneq ($(dtstree),)
 %.dtb: include/config/kernel.release scripts_dtc
        $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
-PHONY += dtbs dtbs_install dt_binding_check
+PHONY += dtbs dtbs_install dtbs_check dt_binding_check
 dtbs dtbs_check: include/config/kernel.release scripts_dtc
        $(Q)$(MAKE) $(build)=$(dtstree)
 
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2020-02-22 19:04 UTC #5 lore raw

PATCH 4/5 kbuild: add dt_binding_check to PHONY in a correct place

In reply to Masahiro Yamada

The dt_binding_check is added to PHONY, but it is visible only when $(dtstree) is not empty. So, it is not specified as phony for ARCH=x86 etc.

Add it to PHONY outside the ifneq ... endif block.

Signed-off-by Masahiro Yamada <[email protected]>

---

 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Makefile +2 -1

index 102710a9228c..83f9b8f6fbaf 100644
@@ -1238,7 +1238,7 @@ ifneq ($(dtstree),)
 %.dtb: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
-PHONY += dtbs dtbs_install dtbs_check dt_binding_check
+PHONY += dtbs dtbs_install dtbs_check
 dtbs dtbs_check: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)

@@ -1258,6 +1258,7 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
 	$(Q)$(MAKE) $(build)=scripts/dtc
 
+PHONY += dt_binding_check
 dt_binding_check: scripts_dtc
 	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings

Signature · 1 line

2.17.1

Masahiro Yamada

author

2020-02-26 17:49 UTC #6 lore raw

Re: [PATCH 4/5] kbuild: add dt_binding_check to PHONY in a correct place

Quoted 9 lines · Masahiro Yamada

The dt_binding_check is added to PHONY, but it is visible only when $(dtstree) is not empty. So, it is not specified as phony for ARCH=x86 etc.
Add it to PHONY outside the ifneq ... endif block.
Signed-off-by Masahiro Yamada <[email protected]>
---

Applied with Rob's Ack.

Quoted 27 lines

 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Makefile +2 -2

index 102710a9228c..83f9b8f6fbaf 100644
@@ -1238,7 +1238,7 @@ ifneq ($(dtstree),)
 %.dtb: include/config/kernel.release scripts_dtc
        $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
-PHONY += dtbs dtbs_install dtbs_check dt_binding_check
+PHONY += dtbs dtbs_install dtbs_check
 dtbs dtbs_check: include/config/kernel.release scripts_dtc
        $(Q)$(MAKE) $(build)=$(dtstree)

@@ -1258,6 +1258,7 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
        $(Q)$(MAKE) $(build)=scripts/dtc
 
+PHONY += dt_binding_check
 dt_binding_check: scripts_dtc
        $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
 
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2020-02-22 19:04 UTC #7 lore raw

PATCH 5/5 kbuild: allow to run dt_binding_check and dtbs_check in a single command

In reply to 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

You cannot do:

  $ make ARCH=arm dt_binding_check dtbs_check

Because CHECK_DTBS is set, dt-doc-validate and dt-extract-example are skipped.

Making it work will be useful for schema writers.

Signed-off-by Masahiro Yamada <[email protected]>

---

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

Documentation/devicetree/bindings/Makefile +2 -4

index 646cb3525373..6efa2094b95e 100644
@@ -25,10 +25,8 @@ DT_DOCS = $(shell \
 
 DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(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
+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))
 
 $(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE
 	$(call if_changed,mk_schema)

Documentation/devicetree/writing-schema.rst +4 -0

index 7635ab230456..220cf464ed77 100644
@@ -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.

Makefile +5 -3

index 83f9b8f6fbaf..59dd768a1c1e 100644
@@ -1243,7 +1243,7 @@ dtbs dtbs_check: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)
 
 dtbs_check: export CHECK_DTBS=1
-dtbs_check: dt_binding_check
+dtbs_check: __dt_binding_check
 
 dtbs_install:
 	$(Q)$(MAKE) $(dtbinst)=$(dtstree)

@@ -1258,8 +1258,10 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
 	$(Q)$(MAKE) $(build)=scripts/dtc
 
-PHONY += dt_binding_check
-dt_binding_check: scripts_dtc
+PHONY += dt_binding_check __dt_binding_check
+dt_binding_check: export CHECK_DT_BINDING=y
+dt_binding_check: __dt_binding_check
+__dt_binding_check: scripts_dtc
 	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
 
 # ---------------------------------------------------------------------------

Signature · 1 line

2.17.1

Masahiro Yamada

author

2020-02-26 06:47 UTC #8 lore raw

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

Quoted 13 lines · 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
You cannot do:
  $ make ARCH=arm dt_binding_check dtbs_check

Quoted 7 lines

Because CHECK_DTBS is set, dt-doc-validate and dt-extract-example are skipped.
Making it work will be useful for schema writers.
Signed-off-by Masahiro Yamada <[email protected]>
---

Sorry, I take back this patch.

'make dtbs_check dt_binding_chec' does not work.

Quoted 67 lines

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

Documentation/devicetree/bindings/Makefile +2 -4

index 646cb3525373..6efa2094b95e 100644
@@ -25,10 +25,8 @@ DT_DOCS = $(shell \
 
 DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(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
+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))
 
 $(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE
        $(call if_changed,mk_schema)

Documentation/devicetree/writing-schema.rst +4 -0

index 7635ab230456..220cf464ed77 100644
@@ -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.

Makefile +5 -4

index 83f9b8f6fbaf..59dd768a1c1e 100644
@@ -1243,7 +1243,7 @@ dtbs dtbs_check: include/config/kernel.release scripts_dtc
        $(Q)$(MAKE) $(build)=$(dtstree)
 
 dtbs_check: export CHECK_DTBS=1
-dtbs_check: dt_binding_check
+dtbs_check: __dt_binding_check
 
 dtbs_install:
        $(Q)$(MAKE) $(dtbinst)=$(dtstree)

@@ -1258,8 +1258,10 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
        $(Q)$(MAKE) $(build)=scripts/dtc
 
-PHONY += dt_binding_check
-dt_binding_check: scripts_dtc
+PHONY += dt_binding_check __dt_binding_check
+dt_binding_check: export CHECK_DT_BINDING=y
+dt_binding_check: __dt_binding_check
+__dt_binding_check: scripts_dtc
        $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
 
 # ---------------------------------------------------------------------------
--

2.17.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Rob Herring

maintainer Acked-by on v1 1/5

2020-02-24 22:52 UTC #9 lore raw

In reply to Masahiro Yamada

Quoted 10 lines · Masahiro Yamada

This if_change_rule is not working; it cannot detect any command line changes.
The reason is because cmd-check in scripts/Kbuild.include compares $(cmd_$@) and $(cmd_$1), but cmd_dtc_dt_yaml does not exist here.
For if_change_rule to work properly, the stem part of cmd_* and rule_* must match. Because this cmd_and_fixdep invokes cmd_dtc, this rule must be named rule_dtc.

Quoted 6 lines

Fixes 4f0e3a57d6eb ("kbuild: Add support for DT binding schema checks")
Signed-off-by Masahiro Yamada <[email protected]>
---
 scripts/Makefile.lib | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

For the series,

Acked-by Rob Herring <[email protected]>

I'm assuming you will take these? If not, I can in the DT tree.

Rob

Masahiro Yamada

author

2020-02-25 04:33 UTC #10 lore raw

Hi Rob,

Quoted 16 lines · Rob Herring

Quoted 14 lines · Masahiro Yamada
This if_change_rule is not working; it cannot detect any command line changes.
The reason is because cmd-check in scripts/Kbuild.include compares $(cmd_$@) and $(cmd_$1), but cmd_dtc_dt_yaml does not exist here.
For if_change_rule to work properly, the stem part of cmd_* and rule_* must match. Because this cmd_and_fixdep invokes cmd_dtc, this rule must be named rule_dtc.
Fixes 4f0e3a57d6eb ("kbuild: Add support for DT binding schema checks")
Signed-off-by Masahiro Yamada <[email protected]>
---

Quoted 8 lines

Quoted 2 lines
 scripts/Makefile.lib | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
For the series,
Acked-by Rob Herring <[email protected]>
I'm assuming you will take these? If not, I can in the DT tree.

Yes, I will queue these to kbuild tree.

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2020-02-26 17:48 UTC #11 lore raw

Quoted 23 lines · Masahiro Yamada

Hi Rob,
Quoted 19 lines · Rob Herring
Quoted 16 lines · Masahiro Yamada
This if_change_rule is not working; it cannot detect any command line changes.
The reason is because cmd-check in scripts/Kbuild.include compares $(cmd_$@) and $(cmd_$1), but cmd_dtc_dt_yaml does not exist here.
For if_change_rule to work properly, the stem part of cmd_* and rule_* must match. Because this cmd_and_fixdep invokes cmd_dtc, this rule must be named rule_dtc.
Fixes 4f0e3a57d6eb ("kbuild: Add support for DT binding schema checks")
Signed-off-by Masahiro Yamada <[email protected]>
---
 scripts/Makefile.lib | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Quoted 9 lines

Quoted 5 lines
For the series,
Acked-by Rob Herring <[email protected]>
I'm assuming you will take these? If not, I can in the DT tree.
Yes, I will queue these to kbuild tree.

Applied with Rob's Ack.

Signature · 2 lines

Best Regards
Masahiro Yamada