| Message ID | 20220727100615.638072-1-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: take into account DT_SCHEMA_FILES changes while checking dtbs | expand |
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 >
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
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.
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
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.
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
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.
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
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) > >
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
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) > > > > > >
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
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/
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)
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(-)