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@xxxxxxxxxxxx>
>>
>> Add documentation for DT properties supported by
>> ps8640 DSI-eDP converter.
>>
>> Signed-off-by: Jitao Shi <jitao.shi@xxxxxxxxxxxx>
>> Acked-by: Rob Herring <robh@xxxxxxxxxx>
>> Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
>> Signed-off-by: Ulrich Hecht <uli@xxxxxxxx>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
> [..]
>> +
>> + 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
>>
>>
>
>
>
