Re: [PATCH v6 01/12] dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

Rob Herring
2020-04-21T19:03:12Z

On Fri, Apr 17, 2020 at 7:16 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Rob,
>
> > Am 16.04.2020 um 22:41 schrieb Rob Herring <robh@kernel.org>:
> >
> > On Wed, 15 Apr 2020 10:35:08 +0200, "H. Nikolaus Schaller" wrote:
> >> The Imagination PVR/SGX GPU is part of several SoC from
> >> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo,
> >> Allwinner A83 and others.
> >>
> >> With this binding, we describe how the SGX processor is
> >> interfaced to the SoC (registers, interrupt etc.).
> >>
> >> In most cases, Clock, Reset and power management is handled
> >> by a parent node or elsewhere (e.g. code in the driver).
> >>
> >> Tested by make dt_binding_check dtbs_check
> >>
> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >> ---
> >> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 122 ++++++++++++++++++
> >> 1 file changed, 122 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
> >>
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml: while parsing a block mapping
> > in "<unicode string>", line 74, column 13
>
> It turned out that there was a stray " in line 74 from the last editing phase.
> Will be fixed in v7.
>
> Would it be possible to make dt_binding_check not only report line/column but the
> contents of the line instead of "<unicode string>"?

This comes from ruamel.yaml module. I believe "<unicode string>" is
supposed to be the type of the data, not what it is. However, it is
possible to get something a bit more helpful, but it depends on which
parser is used. By default we use the C based parser (aka 'safe'). If
we use the round trip parser, we get this:

ruamel.yaml.scanner.ScannerError: while scanning a simple key
in "<unicode string>", line 84, column 5:
maxItems
^ (line: 84)

This can be enabled by passing '-n' (line numbers) to dt-doc-validate.
Currently, you have have to edit the Makefile to do this. The C parser
is a big difference in speed, so I don't want to change the default.

I can probably work around this and dump the erroring line, but I'm
not sure that's always useful. Many errors are indentation related and
those need some context. Plus just dumping the line can be done easily
with sed:

sed -n ${LINE}p <file>

Rob