Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation

From: Geert Uytterhoeven
Date: Mon Mar 07 2022 - 07:20:50 EST

Next message: Tim Chang: "[PATCH 0/4] cpufreq: mediatek: introduce mtk cpufreq"
Previous message: Hao Xiang: "Re: [PATCH v4 1/2] x86/arch_prctl: Fix the ARCH_REQ_XCOMP_PERM implementation"
In reply to: Rob Herring: "Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation"
Next in thread: Rob Herring: "Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Rob,

On Thu, Mar 3, 2022 at 11:43 PM Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxx>

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

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

Next message: Tim Chang: "[PATCH 0/4] cpufreq: mediatek: introduce mtk cpufreq"
Previous message: Hao Xiang: "Re: [PATCH v4 1/2] x86/arch_prctl: Fix the ARCH_REQ_XCOMP_PERM implementation"
In reply to: Rob Herring: "Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation"
Next in thread: Rob Herring: "Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]