[PATCH] dt-bindings: fix wrong use of if_changed_rule

[PATCH] dt-bindings: fix wrong use of if_changed_rule

[Masahiro Yamada]

The intent for if_changed_rule is to re-run the rule when the command
line is changed, but this if_changed_rule does not do anything for it.

$(cmd-check) for this rule is always empty because:

 [1] $(cmd_$@) is empty because .processed-schema.json.cmd does not exist
 [2] $(cmd_$1) is empty because cmd_chkdt is not defined

To address [1], use cmd_and_cmdsave instead of cmd.

To address [2], rename rule_chkdt to rule_mk_schema so that the stem
parts of cmd_* and rule_* match, like commit 7a0496056064 ("kbuild:
fix DT binding schema rule to detect command line changes").

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

Another possibility might be to split out yamllint and chk_bindings
as standalone build rules instead of running them as a side-effect
of the schema build. (but it it up to Rob's preference)


 Documentation/devicetree/bindings/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 1eaccf135b30..bb40205689ea 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -46,10 +46,10 @@ quiet_cmd_mk_schema = SCHEMA  $@
                       $(DT_MK_SCHEMA) -j $(DT_MK_SCHEMA_FLAGS) @$$f > $@ ; \
 		      rm -f $$f
 
-define rule_chkdt
+define rule_mk_schema
 	$(if $(DT_SCHEMA_LINT),$(call cmd,yamllint),)
 	$(call cmd,chk_bindings)
-	$(call cmd,mk_schema)
+	$(call cmd_and_savecmd,mk_schema)
 endef
 
 DT_DOCS = $(patsubst $(srctree)/%,%,$(shell $(find_all_cmd)))
@@ -65,7 +65,7 @@ override DTC_FLAGS := \
 override DT_CHECKER_FLAGS ?=
 
 $(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
-	$(call if_changed_rule,chkdt)
+	$(call if_changed_rule,mk_schema)
 
 always-y += processed-schema.json
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
-- 
2.34.1

Re: [PATCH] dt-bindings: fix wrong use of if_changed_rule

[Rob Herring]

On Thu, Aug 18, 2022 at 12:20:26AM +0900, Masahiro Yamada wrote:
> The intent for if_changed_rule is to re-run the rule when the command
> line is changed, but this if_changed_rule does not do anything for it.

This is the issue with DT_SCHEMA_FILES changes not causing a rebuild?

> $(cmd-check) for this rule is always empty because:
> 
>  [1] $(cmd_$@) is empty because .processed-schema.json.cmd does not exist
>  [2] $(cmd_$1) is empty because cmd_chkdt is not defined
> 
> To address [1], use cmd_and_cmdsave instead of cmd.
> 
> To address [2], rename rule_chkdt to rule_mk_schema so that the stem
> parts of cmd_* and rule_* match, like commit 7a0496056064 ("kbuild:
> fix DT binding schema rule to detect command line changes").
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
> Another possibility might be to split out yamllint and chk_bindings
> as standalone build rules instead of running them as a side-effect
> of the schema build. (but it it up to Rob's preference)

That is the direction I'd like to go. Something like the below patch 
perhaps.

The main issue (or feature?) is that 'dt_binding_lint' and 
'dt_binding_schema' targets are still rerun every time even with the 
dummy target files.

I think the top level makefile can be simplified a bit more with this 
change, but this is what I got to being somewhat functional.

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 1eaccf135b30..ec3d8a926331 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -34,11 +34,13 @@ CHK_DT_DOCS := $(shell $(find_cmd))
 quiet_cmd_yamllint = LINT    $(src)
       cmd_yamllint = ($(find_cmd) | \
                      xargs -n200 -P$$(nproc) \
-		     $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true
+		     $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true; \
+                     touch $(obj)/dt_binding_lint.checked
 
-quiet_cmd_chk_bindings = CHKDT   $@
+quiet_cmd_chk_bindings = CHKDT   $(src)
       cmd_chk_bindings = ($(find_cmd) | \
-                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true
+                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true; \
+                         touch $(obj)/dt_binding_schema.checked
 
 quiet_cmd_mk_schema = SCHEMA  $@
       cmd_mk_schema = f=$$(mktemp) ; \
@@ -46,12 +48,6 @@ quiet_cmd_mk_schema = SCHEMA  $@
                       $(DT_MK_SCHEMA) -j $(DT_MK_SCHEMA_FLAGS) @$$f > $@ ; \
 		      rm -f $$f
 
-define rule_chkdt
-	$(if $(DT_SCHEMA_LINT),$(call cmd,yamllint),)
-	$(call cmd,chk_bindings)
-	$(call cmd,mk_schema)
-endef
-
 DT_DOCS = $(patsubst $(srctree)/%,%,$(shell $(find_all_cmd)))
 
 override DTC_FLAGS := \
@@ -64,8 +60,25 @@ override DTC_FLAGS := \
 # Disable undocumented compatible checks until warning free
 override DT_CHECKER_FLAGS ?=
 
-$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
-	$(call if_changed_rule,chkdt)
+dt_binding_lint: $(obj)/dt_binding_lint.checked
+
+$(obj)/dt_binding_lint.checked: $(CHK_DT_DOCS) $(src)/.yamllint FORCE
+	$(call if_changed,yamllint)
+
+dt_binding_schema: $(obj)/dt_binding_schema.checked
+
+$(obj)/dt_binding_schema.checked: $(CHK_DT_DOCS) check_dtschema_version FORCE
+	$(call if_changed,chk_bindings)
+
+dt_binding_examples: CHECK_DT_BINDING = y
+
+dt_binding_examples: $(obj)/processed-schema.json $(patsubst $(srctree)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
+
+dt_binding_check: dt_binding_lint dt_binding_examples dt_binding_schema
+
+
+$(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
+	$(call if_changed,mk_schema)
 
 always-y += processed-schema.json
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
diff --git a/Makefile b/Makefile
index c7705f749601..0f197e3bd1f9 100644
--- a/Makefile
+++ b/Makefile
@@ -1391,7 +1391,7 @@ dtbs_prepare: include/config/kernel.release scripts_dtc
 
 ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
 export CHECK_DTBS=y
-dtbs: dt_binding_check
+dtbs: dt_binding_schema
 endif
 
 dtbs_check: dtbs
@@ -1409,13 +1409,14 @@ PHONY += scripts_dtc
 scripts_dtc: scripts_basic
 	$(Q)$(MAKE) $(build)=scripts/dtc
 
-ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),)
+ifneq ($(filter dt_binding_examples, $(MAKECMDGOALS)),)
 export CHECK_DT_BINDING=y
 endif
 
-PHONY += dt_binding_check
-dt_binding_check: scripts_dtc
-	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
+DT_BINDING_TARGETS := dt_binding_check dt_binding_lint dt_binding_schema dt_binding_examples
+PHONY += $(DT_BINDING_TARGETS)
+$(DT_BINDING_TARGETS): scripts_dtc
+	$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings $@
 
 # ---------------------------------------------------------------------------
 # Modules

Re: [PATCH] dt-bindings: fix wrong use of if_changed_rule

[Masahiro Yamada]

On Wed, Aug 24, 2022 at 9:23 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Aug 18, 2022 at 12:20:26AM +0900, Masahiro Yamada wrote:
> > The intent for if_changed_rule is to re-run the rule when the command
> > line is changed, but this if_changed_rule does not do anything for it.
>
> This is the issue with DT_SCHEMA_FILES changes not causing a rebuild?
>
> > $(cmd-check) for this rule is always empty because:
> >
> >  [1] $(cmd_$@) is empty because .processed-schema.json.cmd does not exist
> >  [2] $(cmd_$1) is empty because cmd_chkdt is not defined
> >
> > To address [1], use cmd_and_cmdsave instead of cmd.
> >
> > To address [2], rename rule_chkdt to rule_mk_schema so that the stem
> > parts of cmd_* and rule_* match, like commit 7a0496056064 ("kbuild:
> > fix DT binding schema rule to detect command line changes").
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > Another possibility might be to split out yamllint and chk_bindings
> > as standalone build rules instead of running them as a side-effect
> > of the schema build. (but it it up to Rob's preference)
>
> That is the direction I'd like to go. Something like the below patch
> perhaps.
>
> The main issue (or feature?) is that 'dt_binding_lint' and
> 'dt_binding_schema' targets are still rerun every time even with the
> dummy target files.


Not a feature.
It is just because the code is wrong.


You need to add

targets += dt_binding_lint.checked

  or

always-$(CHECK_DT_BINDING) += dt_binding_lint.checked



>
> I think the top level makefile can be simplified a bit more with this
> change, but this is what I got to being somewhat functional.


I do not understand why the top Makefile needs a change.

It is just a matter of adding the timestamp files to always-y
in Documentation/devicetree/bindings/Makefile, isn'ts it?






> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 1eaccf135b30..ec3d8a926331 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -34,11 +34,13 @@ CHK_DT_DOCS := $(shell $(find_cmd))
>  quiet_cmd_yamllint = LINT    $(src)
>        cmd_yamllint = ($(find_cmd) | \
>                       xargs -n200 -P$$(nproc) \
> -                    $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true
> +                    $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true; \
> +                     touch $(obj)/dt_binding_lint.checked
>
> -quiet_cmd_chk_bindings = CHKDT   $@
> +quiet_cmd_chk_bindings = CHKDT   $(src)
>        cmd_chk_bindings = ($(find_cmd) | \
> -                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true
> +                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true; \
> +                         touch $(obj)/dt_binding_schema.checked
>
>  quiet_cmd_mk_schema = SCHEMA  $@
>        cmd_mk_schema = f=$$(mktemp) ; \
> @@ -46,12 +48,6 @@ quiet_cmd_mk_schema = SCHEMA  $@
>                        $(DT_MK_SCHEMA) -j $(DT_MK_SCHEMA_FLAGS) @$$f > $@ ; \
>                       rm -f $$f
>
> -define rule_chkdt
> -       $(if $(DT_SCHEMA_LINT),$(call cmd,yamllint),)
> -       $(call cmd,chk_bindings)
> -       $(call cmd,mk_schema)
> -endef
> -
>  DT_DOCS = $(patsubst $(srctree)/%,%,$(shell $(find_all_cmd)))
>
>  override DTC_FLAGS := \
> @@ -64,8 +60,25 @@ override DTC_FLAGS := \
>  # Disable undocumented compatible checks until warning free
>  override DT_CHECKER_FLAGS ?=
>
> -$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
> -       $(call if_changed_rule,chkdt)
> +dt_binding_lint: $(obj)/dt_binding_lint.checked
> +
> +$(obj)/dt_binding_lint.checked: $(CHK_DT_DOCS) $(src)/.yamllint FORCE
> +       $(call if_changed,yamllint)
> +
> +dt_binding_schema: $(obj)/dt_binding_schema.checked
> +
> +$(obj)/dt_binding_schema.checked: $(CHK_DT_DOCS) check_dtschema_version FORCE
> +       $(call if_changed,chk_bindings)
> +
> +dt_binding_examples: CHECK_DT_BINDING = y
> +
> +dt_binding_examples: $(obj)/processed-schema.json $(patsubst $(srctree)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
> +
> +dt_binding_check: dt_binding_lint dt_binding_examples dt_binding_schema
> +
> +
> +$(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
> +       $(call if_changed,mk_schema)
>
>  always-y += processed-schema.json
>  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
> diff --git a/Makefile b/Makefile
> index c7705f749601..0f197e3bd1f9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1391,7 +1391,7 @@ dtbs_prepare: include/config/kernel.release scripts_dtc
>
>  ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
>  export CHECK_DTBS=y
> -dtbs: dt_binding_check
> +dtbs: dt_binding_schema
>  endif
>
>  dtbs_check: dtbs
> @@ -1409,13 +1409,14 @@ PHONY += scripts_dtc
>  scripts_dtc: scripts_basic
>         $(Q)$(MAKE) $(build)=scripts/dtc
>
> -ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),)
> +ifneq ($(filter dt_binding_examples, $(MAKECMDGOALS)),)
>  export CHECK_DT_BINDING=y
>  endif
>
> -PHONY += dt_binding_check
> -dt_binding_check: scripts_dtc
> -       $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
> +DT_BINDING_TARGETS := dt_binding_check dt_binding_lint dt_binding_schema dt_binding_examples
> +PHONY += $(DT_BINDING_TARGETS)
> +$(DT_BINDING_TARGETS): scripts_dtc
> +       $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings $@
>
>  # ---------------------------------------------------------------------------
>  # Modules



-- 
Best Regards
Masahiro Yamada