* [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs @ 2022-07-27 10:06 Dmitry Baryshkov 2022-07-27 17:36 ` Rob Herring 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-07-27 10:06 UTC (permalink / raw) To: Masahiro Yamada, Michal Marek, Nick Desaulniers Cc: linux-kbuild, linux-kernel, Rob Herring, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm It is useful to be able to recheck dtbs files against a limited set of DT schema files. This can be accomplished by using differnt DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However for some reason if_changed_rule doesn't pick up the rule_dtc changes (and doesn't retrigger the build). Fix this by changing if_changed_rule to if_changed_dep and squashing DTC and dt-validate into a single new command. Then if_changed_dep triggers on DT_SCHEMA_FILES changes and reruns the build/check. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- scripts/Makefile.lib | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index c88b98b5dc44..3df470289382 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) DT_BINDING_DIR := Documentation/devicetree/bindings DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json -quiet_cmd_dtb_check = CHECK $@ - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true +quiet_cmd_dtb = DTC/CHECK $@ + cmd_dtb = $(cmd_dtc) ; $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true +else +quiet_cmd_dtb = $(quiet_cmd_dtc) + cmd_dtb = $(cmd_dtc) endif -define rule_dtc - $(call cmd_and_fixdep,dtc) - $(call cmd,dtb_check) -endef - $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE - $(call if_changed_rule,dtc) + $(call if_changed_dep,dtb) $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) -- 2.35.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-27 10:06 [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs Dmitry Baryshkov @ 2022-07-27 17:36 ` Rob Herring 2022-07-29 5:54 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2022-07-27 17:36 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > It is useful to be able to recheck dtbs files against a limited set of > DT schema files. This can be accomplished by using differnt > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > for some reason if_changed_rule doesn't pick up the rule_dtc changes > (and doesn't retrigger the build). > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > and dt-validate into a single new command. Then if_changed_dep triggers > on DT_SCHEMA_FILES changes and reruns the build/check. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > scripts/Makefile.lib | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index c88b98b5dc44..3df470289382 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > DT_BINDING_DIR := Documentation/devicetree/bindings > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > -quiet_cmd_dtb_check = CHECK $@ > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > +quiet_cmd_dtb = DTC/CHECK $@ This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or always do just 'DTC'. I can fixup when applying. I'll give it a few days for other comments. > + cmd_dtb = $(cmd_dtc) ; $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > +else > +quiet_cmd_dtb = $(quiet_cmd_dtc) > + cmd_dtb = $(cmd_dtc) > endif > > -define rule_dtc > - $(call cmd_and_fixdep,dtc) > - $(call cmd,dtb_check) > -endef > - > $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE > - $(call if_changed_rule,dtc) > + $(call if_changed_dep,dtb) > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) > -- > 2.35.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-27 17:36 ` Rob Herring @ 2022-07-29 5:54 ` Masahiro Yamada 2022-07-29 6:52 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2022-07-29 5:54 UTC (permalink / raw) To: Rob Herring Cc: Dmitry Baryshkov, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > It is useful to be able to recheck dtbs files against a limited set of > > DT schema files. This can be accomplished by using differnt > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > (and doesn't retrigger the build). > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > and dt-validate into a single new command. Then if_changed_dep triggers > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > scripts/Makefile.lib | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index c88b98b5dc44..3df470289382 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > DT_BINDING_DIR := Documentation/devicetree/bindings > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > -quiet_cmd_dtb_check = CHECK $@ > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > +quiet_cmd_dtb = DTC/CHECK $@ > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > always do just 'DTC'. I can fixup when applying. > > I'll give it a few days for other comments. When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. You do not need to re-run dtc. I guess the strangeness comes from the fact that you are trying to do the two different things in a single rule. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-29 5:54 ` Masahiro Yamada @ 2022-07-29 6:52 ` Dmitry Baryshkov 2022-07-29 7:03 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-07-29 6:52 UTC (permalink / raw) To: Masahiro Yamada Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > DT schema files. This can be accomplished by using differnt > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > (and doesn't retrigger the build). > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > scripts/Makefile.lib | 14 ++++++-------- > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index c88b98b5dc44..3df470289382 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > always do just 'DTC'. I can fixup when applying. > > > > I'll give it a few days for other comments. > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > You do not need to re-run dtc. > > I guess the strangeness comes from the fact that you are trying to do the > two different things in a single rule. The issue is that with the current rules the dt-validate isn't re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-29 6:52 ` Dmitry Baryshkov @ 2022-07-29 7:03 ` Masahiro Yamada 2022-07-29 7:46 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2022-07-29 7:03 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > DT schema files. This can be accomplished by using differnt > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > (and doesn't retrigger the build). > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > --- > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > index c88b98b5dc44..3df470289382 100644 > > > > --- a/scripts/Makefile.lib > > > > +++ b/scripts/Makefile.lib > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > always do just 'DTC'. I can fixup when applying. > > > > > > I'll give it a few days for other comments. > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > You do not need to re-run dtc. > > > > I guess the strangeness comes from the fact that you are trying to do the > > two different things in a single rule. > > The issue is that with the current rules the dt-validate isn't > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. Correct. What I said is like this. # touch the timestamp file, %.dtb.checked $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE $(call if_changed_rule,dtb_check) $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE $(call if_changed_rule,dtc) $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) With the dtc/check split, we can avoid unneeded regeneration of %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is changed. One drawback is we track %.dtb.checked and and %.dtb separately, so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 may come back. BTW, we do not check %.dtbo, why? At least, 53182e81f47d4ea0c727c49ad23cb782173ab849 was trying to check %.dtbo > -- > With best wishes > Dmitry -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-29 7:03 ` Masahiro Yamada @ 2022-07-29 7:46 ` Dmitry Baryshkov 2022-08-04 15:27 ` Rob Herring 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-07-29 7:46 UTC (permalink / raw) To: Masahiro Yamada Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > DT schema files. This can be accomplished by using differnt > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > (and doesn't retrigger the build). > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > --- > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > --- a/scripts/Makefile.lib > > > > > +++ b/scripts/Makefile.lib > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > You do not need to re-run dtc. > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > two different things in a single rule. > > > > The issue is that with the current rules the dt-validate isn't > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > Correct. > > What I said is like this. > > # touch the timestamp file, %.dtb.checked > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > $(call if_changed_rule,dtb_check) > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > $(call if_changed_rule,dtc) > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) > > > With the dtc/check split, we can avoid unneeded regeneration of > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > changed. > > > One drawback is we track %.dtb.checked and and %.dtb separately, > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > may come back. It's up to you and Rob, but I'd really prefer a simpler solution here. Regenerating dtbs sounds like a minor pain compared to hacking the top-level Makefile again. What I really like is that if one has CHECK_DTBS=y (for whatever reason), he can not generate dtb without validation. > BTW, we do not check %.dtbo, why? > > At least, 53182e81f47d4ea0c727c49ad23cb782173ab849 > was trying to check %.dtbo The commit ef8795f3f1ce ("dt-bindings: kbuild: Use DTB files for validation") separated .dtb and .dtbo paths. dt-validate is not prepared to be executed on top of the .dtbo file. And this is quite logical. The dtbo is an overlay, a patch. So it doesn't have to follow the schema on its own. We should probably make sure that multi-dtb files generated via fdtoverlay are also validated, but it can go in a separate commit. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-07-29 7:46 ` Dmitry Baryshkov @ 2022-08-04 15:27 ` Rob Herring 2022-09-10 17:35 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2022-08-04 15:27 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > --- > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > --- a/scripts/Makefile.lib > > > > > > +++ b/scripts/Makefile.lib > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > You do not need to re-run dtc. > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > two different things in a single rule. > > > > > > The issue is that with the current rules the dt-validate isn't > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > Correct. > > > > What I said is like this. > > > > # touch the timestamp file, %.dtb.checked > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE Not really a fan of the thousands of files that creates. Maybe if it was turned into something useful like a list of schemas that apply to the dtb. IOW, a dependency list. That would speed up re-running after a schema change. Though if a schema change created new dependencies, that wouldn't work. > > $(call if_changed_rule,dtb_check) > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > $(call if_changed_rule,dtc) > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > $(call if_changed_dep,dtc) > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > changed. > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > may come back. > > It's up to you and Rob, but I'd really prefer a simpler solution here. > Regenerating dtbs sounds like a minor pain compared to hacking the > top-level Makefile again. What I really like is that if one has > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > validation. I lean towards just rebuilding the dtbs. That's pretty quick and ensures we get dtc warnings with schema warnings. In the long run, I would like to make the schema checks not optional to run. The impediment to doing that is lots of warnings (but not not some platforms), adding a tool dependency, and validation time. > > BTW, we do not check %.dtbo, why? > > > > At least, 53182e81f47d4ea0c727c49ad23cb782173ab849 > > was trying to check %.dtbo > > The commit ef8795f3f1ce ("dt-bindings: kbuild: Use DTB files for > validation") separated .dtb and .dtbo paths. dt-validate is not > prepared to be executed on top of the .dtbo file. And this is quite > logical. The dtbo is an overlay, a patch. So it doesn't have to follow > the schema on its own. We should probably make sure that multi-dtb > files generated via fdtoverlay are also validated, but it can go in a > separate commit. It needs some attention to see how well validation of overlays works or not. Rob ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-08-04 15:27 ` Rob Herring @ 2022-09-10 17:35 ` Dmitry Baryshkov 2022-09-15 4:50 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-09-10 17:35 UTC (permalink / raw) To: Rob Herring Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > --- > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > --- a/scripts/Makefile.lib > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > You do not need to re-run dtc. > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > two different things in a single rule. > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > Correct. > > > > > > What I said is like this. > > > > > > # touch the timestamp file, %.dtb.checked > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > Not really a fan of the thousands of files that creates. Maybe if it > was turned into something useful like a list of schemas that apply to > the dtb. IOW, a dependency list. That would speed up re-running after > a schema change. Though if a schema change created new dependencies, > that wouldn't work. > > > > $(call if_changed_rule,dtb_check) > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > $(call if_changed_rule,dtc) > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > $(call if_changed_dep,dtc) > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > changed. > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > may come back. > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > Regenerating dtbs sounds like a minor pain compared to hacking the > > top-level Makefile again. What I really like is that if one has > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > validation. > > I lean towards just rebuilding the dtbs. That's pretty quick and > ensures we get dtc warnings with schema warnings. In the long run, I > would like to make the schema checks not optional to run. The > impediment to doing that is lots of warnings (but not not some > platforms), adding a tool dependency, and validation time. Rob, Masahiro, do we have any conclusion here? I can change my patch, but I'd like to understand in which way I should change it. Fixing/testing yaml changes is a bit painful w/o this change. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-10 17:35 ` Dmitry Baryshkov @ 2022-09-15 4:50 ` Masahiro Yamada 2022-09-15 11:40 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2022-09-15 4:50 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > --- > > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > > --- a/scripts/Makefile.lib > > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > > You do not need to re-run dtc. > > > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > > two different things in a single rule. > > > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > > > Correct. > > > > > > > > What I said is like this. > > > > > > > > # touch the timestamp file, %.dtb.checked > > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > > > Not really a fan of the thousands of files that creates. Maybe if it > > was turned into something useful like a list of schemas that apply to > > the dtb. IOW, a dependency list. That would speed up re-running after > > a schema change. Though if a schema change created new dependencies, > > that wouldn't work. > > > > > > $(call if_changed_rule,dtb_check) > > > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > > $(call if_changed_rule,dtc) > > > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > > $(call if_changed_dep,dtc) > > > > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > > changed. > > > > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > > may come back. > > > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > > Regenerating dtbs sounds like a minor pain compared to hacking the > > > top-level Makefile again. What I really like is that if one has > > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > > validation. > > > > I lean towards just rebuilding the dtbs. That's pretty quick and > > ensures we get dtc warnings with schema warnings. In the long run, I > > would like to make the schema checks not optional to run. The > > impediment to doing that is lots of warnings (but not not some > > platforms), adding a tool dependency, and validation time. > > Rob, Masahiro, do we have any conclusion here? I can change my patch, > but I'd like to understand in which way I should change it. > Fixing/testing yaml changes is a bit painful w/o this change. > > -- > With best wishes > Dmitry I am fine with the patch as long as you fix DTC/CHECK to DTC. Another idea is to re-check the schema every time, like this: diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 3fb6a99e78c4..9fc9f39fb12e 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -375,13 +375,9 @@ quiet_cmd_dtb_check = CHECK $@ cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA ) $@ || true endif -define rule_dtc - $(call cmd_and_fixdep,dtc) - $(call cmd,dtb_check) -endef - $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE - $(call if_changed_rule,dtc) + $(call if_changed_dep,dtc) + $(call cmd,dtb_check) $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) Whatever. -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-15 4:50 ` Masahiro Yamada @ 2022-09-15 11:40 ` Dmitry Baryshkov 2022-09-16 1:09 ` Masahiro Yamada 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-09-15 11:40 UTC (permalink / raw) To: Masahiro Yamada Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Thu, 15 Sept 2022 at 07:51, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > --- > > > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > > > --- a/scripts/Makefile.lib > > > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > > > You do not need to re-run dtc. > > > > > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > > > two different things in a single rule. > > > > > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > > > > > Correct. > > > > > > > > > > What I said is like this. > > > > > > > > > > # touch the timestamp file, %.dtb.checked > > > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > > > > > Not really a fan of the thousands of files that creates. Maybe if it > > > was turned into something useful like a list of schemas that apply to > > > the dtb. IOW, a dependency list. That would speed up re-running after > > > a schema change. Though if a schema change created new dependencies, > > > that wouldn't work. > > > > > > > > $(call if_changed_rule,dtb_check) > > > > > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > > > $(call if_changed_rule,dtc) > > > > > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > > > $(call if_changed_dep,dtc) > > > > > > > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > > > changed. > > > > > > > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > > > may come back. > > > > > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > > > Regenerating dtbs sounds like a minor pain compared to hacking the > > > > top-level Makefile again. What I really like is that if one has > > > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > > > validation. > > > > > > I lean towards just rebuilding the dtbs. That's pretty quick and > > > ensures we get dtc warnings with schema warnings. In the long run, I > > > would like to make the schema checks not optional to run. The > > > impediment to doing that is lots of warnings (but not not some > > > platforms), adding a tool dependency, and validation time. > > > > Rob, Masahiro, do we have any conclusion here? I can change my patch, > > but I'd like to understand in which way I should change it. > > Fixing/testing yaml changes is a bit painful w/o this change. > > > > -- > > With best wishes > > Dmitry > > > > > I am fine with the patch as long as you fix DTC/CHECK to DTC. Ack, I completely forgot about this part. Please excuse me. I'll change it to DTC_CHK (as suggested before) to fit into 7 chars in v2. > > Another idea is to re-check the schema every time, > like this: I think this will add the external build dependency. We tried a similar approach several releases ago and had to revert the change. > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 3fb6a99e78c4..9fc9f39fb12e 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -375,13 +375,9 @@ quiet_cmd_dtb_check = CHECK $@ > cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u > $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA > ) $@ || true > endif > > -define rule_dtc > - $(call cmd_and_fixdep,dtc) > - $(call cmd,dtb_check) > -endef > - > $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE > - $(call if_changed_rule,dtc) > + $(call if_changed_dep,dtc) > + $(call cmd,dtb_check) > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) > > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-15 11:40 ` Dmitry Baryshkov @ 2022-09-16 1:09 ` Masahiro Yamada 2022-09-16 14:21 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Masahiro Yamada @ 2022-09-16 1:09 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Thu, Sep 15, 2022 at 8:40 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 15 Sept 2022 at 07:51, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > > --- > > > > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > > > > --- a/scripts/Makefile.lib > > > > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > > > > You do not need to re-run dtc. > > > > > > > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > > > > two different things in a single rule. > > > > > > > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > > > > > > > Correct. > > > > > > > > > > > > What I said is like this. > > > > > > > > > > > > # touch the timestamp file, %.dtb.checked > > > > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > > > > > > > Not really a fan of the thousands of files that creates. Maybe if it > > > > was turned into something useful like a list of schemas that apply to > > > > the dtb. IOW, a dependency list. That would speed up re-running after > > > > a schema change. Though if a schema change created new dependencies, > > > > that wouldn't work. > > > > > > > > > > $(call if_changed_rule,dtb_check) > > > > > > > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > > > > $(call if_changed_rule,dtc) > > > > > > > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > > > > $(call if_changed_dep,dtc) > > > > > > > > > > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > > > > changed. > > > > > > > > > > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > > > > may come back. > > > > > > > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > > > > Regenerating dtbs sounds like a minor pain compared to hacking the > > > > > top-level Makefile again. What I really like is that if one has > > > > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > > > > validation. > > > > > > > > I lean towards just rebuilding the dtbs. That's pretty quick and > > > > ensures we get dtc warnings with schema warnings. In the long run, I > > > > would like to make the schema checks not optional to run. The > > > > impediment to doing that is lots of warnings (but not not some > > > > platforms), adding a tool dependency, and validation time. > > > > > > Rob, Masahiro, do we have any conclusion here? I can change my patch, > > > but I'd like to understand in which way I should change it. > > > Fixing/testing yaml changes is a bit painful w/o this change. > > > > > > -- > > > With best wishes > > > Dmitry > > > > > > > > > > I am fine with the patch as long as you fix DTC/CHECK to DTC. > > Ack, I completely forgot about this part. Please excuse me. I'll > change it to DTC_CHK (as suggested before) to fit into 7 chars in v2. > > > > > Another idea is to re-check the schema every time, > > like this: > > I think this will add the external build dependency. We tried a > similar approach several releases ago and had to revert the change. If you are talking about 75e895343d5a2fcbdf4cb3d31ab7492bd65925f0, you are completely misunderstanding. Look at my code closely. It is how the sparse checker works with C=2. > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 3fb6a99e78c4..9fc9f39fb12e 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -375,13 +375,9 @@ quiet_cmd_dtb_check = CHECK $@ > > cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u > > $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA > > ) $@ || true > > endif > > > > -define rule_dtc > > - $(call cmd_and_fixdep,dtc) > > - $(call cmd,dtb_check) > > -endef > > - > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE > > - $(call if_changed_rule,dtc) > > + $(call if_changed_dep,dtc) > > + $(call cmd,dtb_check) > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > $(call if_changed_dep,dtc) > > > > > > -- > With best wishes > Dmitry -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-16 1:09 ` Masahiro Yamada @ 2022-09-16 14:21 ` Dmitry Baryshkov 2022-09-16 14:54 ` Rob Herring 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Baryshkov @ 2022-09-16 14:21 UTC (permalink / raw) To: Masahiro Yamada Cc: Rob Herring, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, 16 Sept 2022 at 04:10, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Sep 15, 2022 at 8:40 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Thu, 15 Sept 2022 at 07:51, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > > > --- > > > > > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > > > > > --- a/scripts/Makefile.lib > > > > > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > > > > > You do not need to re-run dtc. > > > > > > > > > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > > > > > two different things in a single rule. > > > > > > > > > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > > > > > > > > > Correct. > > > > > > > > > > > > > > What I said is like this. > > > > > > > > > > > > > > # touch the timestamp file, %.dtb.checked > > > > > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > > > > > > > > > Not really a fan of the thousands of files that creates. Maybe if it > > > > > was turned into something useful like a list of schemas that apply to > > > > > the dtb. IOW, a dependency list. That would speed up re-running after > > > > > a schema change. Though if a schema change created new dependencies, > > > > > that wouldn't work. > > > > > > > > > > > > $(call if_changed_rule,dtb_check) > > > > > > > > > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > > > > > $(call if_changed_rule,dtc) > > > > > > > > > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > > > > > $(call if_changed_dep,dtc) > > > > > > > > > > > > > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > > > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > > > > > changed. > > > > > > > > > > > > > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > > > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > > > > > may come back. > > > > > > > > > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > > > > > Regenerating dtbs sounds like a minor pain compared to hacking the > > > > > > top-level Makefile again. What I really like is that if one has > > > > > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > > > > > validation. > > > > > > > > > > I lean towards just rebuilding the dtbs. That's pretty quick and > > > > > ensures we get dtc warnings with schema warnings. In the long run, I > > > > > would like to make the schema checks not optional to run. The > > > > > impediment to doing that is lots of warnings (but not not some > > > > > platforms), adding a tool dependency, and validation time. > > > > > > > > Rob, Masahiro, do we have any conclusion here? I can change my patch, > > > > but I'd like to understand in which way I should change it. > > > > Fixing/testing yaml changes is a bit painful w/o this change. > > > > > > > > -- > > > > With best wishes > > > > Dmitry > > > > > > > > > > > > > > > I am fine with the patch as long as you fix DTC/CHECK to DTC. > > > > Ack, I completely forgot about this part. Please excuse me. I'll > > change it to DTC_CHK (as suggested before) to fit into 7 chars in v2. > > > > > > > > Another idea is to re-check the schema every time, > > > like this: > > > > I think this will add the external build dependency. We tried a > > similar approach several releases ago and had to revert the change. > > > If you are talking about > 75e895343d5a2fcbdf4cb3d31ab7492bd65925f0, > you are completely misunderstanding. > > Look at my code closely. > It is how the sparse checker works with C=2. Ah, I see. Then I'm fine with either your patch or mine v2 at your (and Rob's) preference. > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index 3fb6a99e78c4..9fc9f39fb12e 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > @@ -375,13 +375,9 @@ quiet_cmd_dtb_check = CHECK $@ > > > cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u > > > $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA > > > ) $@ || true > > > endif > > > > > > -define rule_dtc > > > - $(call cmd_and_fixdep,dtc) > > > - $(call cmd,dtb_check) > > > -endef > > > - > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE > > > - $(call if_changed_rule,dtc) > > > + $(call if_changed_dep,dtc) > > > + $(call cmd,dtb_check) > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > $(call if_changed_dep,dtc) > > > > > > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-16 14:21 ` Dmitry Baryshkov @ 2022-09-16 14:54 ` Rob Herring 2022-09-16 19:37 ` Dmitry Baryshkov 0 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2022-09-16 14:54 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On Fri, Sep 16, 2022 at 9:21 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Fri, 16 Sept 2022 at 04:10, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Thu, Sep 15, 2022 at 8:40 PM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Thu, 15 Sept 2022 at 07:51, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > > > On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > > > > > > > > > > > > > > > On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov > > > > > > > > > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > It is useful to be able to recheck dtbs files against a limited set of > > > > > > > > > > > > DT schema files. This can be accomplished by using differnt > > > > > > > > > > > > DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However > > > > > > > > > > > > for some reason if_changed_rule doesn't pick up the rule_dtc changes > > > > > > > > > > > > (and doesn't retrigger the build). > > > > > > > > > > > > > > > > > > > > > > > > Fix this by changing if_changed_rule to if_changed_dep and squashing DTC > > > > > > > > > > > > and dt-validate into a single new command. Then if_changed_dep triggers > > > > > > > > > > > > on DT_SCHEMA_FILES changes and reruns the build/check. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > > > > > > > --- > > > > > > > > > > > > scripts/Makefile.lib | 14 ++++++-------- > > > > > > > > > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > > > > > > > > index c88b98b5dc44..3df470289382 100644 > > > > > > > > > > > > --- a/scripts/Makefile.lib > > > > > > > > > > > > +++ b/scripts/Makefile.lib > > > > > > > > > > > > @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) > > > > > > > > > > > > DT_BINDING_DIR := Documentation/devicetree/bindings > > > > > > > > > > > > DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json > > > > > > > > > > > > > > > > > > > > > > > > -quiet_cmd_dtb_check = CHECK $@ > > > > > > > > > > > > - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true > > > > > > > > > > > > +quiet_cmd_dtb = DTC/CHECK $@ > > > > > > > > > > > > > > > > > > > > > > This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or > > > > > > > > > > > always do just 'DTC'. I can fixup when applying. > > > > > > > > > > > > > > > > > > > > > > I'll give it a few days for other comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. > > > > > > > > > > You do not need to re-run dtc. > > > > > > > > > > > > > > > > > > > > I guess the strangeness comes from the fact that you are trying to do the > > > > > > > > > > two different things in a single rule. > > > > > > > > > > > > > > > > > > The issue is that with the current rules the dt-validate isn't > > > > > > > > > re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. > > > > > > > > > > > > > > > > Correct. > > > > > > > > > > > > > > > > What I said is like this. > > > > > > > > > > > > > > > > # touch the timestamp file, %.dtb.checked > > > > > > > > $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE > > > > > > > > > > > > Not really a fan of the thousands of files that creates. Maybe if it > > > > > > was turned into something useful like a list of schemas that apply to > > > > > > the dtb. IOW, a dependency list. That would speed up re-running after > > > > > > a schema change. Though if a schema change created new dependencies, > > > > > > that wouldn't work. > > > > > > > > > > > > > > $(call if_changed_rule,dtb_check) > > > > > > > > > > > > > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE > > > > > > > > $(call if_changed_rule,dtc) > > > > > > > > > > > > > > > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > > > > > > > $(call if_changed_dep,dtc) > > > > > > > > > > > > > > > > > > > > > > > > With the dtc/check split, we can avoid unneeded regeneration of > > > > > > > > %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is > > > > > > > > changed. > > > > > > > > > > > > > > > > > > > > > > > > One drawback is we track %.dtb.checked and and %.dtb separately, > > > > > > > > so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 > > > > > > > > may come back. > > > > > > > > > > > > > > It's up to you and Rob, but I'd really prefer a simpler solution here. > > > > > > > Regenerating dtbs sounds like a minor pain compared to hacking the > > > > > > > top-level Makefile again. What I really like is that if one has > > > > > > > CHECK_DTBS=y (for whatever reason), he can not generate dtb without > > > > > > > validation. > > > > > > > > > > > > I lean towards just rebuilding the dtbs. That's pretty quick and > > > > > > ensures we get dtc warnings with schema warnings. In the long run, I > > > > > > would like to make the schema checks not optional to run. The > > > > > > impediment to doing that is lots of warnings (but not not some > > > > > > platforms), adding a tool dependency, and validation time. > > > > > > > > > > Rob, Masahiro, do we have any conclusion here? I can change my patch, > > > > > but I'd like to understand in which way I should change it. > > > > > Fixing/testing yaml changes is a bit painful w/o this change. > > > > > > > > > > -- > > > > > With best wishes > > > > > Dmitry > > > > > > > > > > > > > > > > > > > > I am fine with the patch as long as you fix DTC/CHECK to DTC. > > > > > > Ack, I completely forgot about this part. Please excuse me. I'll > > > change it to DTC_CHK (as suggested before) to fit into 7 chars in v2. > > > > > > > > > > > Another idea is to re-check the schema every time, > > > > like this: > > > > > > I think this will add the external build dependency. We tried a > > > similar approach several releases ago and had to revert the change. > > > > > > If you are talking about > > 75e895343d5a2fcbdf4cb3d31ab7492bd65925f0, > > you are completely misunderstanding. > > > > Look at my code closely. > > It is how the sparse checker works with C=2. > > Ah, I see. Then I'm fine with either your patch or mine v2 at your > (and Rob's) preference. I'd rather not recheck every time. The user can remove the dtbs if they want to do that. Rob ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs 2022-09-16 14:54 ` Rob Herring @ 2022-09-16 19:37 ` Dmitry Baryshkov 0 siblings, 0 replies; 14+ messages in thread From: Dmitry Baryshkov @ 2022-09-16 19:37 UTC (permalink / raw) To: Rob Herring Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Linux Kbuild mailing list, linux-kernel, Krzysztof Kozlowski, Bjorn Andersson, linux-arm-msm On 16/09/2022 17:54, Rob Herring wrote: > On Fri, Sep 16, 2022 at 9:21 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Fri, 16 Sept 2022 at 04:10, Masahiro Yamada <masahiroy@kernel.org> wrote: >>> >>> On Thu, Sep 15, 2022 at 8:40 PM Dmitry Baryshkov >>> <dmitry.baryshkov@linaro.org> wrote: >>>> >>>> On Thu, 15 Sept 2022 at 07:51, Masahiro Yamada <masahiroy@kernel.org> wrote: >>>>> >>>>> On Sun, Sep 11, 2022 at 2:36 AM Dmitry Baryshkov >>>>> >>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>> >>>>>> On Thu, 4 Aug 2022 at 18:27, Rob Herring <robh+dt@kernel.org> wrote: >>>>>>> >>>>>>> On Fri, Jul 29, 2022 at 1:46 AM Dmitry Baryshkov >>>>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>>>> >>>>>>>> On Fri, 29 Jul 2022 at 10:05, Masahiro Yamada <masahiroy@kernel.org> wrote: >>>>>>>>> >>>>>>>>> On Fri, Jul 29, 2022 at 3:53 PM Dmitry Baryshkov >>>>>>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>>>>>> >>>>>>>>>> On Fri, 29 Jul 2022 at 08:55, Masahiro Yamada <masahiroy@kernel.org> wrote: >>>>>>>>>>> >>>>>>>>>>> On Thu, Jul 28, 2022 at 2:36 AM Rob Herring <robh+dt@kernel.org> wrote: >>>>>>>>>>>> >>>>>>>>>>>> On Wed, Jul 27, 2022 at 4:06 AM Dmitry Baryshkov >>>>>>>>>>>> <dmitry.baryshkov@linaro.org> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> It is useful to be able to recheck dtbs files against a limited set of >>>>>>>>>>>>> DT schema files. This can be accomplished by using differnt >>>>>>>>>>>>> DT_SCHEMA_FILES argument values while rerunning make dtbs_check. However >>>>>>>>>>>>> for some reason if_changed_rule doesn't pick up the rule_dtc changes >>>>>>>>>>>>> (and doesn't retrigger the build). >>>>>>>>>>>>> >>>>>>>>>>>>> Fix this by changing if_changed_rule to if_changed_dep and squashing DTC >>>>>>>>>>>>> and dt-validate into a single new command. Then if_changed_dep triggers >>>>>>>>>>>>> on DT_SCHEMA_FILES changes and reruns the build/check. >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>>>>>>>>>> --- >>>>>>>>>>>>> scripts/Makefile.lib | 14 ++++++-------- >>>>>>>>>>>>> 1 file changed, 6 insertions(+), 8 deletions(-) >>>>>>>>>>>>> >>>>>>>>>>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>>>>>>>>>>>> index c88b98b5dc44..3df470289382 100644 >>>>>>>>>>>>> --- a/scripts/Makefile.lib >>>>>>>>>>>>> +++ b/scripts/Makefile.lib >>>>>>>>>>>>> @@ -383,17 +383,15 @@ DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m) >>>>>>>>>>>>> DT_BINDING_DIR := Documentation/devicetree/bindings >>>>>>>>>>>>> DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json >>>>>>>>>>>>> >>>>>>>>>>>>> -quiet_cmd_dtb_check = CHECK $@ >>>>>>>>>>>>> - cmd_dtb_check = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true >>>>>>>>>>>>> +quiet_cmd_dtb = DTC/CHECK $@ >>>>>>>>>>>> >>>>>>>>>>>> This is supposed to be 7 chars or less. DTCCHK or DTC_CHK perhaps. Or >>>>>>>>>>>> always do just 'DTC'. I can fixup when applying. >>>>>>>>>>>> >>>>>>>>>>>> I'll give it a few days for other comments. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> When you change DT_SCHEMA_FILES, re-running dt-validate should be enough. >>>>>>>>>>> You do not need to re-run dtc. >>>>>>>>>>> >>>>>>>>>>> I guess the strangeness comes from the fact that you are trying to do the >>>>>>>>>>> two different things in a single rule. >>>>>>>>>> >>>>>>>>>> The issue is that with the current rules the dt-validate isn't >>>>>>>>>> re-executed on DT_SCHEMA_FILES changes. Thus comes my proposal. >>>>>>>>> >>>>>>>>> Correct. >>>>>>>>> >>>>>>>>> What I said is like this. >>>>>>>>> >>>>>>>>> # touch the timestamp file, %.dtb.checked >>>>>>>>> $(obj)/%.dtb.checked: $(obj)/%.dtb $(DT_TMP_SCHEMA) FORCE >>>>>>> >>>>>>> Not really a fan of the thousands of files that creates. Maybe if it >>>>>>> was turned into something useful like a list of schemas that apply to >>>>>>> the dtb. IOW, a dependency list. That would speed up re-running after >>>>>>> a schema change. Though if a schema change created new dependencies, >>>>>>> that wouldn't work. >>>>>>> >>>>>>>>> $(call if_changed_rule,dtb_check) >>>>>>>>> >>>>>>>>> $(obj)/%.dtb: $(src)/%.dts $(DTC) $FORCE >>>>>>>>> $(call if_changed_rule,dtc) >>>>>>>>> >>>>>>>>> $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE >>>>>>>>> $(call if_changed_dep,dtc) >>>>>>>>> >>>>>>>>> >>>>>>>>> With the dtc/check split, we can avoid unneeded regeneration of >>>>>>>>> %.dtb when DT_TMP_SCHEMA or DT_SCHEMA_FILES is >>>>>>>>> changed. >>>>>>>>> >>>>>>>>> >>>>>>>>> One drawback is we track %.dtb.checked and and %.dtb separately, >>>>>>>>> so something like 53182e81f47d4ea0c727c49ad23cb782173ab849 >>>>>>>>> may come back. >>>>>>>> >>>>>>>> It's up to you and Rob, but I'd really prefer a simpler solution here. >>>>>>>> Regenerating dtbs sounds like a minor pain compared to hacking the >>>>>>>> top-level Makefile again. What I really like is that if one has >>>>>>>> CHECK_DTBS=y (for whatever reason), he can not generate dtb without >>>>>>>> validation. >>>>>>> >>>>>>> I lean towards just rebuilding the dtbs. That's pretty quick and >>>>>>> ensures we get dtc warnings with schema warnings. In the long run, I >>>>>>> would like to make the schema checks not optional to run. The >>>>>>> impediment to doing that is lots of warnings (but not not some >>>>>>> platforms), adding a tool dependency, and validation time. >>>>>> >>>>>> Rob, Masahiro, do we have any conclusion here? I can change my patch, >>>>>> but I'd like to understand in which way I should change it. >>>>>> Fixing/testing yaml changes is a bit painful w/o this change. >>>>>> >>>>>> -- >>>>>> With best wishes >>>>>> Dmitry >>>>> >>>>> >>>>> >>>>> >>>>> I am fine with the patch as long as you fix DTC/CHECK to DTC. >>>> >>>> Ack, I completely forgot about this part. Please excuse me. I'll >>>> change it to DTC_CHK (as suggested before) to fit into 7 chars in v2. >>>> >>>>> >>>>> Another idea is to re-check the schema every time, >>>>> like this: >>>> >>>> I think this will add the external build dependency. We tried a >>>> similar approach several releases ago and had to revert the change. >>> >>> >>> If you are talking about >>> 75e895343d5a2fcbdf4cb3d31ab7492bd65925f0, >>> you are completely misunderstanding. >>> >>> Look at my code closely. >>> It is how the sparse checker works with C=2. >> >> Ah, I see. Then I'm fine with either your patch or mine v2 at your >> (and Rob's) preference. > > I'd rather not recheck every time. The user can remove the dtbs if > they want to do that. Then [1] should be the better fit. [1] https://lore.kernel.org/linux-kernel/20220915114422.79378-1-dmitry.baryshkov@linaro.org/ -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-09-16 19:37 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-27 10:06 [PATCH] kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs Dmitry Baryshkov 2022-07-27 17:36 ` Rob Herring 2022-07-29 5:54 ` Masahiro Yamada 2022-07-29 6:52 ` Dmitry Baryshkov 2022-07-29 7:03 ` Masahiro Yamada 2022-07-29 7:46 ` Dmitry Baryshkov 2022-08-04 15:27 ` Rob Herring 2022-09-10 17:35 ` Dmitry Baryshkov 2022-09-15 4:50 ` Masahiro Yamada 2022-09-15 11:40 ` Dmitry Baryshkov 2022-09-16 1:09 ` Masahiro Yamada 2022-09-16 14:21 ` Dmitry Baryshkov 2022-09-16 14:54 ` Rob Herring 2022-09-16 19:37 ` Dmitry Baryshkov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.