[v1,32/36] dt-bindings: display: convert sharp,ls037v7dw01 to DT Schema
diff mbox series

Message ID 20200315134416.16527-33-sam@ravnborg.org
State New
Headers show
Series
  • dt-bindings: display: convert remaning panel bindings to DT Schema
Related show

Commit Message

Sam Ravnborg March 15, 2020, 1:44 p.m. UTC
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 .../display/panel/sharp,ls037v7dw01.txt       | 43 ------------
 .../display/panel/sharp,ls037v7dw01.yaml      | 66 +++++++++++++++++++
 2 files changed, 66 insertions(+), 43 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml

Comments

Rob Herring March 19, 2020, 3:07 a.m. UTC | #1
On Sun, Mar 15, 2020 at 02:44:12PM +0100, Sam Ravnborg wrote:
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
>  .../display/panel/sharp,ls037v7dw01.txt       | 43 ------------
>  .../display/panel/sharp,ls037v7dw01.yaml      | 66 +++++++++++++++++++
>  2 files changed, 66 insertions(+), 43 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
> deleted file mode 100644
> index 0cc8981e9d49..000000000000
> --- a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -SHARP LS037V7DW01 TFT-LCD panel
> -===================================
> -
> -Required properties:
> -- compatible: "sharp,ls037v7dw01"
> -
> -Optional properties:
> -- label: a symbolic name for the panel
> -- enable-gpios: a GPIO spec for the optional enable pin.
> -  This pin is the INI pin as specified in the LS037V7DW01.pdf file.
> -- reset-gpios: a GPIO spec for the optional reset pin.
> -  This pin is the RESB pin as specified in the LS037V7DW01.pdf file.
> -- mode-gpios: a GPIO
> -  ordered MO, LR, and UD as specified in the LS037V7DW01.pdf file.
> -
> -Required nodes:
> -- Video port for DPI input
> -
> -This panel can have zero to five GPIOs to configure to 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.
> -
> -Example
> --------
> -
> -Example when connected to a omap2+ based device:
> -
> -lcd0: display {
> -	compatible = "sharp,ls037v7dw01";
> -	power-supply = <&lcd_3v3>;
> -	enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>;	/* gpio152, lcd INI */
> -	reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>;	/* gpio155, lcd RESB */
> -	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 */
> -
> -	port {
> -		lcd_in: endpoint {
> -			remote-endpoint = <&dpi_out>;
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml
> new file mode 100644
> index 000000000000..56bd510ae398
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/sharp,ls037v7dw01.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SHARP LS037V7DW01 TFT-LCD panel
> +
> +description: |
> +  This panel can have zero to five GPIOs to configure to 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.
> +
> +maintainers:
> +  - Tony Lindgren <tony@atomide.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: sharp,ls037v7dw01
> +
> +  label: true
> +  enable-gpios: true
> +  reset-gpios: true
> +  port: true
> +  power-supply: true
> +
> +  mode-gpios:
> +    description: |
> +      GPIO ordered MO, LR, and UD as specified in LS037V7DW01.pdf

3 or...

> +      This panel can have zero to five GPIOs to configure to

5?

> +      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

> +
> +required:
> +  - compatible
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    lcd0: display {
> +        compatible = "sharp,ls037v7dw01";
> +        power-supply = <&lcd_3v3>;
> +        enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>;    /* gpio152, lcd INI */
> +        reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>;     /* gpio155, lcd RESB */
> +        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 */
> +
> +        port {
> +            lcd_in: endpoint {
> +                remote-endpoint = <&dpi_out>;
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.20.1
>
Sam Ravnborg March 29, 2020, 7:03 p.m. UTC | #2
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:


> > +        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>;

But the following is not OK:
    mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, <&gpio1 2 GPIO_ACTIVE_HIGH>;

Any hints how to specify the binding to prevent the above?
I have tried a few combinations - but they do not catch this.
So my binding attempts are not restrictive enough.

Any hints how to describe this properly?

	Sam
Rob Herring March 31, 2020, 5:20 p.m. UTC | #3
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.

If that's not working, then it's a bug in the tooling. Please confirm
and I'll investigate.

Rob
Sam Ravnborg March 31, 2020, 7:13 p.m. UTC | #4
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 */

So if I get you right this is a bug in the tooling.
I have updated the tooling a few days ago, should be on the latest.

In the actual example I go for the snip you see above.

	Sam
Rob Herring March 31, 2020, 8:57 p.m. UTC | #5
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

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
deleted file mode 100644
index 0cc8981e9d49..000000000000
--- a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.txt
+++ /dev/null
@@ -1,43 +0,0 @@ 
-SHARP LS037V7DW01 TFT-LCD panel
-===================================
-
-Required properties:
-- compatible: "sharp,ls037v7dw01"
-
-Optional properties:
-- label: a symbolic name for the panel
-- enable-gpios: a GPIO spec for the optional enable pin.
-  This pin is the INI pin as specified in the LS037V7DW01.pdf file.
-- reset-gpios: a GPIO spec for the optional reset pin.
-  This pin is the RESB pin as specified in the LS037V7DW01.pdf file.
-- mode-gpios: a GPIO
-  ordered MO, LR, and UD as specified in the LS037V7DW01.pdf file.
-
-Required nodes:
-- Video port for DPI input
-
-This panel can have zero to five GPIOs to configure to 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.
-
-Example
--------
-
-Example when connected to a omap2+ based device:
-
-lcd0: display {
-	compatible = "sharp,ls037v7dw01";
-	power-supply = <&lcd_3v3>;
-	enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>;	/* gpio152, lcd INI */
-	reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>;	/* gpio155, lcd RESB */
-	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 */
-
-	port {
-		lcd_in: endpoint {
-			remote-endpoint = <&dpi_out>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml
new file mode 100644
index 000000000000..56bd510ae398
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sharp,ls037v7dw01.yaml
@@ -0,0 +1,66 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/sharp,ls037v7dw01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SHARP LS037V7DW01 TFT-LCD panel
+
+description: |
+  This panel can have zero to five GPIOs to configure to 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.
+
+maintainers:
+  - Tony Lindgren <tony@atomide.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: sharp,ls037v7dw01
+
+  label: true
+  enable-gpios: true
+  reset-gpios: true
+  port: true
+  power-supply: true
+
+  mode-gpios:
+    description: |
+      GPIO ordered MO, LR, and UD as specified in LS037V7DW01.pdf
+      This panel can have zero to five GPIOs to configure to
+      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.
+
+required:
+  - compatible
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    lcd0: display {
+        compatible = "sharp,ls037v7dw01";
+        power-supply = <&lcd_3v3>;
+        enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>;    /* gpio152, lcd INI */
+        reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>;     /* gpio155, lcd RESB */
+        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 */
+
+        port {
+            lcd_in: endpoint {
+                remote-endpoint = <&dpi_out>;
+            };
+        };
+    };
+
+...