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;
MIME-Version: 1.0
References: <20220303224237.2497570-1-robh@kernel.org> <20220303224237.2497570-3-robh@kernel.org>
 <CAMuHMdUzu2PMN7ZerX=D0KsiqKZFDW5dVe3BY77wR=H-grrPVA@mail.gmail.com> <YiY6LNfEGhz1hSYg@robh.at.kernel.org>
In-Reply-To: <YiY6LNfEGhz1hSYg@robh.at.kernel.org>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Tue, 8 Mar 2022 09:37:58 +0100
Message-ID: <CAMuHMdVFexJ=NT4wX4v2s3K--znQCiahskbGtEci3xwgsyEQWA@mail.gmail.com>
Subject: Re: [PATCH 2/2] dt-bindings: kbuild: Use DTB files for validation
To:     Rob Herring <robh@kernel.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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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