| Message ID | 20200306152031.14212-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [RESEND,v7] dt-bindings: display: Add idk-2121wr binding | expand |
On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote: > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > 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 <fabrizio.castro@bp.renesas.com> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > 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.
Hi Rob, On Mon, Mar 9, 2020 at 8:32 PM Rob Herring <robh@kernel.org> wrote: > > On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote: > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > 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 <fabrizio.castro@bp.renesas.com> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > 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.
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 <robh@kernel.org> wrote: > > > > On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote: > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > 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 <fabrizio.castro@bp.renesas.com> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > 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
Hi Sam, On Mon, Mar 9, 2020 at 9:47 PM Sam Ravnborg <sam@ravnborg.org> 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 <robh@kernel.org> wrote: > > > > > > On Fri, 6 Mar 2020 15:20:31 +0000, Lad Prabhakar wrote: > > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > > > 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 <fabrizio.castro@bp.renesas.com> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > --- > > > > 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
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 <prabhakar.mahadev-lad.rj@bp.renesas.com> + - Thierry Reding <thierry.reding@gmail.com> + +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>; + }; + }; + }; + }; + +...