[PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Maxime Ripard]

The binding mentions that all the drivers using that driver must use a
vendor-specific compatible but never enforces it, nor documents the
vendor-specific compatibles.

Let's make we document all of them, and that the binding will create an
error if we add one that isn't.

Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
index 49460c9dceea..d1513111eb48 100644
--- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
+++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
@@ -31,12 +31,18 @@ allOf:
 
 properties:
   compatible:
-    contains:
-      const: panel-lvds
-    description:
-      Shall contain "panel-lvds" in addition to a mandatory panel-specific
-      compatible string defined in individual panel bindings. The "panel-lvds"
-      value shall never be used on its own.
+    items:
+      - enum:
+          - advantech,idk-1110wr
+          - advantech,idk-2121wr
+          - auo,b101ew05
+          - innolux,ee101ia-01d
+          - mitsubishi,aa104xd12
+          - mitsubishi,aa121td01
+          - sgd,gktw70sdae4se
+          - sharp,lq150x1lg11
+          - tbs,a711-panel
+      - const: panel-lvds
 
   data-mapping:
     enum:
-- 
2.31.1

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Sam Ravnborg]

Hi Maxime,
On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> The binding mentions that all the drivers using that driver must use a
> vendor-specific compatible but never enforces it, nor documents the
> vendor-specific compatibles.
> 
> Let's make we document all of them, and that the binding will create an
> error if we add one that isn't.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> index 49460c9dceea..d1513111eb48 100644
> --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> @@ -31,12 +31,18 @@ allOf:
>  
>  properties:
>    compatible:
> -    contains:
> -      const: panel-lvds
> -    description:
> -      Shall contain "panel-lvds" in addition to a mandatory panel-specific
> -      compatible string defined in individual panel bindings. The "panel-lvds"
> -      value shall never be used on its own.
> +    items:
> +      - enum:
> +          - advantech,idk-1110wr
> +          - advantech,idk-2121wr
> +          - auo,b101ew05
> +          - innolux,ee101ia-01d
> +          - mitsubishi,aa104xd12
> +          - mitsubishi,aa121td01
> +          - sgd,gktw70sdae4se
> +          - sharp,lq150x1lg11
> +          - tbs,a711-panel
> +      - const: panel-lvds

I can see there is already a dedicated binding for sharp,lq150x1lg11
I picked this randomly - did not check the rest.

I think we are in for troubles if we have two bindings with the same
compatible...

	Sam

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Rob Herring]

On Wed, 21 Jul 2021 16:03:40 +0200, Maxime Ripard wrote:
> The binding mentions that all the drivers using that driver must use a
> vendor-specific compatible but never enforces it, nor documents the
> vendor-specific compatibles.
> 
> Let's make we document all of them, and that the binding will create an
> error if we add one that isn't.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: compatible: ['sharp,lq150x1lg11'] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: 'data-mapping' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: 'width-mm' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: 'height-mm' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: 'panel-timing' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.example.dt.yaml: panel: 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lvds.yaml
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1508254

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Rob Herring]

On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> The binding mentions that all the drivers using that driver must use a
> vendor-specific compatible but never enforces it, nor documents the
> vendor-specific compatibles.
> 
> Let's make we document all of them, and that the binding will create an
> error if we add one that isn't.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> index 49460c9dceea..d1513111eb48 100644
> --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> @@ -31,12 +31,18 @@ allOf:
>  
>  properties:
>    compatible:
> -    contains:
> -      const: panel-lvds
> -    description:
> -      Shall contain "panel-lvds" in addition to a mandatory panel-specific
> -      compatible string defined in individual panel bindings. The "panel-lvds"
> -      value shall never be used on its own.
> +    items:
> +      - enum:
> +          - advantech,idk-1110wr

At least this one is documented elsewhere. You can add 'minItems: 2' if 
you want to just enforce having 2 compatibles. Or do:

items:
  - {}
  - const: panel-lvds

Which also enforces the order.

> +          - advantech,idk-2121wr
> +          - auo,b101ew05
> +          - innolux,ee101ia-01d
> +          - mitsubishi,aa104xd12
> +          - mitsubishi,aa121td01
> +          - sgd,gktw70sdae4se
> +          - sharp,lq150x1lg11
> +          - tbs,a711-panel
> +      - const: panel-lvds
>  
>    data-mapping:
>      enum:
> -- 
> 2.31.1
> 
> 

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

Hi Rob, Sam,

On Wed, Jul 21, 2021 at 08:29:47PM -0600, Rob Herring wrote:
> On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> > The binding mentions that all the drivers using that driver must use a
> > vendor-specific compatible but never enforces it, nor documents the
> > vendor-specific compatibles.
> > 
> > Let's make we document all of them, and that the binding will create an
> > error if we add one that isn't.
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > index 49460c9dceea..d1513111eb48 100644
> > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > @@ -31,12 +31,18 @@ allOf:
> >  
> >  properties:
> >    compatible:
> > -    contains:
> > -      const: panel-lvds
> > -    description:
> > -      Shall contain "panel-lvds" in addition to a mandatory panel-specific
> > -      compatible string defined in individual panel bindings. The "panel-lvds"
> > -      value shall never be used on its own.
> > +    items:
> > +      - enum:
> > +          - advantech,idk-1110wr
> 
> At least this one is documented elsewhere.

Indeed, I missed it.

> You can add 'minItems: 2' if you want to just enforce having 2 compatibles. Or do:
> 
> items:
>   - {}
>   - const: panel-lvds
> 
> Which also enforces the order.

It's not just about the order since a missing compatible will also raise
a warning.

Some of those panels have a binding of their own, but some probably
won't (and I can't find anything specific about the one I'm most
interested in: tbs,a711-panel)

Can we have something like:

compatible:
  oneOf:
    - items:
      - enum:
	- tbs,a711-panel
      - const: panel-lvds

    - items:
      - {}
      - const: panel-lvds

That would work for both cases I guess?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Rob Herring]

On Wed, Aug 18, 2021 at 7:43 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob, Sam,
>
> On Wed, Jul 21, 2021 at 08:29:47PM -0600, Rob Herring wrote:
> > On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> > > The binding mentions that all the drivers using that driver must use a
> > > vendor-specific compatible but never enforces it, nor documents the
> > > vendor-specific compatibles.
> > >
> > > Let's make we document all of them, and that the binding will create an
> > > error if we add one that isn't.
> > >
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Sam Ravnborg <sam@ravnborg.org>
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > ---
> > >  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
> > >  1 file changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > index 49460c9dceea..d1513111eb48 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > @@ -31,12 +31,18 @@ allOf:
> > >
> > >  properties:
> > >    compatible:
> > > -    contains:
> > > -      const: panel-lvds
> > > -    description:
> > > -      Shall contain "panel-lvds" in addition to a mandatory panel-specific
> > > -      compatible string defined in individual panel bindings. The "panel-lvds"
> > > -      value shall never be used on its own.
> > > +    items:
> > > +      - enum:
> > > +          - advantech,idk-1110wr
> >
> > At least this one is documented elsewhere.
>
> Indeed, I missed it.
>
> > You can add 'minItems: 2' if you want to just enforce having 2 compatibles. Or do:
> >
> > items:
> >   - {}
> >   - const: panel-lvds
> >
> > Which also enforces the order.
>
> It's not just about the order since a missing compatible will also raise
> a warning.
>
> Some of those panels have a binding of their own, but some probably
> won't (and I can't find anything specific about the one I'm most
> interested in: tbs,a711-panel)
>
> Can we have something like:
>
> compatible:
>   oneOf:
>     - items:
>       - enum:
>         - tbs,a711-panel
>       - const: panel-lvds
>
>     - items:
>       - {}
>       - const: panel-lvds
>
> That would work for both cases I guess?

No, both conditions will be true. If you use 'anyOf', then we're never
really checking the specific compatible.

I think the problem here is trying to mix a common binding (aka an
incomplete collection of properties) and a specific binding. The
former is characterized with 'additionalProperties: true' as we have
here. You need to create a 'panel-simple-lvds.yaml' schema file that
references this one, defines all the 'simple' compatibles, and sets
'unevaluatedProperties: false'.

Rob

Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]

Hi,

On Wed, Aug 18, 2021 at 08:48:46AM -0500, Rob Herring wrote:
> On Wed, Aug 18, 2021 at 7:43 AM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi Rob, Sam,
> >
> > On Wed, Jul 21, 2021 at 08:29:47PM -0600, Rob Herring wrote:
> > > On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> > > > The binding mentions that all the drivers using that driver must use a
> > > > vendor-specific compatible but never enforces it, nor documents the
> > > > vendor-specific compatibles.
> > > >
> > > > Let's make we document all of them, and that the binding will create an
> > > > error if we add one that isn't.
> > > >
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Cc: Sam Ravnborg <sam@ravnborg.org>
> > > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > > ---
> > > >  .../bindings/display/panel/lvds.yaml           | 18 ++++++++++++------
> > > >  1 file changed, 12 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > > index 49460c9dceea..d1513111eb48 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > > > @@ -31,12 +31,18 @@ allOf:
> > > >
> > > >  properties:
> > > >    compatible:
> > > > -    contains:
> > > > -      const: panel-lvds
> > > > -    description:
> > > > -      Shall contain "panel-lvds" in addition to a mandatory panel-specific
> > > > -      compatible string defined in individual panel bindings. The "panel-lvds"
> > > > -      value shall never be used on its own.
> > > > +    items:
> > > > +      - enum:
> > > > +          - advantech,idk-1110wr
> > >
> > > At least this one is documented elsewhere.
> >
> > Indeed, I missed it.
> >
> > > You can add 'minItems: 2' if you want to just enforce having 2 compatibles. Or do:
> > >
> > > items:
> > >   - {}
> > >   - const: panel-lvds
> > >
> > > Which also enforces the order.
> >
> > It's not just about the order since a missing compatible will also raise
> > a warning.
> >
> > Some of those panels have a binding of their own, but some probably
> > won't (and I can't find anything specific about the one I'm most
> > interested in: tbs,a711-panel)
> >
> > Can we have something like:
> >
> > compatible:
> >   oneOf:
> >     - items:
> >       - enum:
> >         - tbs,a711-panel
> >       - const: panel-lvds
> >
> >     - items:
> >       - {}
> >       - const: panel-lvds
> >
> > That would work for both cases I guess?
> 
> No, both conditions will be true. If you use 'anyOf', then we're never
> really checking the specific compatible.
> 
> I think the problem here is trying to mix a common binding (aka an
> incomplete collection of properties) and a specific binding.

I'm not entirely sure why we have specific bindings for this in the
first place.

We currently have 6 specific bindings, and for 5 of them the only
specific thing in there are the data-mapping value to force and their
dimension.

I'd argue that the dimension shouldn't even be set in stone: you could
very well imagine a screen with exactly the same timings but a different
size. We would consider it compatible.

And the data-mapping can be dealt with with an if clause fairly easily.

And for the last one, the specific thing about it is that it's using a
dual-link output, which is a generic binding and could thus be described
in panel-lvds too.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 11/54] dt-bindings: display: simple-bridge: Add corpro, gm7123 compatible

[Maxime Ripard]

The corpro,gm7123 was in use in a DT but was never properly documented,
let's add it.

Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>

---

Changes from v1:
  - Removed the dumb-vga-dac compatible from the list
---
 .../devicetree/bindings/display/bridge/simple-bridge.yaml      | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
index 6c7b577fd471..43cf4df9811a 100644
--- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml
@@ -22,6 +22,9 @@ properties:
               - ti,ths8134a
               - ti,ths8134b
           - const: ti,ths8134
+      - items:
+          - const: corpro,gm7123
+          - const: adi,adv7123
       - enum:
           - adi,adv7123
           - dumb-vga-dac
-- 
2.31.1

Re: [PATCH 11/54] dt-bindings: display: simple-bridge: Add corpro, gm7123 compatible

[Sam Ravnborg]

On Wed, Jul 21, 2021 at 04:03:41PM +0200, Maxime Ripard wrote:
> The corpro,gm7123 was in use in a DT but was never properly documented,
> let's add it.
> 
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH 11/54] dt-bindings: display: simple-bridge: Add corpro, gm7123 compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On Wed, Jul 21, 2021 at 04:16:13PM +0200, Sam Ravnborg wrote:
> On Wed, Jul 21, 2021 at 04:03:41PM +0200, Maxime Ripard wrote:
> > The corpro,gm7123 was in use in a DT but was never properly documented,
> > let's add it.
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Applied to drm-misc-next, thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]