Re: [PATCH v22 1/2] Documentation: bridge: Add documentation for ps8640 DT properties

Hi Ezequiel,

On 26/12/19 15:27, Ezequiel Garcia wrote:

Hi Enric, Rob,

On Mon, 2019-12-23 at 15:35 +0100, Enric Balletbo i Serra wrote:

...

From: Jitao Shi jitao.shi@mediatek.com

Add documentation for DT properties supported by ps8640 DSI-eDP converter.

Signed-off-by: Jitao Shi jitao.shi@mediatek.com Acked-by: Rob Herring robh@kernel.org Reviewed-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Ulrich Hecht uli@fpond.eu Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com

[..]

...

• ports:
• type: object
• description:

•  A node containing DSI input & output port nodes with endpoint
  

•  definitions as documented in
  

•  Documentation/devicetree/bindings/media/video-interfaces.txt
  

•  Documentation/devicetree/bindings/graph.txt
  

• properties:

•  port@0:
  

•    type: object
  

•    description: |
  

•      Video port for DSI input
  

•  port@1:
  

•    type: object
  

•    description: |
  

•      Video port for eDP output (panel or connector).
  

• required:

•  - port@0
  

Is it correct to require port@0 ? This could be called port@1 or port@2, and IIUC it should bind the same.

My understanding is that at least the Video port for DSI input is required, which makes sense, otherwise you have the chip connected nowhere. port@1 is optional because it could be connected to a eDP panel or can just be a connector.

About your second question, I am not sure I understand you. You mean that have a DT like this should work?

ports { #address-cells = <1>; #size-cells = <0>;

port@1 { reg = <0>; ps8640_in: endpoint { remote-endpoint = <&dsi0_out>; }; };

port@2 { reg = <1>; ps8640_out: endpoint { remote-endpoint = <&panel_in>; }; }; };

Probably yes, because the driver what really looks is the register value, but that's odd and probably a bad practice. Also if I am not wrong the convention is name the nodes with port@<reg property> (like we do in i2c devices for example)

port@0 is the label that has the register value to 0. port@1 is the label that has the register value to 1. ...

Thanks, Enric

Thanks, Ezequiel

...

+required:

• • compatible
• • reg
• • powerdown-gpios
• • reset-gpios
• • vdd12-supply
• • vdd33-supply
• • ports

+additionalProperties: false

+examples:

• • |
• #include <dt-bindings/gpio/gpio.h>
• i2c0 {

•    #address-cells = <1>;
  

•    #size-cells = <0>;
  

•    ps8640: edp-bridge@18 {
  

•        compatible = "parade,ps8640";
  

•        reg = <0x18>;
  

•        powerdown-gpios = <&pio 116 GPIO_ACTIVE_LOW>;
  

•        reset-gpios = <&pio 115 GPIO_ACTIVE_LOW>;
  

•        vdd12-supply = <&ps8640_fixed_1v2>;
  

•        vdd33-supply = <&mt6397_vgp2_reg>;
  

•        ports {
  

•            #address-cells = <1>;
  

•            #size-cells = <0>;
  

•            port@0 {
  

•                reg = <0>;
  

•                ps8640_in: endpoint {
  

•                    remote-endpoint = <&dsi0_out>;
  

•                };
  

•            };
  

•            port@1 {
  

•                reg = <1>;
  

•                ps8640_out: endpoint {
  

•                    remote-endpoint = <&panel_in>;
  

•               };
  

•            };
  

•        };
  

•    };
  

• };

-- 2.20.1