Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Andrzej Hajda <a.hajda@samsung.com>]

On 28.03.2018 18:06, Li Jun wrote:
> Add bingdings supported by current typec driver, so user can pass
> all those properties via dt.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  .../bindings/connector/usb-connector.txt           | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..922f22b 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,29 @@ Optional properties:
>  - type: size of the connector, should be specified in case of USB-A, USB-B
>    non-fullsize connectors: "mini", "micro".
>  
> +Optional properties for usb-c-connector:
> +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> +  connector has power support.
> +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> +  connector supports USB data.
> +
> +Required properties for usb-c-connector with power delivery support:
> +- source-pdos: An array of u32 with each entry providing supported power
> +  source data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> +  Source_Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- sink-pdos: An array of u32 with each entry providing supported power
> +  sink data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> +  Sink Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> +- op-sink-microwatt-hours: Sink required operating power in micro
> +  watt-hours, if source offered power is less then it, Capability Mismatch
> +  is set, required for power sink and power dual role.

I have lurked into specs and I am not sure what is the relation of this
field with PD protocol, there is Minimum Operating Power, Maximum
Operating Power and Operating Power present in different RDOs, there are
also similar fields in sink PDOs.
I guess it can be one of them, which one? why other ones are not
provided? What if this or any other property can be calculated only in
runtime?
I am not sure if any of them should be marked "required".

> +
>  Required nodes:
>  - any data bus to the connector should be modeled using the OF graph bindings
>    specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +96,19 @@ ccic: s2mm005@33 {
>  		};
>  	};
>  };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		power-type = "dual";
> +		try-power-role = "sink";
> +		source-pdos = <0x380190c8>;

I understand from DT specification point of view cryptic numbers are OK,
but for sake of readability I strongly suggest use/define macros similar
to the ones present already in kernel:
    source-pdos = < PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >;

It much less error prone, and makes review process much more easier.

Regards
Andrzej

> +		sink-pdos = <0x380190c8 0x3802d0c8>;
> +		op-sink-microwatt-hours = <9000000>;
> +	};
> +};


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel