Re: [PATCH v2 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

[Rob Herring <robh@xxxxxxxxxx>]

On Thu, Jan 27, 2022 at 10:37:22AM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Jan 25, 2022 at 06:56:58PM +0100, Noralf Trønnes wrote:
> > Add binding for MIPI DBI compatible SPI panels.
> > 
> > v2:
> > - Fix path for panel-common.yaml
> > - Use unevaluatedProperties
> > - Drop properties which are in the allOf section
> > - Drop model property (Rob)
> > 
> > Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > ---
> >  .../display/panel/panel-mipi-dbi-spi.yaml     | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > new file mode 100644
> > index 000000000000..b7cbeea0f8aa
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MIPI DBI SPI Panels Device Tree Bindings
> > +
> > +maintainers:
> > +  - Noralf Trønnes <noralf@xxxxxxxxxxx>
> > +
> > +description:
> > +  This binding is for display panels using a MIPI DBI controller
> > +  in SPI mode.
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: panel-mipi-dbi-spi
> 
> You need contains here, otherwise it will error out if you have two compatibles.

Shouldn't it always have 2?

Either way, this has to be split up between a common, shareable schema 
and specific, complete schema(s). Like this:

- A schema for everything common (that allows additional properties)

- A schema for 'panel-mipi-dbi-spi' referencing the common schema plus 
  'unevaluatedProperties: false'

- Schemas for panels with their own additional properties (regulators, 
  GPIOs, etc.)

LVDS was restructured like this IIRC.

> > +  write-only:
> > +    type: boolean
> > +    description:
> > +      Controller is not readable (ie. MISO is not wired up).
> > +
> > +  dc-gpios:
> > +    maxItems: 1
> > +    description: |
> > +      Controller data/command selection (D/CX) in 4-line SPI mode.
> > +      If not set, the controller is in 3-line SPI mode.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    spi {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            display@0{
> > +                    compatible = "panel-mipi-dbi-spi";
> 
> We should have two compatibles in the example too
> 
> Maxime