Re: [PATCH v1 32/36] dt-bindings: display: convert sharp,ls037v7dw01 to DT Schema

From: Rob Herring <robh@kernel.org>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	devicetree@vger.kernel.org,
	Alexandre Courbot <acourbot@nvidia.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Brian Masney <masneyb@onstation.org>,
	Chris Zhong <zyw@rock-chips.com>,
	Douglas Anderson <dianders@chromium.org>,
	Guido Gunther <agx@sigxcpu.org>, Heiko Schocher <hs@denx.de>,
	Nikolaus Schaller <hns@goldelico.com>,
	Hoegeun Kwon <hoegeun.kwon@samsung.com>,
	Jagan Teki <jagan@amarulasolutions.com>,
	Jerry Han <hanxu5@huaqin.corp-partner.google.com>,
	Jonathan Bakker <xc-racer2@live.ca>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Lin Huang <hl@rock-chips.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	Marco Franchi <marco.franchi@nxp.com>,
	Marek Belisko <marek@goldelico.com>,
	Mark Brown <broonie@kernel.org>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Maxime Ripard <mripard@kernel.org>,
	Nickey Yang <nickey.yang@rock-chips.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Peter Rosin <peda@axentia.se>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Purism Kernel Team <kernel@puri.sm>,
	Robert Chiras <robert.chiras@nxp.com>,
	Sandeep Panda <spanda@codeaurora.org>,
	Stefan Mavrodiev <stefan@olimex.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Tony Lindgren <tony@atomide.com>,
	Vinay Simha BN <simhavcs@gmail.com>,
	Werner Johansson <werner.johansson@sonymobile.com>
Subject: Re: [PATCH v1 32/36] dt-bindings: display: convert sharp,ls037v7dw01 to DT Schema
Date: Tue, 31 Mar 2020 14:57:29 -0600	[thread overview]
Message-ID: <CAL_Jsq+S1yjX3aH7jYiMGUsgvwCju2KgCvn57Pv9DYZ4LfHa7Q@mail.gmail.com> (raw)
In-Reply-To: <20200331191353.GA14267@ravnborg.org>

On Tue, Mar 31, 2020 at 1:14 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Tue, Mar 31, 2020 at 11:20:13AM -0600, Rob Herring wrote:
> > On Sun, Mar 29, 2020 at 1:04 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > >
> > > Hi Rob.
> > >
> > > > > +
> > > > > +  mode-gpios:
> > > > > +    description: |
> > > > > +      GPIO ordered MO, LR, and UD as specified in LS037V7DW01.pdf
> > > >
> > > > 3 or...
> > > >
> > > > > +      change configuration between QVGA and VGA mode and the
> > > > > +      scan direction. As these pins can be also configured
> > > > > +      with external pulls, all the GPIOs are considered
> > > > > +      optional with holes in the array.
> > > >
> > > > minItems: 3
> > > > maxItems: 5
> > >
> > > This binding can specify up to three GPIOs like this:
> >
> > So it should be:
> >
> > minItems: 1
> > maxItems: 3
> >
> > > > > +        mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH        /* gpio154, lcd MO */
> > > > > +                      &gpio1 2 GPIO_ACTIVE_HIGH         /* gpio2, lcd LR */
> > > > > +                      &gpio1 3 GPIO_ACTIVE_HIGH>;       /* gpio3, lcd UD */
> > >
> > > They are in the linux kernel driver accessed like this:
> > >
> > >     devm_gpiod_get_index(&pdev->dev, "mode", 2, GPIOD_OUT_LOW);
> > >
> > > The following is OK in the DT file:
> > >
> > >     mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>;
> > >
> > >     mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH
> > >                   &gpio1 2 GPIO_ACTIVE_HIGH>;
> > >
> > >     mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH
> > >                   &gpio1 2 GPIO_ACTIVE_HIGH
> > >                   &gpio1 3 GPIO_ACTIVE_HIGH>;
> >
> > With the above, the 2nd 2 should fail...
> >
> > > But the following is not OK:
> > >     mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, <&gpio1 2 GPIO_ACTIVE_HIGH>;
> >
> > And this should pass. We want phandle+arg type properties to be
> > bracketed like this.
>
> OK, so if I get you right you say that we should accept the:
> <phandle+arg>, <phandle+arg> ... syntax.
>
> And then ignore that current DT files uses:
> <phandle+arg phandle+arg>
>
>
> A binding like this:
>  mode-gpios:
>     minItems: 1
>     maxItems: 3
>     description: |
>       GPIO ordered MO, LR, and UD as specified in LS037V7DW01.pdf
>       This panel can have zero to three GPIOs to configure to
>
>
> Do not error out when the example looks like this:
>
>         mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH        /* gpio154, lcd MO */
>                       &gpio1 2 GPIO_ACTIVE_HIGH         /* gpio2, lcd LR */
>                       &gpio1 3 GPIO_ACTIVE_HIGH>;       /* gpio3, lcd UD */

That's because we can't distinguish between this and 1 entry as the
schema doesn't have visibility of what #gpio-cells value is. dtc does
check that the cell sizes are correct. We'll need to somehow combine
that and the schema to check this form correctly.

>
> So if I get you right this is a bug in the tooling.

Limitation I guess. I thought you where saying the bracketed form was
not working.

Rob