Re: [RFC PATCH 1/4] dt-bindings: add bindings for USB physical connector

[Laurent Pinchart]

Hi Andrzej,

Thank you for the patch.

On Thursday, 28 September 2017 16:07:27 EEST Andrzej Hajda wrote:
> These bindings allows to describe most known standard USB connectors
> and it should be possible to extend it if necessary.
> USB connectors, beside USB can be used to route other protocols,
> for example UART, Audio, MHL. In such case every device passing data
> through the connector should have appropriate graph bindings.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> ---
> There are few things for discussion (IMO):
> 1. vendor specific connectors, I have added them here, but maybe better is
>    to place them in separate files.

It's useful to have one vendor-specific compatible string to be used in the 
example. We could split vendor-specific connectors to separate files later if 
needed, but for now I'm fine keeping them here.

> 2. physical connector description - I have split it to three properties:
>    type(a,b,ab,c), max-mode(ls,fs,hs,ss,ss+), size(mini,micro,powered).
>    This tripled is able to describe all USB-standard connectors, but there
>    are also impossible combinations, for example(c, *, micro). Maybe better
>    would be to just enumerate all possible connectors in include file.

I don't have a strong opinion on this. The three properties are nicely 
descriptive. You might want to list the valid combinations in the bindings 
though.

> 3. Numbering of port/remote nodes, currently only 0 is assigned for
> Interface Controller. Maybe other functions should be also assigned:
>    HS, SS, CC, SBU, ... whatever. Maybe functions should be described
>    as an additional property of remote node?

Given that one of the main reasons this binding is needed is to describe MHL 
connection to a USB connector, I think we'll need to define additional 
functions, yes. I'm not sure yet how that should look like though.

> ---
>  .../bindings/connector/usb-connector.txt           | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/connector/usb-connector.txt
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt
> b/Documentation/devicetree/bindings/connector/usb-connector.txt new file
> mode 100644
> index 000000000000..f3a4e85122d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -0,0 +1,49 @@
> +USB Connector
> +=============
> +
> +Required properties:
> +- compatible: "usb-connector"
> +  connectors with vendor specific extensions can add one of additional
> +  compatibles:
> +    "samsung,usb-connector-11pin": 11-pin Samsung micro-USB connector
> +- type: the USB connector type: "a", "b", "ab", "c"
> +- max-mode: max USB speed mode supported by the connector:
> +  "ls", "fs", "hs", "ss", "ss+"
> +
> +Optional properties:
> +- label: a symbolic name for the connector
> +- size: size of the connector, should be specified in case of
> +  non-standard USB connectors: "mini", "micro", "powered"

"non-standard" sounds like "vendor-specific", while I assume you're talking 
about the size. The USB specification uses the term "standard" for this 
purpose, so it's hard to use another one that would convey the right meaning 
precisely. Maybe "non-standard ('large') USB connector sizes" ?

> +Required nodes:
> +- any data bus to the connector should be modeled using the
> +  OF graph bindings specified in bindings/graph.txt.
> +  There should be exactly one port with at least one endpoint to
> +  different device nodes. The first endpoint (reg = <0>) should
> +  point to USB Interface Controller.
> +
> +Example
> +-------
> +
> +musb_con: connector {
> +	compatible = "samsung,usb-connector-11pin", "usb-connector";
> +	label = "usb";
> +	type = "b";
> +	size = "micro";
> +	max-mode = "hs";
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		musb_con_usb_in: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&muic_usb_out>;
> +		};
> +
> +		musb_con_mhl_in: endpoint@1 {
> +			reg = <1>;
> +			remote-endpoint = <&mhl_out>;
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart