diff mbox series

[v3,1/7] dt-bindings: display: panel: Add himax hx83102 panel bindings

Message ID 20240424023010.2099949-2-yangcong5@huaqin.corp-partner.google.com (mailing list archive)
State New, archived
Headers show
Series Break out as separate driver and add BOE nv110wum-l60 IVO t109nw41 MIPI-DSI panel | expand

Commit Message

cong yang April 24, 2024, 2:30 a.m. UTC 
In V1, discussed with Doug and Linus [1], we need break out as separate
driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
controller, they have some common CMDS. So add new documentation for
this panels.

[1]: https://lore.kernel.org/all/CACRpkdbzYZAS0=zBQJUC4CB2wj4s1h6n6aSAZQvdMV95r3zRUw@mail.gmail.com

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
Chage since V3:

- Update commit message.

V2: https://lore.kernel.org/all/20240422090310.3311429-2-yangcong5@huaqin.corp-partner.google.com

---
 .../display/panel/boe,tv101wum-nl6.yaml       |  2 -
 .../bindings/display/panel/himax,hx83102.yaml | 73 +++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml

Comments

Conor Dooley April 24, 2024, 4:55 p.m. UTC | #1 
On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> In V1, discussed with Doug and Linus [1], we need break out as separate
> driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
> and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
> controller, they have some common CMDS. So add new documentation for
> this panels.

It'd be good to note in the commit message that the 3v3 supply is not
present on these panels, given it was present in the other binding and
not here.

> [1]: https://lore.kernel.org/all/CACRpkdbzYZAS0=zBQJUC4CB2wj4s1h6n6aSAZQvdMV95r3zRUw@mail.gmail.com
> 
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
> Chage since V3:
> 
> - Update commit message.
> 
> V2: https://lore.kernel.org/all/20240422090310.3311429-2-yangcong5@huaqin.corp-partner.google.com
> 
> ---
>  .../display/panel/boe,tv101wum-nl6.yaml       |  2 -
>  .../bindings/display/panel/himax,hx83102.yaml | 73 +++++++++++++++++++
>  2 files changed, 73 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> index 906ef62709b8..53fb35f5c9de 100644
> --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> @@ -32,8 +32,6 @@ properties:
>        - innolux,hj110iz-01a
>          # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
>        - starry,2081101qfh032011-53g
> -        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> -      - starry,himax83102-j02
>          # STARRY ili9882t 10.51" WUXGA TFT LCD panel
>        - starry,ili9882t
>  
> diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> new file mode 100644
> index 000000000000..2e0cd6998ba8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml

Filename matching a compatible please. What you've done here makes it
seem like there's a fallback compatible missing, given this looks like
the LCD panel controller and the starry compatible below is an LCD panel.

> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax HX83102 MIPI-DSI LCD panel controller
> +
> +maintainers:
> +  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> +      - starry,himax83102-j02
> +
> +  reg:
> +    description: the virtual channel number of a DSI peripheral
> +
> +  enable-gpios:
> +    description: a GPIO spec for the enable pin
> +
> +  pp1800-supply:
> +    description: core voltage supply
> +
> +  avdd-supply:
> +    description: phandle of the regulator that provides positive voltage
> +
> +  avee-supply:
> +    description: phandle of the regulator that provides negative voltage
> +
> +  backlight:
> +    description: phandle of the backlight device attached to the panel

I'm not sure why this was given a description when port or rotation
was not.

Otherwise, this looks fine to me.

Cheers,
Conor.

> +
> +  port: true
> +  rotation: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - enable-gpios
> +  - pp1800-supply
> +  - avdd-supply
> +  - avee-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dsi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        panel@0 {
> +            compatible = "starry,himax83102-j02";
> +            reg = <0>;
> +            enable-gpios = <&pio 45 0>;
> +            avdd-supply = <&ppvarn_lcd>;
> +            avee-supply = <&ppvarp_lcd>;
> +            pp1800-supply = <&pp1800_lcd>;
> +            backlight = <&backlight_lcd0>;
> +            port {
> +                panel_in: endpoint {
> +                    remote-endpoint = <&dsi_out>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.25.1
>
cong yang April 25, 2024, 6:03 a.m. UTC | #2 
Hi,

Thanks for review.

Conor Dooley <conor@kernel.org> 于2024年4月25日周四 00:55写道：
>
> On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> > In V1, discussed with Doug and Linus [1], we need break out as separate
> > driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
> > and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
> > controller, they have some common CMDS. So add new documentation for
> > this panels.
>
> It'd be good to note in the commit message that the 3v3 supply is not
> present on these panels, given it was present in the other binding and
> not here.

Got it, fix in V4,thanks.

>
> > [1]: https://lore.kernel.org/all/CACRpkdbzYZAS0=zBQJUC4CB2wj4s1h6n6aSAZQvdMV95r3zRUw@mail.gmail.com
> >
> > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > ---
> > Chage since V3:
> >
> > - Update commit message.
> >
> > V2: https://lore.kernel.org/all/20240422090310.3311429-2-yangcong5@huaqin.corp-partner.google.com
> >
> > ---
> >  .../display/panel/boe,tv101wum-nl6.yaml       |  2 -
> >  .../bindings/display/panel/himax,hx83102.yaml | 73 +++++++++++++++++++
> >  2 files changed, 73 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > index 906ef62709b8..53fb35f5c9de 100644
> > --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > @@ -32,8 +32,6 @@ properties:
> >        - innolux,hj110iz-01a
> >          # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
> >        - starry,2081101qfh032011-53g
> > -        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > -      - starry,himax83102-j02
> >          # STARRY ili9882t 10.51" WUXGA TFT LCD panel
> >        - starry,ili9882t
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> > new file mode 100644
> > index 000000000000..2e0cd6998ba8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
>
> Filename matching a compatible please. What you've done here makes it
> seem like there's a fallback compatible missing, given this looks like
> the LCD panel controller and the starry compatible below is an LCD panel.

So change the filename to starry,himax83102-j02.yaml?

>
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Himax HX83102 MIPI-DSI LCD panel controller
> > +
> > +maintainers:
> > +  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > +      - starry,himax83102-j02
> > +
> > +  reg:
> > +    description: the virtual channel number of a DSI peripheral
> > +
> > +  enable-gpios:
> > +    description: a GPIO spec for the enable pin
> > +
> > +  pp1800-supply:
> > +    description: core voltage supply
> > +
> > +  avdd-supply:
> > +    description: phandle of the regulator that provides positive voltage
> > +
> > +  avee-supply:
> > +    description: phandle of the regulator that provides negative voltage
> > +
> > +  backlight:
> > +    description: phandle of the backlight device attached to the panel
>
> I'm not sure why this was given a description when port or rotation
> was not.

So change it to backlight: true ?

Thanks.

>
> Otherwise, this looks fine to me.
>
> Cheers,
> Conor.
>
> > +
> > +  port: true
> > +  rotation: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - enable-gpios
> > +  - pp1800-supply
> > +  - avdd-supply
> > +  - avee-supply
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    dsi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        panel@0 {
> > +            compatible = "starry,himax83102-j02";
> > +            reg = <0>;
> > +            enable-gpios = <&pio 45 0>;
> > +            avdd-supply = <&ppvarn_lcd>;
> > +            avee-supply = <&ppvarp_lcd>;
> > +            pp1800-supply = <&pp1800_lcd>;
> > +            backlight = <&backlight_lcd0>;
> > +            port {
> > +                panel_in: endpoint {
> > +                    remote-endpoint = <&dsi_out>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > --
> > 2.25.1
> >
Conor Dooley April 26, 2024, 5:06 p.m. UTC | #3 
On Thu, Apr 25, 2024 at 02:03:24PM +0800, cong yang wrote:
> Conor Dooley <conor@kernel.org> 于2024年4月25日周四 00:55写道：
> > On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:

> > > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> >
> > Filename matching a compatible please. What you've done here makes it
> > seem like there's a fallback compatible missing, given this looks like
> > the LCD panel controller and the starry compatible below is an LCD panel.
> 
> So change the filename to starry,himax83102-j02.yaml?

IDK chief, are you missing a fallback or not?

> 
> >
> > > @@ -0,0 +1,73 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Himax HX83102 MIPI-DSI LCD panel controller

Because the title here makes it seem like there should be.

> > > +maintainers:
> > > +  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > > +
> > > +allOf:
> > > +  - $ref: panel-common.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > > +      - starry,himax83102-j02
> > > +
> > > +  reg:
> > > +    description: the virtual channel number of a DSI peripheral
> > > +
> > > +  enable-gpios:
> > > +    description: a GPIO spec for the enable pin
> > > +
> > > +  pp1800-supply:
> > > +    description: core voltage supply
> > > +
> > > +  avdd-supply:
> > > +    description: phandle of the regulator that provides positive voltage
> > > +
> > > +  avee-supply:
> > > +    description: phandle of the regulator that provides negative voltage
> > > +
> > > +  backlight:
> > > +    description: phandle of the backlight device attached to the panel
> >
> > I'm not sure why this was given a description when port or rotation
> > was not.
> 
> So change it to backlight: true ?

Sure? It is just a repeat of something already described in
panel-common.
cong yang April 28, 2024, 6:12 a.m. UTC | #4 
Hi,

Conor Dooley <conor@kernel.org> 于2024年4月27日周六 01:06写道：
>
> On Thu, Apr 25, 2024 at 02:03:24PM +0800, cong yang wrote:
> > Conor Dooley <conor@kernel.org> 于2024年4月25日周四 00:55写道：
> > > On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
>
> > > > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> > >
> > > Filename matching a compatible please. What you've done here makes it
> > > seem like there's a fallback compatible missing, given this looks like
> > > the LCD panel controller and the starry compatible below is an LCD panel.
> >
> > So change the filename to starry,himax83102-j02.yaml?
>
> IDK chief, are you missing a fallback or not?

Ohn, I see.  Like this. Thanks.

properties:
  compatible:
    items:
      - enum:
          - starry,himax83102-j02
      - const: himax,hx83102

>
> >
> > >
> > > > @@ -0,0 +1,73 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Himax HX83102 MIPI-DSI LCD panel controller
>
> Because the title here makes it seem like there should be.
>
> > > > +maintainers:
> > > > +  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > > > +
> > > > +allOf:
> > > > +  - $ref: panel-common.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > > > +      - starry,himax83102-j02
> > > > +
> > > > +  reg:
> > > > +    description: the virtual channel number of a DSI peripheral
> > > > +
> > > > +  enable-gpios:
> > > > +    description: a GPIO spec for the enable pin
> > > > +
> > > > +  pp1800-supply:
> > > > +    description: core voltage supply
> > > > +
> > > > +  avdd-supply:
> > > > +    description: phandle of the regulator that provides positive voltage
> > > > +
> > > > +  avee-supply:
> > > > +    description: phandle of the regulator that provides negative voltage
> > > > +
> > > > +  backlight:
> > > > +    description: phandle of the backlight device attached to the panel
> > >
> > > I'm not sure why this was given a description when port or rotation
> > > was not.
> >
> > So change it to backlight: true ?
>
> Sure? It is just a repeat of something already described in
> panel-common.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
index 906ef62709b8..53fb35f5c9de 100644
--- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
+++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
@@ -32,8 +32,6 @@  properties:
       - innolux,hj110iz-01a
         # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
       - starry,2081101qfh032011-53g
-        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
-      - starry,himax83102-j02
         # STARRY ili9882t 10.51" WUXGA TFT LCD panel
       - starry,ili9882t
 
diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
new file mode 100644
index 000000000000..2e0cd6998ba8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
@@ -0,0 +1,73 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Himax HX83102 MIPI-DSI LCD panel controller
+
+maintainers:
+  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    enum:
+        # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
+      - starry,himax83102-j02
+
+  reg:
+    description: the virtual channel number of a DSI peripheral
+
+  enable-gpios:
+    description: a GPIO spec for the enable pin
+
+  pp1800-supply:
+    description: core voltage supply
+
+  avdd-supply:
+    description: phandle of the regulator that provides positive voltage
+
+  avee-supply:
+    description: phandle of the regulator that provides negative voltage
+
+  backlight:
+    description: phandle of the backlight device attached to the panel
+
+  port: true
+  rotation: true
+
+required:
+  - compatible
+  - reg
+  - enable-gpios
+  - pp1800-supply
+  - avdd-supply
+  - avee-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "starry,himax83102-j02";
+            reg = <0>;
+            enable-gpios = <&pio 45 0>;
+            avdd-supply = <&ppvarn_lcd>;
+            avee-supply = <&ppvarp_lcd>;
+            pp1800-supply = <&pp1800_lcd>;
+            backlight = <&backlight_lcd0>;
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&dsi_out>;
+                };
+            };
+        };
+    };
+
+...