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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

On Mon, Mar 7, 2022 at 6:00 PM Rob Herring <robh@xxxxxxxxxx> 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@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.
>
> 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@xxxxxxxxx>

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux