2020-03-06 15:21:35

by Lad, Prabhakar

[permalink] [raw]
Subject: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding

From: Fabrizio Castro <[email protected]>

Add binding for the idk-2121wr LVDS panel from Advantech.

Some panel-specific documentation can be found here:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm

Signed-off-by: Fabrizio Castro <[email protected]>
Signed-off-by: Lad Prabhakar <[email protected]>
---
Apologies for flooding in I missed to add the ML email-ids for the earlier
version so resending it.

Hi All,

This patch is part of series [1] ("Add dual-LVDS panel support to EK874),
all the patches have been accepted from it except this one. I have fixed
Rob's comments in this version of the patch.

[1] https://patchwork.kernel.org/cover/11297589/

v6->7
* Added reference to lvds.yaml
* Changed maintainer to myself
* Switched to dual license
* Dropped required properties except for ports as rest are already listed
in lvds.panel
* Dropped Reviewed-by tag of Laurent, due to the changes made it might not
be valid.

v5->v6:
* No change

v4->v5:
* No change

v3->v4:
* Absorbed patch "dt-bindings: display: Add bindings for LVDS
bus-timings"
* Big restructuring after Rob's and Laurent's comments

v2->v3:
* New patch

.../display/panel/advantech,idk-2121wr.yaml | 120 +++++++++++++++++++++
1 file changed, 120 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
new file mode 100644
index 0000000..b05df05
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/advantech,idk-2121wr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Advantech IDK-2121WR 21.5" Full-HD dual-LVDS panel
+
+maintainers:
+ - Lad Prabhakar <[email protected]>
+ - Thierry Reding <[email protected]>
+
+description: |
+ The IDK-2121WR from Advantech is a Full-HD dual-LVDS panel.
+ A dual-LVDS interface is a dual-link connection with even pixels traveling
+ on one link, and with odd pixels traveling on the other link.
+
+ The panel expects odd pixels on the first port, and even pixels on the
+ second port, therefore the ports must be marked accordingly (with either
+ dual-lvds-odd-pixels or dual-lvds-even-pixels).
+
+allOf:
+ - $ref: lvds.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: advantech,idk-2121wr
+ - {} # panel-lvds, but not listed here to avoid false select
+
+ width-mm:
+ const: 476
+
+ height-mm:
+ const: 268
+
+ data-mapping:
+ const: vesa-24
+
+ ports:
+ type: object
+ properties:
+ port@0:
+ type: object
+ description: The sink for odd pixels.
+ properties:
+ reg:
+ const: 0
+
+ dual-lvds-odd-pixels: true
+
+ required:
+ - reg
+ - dual-lvds-odd-pixels
+
+ port@1:
+ type: object
+ description: The sink for even pixels.
+ properties:
+ reg:
+ const: 1
+
+ dual-lvds-even-pixels: true
+
+ required:
+ - reg
+ - dual-lvds-even-pixels
+
+ panel-timing: true
+
+additionalProperties: false
+
+required:
+ - ports
+
+examples:
+ - |+
+ panel-lvds {
+ compatible = "advantech,idk-2121wr", "panel-lvds";
+
+ width-mm = <476>;
+ height-mm = <268>;
+
+ data-mapping = "vesa-24";
+
+ panel-timing {
+ clock-frequency = <148500000>;
+ hactive = <1920>;
+ vactive = <1080>;
+ hsync-len = <44>;
+ hfront-porch = <88>;
+ hback-porch = <148>;
+ vfront-porch = <4>;
+ vback-porch = <36>;
+ vsync-len = <5>;
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dual-lvds-odd-pixels;
+ panel_in0: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dual-lvds-even-pixels;
+ panel_in1: endpoint {
+ remote-endpoint = <&lvds1_out>;
+ };
+ };
+ };
+ };
+
+...
--
2.7.4


2020-03-09 20:33:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding

On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote:
> From: Fabrizio Castro <[email protected]>
>
> Add binding for the idk-2121wr LVDS panel from Advantech.
>
> Some panel-specific documentation can be found here:
> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
>
> Signed-off-by: Fabrizio Castro <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Apologies for flooding in I missed to add the ML email-ids for the earlier
> version so resending it.
>
> Hi All,
>
> This patch is part of series [1] ("Add dual-LVDS panel support to EK874),
> all the patches have been accepted from it except this one. I have fixed
> Rob's comments in this version of the patch.
>
> [1] https://patchwork.kernel.org/cover/11297589/
>
> v6->7
> * Added reference to lvds.yaml
> * Changed maintainer to myself
> * Switched to dual license
> * Dropped required properties except for ports as rest are already listed
> in lvds.panel
> * Dropped Reviewed-by tag of Laurent, due to the changes made it might not
> be valid.
>
> v5->v6:
> * No change
>
> v4->v5:
> * No change
>
> v3->v4:
> * Absorbed patch "dt-bindings: display: Add bindings for LVDS
> bus-timings"
> * Big restructuring after Rob's and Laurent's comments
>
> v2->v3:
> * New patch
>
> .../display/panel/advantech,idk-2121wr.yaml | 120 +++++++++++++++++++++
> 1 file changed, 120 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
>

My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property

See https://patchwork.ozlabs.org/patch/1250384
Please check and re-submit.

2020-03-09 21:24:46

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding

Hi Rob,

On Mon, Mar 9, 2020 at 8:32 PM Rob Herring <[email protected]> wrote:
>
> On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote:
> > From: Fabrizio Castro <[email protected]>
> >
> > Add binding for the idk-2121wr LVDS panel from Advantech.
> >
> > Some panel-specific documentation can be found here:
> > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> >
> > Signed-off-by: Fabrizio Castro <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > Apologies for flooding in I missed to add the ML email-ids for the earlier
> > version so resending it.
> >
> > Hi All,
> >
> > This patch is part of series [1] ("Add dual-LVDS panel support to EK874),
> > all the patches have been accepted from it except this one. I have fixed
> > Rob's comments in this version of the patch.
> >
> > [1] https://patchwork.kernel.org/cover/11297589/
> >
> > v6->7
> > * Added reference to lvds.yaml
> > * Changed maintainer to myself
> > * Switched to dual license
> > * Dropped required properties except for ports as rest are already listed
> > in lvds.panel
> > * Dropped Reviewed-by tag of Laurent, due to the changes made it might not
> > be valid.
> >
> > v5->v6:
> > * No change
> >
> > v4->v5:
> > * No change
> >
> > v3->v4:
> > * Absorbed patch "dt-bindings: display: Add bindings for LVDS
> > bus-timings"
> > * Big restructuring after Rob's and Laurent's comments
> >
> > v2->v3:
> > * New patch
> >
> > .../display/panel/advantech,idk-2121wr.yaml | 120 +++++++++++++++++++++
> > 1 file changed, 120 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
>
This panel is a dual channel LVDS, as a result the root port is called as
ports instead of port and the child node port@0 and port@1 are used for
even and odd pixels, hence binding has required property as ports instead
of port.

Cheers,
--Prabhakar

> See https://patchwork.ozlabs.org/patch/1250384
> Please check and re-submit.

2020-03-09 21:48:18

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding

Hi Prabhakar

On Mon, Mar 09, 2020 at 09:23:24PM +0000, Lad, Prabhakar wrote:
> Hi Rob,
>
> On Mon, Mar 9, 2020 at 8:32 PM Rob Herring <[email protected]> wrote:
> >
> > On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote:
> > > From: Fabrizio Castro <[email protected]>
> > >
> > > Add binding for the idk-2121wr LVDS panel from Advantech.
> > >
> > > Some panel-specific documentation can be found here:
> > > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> > >
> > > Signed-off-by: Fabrizio Castro <[email protected]>
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > ---
> > > Apologies for flooding in I missed to add the ML email-ids for the earlier
> > > version so resending it.
> > >
> > > Hi All,
> > >
> > > This patch is part of series [1] ("Add dual-LVDS panel support to EK874),
> > > all the patches have been accepted from it except this one. I have fixed
> > > Rob's comments in this version of the patch.
> > >
> > > [1] https://patchwork.kernel.org/cover/11297589/
> > >
> > > v6->7
> > > * Added reference to lvds.yaml
> > > * Changed maintainer to myself
> > > * Switched to dual license
> > > * Dropped required properties except for ports as rest are already listed
> > > in lvds.panel
> > > * Dropped Reviewed-by tag of Laurent, due to the changes made it might not
> > > be valid.
> > >
> > > v5->v6:
> > > * No change
> > >
> > > v4->v5:
> > > * No change
> > >
> > > v3->v4:
> > > * Absorbed patch "dt-bindings: display: Add bindings for LVDS
> > > bus-timings"
> > > * Big restructuring after Rob's and Laurent's comments
> > >
> > > v2->v3:
> > > * New patch
> > >
> > > .../display/panel/advantech,idk-2121wr.yaml | 120 +++++++++++++++++++++
> > > 1 file changed, 120 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
> >
> This panel is a dual channel LVDS, as a result the root port is called as
> ports instead of port and the child node port@0 and port@1 are used for
> even and odd pixels, hence binding has required property as ports instead
> of port.

What goes wrong is that you have a ref to lvds.yaml - and thus you get
also required from that file.

So basically - I think this binding should not have a ref to lvds.yaml -
as the binding needs to be different.

Sam

2020-03-09 22:29:45

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding

Hi Sam,

On Mon, Mar 9, 2020 at 9:47 PM Sam Ravnborg <[email protected]> wrote:
>
> Hi Prabhakar
>
> On Mon, Mar 09, 2020 at 09:23:24PM +0000, Lad, Prabhakar wrote:
> > Hi Rob,
> >
> > On Mon, Mar 9, 2020 at 8:32 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote:
> > > > From: Fabrizio Castro <[email protected]>
> > > >
> > > > Add binding for the idk-2121wr LVDS panel from Advantech.
> > > >
> > > > Some panel-specific documentation can be found here:
> > > > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> > > >
> > > > Signed-off-by: Fabrizio Castro <[email protected]>
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > ---
> > > > Apologies for flooding in I missed to add the ML email-ids for the earlier
> > > > version so resending it.
> > > >
> > > > Hi All,
> > > >
> > > > This patch is part of series [1] ("Add dual-LVDS panel support to EK874),
> > > > all the patches have been accepted from it except this one. I have fixed
> > > > Rob's comments in this version of the patch.
> > > >
> > > > [1] https://patchwork.kernel.org/cover/11297589/
> > > >
> > > > v6->7
> > > > * Added reference to lvds.yaml
> > > > * Changed maintainer to myself
> > > > * Switched to dual license
> > > > * Dropped required properties except for ports as rest are already listed
> > > > in lvds.panel
> > > > * Dropped Reviewed-by tag of Laurent, due to the changes made it might not
> > > > be valid.
> > > >
> > > > v5->v6:
> > > > * No change
> > > >
> > > > v4->v5:
> > > > * No change
> > > >
> > > > v3->v4:
> > > > * Absorbed patch "dt-bindings: display: Add bindings for LVDS
> > > > bus-timings"
> > > > * Big restructuring after Rob's and Laurent's comments
> > > >
> > > > v2->v3:
> > > > * New patch
> > > >
> > > > .../display/panel/advantech,idk-2121wr.yaml | 120 +++++++++++++++++++++
> > > > 1 file changed, 120 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> > > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
> > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property
> > >
> > This panel is a dual channel LVDS, as a result the root port is called as
> > ports instead of port and the child node port@0 and port@1 are used for
> > even and odd pixels, hence binding has required property as ports instead
> > of port.
>
> What goes wrong is that you have a ref to lvds.yaml - and thus you get
> also required from that file.
>
Agreed.

> So basically - I think this binding should not have a ref to lvds.yaml -
> as the binding needs to be different.
>
Yes makes sense, will post a v8 dropping the reference to lvds.yaml

Cheers,
--Prabhakar

> Sam