[PATCH v3 2/2] dt-bindings: panel: Introduce a panel-lvds binding

[Maxime Ripard]

On Wed, Feb 02, 2022 at 02:47:14PM +0200, Laurent Pinchart wrote:
> On Wed, Feb 02, 2022 at 10:48:45AM +0100, Maxime Ripard wrote:
> > On Thu, Jan 27, 2022 at 03:22:15PM +0100, Maxime Ripard wrote:
> > > On Tue, Jan 11, 2022 at 03:05:10PM +0200, Laurent Pinchart wrote:
> > > > On Tue, Jan 11, 2022 at 12:06:35PM +0100, Maxime Ripard wrote:
> > > > > Following the previous patch, let's introduce a generic panel-lvds
> > > > > binding that documents the panels that don't have any particular
> > > > > constraint documented.
> > > > > 
> > > > > Reviewed-by: Rob Herring <robh at kernel.org>
> > > > > Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> > > > > 
> > > > > ---
> > > > > 
> > > > > Changes from v2:
> > > > >   - Added a MAINTAINERS entry
> > > > > 
> > > > > Changes from v1:
> > > > >   - Added missing compatible
> > > > >   - Fixed lint
> > > > > ---
> > > > >  .../bindings/display/panel/panel-lvds.yaml    | 57 +++++++++++++++++++
> > > > >  MAINTAINERS                                   |  1 +
> > > > >  2 files changed, 58 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..fcc50db6a812
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml
> > > > > @@ -0,0 +1,57 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Generic LVDS Display Panel Device Tree Bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>
> > > > > +  - Thierry Reding <thierry.reding at gmail.com>
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: panel-common.yaml#
> > > > > +  - $ref: /schemas/display/lvds.yaml/#
> > > > > +
> > > > > +select:
> > > > > +  properties:
> > > > > +    compatible:
> > > > > +      contains:
> > > > > +        const: panel-lvds
> > > > > +
> > > > > +  not:
> > > > > +    properties:
> > > > > +      compatible:
> > > > > +        contains:
> > > > > +          enum:
> > > > > +            - advantech,idk-1110wr
> > > > > +            - advantech,idk-2121wr
> > > > > +            - innolux,ee101ia-01d
> > > > > +            - mitsubishi,aa104xd12
> > > > > +            - mitsubishi,aa121td01
> > > > > +            - sgd,gktw70sdae4se
> > > > 
> > > > I still don't like this :-( Couldn't we instead do
> > > > 
> > > > select:
> > > >   properties:
> > > >     compatible:
> > > >       contains:
> > > >         enum:
> > > >           - auo,b101ew05
> > > >           - tbs,a711-panel
> > > > 
> > > > ?
> > > 
> > > That works too, I'll send another version.
> > 
> > Actually, no, it doesn't work.
> > 
> > If we do this, if we were to have a panel that has panel-lvds but none
> > of the other compatible (because of a typo, or downright invalid
> > binding) we won't validate it and report any error.
> > 
> > I'll merge this version (together with the v4 version of patch 1)
> 
> I'm sorry but I *really* *really* dislike this. Having to list all other
> compatible values in this file is a sign that something is wrong in the
> validation infrastructure. People will forget to update it when adding
> new bindings, and will get confused by the result. If I were a
> maintainer for DT bindings I'd nack this.

The validation infrastructure is what it is, and we can't change that.
Rewriting one from scratch isn't reasonable either. That being said, the
*only* case where this has been a problem are the panels because there's
so many pointless schemas which should really be a single schema.

That's the root cause.

I tried to merge all of them, but once again panels seem to be special,
and it was shot down. So be it. But at the end of the day, there's not a
lot of solutions to do what we are doing for every other case out there.

> If a DT has panel-lvds and no other compatible string, or invalid ones,
> won't the validation report that the compatible isn't understood ? I
> think that would be enough.

That's just worse. How would you not get confused if there's an error
that the compatible isn't documented, you search for it, and it's
actually documented there?

We really have two solutions:

  - Either we merge all the panel-lvds schemas in one,

  - Or we have this.

The first was shot down, only the latter remains.

Maxime