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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >
> > 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