| Message ID | 20220706114407.1507412-1-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: allow validating individual dtb files against schema | expand |
Am 06.07.22 um 13:44 schrieb Dmitry Baryshkov: > While it is possible to validate all generated dtb files against the > schema, it typically results in huge pile of warnings. While working on > a platform it is quite useful to validate just a single file against > schema. > > Allow specifying CHECK_DTBS=1 on a make command line to enable > validation while building dtb files. This reuses the infrastructure > existing for `make dtbs_check`, making dtbs_check a shortcut for > `make CHECK_DTBS=1 dt_binding_check dtbs`. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Tom Rini <trini@konsulko.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: linux-kbuild@vger.kernel.org > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> This is really useful, thanks! Exactly what I was looking for. May I suggest to add a line about this new option to Documentation/devicetree/bindings/writing-schema.rst? Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> > --- > > Changes since v1: > - Added dependency to rebuild schema if `make dtbs` was used. > > --- > Makefile | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index 9aa7de1ca58f..5a9858aa4934 100644 > --- a/Makefile > +++ b/Makefile > @@ -1464,14 +1464,18 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +ifneq ($(CHECK_DTBS),) > +DT_TMP_BINDING := dt_binding > +endif > + > +%.dtb: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > -%.dtbo: include/config/kernel.release scripts_dtc > +%.dtbo: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > PHONY += dtbs dtbs_install dtbs_check > -dtbs: include/config/kernel.release scripts_dtc > +dtbs: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > @@ -1498,8 +1502,10 @@ ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) > export CHECK_DT_BINDING=y > endif > > -PHONY += dt_binding_check > -dt_binding_check: scripts_dtc > +dt_binding_check: dt_binding > + > +PHONY += dt_binding > +dt_binding: scripts_dtc > $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings > > # --------------------------------------------------------------------------- > @@ -1774,6 +1780,10 @@ help: > @echo ' 3: more obscure warnings, can most likely be ignored' > @echo ' e: warnings are being treated as errors' > @echo ' Multiple levels can be combined with W=12 or W=123' > + @$(if $(dtstree), \ > + echo ' make CHECK_DTBS=1 [targets] Check all generated dtb files against schema'; \ > + echo ' This can be applied both to "dtbs" and to individual "foo.dtb" targets' ; \ > + ) > @echo '' > @echo 'Execute "make" or "make all" to build all targets marked with [*] ' > @echo 'For further info see the ./README file' >
On Wed, Jul 6, 2022 at 8:44 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > While it is possible to validate all generated dtb files against the > schema, it typically results in huge pile of warnings. While working on > a platform it is quite useful to validate just a single file against > schema. > > Allow specifying CHECK_DTBS=1 on a make command line to enable > validation while building dtb files. This reuses the infrastructure > existing for `make dtbs_check`, making dtbs_check a shortcut for > `make CHECK_DTBS=1 dt_binding_check dtbs`. > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Tom Rini <trini@konsulko.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: linux-kbuild@vger.kernel.org > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > > Changes since v1: > - Added dependency to rebuild schema if `make dtbs` was used. > > --- > Makefile | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index 9aa7de1ca58f..5a9858aa4934 100644 > --- a/Makefile > +++ b/Makefile > @@ -1464,14 +1464,18 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +ifneq ($(CHECK_DTBS),) > +DT_TMP_BINDING := dt_binding > +endif > + > +%.dtb: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > -%.dtbo: include/config/kernel.release scripts_dtc > +%.dtbo: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > PHONY += dtbs dtbs_install dtbs_check > -dtbs: include/config/kernel.release scripts_dtc > +dtbs: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > $(Q)$(MAKE) $(build)=$(dtstree) > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > @@ -1498,8 +1502,10 @@ ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) > export CHECK_DT_BINDING=y > endif > > -PHONY += dt_binding_check > -dt_binding_check: scripts_dtc > +dt_binding_check: dt_binding > + > +PHONY += dt_binding > +dt_binding: scripts_dtc > $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings > > # --------------------------------------------------------------------------- > @@ -1774,6 +1780,10 @@ help: > @echo ' 3: more obscure warnings, can most likely be ignored' > @echo ' e: warnings are being treated as errors' > @echo ' Multiple levels can be combined with W=12 or W=123' > + @$(if $(dtstree), \ > + echo ' make CHECK_DTBS=1 [targets] Check all generated dtb files against schema'; \ > + echo ' This can be applied both to "dtbs" and to individual "foo.dtb" targets' ; \ > + ) > @echo '' > @echo 'Execute "make" or "make all" to build all targets marked with [*] ' > @echo 'For further info see the ./README file' > -- > 2.35.1 > I think the idea seems OK to me, but we can make it simpler. First, apply the following clean-up patch to reduce the code duplication. https://lore.kernel.org/all/20220716093122.137494-1-masahiroy@kernel.org/T/#u Then, apply the attached patch.diff Please try it.
On Sat, 16 Jul 2022 at 12:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Jul 6, 2022 at 8:44 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > While it is possible to validate all generated dtb files against the > > schema, it typically results in huge pile of warnings. While working on > > a platform it is quite useful to validate just a single file against > > schema. > > > > Allow specifying CHECK_DTBS=1 on a make command line to enable > > validation while building dtb files. This reuses the infrastructure > > existing for `make dtbs_check`, making dtbs_check a shortcut for > > `make CHECK_DTBS=1 dt_binding_check dtbs`. > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Tom Rini <trini@konsulko.com> > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Cc: linux-kbuild@vger.kernel.org > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > > > Changes since v1: > > - Added dependency to rebuild schema if `make dtbs` was used. > > > > --- > > Makefile | 20 +++++++++++++++----- > > 1 file changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 9aa7de1ca58f..5a9858aa4934 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1464,14 +1464,18 @@ endif > > > > ifneq ($(dtstree),) > > > > -%.dtb: include/config/kernel.release scripts_dtc > > +ifneq ($(CHECK_DTBS),) > > +DT_TMP_BINDING := dt_binding > > +endif > > + > > +%.dtb: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > -%.dtbo: include/config/kernel.release scripts_dtc > > +%.dtbo: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > PHONY += dtbs dtbs_install dtbs_check > > -dtbs: include/config/kernel.release scripts_dtc > > +dtbs: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) > > $(Q)$(MAKE) $(build)=$(dtstree) > > > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > > @@ -1498,8 +1502,10 @@ ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) > > export CHECK_DT_BINDING=y > > endif > > > > -PHONY += dt_binding_check > > -dt_binding_check: scripts_dtc > > +dt_binding_check: dt_binding > > + > > +PHONY += dt_binding > > +dt_binding: scripts_dtc > > $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings > > > > # --------------------------------------------------------------------------- > > @@ -1774,6 +1780,10 @@ help: > > @echo ' 3: more obscure warnings, can most likely be ignored' > > @echo ' e: warnings are being treated as errors' > > @echo ' Multiple levels can be combined with W=12 or W=123' > > + @$(if $(dtstree), \ > > + echo ' make CHECK_DTBS=1 [targets] Check all generated dtb files against schema'; \ > > + echo ' This can be applied both to "dtbs" and to individual "foo.dtb" targets' ; \ > > + ) > > @echo '' > > @echo 'Execute "make" or "make all" to build all targets marked with [*] ' > > @echo 'For further info see the ./README file' > > -- > > 2.35.1 > > > > > I think the idea seems OK to me, but we can make it simpler. > > > First, apply the following clean-up patch to reduce the code duplication. > https://lore.kernel.org/all/20220716093122.137494-1-masahiroy@kernel.org/T/#u > > > Then, apply the attached patch.diff > > Please try it. Please excuse me, it took me a bit to get back to the issue and test your patch. It works like a charm, feel free to add while posting it: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> -- With best wishes Dmitry
On Sat, 16 Jul 2022 at 12:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > First, apply the following clean-up patch to reduce the code duplication. > https://lore.kernel.org/all/20220716093122.137494-1-masahiroy@kernel.org/T/#u > The cleanup patch has been merged into 6.0. > Then, apply the attached patch.diff But the patch you proposed didn't seem to be posted. Do you still intend to post it? -- With best wishes Dmitry
Hi Masahiro, On 25.10.22 21:07, Dmitry Baryshkov wrote: > On Sat, 16 Jul 2022 at 12:38, Masahiro Yamada <masahiroy@kernel.org> wrote: >> First, apply the following clean-up patch to reduce the code duplication. >> https://lore.kernel.org/all/20220716093122.137494-1-masahiroy@kernel.org/T/#u >> > > The cleanup patch has been merged into 6.0. > >> Then, apply the attached patch.diff > > But the patch you proposed didn't seem to be posted. Do you still > intend to post it? Can you formally post the patch you proposed above? It is very useful and works just fine for me. Thanks Frieder
On Thu, Feb 2, 2023 at 12:51 AM Frieder Schrempf <frieder.schrempf@kontron.de> wrote: > > Hi Masahiro, > > On 25.10.22 21:07, Dmitry Baryshkov wrote: > > On Sat, 16 Jul 2022 at 12:38, Masahiro Yamada <masahiroy@kernel.org> wrote: > >> First, apply the following clean-up patch to reduce the code duplication. > >> https://lore.kernel.org/all/20220716093122.137494-1-masahiroy@kernel.org/T/#u > >> > > > > The cleanup patch has been merged into 6.0. > > > >> Then, apply the attached patch.diff > > > > But the patch you proposed didn't seem to be posted. Do you still > > intend to post it? > > Can you formally post the patch you proposed above? > It is very useful and works just fine for me. > > Thanks > Frieder Rob applied an equivalent patch (with help message added). commit ec201955a53be4b57a467f7160724ff06289cead Author: Rob Herring <robh@kernel.org> Date: Mon Dec 19 19:32:33 2022 -0600 kbuild: Optionally enable schema checks for %.dtb targets -- Best Regards Masahiro Yamada
On 01.02.23 17:08, Masahiro Yamada wrote: > On Thu, Feb 2, 2023 at 12:51 AM Frieder Schrempf > <frieder.schrempf@kontron.de> wrote: >> >> Hi Masahiro, >> >> On 25.10.22 21:07, Dmitry Baryshkov wrote: >>> On Sat, 16 Jul 2022 at 12:38, Masahiro Yamada <masahiroy@kernel.org> wrote: >>>> First, apply the following clean-up patch to reduce the code duplication. >>>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220716093122.137494-1-masahiroy%40kernel.org%2FT%2F%23u&data=05%7C01%7Cfrieder.schrempf%40kontron.de%7C95bf7b3a216841e1630408db046ea810%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C638108645530201844%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6aAT%2Bzng%2B7CWd8wWPVzvvgJRpRcG2dAeW%2FKDUoUNKzg%3D&reserved=0 >>>> >>> >>> The cleanup patch has been merged into 6.0. >>> >>>> Then, apply the attached patch.diff >>> >>> But the patch you proposed didn't seem to be posted. Do you still >>> intend to post it? >> >> Can you formally post the patch you proposed above? >> It is very useful and works just fine for me. >> >> Thanks >> Frieder > > > > Rob applied an equivalent patch > (with help message added). > > > > commit ec201955a53be4b57a467f7160724ff06289cead > Author: Rob Herring <robh@kernel.org> > Date: Mon Dec 19 19:32:33 2022 -0600 > > kbuild: Optionally enable schema checks for %.dtb targets > Ok, great! I forgot to check linux-next. Thanks!
diff --git a/Makefile b/Makefile index 9aa7de1ca58f..5a9858aa4934 100644 --- a/Makefile +++ b/Makefile @@ -1464,14 +1464,18 @@ endif ifneq ($(dtstree),) -%.dtb: include/config/kernel.release scripts_dtc +ifneq ($(CHECK_DTBS),) +DT_TMP_BINDING := dt_binding +endif + +%.dtb: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ -%.dtbo: include/config/kernel.release scripts_dtc +%.dtbo: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ PHONY += dtbs dtbs_install dtbs_check -dtbs: include/config/kernel.release scripts_dtc +dtbs: include/config/kernel.release scripts_dtc $(DT_TMP_BINDING) $(Q)$(MAKE) $(build)=$(dtstree) ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) @@ -1498,8 +1502,10 @@ ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) export CHECK_DT_BINDING=y endif -PHONY += dt_binding_check -dt_binding_check: scripts_dtc +dt_binding_check: dt_binding + +PHONY += dt_binding +dt_binding: scripts_dtc $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings # --------------------------------------------------------------------------- @@ -1774,6 +1780,10 @@ help: @echo ' 3: more obscure warnings, can most likely be ignored' @echo ' e: warnings are being treated as errors' @echo ' Multiple levels can be combined with W=12 or W=123' + @$(if $(dtstree), \ + echo ' make CHECK_DTBS=1 [targets] Check all generated dtb files against schema'; \ + echo ' This can be applied both to "dtbs" and to individual "foo.dtb" targets' ; \ + ) @echo '' @echo 'Execute "make" or "make all" to build all targets marked with [*] ' @echo 'For further info see the ./README file'
While it is possible to validate all generated dtb files against the schema, it typically results in huge pile of warnings. While working on a platform it is quite useful to validate just a single file against schema. Allow specifying CHECK_DTBS=1 on a make command line to enable validation while building dtb files. This reuses the infrastructure existing for `make dtbs_check`, making dtbs_check a shortcut for `make CHECK_DTBS=1 dt_binding_check dtbs`. Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Tom Rini <trini@konsulko.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: linux-kbuild@vger.kernel.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes since v1: - Added dependency to rebuild schema if `make dtbs` was used. --- Makefile | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)