linux-kernel - Re: [PATCH v4] kbuild: Add support for DT binding schema checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_Jsq+5RpZ2WmxnFz-wa=NTGQ2=6MqjdysmXH8PTrRfke8-=A@mail.gmail.com>
Date:   Mon, 28 Jan 2019 09:49:04 -0600
From:   Rob Herring <robh@...nel.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Sean Hudson <darknighte@...knighte.com>,
        Frank Rowand <frowand.list@...il.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Grant Likely <grant.likely@....com>,
        Kumar Gala <kumar.gala@...aro.org>, arm-soc <arm@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Mark Rutland <mark.rutland@....com>,
        Michal Marek <michal.lkml@...kovi.net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        linux-kbuild <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH v4] kbuild: Add support for DT binding schema checks

On Mon, Jan 28, 2019 at 2:59 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> Hi Rob,
>
> On Sun, Jan 27, 2019 at 4:00 AM Rob Herring <robh@...nel.org> wrote:
> > On Wed, Jan 23, 2019 at 9:33 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > > On Tue, Dec 11, 2018 at 9:24 PM Rob Herring <robh@...nel.org> wrote:
> > > > This adds the build infrastructure for checking DT binding schema
> > > > documents and validating dts files using the binding schema.
> > > >
> > > > Check DT binding schema documents:
> > > > make dt_binding_check
> > > >
> > > > Build dts files and check using DT binding schema:
> > > > make dtbs_check
> > > >
> > > > Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to
> > > > use for validation. This makes it easier to find and fix errors
> > > > generated by a specific schema.
> > > >
> > > > Currently, the validation targets are separate from a normal build to
> > > > avoid a hard dependency on the external DT schema project and because
> > > > there are lots of warnings generated.
> > >
> > > Thanks, I'm giving this a try, and get errors like:
> > >
> > >       DTC     arch/arm/boot/dts/emev2-kzm9d.dt.yaml
> > >     FATAL ERROR: No markers present in property 'cpu0' value
> > >
> > > and
> > >
> > >       DTC     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
> > >     FATAL ERROR: No markers present in property 'audio_clk_a' value
> > >
> > > Do you have a clue?
> >
> > That's really strange because those aren't even properties. Are other
> > dts files okay? This is the in tree dtc?
> >
> > The only time you should be missing markers is if you did a dts -> dts
> > -> dt.yaml.
>
> Found it: make dtbs_check doesn't play well with my local change to
> add symbols for DT overlays:

Now that makes sense.

>     --- a/scripts/Makefile.lib
>     +++ b/scripts/Makefile.lib
>     @@ -285,6 +285,10 @@ cmd_dt_S_dtb=
>              \
>      $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>             $(call if_changed,dt_S_dtb)
>
>     +ifeq ($(CONFIG_OF_OVERLAY),y)
>     +DTC_FLAGS += -@
>     +endif
>     +
>      quiet_cmd_dtc = DTC     $@
>      cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>             $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o
> $(dtc-tmp) $< ; \
>
> Do you see a way to handle that better?

We need to have the code that generates these properties to also add
markers. Or we could drop the __symbols__ nodes on YAML output. Or
ignore the option when doing YAML output.

> Apart from a few expected issues, I'm seeing one other strange message:
>
>     arch/arm/boot/dts/sh73a0-kzm9g.dt.yaml: interrupts: [[2, 4], [3,
> 4]] is too long
>
> This is the interrupts property in the adi,adxl345 node in
> arch/arm/boot/dts/sh73a0-kzm9g.dts.
> Apparently the check complains if more than one interrupt is listed here.
> Is this a known issue?

There are lots of warnings... I've gone thru and checked some to make
sure they are valid, but certainly not all. There's probably some
cases that are too strict too.

This one is because this device is listed in trivial-devices.yaml and
you can't have 2 interrupts for a trivial device (because you need to
define the interrupt order). Looks like we have a binding doc for it
too, so we should just remove it from trivial-devices.yaml.

There are lots of '... is too (long|short)' messages because of how
the dts file property values are bracketed. This used to not matter,
but is significant in the YAML output. I have a dtc patch to give
warnings on all these (and dtc will give source location).

Rob