linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] dt-bindings: DTB based validation
@ 2022-03-03 22:42 Rob Herring
  2022-03-03 22:42 ` [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate Rob Herring
  2022-03-03 22:42 ` [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation Rob Herring
  0 siblings, 2 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-03 22:42 UTC (permalink / raw)
  To: Masahiro Yamada, Krzysztof Kozlowski
  Cc: Geert Uytterhoeven, Laurent Pinchart, Maxime Ripard,
	linux-kernel, devicetree

This series switches the kernel's DT schema validation from YAML encoded
DT files to using DTB files directly. See patch 2 for the full reasoning
of why. The diffstat also shows this is a nice simplification (at least 
from the kernel side). Patch 1 is further reworking of how 
DT_SCHEMA_FILES works and builds on [1].

Overall, the build time is about the same (still slow) though we do save 
a dtc call for dtbs_check. Extracting the type information is not cached 
in any way, so there's some opportunity for a slight optimization there. 

Switching to DTB validation found various issues in the bindings and 
examples. Patches for those issues have already been posted and applied 
over the last month or so.

Rob

[1] https://lore.kernel.org/all/20220228201006.1484903-1-robh@kernel.org/

Rob Herring (2):
  dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  dt-bindings: kbuild: Use DTB files for validation

 Documentation/devicetree/bindings/Makefile    | 35 ++++---------------
 .../devicetree/bindings/writing-schema.rst    | 12 -------
 scripts/Makefile.lib                          | 23 +++++-------
 scripts/dtc/Makefile                          | 13 -------
 scripts/dtc/update-dtc-source.sh              |  2 +-
 5 files changed, 15 insertions(+), 70 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-03 22:42 [PATCH 0/2] dt-bindings: DTB based validation Rob Herring
@ 2022-03-03 22:42 ` Rob Herring
  2022-03-04  9:32   ` Geert Uytterhoeven
  2022-03-04 11:48   ` Laurent Pinchart
  2022-03-03 22:42 ` [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation Rob Herring
  1 sibling, 2 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-03 22:42 UTC (permalink / raw)
  To: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek, Nick Desaulniers
  Cc: Geert Uytterhoeven, Laurent Pinchart, Maxime Ripard,
	linux-kernel, devicetree, linux-kbuild

In preparation for supporting validation of DTB files, the full
processed schema will always be needed in order to extract type
information from it. Therefore, the processed schema containing only
what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
gained an option, -l or --limit, to specify which schema(s) to use for
validation.

As the command line option is new, we the minimum dtschema version must be
updated.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/Makefile | 28 +++-------------------
 scripts/Makefile.lib                       |  3 +--
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 61ec18ecc931..246ba0ecab64 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
 DT_SCHEMA_LINT := $(shell which yamllint || \
   echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
 
-DT_SCHEMA_MIN_VERSION = 2021.2.1
+DT_SCHEMA_MIN_VERSION = 2022.3
 
 PHONY += check_dtschema_version
 check_dtschema_version:
@@ -25,9 +25,6 @@ quiet_cmd_extract_ex = DTEX    $@
 $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
 	$(call if_changed,extract_ex)
 
-# Use full schemas when checking %.example.dts
-DT_TMP_SCHEMA := $(obj)/processed-schema-examples.json
-
 find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
 		-name 'processed-schema*' ! \
 		-name '*.example.dt.yaml' \)
@@ -70,29 +67,10 @@ override DTC_FLAGS := \
 # Disable undocumented compatible checks until warning free
 override DT_CHECKER_FLAGS ?=
 
-$(obj)/processed-schema-examples.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
+$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
 	$(call if_changed_rule,chkdt)
 
-ifeq ($(DT_SCHEMA_FILES),)
-
-# Unless DT_SCHEMA_FILES is specified, use the full schema for dtbs_check too.
-# Just copy processed-schema-examples.json
-
-$(obj)/processed-schema.json: $(obj)/processed-schema-examples.json FORCE
-	$(call if_changed,copy)
-
-else
-
-# If DT_SCHEMA_FILES is specified, use it for processed-schema.json
-
-$(obj)/processed-schema.json: DT_MK_SCHEMA_FLAGS := -u
-$(obj)/processed-schema.json: $(CHK_DT_DOCS) check_dtschema_version FORCE
-	$(call if_changed,mk_schema)
-
-endif
-
-always-$(CHECK_DT_BINDING) += processed-schema-examples.json
-always-$(CHECK_DTBS)       += processed-schema.json
+always-y += processed-schema.json
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 79be57fdd32a..9f1e8442564e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -361,9 +361,8 @@ $(multi-dtb-y): FORCE
 $(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
 
 DT_CHECKER ?= dt-validate
-DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),,-m)
+DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
 DT_BINDING_DIR := Documentation/devicetree/bindings
-# DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
 DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
 
 quiet_cmd_dtb_check =	CHECK   $@
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-03 22:42 [PATCH 0/2] dt-bindings: DTB based validation Rob Herring
  2022-03-03 22:42 ` [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate Rob Herring
@ 2022-03-03 22:42 ` Rob Herring
  2022-03-04 11:55   ` Laurent Pinchart
  2022-03-07 12:20   ` Geert Uytterhoeven
  1 sibling, 2 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-03 22:42 UTC (permalink / raw)
  To: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand
  Cc: Geert Uytterhoeven, Laurent Pinchart, Maxime Ripard,
	linux-kernel, devicetree, linux-kbuild

Switch the DT validation to use DTB files directly instead of a DTS to
YAML conversion.

The original motivation for supporting validation on DTB files was to
enable running validation on a running system (e.g. 'dt-validate
/sys/firmware/fdt') or other cases where the original source DTS is not
available.

The YAML format was not without issues. Using DTBs with the schema type
information solves some of those problems. The YAML format relies on the
DTS source level information including bracketing of properties, size
directives, and phandle tags all of which are lost in a DTB file. While
standardizing the bracketing is a good thing, it does cause a lot of
extra warnings and churn to fix them.

Another issue has been signed types are not validated correctly as sign
information is not propagated to YAML. Using the schema type information
allows for proper handling of signed types. YAML also can't represent
the full range of 64-bit integers as numbers are stored as floats by
most/all parsers.

The DTB validation works by decoding property values using the type
information in the schemas themselves. The main corner case this does
not work for is matrix types where neither dimension is fixed. For
now, checking the dimensions in these cases are skipped.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/Makefile    |  7 +++----
 .../devicetree/bindings/writing-schema.rst    | 12 -----------
 scripts/Makefile.lib                          | 20 +++++++------------
 scripts/dtc/Makefile                          | 13 ------------
 scripts/dtc/update-dtc-source.sh              |  2 +-
 5 files changed, 11 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 246ba0ecab64..b8bd6a8ec43a 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -26,8 +26,7 @@ $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
 	$(call if_changed,extract_ex)
 
 find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
-		-name 'processed-schema*' ! \
-		-name '*.example.dt.yaml' \)
+		-name 'processed-schema*' \)
 
 find_cmd = $(find_all_cmd) | grep -F "$(DT_SCHEMA_FILES)"
 CHK_DT_DOCS := $(shell $(find_cmd))
@@ -72,9 +71,9 @@ $(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version
 
 always-y += processed-schema.json
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
-always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
+always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
 
 # Hack: avoid 'Argument list too long' error for 'make clean'. Remove most of
 # build artifacts here before they are processed by scripts/Makefile.clean
 clean-files = $(shell find $(obj) \( -name '*.example.dts' -o \
-			-name '*.example.dt.yaml' \) -delete 2>/dev/null)
+			-name '*.example.dtb' \) -delete 2>/dev/null)
diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst
index 3b00fe981494..b4258bf81be5 100644
--- a/Documentation/devicetree/bindings/writing-schema.rst
+++ b/Documentation/devicetree/bindings/writing-schema.rst
@@ -123,18 +123,6 @@ project can be installed with pip::
 Several executables (dt-doc-validate, dt-mk-schema, dt-validate) will be
 installed. Ensure they are in your PATH (~/.local/bin by default).
 
-dtc must also be built with YAML output support enabled. This requires that
-libyaml and its headers be installed on the host system. For some distributions
-that involves installing the development package, such as:
-
-Debian::
-
-  apt-get install libyaml-dev
-
-Fedora::
-
-  dnf -y install libyaml-devel
-
 Running checks
 ~~~~~~~~~~~~~~
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 9f1e8442564e..4629af60160b 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -87,11 +87,6 @@ base-dtb-y := $(foreach m, $(multi-dtb-y), $(firstword $(call suffix-search, $m,
 
 always-y			+= $(dtb-y)
 
-ifneq ($(CHECK_DTBS),)
-always-y += $(patsubst %.dtb,%.dt.yaml, $(real-dtb-y))
-always-y += $(patsubst %.dtbo,%.dt.yaml, $(real-dtb-y))
-endif
-
 # Add subdir path
 
 extra-y		:= $(addprefix $(obj)/,$(extra-y))
@@ -347,12 +342,6 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
-$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
-	$(call if_changed_dep,dtc)
-
-$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
-	$(call if_changed_dep,dtc)
-
 quiet_cmd_fdtoverlay = DTOVL   $@
       cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
 
@@ -365,17 +354,22 @@ 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
 
+ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
 quiet_cmd_dtb_check =	CHECK   $@
-      cmd_dtb_check =	$(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
+      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)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
+$(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
 	$(call if_changed_rule,dtc)
 
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+	$(call if_changed_dep,dtc)
+
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
 
 # Bzip2
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 1cba78e1dce6..4d32b9497da9 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -17,20 +17,7 @@ fdtoverlay-objs	:= $(libfdt) fdtoverlay.o util.o
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
-
-ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
-ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
-$(error dtc needs libyaml for DT schema validation support. \
-	Install the necessary libyaml development package.)
-endif
 HOST_EXTRACFLAGS += -DNO_YAML
-else
-dtc-objs	+= yamltree.o
-# To include <yaml.h> installed in a non-default path
-HOSTCFLAGS_yamltree.o := $(shell pkg-config --cflags yaml-0.1)
-# To link libyaml installed in a non-default path
-HOSTLDLIBS_dtc	:= $(shell pkg-config --libs yaml-0.1)
-endif
 
 # Generated files need one more search path to include headers in source tree
 HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
index 32ff17ffd089..94627541533e 100755
--- a/scripts/dtc/update-dtc-source.sh
+++ b/scripts/dtc/update-dtc-source.sh
@@ -32,7 +32,7 @@ DTC_UPSTREAM_PATH=`pwd`/../dtc
 DTC_LINUX_PATH=`pwd`/scripts/dtc
 
 DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c \
-		srcpos.h treesource.c util.c util.h version_gen.h yamltree.c \
+		srcpos.h treesource.c util.c util.h version_gen.h \
 		dtc-lexer.l dtc-parser.y"
 LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
 		fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-03 22:42 ` [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate Rob Herring
@ 2022-03-04  9:32   ` Geert Uytterhoeven
  2022-03-04 13:57     ` Rob Herring
  2022-03-04 11:48   ` Laurent Pinchart
  1 sibling, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2022-03-04  9:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

Hi Rob,

On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> In preparation for supporting validation of DTB files, the full
> processed schema will always be needed in order to extract type
> information from it. Therefore, the processed schema containing only
> what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> gained an option, -l or --limit, to specify which schema(s) to use for
> validation.
>
> As the command line option is new, we the minimum dtschema version must be
> updated.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Rob Herring <robh@kernel.org>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
>  DT_SCHEMA_LINT := $(shell which yamllint || \
>    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
>
> -DT_SCHEMA_MIN_VERSION = 2021.2.1
> +DT_SCHEMA_MIN_VERSION = 2022.3

This doesn't work as-is, as that version hasn't been tagged yet ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-03 22:42 ` [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate Rob Herring
  2022-03-04  9:32   ` Geert Uytterhoeven
@ 2022-03-04 11:48   ` Laurent Pinchart
  2022-03-04 14:03     ` Rob Herring
  1 sibling, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2022-03-04 11:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Geert Uytterhoeven, Maxime Ripard,
	linux-kernel, devicetree, linux-kbuild

On Thu, Mar 03, 2022 at 04:42:36PM -0600, Rob Herring wrote:
> In preparation for supporting validation of DTB files, the full
> processed schema will always be needed in order to extract type
> information from it. Therefore, the processed schema containing only
> what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> gained an option, -l or --limit, to specify which schema(s) to use for
> validation.
> 
> As the command line option is new, we the minimum dtschema version must be
> updated.
> 
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/Makefile | 28 +++-------------------
>  scripts/Makefile.lib                       |  3 +--
>  2 files changed, 4 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 61ec18ecc931..246ba0ecab64 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
>  DT_SCHEMA_LINT := $(shell which yamllint || \
>    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
>  
> -DT_SCHEMA_MIN_VERSION = 2021.2.1
> +DT_SCHEMA_MIN_VERSION = 2022.3
>  
>  PHONY += check_dtschema_version
>  check_dtschema_version:
> @@ -25,9 +25,6 @@ quiet_cmd_extract_ex = DTEX    $@
>  $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
>  	$(call if_changed,extract_ex)
>  
> -# Use full schemas when checking %.example.dts
> -DT_TMP_SCHEMA := $(obj)/processed-schema-examples.json
> -
>  find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
>  		-name 'processed-schema*' ! \
>  		-name '*.example.dt.yaml' \)
> @@ -70,29 +67,10 @@ override DTC_FLAGS := \
>  # Disable undocumented compatible checks until warning free
>  override DT_CHECKER_FLAGS ?=
>  
> -$(obj)/processed-schema-examples.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
> +$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
>  	$(call if_changed_rule,chkdt)
>  
> -ifeq ($(DT_SCHEMA_FILES),)
> -
> -# Unless DT_SCHEMA_FILES is specified, use the full schema for dtbs_check too.
> -# Just copy processed-schema-examples.json
> -
> -$(obj)/processed-schema.json: $(obj)/processed-schema-examples.json FORCE
> -	$(call if_changed,copy)
> -
> -else
> -
> -# If DT_SCHEMA_FILES is specified, use it for processed-schema.json
> -
> -$(obj)/processed-schema.json: DT_MK_SCHEMA_FLAGS := -u
> -$(obj)/processed-schema.json: $(CHK_DT_DOCS) check_dtschema_version FORCE
> -	$(call if_changed,mk_schema)
> -
> -endif
> -
> -always-$(CHECK_DT_BINDING) += processed-schema-examples.json
> -always-$(CHECK_DTBS)       += processed-schema.json
> +always-y += processed-schema.json
>  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
>  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
>  
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 79be57fdd32a..9f1e8442564e 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -361,9 +361,8 @@ $(multi-dtb-y): FORCE
>  $(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
>  
>  DT_CHECKER ?= dt-validate
> -DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),,-m)
> +DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
>  DT_BINDING_DIR := Documentation/devicetree/bindings
> -# DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
>  DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.json

This could now use := instead of ?=

Apart from the fact that 2022.3 hasn't been tagged yet as pointed out by
Geert, this looks fine to me (but I'm no expert in this area).

>  
>  quiet_cmd_dtb_check =	CHECK   $@

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-03 22:42 ` [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation Rob Herring
@ 2022-03-04 11:55   ` Laurent Pinchart
  2022-03-04 14:07     ` Rob Herring
  2022-03-07 12:20   ` Geert Uytterhoeven
  1 sibling, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2022-03-04 11:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand, Geert Uytterhoeven,
	Maxime Ripard, linux-kernel, devicetree, linux-kbuild

Hi Rob,

Thank you for the patch.

On Thu, Mar 03, 2022 at 04:42:37PM -0600, Rob Herring wrote:
> Switch the DT validation to use DTB files directly instead of a DTS to
> YAML conversion.
> 
> The original motivation for supporting validation on DTB files was to
> enable running validation on a running system (e.g. 'dt-validate
> /sys/firmware/fdt') or other cases where the original source DTS is not
> available.
> 
> The YAML format was not without issues. Using DTBs with the schema type
> information solves some of those problems. The YAML format relies on the
> DTS source level information including bracketing of properties, size
> directives, and phandle tags all of which are lost in a DTB file. While
> standardizing the bracketing is a good thing, it does cause a lot of
> extra warnings and churn to fix them.

It does indeed, but it's a bit sad to let that feature go :-S

It's nice to drop the dependency on libyaml though, and the ability to
validate a DTB is nice too, so I think it's worth it.

> Another issue has been signed types are not validated correctly as sign
> information is not propagated to YAML. Using the schema type information
> allows for proper handling of signed types. YAML also can't represent
> the full range of 64-bit integers as numbers are stored as floats by
> most/all parsers.
> 
> The DTB validation works by decoding property values using the type
> information in the schemas themselves. The main corner case this does
> not work for is matrix types where neither dimension is fixed. For
> now, checking the dimensions in these cases are skipped.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/Makefile    |  7 +++----
>  .../devicetree/bindings/writing-schema.rst    | 12 -----------
>  scripts/Makefile.lib                          | 20 +++++++------------
>  scripts/dtc/Makefile                          | 13 ------------
>  scripts/dtc/update-dtc-source.sh              |  2 +-
>  5 files changed, 11 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 246ba0ecab64..b8bd6a8ec43a 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -26,8 +26,7 @@ $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
>  	$(call if_changed,extract_ex)
>  
>  find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
> -		-name 'processed-schema*' ! \
> -		-name '*.example.dt.yaml' \)
> +		-name 'processed-schema*' \)
>  
>  find_cmd = $(find_all_cmd) | grep -F "$(DT_SCHEMA_FILES)"
>  CHK_DT_DOCS := $(shell $(find_cmd))
> @@ -72,9 +71,9 @@ $(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version
>  
>  always-y += processed-schema.json
>  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
> -always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
> +always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
>  
>  # Hack: avoid 'Argument list too long' error for 'make clean'. Remove most of
>  # build artifacts here before they are processed by scripts/Makefile.clean
>  clean-files = $(shell find $(obj) \( -name '*.example.dts' -o \
> -			-name '*.example.dt.yaml' \) -delete 2>/dev/null)
> +			-name '*.example.dtb' \) -delete 2>/dev/null)
> diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst
> index 3b00fe981494..b4258bf81be5 100644
> --- a/Documentation/devicetree/bindings/writing-schema.rst
> +++ b/Documentation/devicetree/bindings/writing-schema.rst
> @@ -123,18 +123,6 @@ project can be installed with pip::
>  Several executables (dt-doc-validate, dt-mk-schema, dt-validate) will be
>  installed. Ensure they are in your PATH (~/.local/bin by default).
>  
> -dtc must also be built with YAML output support enabled. This requires that
> -libyaml and its headers be installed on the host system. For some distributions
> -that involves installing the development package, such as:
> -
> -Debian::
> -
> -  apt-get install libyaml-dev
> -
> -Fedora::
> -
> -  dnf -y install libyaml-devel
> -
>  Running checks
>  ~~~~~~~~~~~~~~
>  
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 9f1e8442564e..4629af60160b 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -87,11 +87,6 @@ base-dtb-y := $(foreach m, $(multi-dtb-y), $(firstword $(call suffix-search, $m,
>  
>  always-y			+= $(dtb-y)
>  
> -ifneq ($(CHECK_DTBS),)
> -always-y += $(patsubst %.dtb,%.dt.yaml, $(real-dtb-y))
> -always-y += $(patsubst %.dtbo,%.dt.yaml, $(real-dtb-y))
> -endif
> -
>  # Add subdir path
>  
>  extra-y		:= $(addprefix $(obj)/,$(extra-y))
> @@ -347,12 +342,6 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
>  		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
>  	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>  
> -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> -	$(call if_changed_dep,dtc)
> -
> -$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> -	$(call if_changed_dep,dtc)
> -
>  quiet_cmd_fdtoverlay = DTOVL   $@
>        cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
>  
> @@ -365,17 +354,22 @@ 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
>  
> +ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
>  quiet_cmd_dtb_check =	CHECK   $@
> -      cmd_dtb_check =	$(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
> +      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)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
>  	$(call if_changed_rule,dtc)
>  
> +$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> +	$(call if_changed_dep,dtc)
> +
>  dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
>  
>  # Bzip2
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 1cba78e1dce6..4d32b9497da9 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -17,20 +17,7 @@ fdtoverlay-objs	:= $(libfdt) fdtoverlay.o util.o
>  
>  # Source files need to get at the userspace version of libfdt_env.h to compile
>  HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
> -
> -ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
> -ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
> -$(error dtc needs libyaml for DT schema validation support. \
> -	Install the necessary libyaml development package.)
> -endif
>  HOST_EXTRACFLAGS += -DNO_YAML
> -else
> -dtc-objs	+= yamltree.o
> -# To include <yaml.h> installed in a non-default path
> -HOSTCFLAGS_yamltree.o := $(shell pkg-config --cflags yaml-0.1)
> -# To link libyaml installed in a non-default path
> -HOSTLDLIBS_dtc	:= $(shell pkg-config --libs yaml-0.1)
> -endif
>  
>  # Generated files need one more search path to include headers in source tree
>  HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
> diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
> index 32ff17ffd089..94627541533e 100755
> --- a/scripts/dtc/update-dtc-source.sh
> +++ b/scripts/dtc/update-dtc-source.sh
> @@ -32,7 +32,7 @@ DTC_UPSTREAM_PATH=`pwd`/../dtc
>  DTC_LINUX_PATH=`pwd`/scripts/dtc
>  
>  DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c \
> -		srcpos.h treesource.c util.c util.h version_gen.h yamltree.c \
> +		srcpos.h treesource.c util.c util.h version_gen.h \
>  		dtc-lexer.l dtc-parser.y"
>  LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
>  		fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04  9:32   ` Geert Uytterhoeven
@ 2022-03-04 13:57     ` Rob Herring
  2022-03-04 14:05       ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2022-03-04 13:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

On Fri, Mar 04, 2022 at 10:32:29AM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > In preparation for supporting validation of DTB files, the full
> > processed schema will always be needed in order to extract type
> > information from it. Therefore, the processed schema containing only
> > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > gained an option, -l or --limit, to specify which schema(s) to use for
> > validation.
> >
> > As the command line option is new, we the minimum dtschema version must be
> > updated.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/Makefile
> > +++ b/Documentation/devicetree/bindings/Makefile
> > @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
> >  DT_SCHEMA_LINT := $(shell which yamllint || \
> >    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
> >
> > -DT_SCHEMA_MIN_VERSION = 2021.2.1
> > +DT_SCHEMA_MIN_VERSION = 2022.3
> 
> This doesn't work as-is, as that version hasn't been tagged yet ;-)

I had to make sure people are paying attention. You win the prize. :)

It's there now.

Rob

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04 11:48   ` Laurent Pinchart
@ 2022-03-04 14:03     ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-04 14:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Geert Uytterhoeven, Maxime Ripard,
	linux-kernel, devicetree, linux-kbuild

On Fri, Mar 04, 2022 at 01:48:37PM +0200, Laurent Pinchart wrote:
> On Thu, Mar 03, 2022 at 04:42:36PM -0600, Rob Herring wrote:
> > In preparation for supporting validation of DTB files, the full
> > processed schema will always be needed in order to extract type
> > information from it. Therefore, the processed schema containing only
> > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > gained an option, -l or --limit, to specify which schema(s) to use for
> > validation.
> > 
> > As the command line option is new, we the minimum dtschema version must be
> > updated.
> > 
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  Documentation/devicetree/bindings/Makefile | 28 +++-------------------
> >  scripts/Makefile.lib                       |  3 +--
> >  2 files changed, 4 insertions(+), 27 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> > index 61ec18ecc931..246ba0ecab64 100644
> > --- a/Documentation/devicetree/bindings/Makefile
> > +++ b/Documentation/devicetree/bindings/Makefile
> > @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
> >  DT_SCHEMA_LINT := $(shell which yamllint || \
> >    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
> >  
> > -DT_SCHEMA_MIN_VERSION = 2021.2.1
> > +DT_SCHEMA_MIN_VERSION = 2022.3
> >  
> >  PHONY += check_dtschema_version
> >  check_dtschema_version:
> > @@ -25,9 +25,6 @@ quiet_cmd_extract_ex = DTEX    $@
> >  $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
> >  	$(call if_changed,extract_ex)
> >  
> > -# Use full schemas when checking %.example.dts
> > -DT_TMP_SCHEMA := $(obj)/processed-schema-examples.json
> > -
> >  find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
> >  		-name 'processed-schema*' ! \
> >  		-name '*.example.dt.yaml' \)
> > @@ -70,29 +67,10 @@ override DTC_FLAGS := \
> >  # Disable undocumented compatible checks until warning free
> >  override DT_CHECKER_FLAGS ?=
> >  
> > -$(obj)/processed-schema-examples.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
> > +$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
> >  	$(call if_changed_rule,chkdt)
> >  
> > -ifeq ($(DT_SCHEMA_FILES),)
> > -
> > -# Unless DT_SCHEMA_FILES is specified, use the full schema for dtbs_check too.
> > -# Just copy processed-schema-examples.json
> > -
> > -$(obj)/processed-schema.json: $(obj)/processed-schema-examples.json FORCE
> > -	$(call if_changed,copy)
> > -
> > -else
> > -
> > -# If DT_SCHEMA_FILES is specified, use it for processed-schema.json
> > -
> > -$(obj)/processed-schema.json: DT_MK_SCHEMA_FLAGS := -u
> > -$(obj)/processed-schema.json: $(CHK_DT_DOCS) check_dtschema_version FORCE
> > -	$(call if_changed,mk_schema)
> > -
> > -endif
> > -
> > -always-$(CHECK_DT_BINDING) += processed-schema-examples.json
> > -always-$(CHECK_DTBS)       += processed-schema.json
> > +always-y += processed-schema.json
> >  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
> >  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
> >  
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 79be57fdd32a..9f1e8442564e 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -361,9 +361,8 @@ $(multi-dtb-y): FORCE
> >  $(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
> >  
> >  DT_CHECKER ?= dt-validate
> > -DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),,-m)
> > +DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
> >  DT_BINDING_DIR := Documentation/devicetree/bindings
> > -# DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
> >  DT_TMP_SCHEMA ?= $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
> 
> This could now use := instead of ?=

Yes, though I think it is possible we may still want to override it. 
Other than debugging perhaps, I don't have an immediate reason.

> Apart from the fact that 2022.3 hasn't been tagged yet as pointed out by
> Geert, this looks fine to me (but I'm no expert in this area).

It's now there.

Rob

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04 13:57     ` Rob Herring
@ 2022-03-04 14:05       ` Geert Uytterhoeven
  2022-03-04 14:30         ` Rob Herring
  0 siblings, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2022-03-04 14:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

Hi Rob,

On Fri, Mar 4, 2022 at 2:57 PM Rob Herring <robh@kernel.org> wrote:
> On Fri, Mar 04, 2022 at 10:32:29AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > > In preparation for supporting validation of DTB files, the full
> > > processed schema will always be needed in order to extract type
> > > information from it. Therefore, the processed schema containing only
> > > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > > gained an option, -l or --limit, to specify which schema(s) to use for
> > > validation.
> > >
> > > As the command line option is new, we the minimum dtschema version must be
> > > updated.
> > >
> > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> >
> > Thanks for your patch!
> >
> > > --- a/Documentation/devicetree/bindings/Makefile
> > > +++ b/Documentation/devicetree/bindings/Makefile
> > > @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
> > >  DT_SCHEMA_LINT := $(shell which yamllint || \
> > >    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
> > >
> > > -DT_SCHEMA_MIN_VERSION = 2021.2.1
> > > +DT_SCHEMA_MIN_VERSION = 2022.3
> >
> > This doesn't work as-is, as that version hasn't been tagged yet ;-)
>
> I had to make sure people are paying attention. You win the prize. :)
>
> It's there now.

Thanks, confirmed.

With this series applied, the various salvator-xs DTS files are now
throwing up:

    sata: size (19) error for type phandle
    backlight: size (11) error for type phandle

I tried to find out what caused that, but couldn't find it.
Do you have a clue?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-04 11:55   ` Laurent Pinchart
@ 2022-03-04 14:07     ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-04 14:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand, Geert Uytterhoeven,
	Maxime Ripard, linux-kernel, devicetree, linux-kbuild

On Fri, Mar 04, 2022 at 01:55:59PM +0200, Laurent Pinchart wrote:
> Hi Rob,
> 
> Thank you for the patch.
> 
> On Thu, Mar 03, 2022 at 04:42:37PM -0600, Rob Herring wrote:
> > Switch the DT validation to use DTB files directly instead of a DTS to
> > YAML conversion.
> > 
> > The original motivation for supporting validation on DTB files was to
> > enable running validation on a running system (e.g. 'dt-validate
> > /sys/firmware/fdt') or other cases where the original source DTS is not
> > available.
> > 
> > The YAML format was not without issues. Using DTBs with the schema type
> > information solves some of those problems. The YAML format relies on the
> > DTS source level information including bracketing of properties, size
> > directives, and phandle tags all of which are lost in a DTB file. While
> > standardizing the bracketing is a good thing, it does cause a lot of
> > extra warnings and churn to fix them.
> 
> It does indeed, but it's a bit sad to let that feature go :-S

I agree, but I think that checking is better served with a DTS linter.

Rob

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04 14:05       ` Geert Uytterhoeven
@ 2022-03-04 14:30         ` Rob Herring
  2022-03-04 14:49           ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2022-03-04 14:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

On Fri, Mar 04, 2022 at 03:05:13PM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Fri, Mar 4, 2022 at 2:57 PM Rob Herring <robh@kernel.org> wrote:
> > On Fri, Mar 04, 2022 at 10:32:29AM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > > > In preparation for supporting validation of DTB files, the full
> > > > processed schema will always be needed in order to extract type
> > > > information from it. Therefore, the processed schema containing only
> > > > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > > > gained an option, -l or --limit, to specify which schema(s) to use for
> > > > validation.
> > > >
> > > > As the command line option is new, we the minimum dtschema version must be
> > > > updated.
> > > >
> > > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/Documentation/devicetree/bindings/Makefile
> > > > +++ b/Documentation/devicetree/bindings/Makefile
> > > > @@ -6,7 +6,7 @@ DT_MK_SCHEMA ?= dt-mk-schema
> > > >  DT_SCHEMA_LINT := $(shell which yamllint || \
> > > >    echo "warning: yamllint not installed, skipping. To install, run 'pip install yamllint'" >&2)
> > > >
> > > > -DT_SCHEMA_MIN_VERSION = 2021.2.1
> > > > +DT_SCHEMA_MIN_VERSION = 2022.3
> > >
> > > This doesn't work as-is, as that version hasn't been tagged yet ;-)
> >
> > I had to make sure people are paying attention. You win the prize. :)
> >
> > It's there now.
> 
> Thanks, confirmed.
> 
> With this series applied, the various salvator-xs DTS files are now
> throwing up:
> 
>     sata: size (19) error for type phandle
>     backlight: size (11) error for type phandle

Those come from the code decoding the properties[1]. Unfortunately, I 
haven't come up with a prettier way to report those with the filename. I 
may just remove it because if decoding the property fails, we'll get 
schema errors later on anyways.

But I don't see any 'sata' properties in the DTS files and 'backlight' 
is a node. Are you building with '-@'? I probably need to skip 
__symbols__ nodes. The overlay side is handled because examples are 
built as overlays (to allow unresolved phandles).

Rob

[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/dtb.py#L149

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04 14:30         ` Rob Herring
@ 2022-03-04 14:49           ` Geert Uytterhoeven
  2022-03-10 15:49             ` Rob Herring
  0 siblings, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2022-03-04 14:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

Hi Rob,

On Fri, Mar 4, 2022 at 3:31 PM Rob Herring <robh@kernel.org> wrote:
> On Fri, Mar 04, 2022 at 03:05:13PM +0100, Geert Uytterhoeven wrote:
> > On Fri, Mar 4, 2022 at 2:57 PM Rob Herring <robh@kernel.org> wrote:
> > > On Fri, Mar 04, 2022 at 10:32:29AM +0100, Geert Uytterhoeven wrote:
> > > > On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > > > > In preparation for supporting validation of DTB files, the full
> > > > > processed schema will always be needed in order to extract type
> > > > > information from it. Therefore, the processed schema containing only
> > > > > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > > > > gained an option, -l or --limit, to specify which schema(s) to use for
> > > > > validation.
> > > > >
> > > > > As the command line option is new, we the minimum dtschema version must be
> > > > > updated.
> > > > >
> > > > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > > > Signed-off-by: Rob Herring <robh@kernel.org>

> > With this series applied, the various salvator-xs DTS files are now
> > throwing up:
> >
> >     sata: size (19) error for type phandle
> >     backlight: size (11) error for type phandle
>
> Those come from the code decoding the properties[1]. Unfortunately, I
> haven't come up with a prettier way to report those with the filename. I
> may just remove it because if decoding the property fails, we'll get
> schema errors later on anyways.
>
> But I don't see any 'sata' properties in the DTS files and 'backlight'
> is a node. Are you building with '-@'? I probably need to skip
> __symbols__ nodes. The overlay side is handled because examples are
> built as overlays (to allow unresolved phandles).

Yes I am. Dropping the "-@" fixes the issue.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-03 22:42 ` [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation Rob Herring
  2022-03-04 11:55   ` Laurent Pinchart
@ 2022-03-07 12:20   ` Geert Uytterhoeven
  2022-03-07 17:00     ` Rob Herring
  1 sibling, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2022-03-07 12:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

Hi Rob,

On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> Switch the DT validation to use DTB files directly instead of a DTS to
> YAML conversion.
>
> The original motivation for supporting validation on DTB files was to
> enable running validation on a running system (e.g. 'dt-validate
> /sys/firmware/fdt') or other cases where the original source DTS is not
> available.
>
> The YAML format was not without issues. Using DTBs with the schema type
> information solves some of those problems. The YAML format relies on the
> DTS source level information including bracketing of properties, size
> directives, and phandle tags all of which are lost in a DTB file. While
> standardizing the bracketing is a good thing, it does cause a lot of
> extra warnings and churn to fix them.
>
> Another issue has been signed types are not validated correctly as sign
> information is not propagated to YAML. Using the schema type information
> allows for proper handling of signed types. YAML also can't represent
> the full range of 64-bit integers as numbers are stored as floats by
> most/all parsers.
>
> The DTB validation works by decoding property values using the type
> information in the schemas themselves. The main corner case this does
> not work for is matrix types where neither dimension is fixed. For
> now, checking the dimensions in these cases are skipped.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

Thanks for your patch!

While investigating why a newly added device node to DTS was not
instantiated as a platform device, I discovered an issue with this
patch: "make dtbs" no longer rebuilds DTB files that need a rebuild.

How to reproduce:

    $ git checkout next-20220307
    # apply this series and its dependency:
    # dt-bindings: kbuild: Support partial matches with DT_SCHEMA_FILES
    # dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
    # dt-bindings: kbuild: Use DTB files for validation
    $ make ARCH=arm shmobile_defconfig
    $ make ARCH=arm dtbs
    $ touch arch/arm/boot/dts/r8a7791.dtsi
    $ make ARCH=arm dtbs
    # The above command does NOT cause:
    # DTC     arch/arm/boot/dts/r8a7791-koelsch.dtb
    # DTC     arch/arm/boot/dts/r8a7791-porter.dtb

I don't see anything wrong with this patch at first sight, though.

> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -26,8 +26,7 @@ $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
>         $(call if_changed,extract_ex)
>
>  find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
> -               -name 'processed-schema*' ! \
> -               -name '*.example.dt.yaml' \)
> +               -name 'processed-schema*' \)
>
>  find_cmd = $(find_all_cmd) | grep -F "$(DT_SCHEMA_FILES)"
>  CHK_DT_DOCS := $(shell $(find_cmd))
> @@ -72,9 +71,9 @@ $(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version
>
>  always-y += processed-schema.json
>  always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
> -always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dt.yaml, $(CHK_DT_DOCS))
> +always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
>
>  # Hack: avoid 'Argument list too long' error for 'make clean'. Remove most of
>  # build artifacts here before they are processed by scripts/Makefile.clean
>  clean-files = $(shell find $(obj) \( -name '*.example.dts' -o \
> -                       -name '*.example.dt.yaml' \) -delete 2>/dev/null)
> +                       -name '*.example.dtb' \) -delete 2>/dev/null)
> diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst
> index 3b00fe981494..b4258bf81be5 100644
> --- a/Documentation/devicetree/bindings/writing-schema.rst
> +++ b/Documentation/devicetree/bindings/writing-schema.rst
> @@ -123,18 +123,6 @@ project can be installed with pip::
>  Several executables (dt-doc-validate, dt-mk-schema, dt-validate) will be
>  installed. Ensure they are in your PATH (~/.local/bin by default).
>
> -dtc must also be built with YAML output support enabled. This requires that
> -libyaml and its headers be installed on the host system. For some distributions
> -that involves installing the development package, such as:
> -
> -Debian::
> -
> -  apt-get install libyaml-dev
> -
> -Fedora::
> -
> -  dnf -y install libyaml-devel
> -
>  Running checks
>  ~~~~~~~~~~~~~~
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 9f1e8442564e..4629af60160b 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -87,11 +87,6 @@ base-dtb-y := $(foreach m, $(multi-dtb-y), $(firstword $(call suffix-search, $m,
>
>  always-y                       += $(dtb-y)
>
> -ifneq ($(CHECK_DTBS),)
> -always-y += $(patsubst %.dtb,%.dt.yaml, $(real-dtb-y))
> -always-y += $(patsubst %.dtbo,%.dt.yaml, $(real-dtb-y))
> -endif
> -
>  # Add subdir path
>
>  extra-y                := $(addprefix $(obj)/,$(extra-y))
> @@ -347,12 +342,6 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
>                 -d $(depfile).dtc.tmp $(dtc-tmp) ; \
>         cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
>
> -$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> -       $(call if_changed_dep,dtc)
> -
> -$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> -       $(call if_changed_dep,dtc)
> -
>  quiet_cmd_fdtoverlay = DTOVL   $@
>        cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
>
> @@ -365,17 +354,22 @@ 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
>
> +ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
>  quiet_cmd_dtb_check =  CHECK   $@
> -      cmd_dtb_check =  $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@
> +      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)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
>         $(call if_changed_rule,dtc)
>
> +$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> +       $(call if_changed_dep,dtc)
> +
>  dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
>
>  # Bzip2
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 1cba78e1dce6..4d32b9497da9 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -17,20 +17,7 @@ fdtoverlay-objs      := $(libfdt) fdtoverlay.o util.o
>
>  # Source files need to get at the userspace version of libfdt_env.h to compile
>  HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
> -
> -ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
> -ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
> -$(error dtc needs libyaml for DT schema validation support. \
> -       Install the necessary libyaml development package.)
> -endif
>  HOST_EXTRACFLAGS += -DNO_YAML
> -else
> -dtc-objs       += yamltree.o
> -# To include <yaml.h> installed in a non-default path
> -HOSTCFLAGS_yamltree.o := $(shell pkg-config --cflags yaml-0.1)
> -# To link libyaml installed in a non-default path
> -HOSTLDLIBS_dtc := $(shell pkg-config --libs yaml-0.1)
> -endif
>
>  # Generated files need one more search path to include headers in source tree
>  HOSTCFLAGS_dtc-lexer.lex.o := -I $(srctree)/$(src)
> diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
> index 32ff17ffd089..94627541533e 100755
> --- a/scripts/dtc/update-dtc-source.sh
> +++ b/scripts/dtc/update-dtc-source.sh
> @@ -32,7 +32,7 @@ DTC_UPSTREAM_PATH=`pwd`/../dtc
>  DTC_LINUX_PATH=`pwd`/scripts/dtc
>
>  DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c \
> -               srcpos.h treesource.c util.c util.h version_gen.h yamltree.c \
> +               srcpos.h treesource.c util.c util.h version_gen.h \
>                 dtc-lexer.l dtc-parser.y"
>  LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
>                 fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \
> --
> 2.32.0

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-07 12:20   ` Geert Uytterhoeven
@ 2022-03-07 17:00     ` Rob Herring
  2022-03-08  8:37       ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2022-03-07 17:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

On Mon, Mar 07, 2022 at 01:20:29PM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > Switch the DT validation to use DTB files directly instead of a DTS to
> > YAML conversion.
> >
> > The original motivation for supporting validation on DTB files was to
> > enable running validation on a running system (e.g. 'dt-validate
> > /sys/firmware/fdt') or other cases where the original source DTS is not
> > available.
> >
> > The YAML format was not without issues. Using DTBs with the schema type
> > information solves some of those problems. The YAML format relies on the
> > DTS source level information including bracketing of properties, size
> > directives, and phandle tags all of which are lost in a DTB file. While
> > standardizing the bracketing is a good thing, it does cause a lot of
> > extra warnings and churn to fix them.
> >
> > Another issue has been signed types are not validated correctly as sign
> > information is not propagated to YAML. Using the schema type information
> > allows for proper handling of signed types. YAML also can't represent
> > the full range of 64-bit integers as numbers are stored as floats by
> > most/all parsers.
> >
> > The DTB validation works by decoding property values using the type
> > information in the schemas themselves. The main corner case this does
> > not work for is matrix types where neither dimension is fixed. For
> > now, checking the dimensions in these cases are skipped.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> Thanks for your patch!
> 
> While investigating why a newly added device node to DTS was not
> instantiated as a platform device, I discovered an issue with this
> patch: "make dtbs" no longer rebuilds DTB files that need a rebuild.
> 
> How to reproduce:
> 
>     $ git checkout next-20220307
>     # apply this series and its dependency:
>     # dt-bindings: kbuild: Support partial matches with DT_SCHEMA_FILES
>     # dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
>     # dt-bindings: kbuild: Use DTB files for validation
>     $ make ARCH=arm shmobile_defconfig
>     $ make ARCH=arm dtbs
>     $ touch arch/arm/boot/dts/r8a7791.dtsi
>     $ make ARCH=arm dtbs
>     # The above command does NOT cause:
>     # DTC     arch/arm/boot/dts/r8a7791-koelsch.dtb
>     # DTC     arch/arm/boot/dts/r8a7791-porter.dtb
> 
> I don't see anything wrong with this patch at first sight, though.

Was this a clean tree?

I think I reproduced it, but then couldn't... But then after a 'make 
clean', 'make dtbs' would error out. I think the issue in both cases was 
processed-schema.json always a dependency when it should be conditional 
on 'dtbs_check'. The patch below fixes that. Can you give it a try too.


diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 4629af60160b..9d5320a47ef8 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -349,12 +349,12 @@ $(multi-dtb-y): FORCE
        $(call if_changed,fdtoverlay)
 $(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
 
+ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
 DT_CHECKER ?= dt-validate
 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
+DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
 
-ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
 quiet_cmd_dtb_check =  CHECK   $@
       cmd_dtb_check =  $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true
 endif

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
  2022-03-07 17:00     ` Rob Herring
@ 2022-03-08  8:37       ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2022-03-08  8:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Frank Rowand, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

Hi Rob,

On Mon, Mar 7, 2022 at 6:00 PM Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 07, 2022 at 01:20:29PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > > Switch the DT validation to use DTB files directly instead of a DTS to
> > > YAML conversion.
> > >
> > > The original motivation for supporting validation on DTB files was to
> > > enable running validation on a running system (e.g. 'dt-validate
> > > /sys/firmware/fdt') or other cases where the original source DTS is not
> > > available.
> > >
> > > The YAML format was not without issues. Using DTBs with the schema type
> > > information solves some of those problems. The YAML format relies on the
> > > DTS source level information including bracketing of properties, size
> > > directives, and phandle tags all of which are lost in a DTB file. While
> > > standardizing the bracketing is a good thing, it does cause a lot of
> > > extra warnings and churn to fix them.
> > >
> > > Another issue has been signed types are not validated correctly as sign
> > > information is not propagated to YAML. Using the schema type information
> > > allows for proper handling of signed types. YAML also can't represent
> > > the full range of 64-bit integers as numbers are stored as floats by
> > > most/all parsers.
> > >
> > > The DTB validation works by decoding property values using the type
> > > information in the schemas themselves. The main corner case this does
> > > not work for is matrix types where neither dimension is fixed. For
> > > now, checking the dimensions in these cases are skipped.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> >
> > Thanks for your patch!
> >
> > While investigating why a newly added device node to DTS was not
> > instantiated as a platform device, I discovered an issue with this
> > patch: "make dtbs" no longer rebuilds DTB files that need a rebuild.
> >
> > How to reproduce:
> >
> >     $ git checkout next-20220307
> >     # apply this series and its dependency:
> >     # dt-bindings: kbuild: Support partial matches with DT_SCHEMA_FILES
> >     # dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
> >     # dt-bindings: kbuild: Use DTB files for validation
> >     $ make ARCH=arm shmobile_defconfig
> >     $ make ARCH=arm dtbs
> >     $ touch arch/arm/boot/dts/r8a7791.dtsi
> >     $ make ARCH=arm dtbs
> >     # The above command does NOT cause:
> >     # DTC     arch/arm/boot/dts/r8a7791-koelsch.dtb
> >     # DTC     arch/arm/boot/dts/r8a7791-porter.dtb
> >
> > I don't see anything wrong with this patch at first sight, though.
>
> Was this a clean tree?

The above was a clean tree.

> I think I reproduced it, but then couldn't... But then after a 'make
> clean', 'make dtbs' would error out. I think the issue in both cases was
> processed-schema.json always a dependency when it should be conditional
> on 'dtbs_check'. The patch below fixes that. Can you give it a try too.

I first saw the issue in my normal work tree.
However, I cannot reproduce it in that tree anymore :-(

I can reproduce it in the clean tree after following the instructions
above, and your patch fixes that, so
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -349,12 +349,12 @@ $(multi-dtb-y): FORCE
>         $(call if_changed,fdtoverlay)
>  $(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
>
> +ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
>  DT_CHECKER ?= dt-validate
>  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
> +DT_TMP_SCHEMA := $(objtree)/$(DT_BINDING_DIR)/processed-schema.json
>
> -ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
>  quiet_cmd_dtb_check =  CHECK   $@
>        cmd_dtb_check =  $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) -p $(DT_TMP_SCHEMA) $@ || true
>  endif

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate
  2022-03-04 14:49           ` Geert Uytterhoeven
@ 2022-03-10 15:49             ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2022-03-10 15:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Masahiro Yamada, Krzysztof Kozlowski, Michal Marek,
	Nick Desaulniers, Laurent Pinchart, Maxime Ripard,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kbuild

On Fri, Mar 04, 2022 at 03:49:20PM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Fri, Mar 4, 2022 at 3:31 PM Rob Herring <robh@kernel.org> wrote:
> > On Fri, Mar 04, 2022 at 03:05:13PM +0100, Geert Uytterhoeven wrote:
> > > On Fri, Mar 4, 2022 at 2:57 PM Rob Herring <robh@kernel.org> wrote:
> > > > On Fri, Mar 04, 2022 at 10:32:29AM +0100, Geert Uytterhoeven wrote:
> > > > > On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@kernel.org> wrote:
> > > > > > In preparation for supporting validation of DTB files, the full
> > > > > > processed schema will always be needed in order to extract type
> > > > > > information from it. Therefore, the processed schema containing only
> > > > > > what DT_SCHEMA_FILES specifies won't work. Instead, dt-validate has
> > > > > > gained an option, -l or --limit, to specify which schema(s) to use for
> > > > > > validation.
> > > > > >
> > > > > > As the command line option is new, we the minimum dtschema version must be
> > > > > > updated.
> > > > > >
> > > > > > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> > > With this series applied, the various salvator-xs DTS files are now
> > > throwing up:
> > >
> > >     sata: size (19) error for type phandle
> > >     backlight: size (11) error for type phandle
> >
> > Those come from the code decoding the properties[1]. Unfortunately, I
> > haven't come up with a prettier way to report those with the filename. I
> > may just remove it because if decoding the property fails, we'll get
> > schema errors later on anyways.
> >
> > But I don't see any 'sata' properties in the DTS files and 'backlight'
> > is a node. Are you building with '-@'? I probably need to skip
> > __symbols__ nodes. The overlay side is handled because examples are
> > built as overlays (to allow unresolved phandles).
> 
> Yes I am. Dropping the "-@" fixes the issue.

FYI, now fixed in v2022.03.1.

Rob


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-03-10 15:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 22:42 [PATCH 0/2] dt-bindings: DTB based validation Rob Herring
2022-03-03 22:42 ` [PATCH 1/2] dt-bindings: kbuild: Pass DT_SCHEMA_FILES to dt-validate Rob Herring
2022-03-04  9:32   ` Geert Uytterhoeven
2022-03-04 13:57     ` Rob Herring
2022-03-04 14:05       ` Geert Uytterhoeven
2022-03-04 14:30         ` Rob Herring
2022-03-04 14:49           ` Geert Uytterhoeven
2022-03-10 15:49             ` Rob Herring
2022-03-04 11:48   ` Laurent Pinchart
2022-03-04 14:03     ` Rob Herring
2022-03-03 22:42 ` [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation Rob Herring
2022-03-04 11:55   ` Laurent Pinchart
2022-03-04 14:07     ` Rob Herring
2022-03-07 12:20   ` Geert Uytterhoeven
2022-03-07 17:00     ` Rob Herring
2022-03-08  8:37       ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).