Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 7 Mar 2022 11:00:28 -0600
From:   Rob Herring <robh@kernel.org>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Masahiro Yamada <masahiroy@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Michal Marek <michal.lkml@markovi.net>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Frank Rowand <frowand.list@gmail.com>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Maxime Ripard <maxime@cerno.tech>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        linux-kbuild <linux-kbuild@vger.kernel.org>
Subject: Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
Message-ID: <YiY6LNfEGhz1hSYg@robh.at.kernel.org>
References: <20220303224237.2497570-1-robh@kernel.org>
 <20220303224237.2497570-3-robh@kernel.org>
 <CAMuHMdUzu2PMN7ZerX=D0KsiqKZFDW5dVe3BY77wR=H-grrPVA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMuHMdUzu2PMN7ZerX=D0KsiqKZFDW5dVe3BY77wR=H-grrPVA@mail.gmail.com>
Precedence: bulk

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