Re: [PATCH v2 2/2] dt-bindings: one file of all simple DSI panels

[Rob Herring <robh@kernel.org>]

On Tue, Jan 7, 2020 at 9:44 AM Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
>
> Le jeu. 2 janv. 2020 à 11:17, Sam Ravnborg <sam@ravnborg.org> a écrit :
> >
> > To complement panel-simple.yaml, create panel-simple-dsi.yaml.
> > panel-simple-dsi-yaml are for all simple DSP panels with a single
> > power-supply and optional backlight / enable GPIO.
> >
> > Migrate panasonic,vvx10f034n00 over to the new file.
> >
> > The objectives with one file for all the simple DSI panels are:
> >     - Make it simpler to add bindings for simple DSI panels
> >     - Keep the number of bindings file lower
> >     - Keep the binding documentation for simple DSI panels more consistent
> >
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Yannick Fertre <yannick.fertre@st.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: devicetree@vger.kernel.org
> > ---
> >  .../display/panel/panasonic,vvx10f034n00.txt  | 20 ------
> >  .../display/panel/panel-simple-dsi.yaml       | 67 +++++++++++++++++++
> >  2 files changed, 67 insertions(+), 20 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/display/panel/panasonic,vvx10f034n00.txt
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/panasonic,vvx10f034n00.txt b/Documentation/devicetree/bindings/display/panel/panasonic,vvx10f034n00.txt
> > deleted file mode 100644
> > index 37dedf6a6702..000000000000
> > --- a/Documentation/devicetree/bindings/display/panel/panasonic,vvx10f034n00.txt
> > +++ /dev/null
> > @@ -1,20 +0,0 @@
> > -Panasonic 10" WUXGA TFT LCD panel
> > -
> > -Required properties:
> > -- compatible: should be "panasonic,vvx10f034n00"
> > -- reg: DSI virtual channel of the peripheral
> > -- power-supply: phandle of the regulator that provides the supply voltage
> > -
> > -Optional properties:
> > -- backlight: phandle of the backlight device attached to the panel
> > -
> > -Example:
> > -
> > -       mdss_dsi@fd922800 {
> > -               panel@0 {
> > -                       compatible = "panasonic,vvx10f034n00";
> > -                       reg = <0>;
> > -                       power-supply = <&vreg_vsp>;
> > -                       backlight = <&lp8566_wled>;
> > -               };
> > -       };
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> > new file mode 100644
> > index 000000000000..05c52390269e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/panel-simple-dsi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Simple DSI panels with a single power-supply
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Sam Ravnborg <sam@ravnborg.org>
> > +
> > +description: |
> > +  This binding file is a collection of the DSI panels that
> > +  requires only a single power-supply.
> > +  There are optionally a backlight and an enable GPIO.
> > +  The panel may use an OF graph binding for the association to the display,
> > +  or it may be a direct child node of the display.
> > +
> > +  If the panel is more advanced a dedicated binding file is required.
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +
> > +properties:
> > +
> > +  compatible:
> > +    enum:
> > +    # compatible must be listed in alphabetical order, ordered by compatible.
> > +    # The description in the comment is mandatory for each compatible.
> > +
> > +    # Panasonic 10" WUXGA TFT LCD panel
> > +    - panasonic,vvx10f034n00
>
> Hi Sam,
>
> I have tested your patch with these 2 dsi panels:
> # Orise Tech OTM8009A is a 3.97" 480x800 TFT LCD
>   - orisetech,otm8009a
>  # Raydium Semiconductor Corporation RM68200 is a 5.5" 720x1280 TFT LCD
>    - raydium,rm68200
>
> It is close to be fine for me but I have minors comments below.
>
> Benjamin
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: DSI virtual channel
> > +
> > +  backlight: true
> > +  enable-gpios: true
> > +  port: true
> > +  power-supply: true
>
> add reset-gpios: true to support orisetech panel

Nope. If not a single GPIO and single supply, not a simple panel.

Maybe reset could be allowed, but we have to draw the line somewhere.

> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - power-supply
>
> power-supply should optional

The panel works without power? The dts should have a fixed supply if
not controllable.

Here's the problem. If it is not required, then panels with multiple
supplies will get added here because they didn't care to begin with.
Then when someone decides to think about the supplies it will have to
be moved. Bindings need to be complete from the start.

Rob