diff mbox series

dt-bindings: usb: typec-tcpci: convert to DT schema format

Message ID 20230323065824.3857573-1-peng.fan@oss.nxp.com
State New
Headers
Series dt-bindings: usb: typec-tcpci: convert to DT schema format |

Commit Message

Peng Fan (OSS) March 23, 2023, 6:58 a.m. UTC 
  From: Peng Fan <peng.fan@nxp.com>

Convert the binding to DT schema format. The default speed is HS,
so add a dummy port@0 in the example.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V1:
 The default speed is HS, so port@0 is not added for some device tree,
 however the usb-c-connector requires port@0. Not sure we should drop
 the required port@0 from usb-c-connector schema or add a dummy port@0
 for tcpci as what this patch does.

 .../devicetree/bindings/usb/typec-tcpci.txt   | 49 ------------
 .../devicetree/bindings/usb/typec-tcpci.yaml  | 80 +++++++++++++++++++
 2 files changed, 80 insertions(+), 49 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
 create mode 100644 Documentation/devicetree/bindings/usb/typec-tcpci.yaml

Comments

Krzysztof Kozlowski March 27, 2023, 7:41 a.m. UTC | #1 
On 23/03/2023 07:58, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Convert the binding to DT schema format. The default speed is HS,
> so add a dummy port@0 in the example.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V1:
>  The default speed is HS, so port@0 is not added for some device tree,
>  however the usb-c-connector requires port@0. Not sure we should drop
>  the required port@0 from usb-c-connector schema or add a dummy port@0
>  for tcpci as what this patch does.

imx8mq-librem5-devkit has full port@0 so just use similar approach.

> 
>  .../devicetree/bindings/usb/typec-tcpci.txt   | 49 ------------
>  .../devicetree/bindings/usb/typec-tcpci.yaml  | 80 +++++++++++++++++++
>  2 files changed, 80 insertions(+), 49 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
>  create mode 100644 Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> deleted file mode 100644
> index 2082522b1c32..000000000000
> --- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -TCPCI(Typec port cotroller interface) binding
> ----------------------------------------------
> -
> -Required properties:
> -- compatible:       should be set one of following:
> -		    - "nxp,ptn5110" for NXP USB PD TCPC PHY IC ptn5110.
> -
> -- reg:              the i2c slave address of typec port controller device.
> -- interrupt-parent: the phandle to the interrupt controller which provides
> -                    the interrupt.
> -- interrupts:       interrupt specification for tcpci alert.
> -
> -Required sub-node:
> -- connector: The "usb-c-connector" attached to the tcpci chip, the bindings
> -  of connector node are specified in
> -  Documentation/devicetree/bindings/connector/usb-connector.yaml
> -
> -Example:
> -
> -ptn5110@50 {
> -	compatible = "nxp,ptn5110";
> -	reg = <0x50>;
> -	interrupt-parent = <&gpio3>;
> -	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> -
> -	usb_con: connector {
> -		compatible = "usb-c-connector";
> -		label = "USB-C";
> -		data-role = "dual";
> -		power-role = "dual";
> -		try-power-role = "sink";
> -		source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
> -		sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
> -			     PDO_VAR(5000, 12000, 2000)>;
> -		op-sink-microwatt = <10000000>;
> -
> -		ports {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			port@1 {
> -				reg = <1>;
> -				usb_con_ss: endpoint {
> -					remote-endpoint = <&usb3_data_ss>;
> -				};
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.yaml b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> new file mode 100644
> index 000000000000..067d3b032e3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml

Is this a binding for PTN5110 or for generic tcpci? Looks like the
first, thus name should be rather device specific, so nxp,ptn5110.
Specially that there are other tcpci chips in separate bindings.

> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/typec-tcpci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TCPCI(Typec port cotroller interface)

Also title should be changed to device specific.

> +
> +maintainers:
> +  - Li Jun <jun.li@nxp.com>
> +
> +properties:
> +  compatible:
> +    const: nxp,ptn5110
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  port:
> +    description: This port is used for usb role switch.
> +    $ref: /schemas/graph.yaml#/properties/port

I don't think
> +
> +  connector:
> +    type: object
> +    $ref: /schemas/connector/usb-connector.yaml#
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - connector
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/usb/pd.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ptn5110@50 {

tcpci or actually more popular: usb-typec

> +            compatible = "nxp,ptn5110";
> +            reg = <0x50>;
> +            interrupt-parent = <&gpio3>;


Best regards,
Krzysztof
Jun Li March 28, 2023, 9:24 a.m. UTC | #2 
Hi Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Monday, March 27, 2023 3:41 PM
> To: Peng Fan (OSS) <peng.fan@oss.nxp.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org; Jun Li
> <jun.li@nxp.com>
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; Peng Fan <peng.fan@nxp.com>
> Subject: Re: [PATCH] dt-bindings: usb: typec-tcpci: convert to DT schema
> format
> 
> On 23/03/2023 07:58, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Convert the binding to DT schema format. The default speed is HS, so
> > add a dummy port@0 in the example.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > V1:
> >  The default speed is HS, so port@0 is not added for some device tree,
> > however the usb-c-connector requires port@0. Not sure we should drop
> > the required port@0 from usb-c-connector schema or add a dummy port@0
> > for tcpci as what this patch does.
> 
> imx8mq-librem5-devkit has full port@0 so just use similar approach.
> 
> >
> >  .../devicetree/bindings/usb/typec-tcpci.txt   | 49 ------------
> >  .../devicetree/bindings/usb/typec-tcpci.yaml  | 80
> > +++++++++++++++++++
> >  2 files changed, 80 insertions(+), 49 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > deleted file mode 100644
> > index 2082522b1c32..000000000000
> > --- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > +++ /dev/null
> > @@ -1,49 +0,0 @@
> > -TCPCI(Typec port cotroller interface) binding
> > ----------------------------------------------
> > -
> > -Required properties:
> > -- compatible:       should be set one of following:
> > -		    - "nxp,ptn5110" for NXP USB PD TCPC PHY IC ptn5110.
> > -
> > -- reg:              the i2c slave address of typec port controller device.
> > -- interrupt-parent: the phandle to the interrupt controller which provides
> > -                    the interrupt.
> > -- interrupts:       interrupt specification for tcpci alert.
> > -
> > -Required sub-node:
> > -- connector: The "usb-c-connector" attached to the tcpci chip, the
> > bindings
> > -  of connector node are specified in
> > -  Documentation/devicetree/bindings/connector/usb-connector.yaml
> > -
> > -Example:
> > -
> > -ptn5110@50 {
> > -	compatible = "nxp,ptn5110";
> > -	reg = <0x50>;
> > -	interrupt-parent = <&gpio3>;
> > -	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> > -
> > -	usb_con: connector {
> > -		compatible = "usb-c-connector";
> > -		label = "USB-C";
> > -		data-role = "dual";
> > -		power-role = "dual";
> > -		try-power-role = "sink";
> > -		source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
> > -		sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
> > -			     PDO_VAR(5000, 12000, 2000)>;
> > -		op-sink-microwatt = <10000000>;
> > -
> > -		ports {
> > -			#address-cells = <1>;
> > -			#size-cells = <0>;
> > -
> > -			port@1 {
> > -				reg = <1>;
> > -				usb_con_ss: endpoint {
> > -					remote-endpoint = <&usb3_data_ss>;
> > -				};
> > -			};
> > -		};
> > -	};
> > -};
> > diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> > b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> > new file mode 100644
> > index 000000000000..067d3b032e3d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> 
> Is this a binding for PTN5110 or for generic tcpci? Looks like the first,
> thus name should be rather device specific, so nxp,ptn5110.
> Specially that there are other tcpci chips in separate bindings.

This binding doc is target for generic tcpci, ptn5110 is the one
fully compliance with tcpci spec, if change it to be only specific
to nxp,ptn5110, my understanding is then other chips need duplicate
a binding doc even common tcpci binding and driver is enough for them.

Thanks
Li Jun 
 
> 
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fusb%2Ftypec-tcpci.yaml%23&data=05%7C01%7Cjun.l
> >
> +i%40nxp.com%7Ca2cf429c972a419ad86608db2e96a0f4%7C686ea1d3bc2b4c6fa92c
> >
> +d99c5c301635%7C0%7C0%7C638154996697929798%7CUnknown%7CTWFpbGZsb3d8eyJ
> >
> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> >
> +0%7C%7C%7C&sdata=euh5zZw%2FL53DHjrl6uUP8JqZNARczFDYC6QNaCYrYfk%3D&res
> > +erved=0
> > +$schema:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cjun.li%40nxp.
> >
> +com%7Ca2cf429c972a419ad86608db2e96a0f4%7C686ea1d3bc2b4c6fa92cd99c5c30
> >
> +1635%7C0%7C0%7C638154996697929798%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> >
> +wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> > +7C&sdata=x3NzSCn1gUc8KtwoWSGiXh%2FDoqsStnsrNcTBg4FTY9M%3D&reserved=0
> > +
> > +title: TCPCI(Typec port cotroller interface)
> 
> Also title should be changed to device specific.
> 
> > +
> > +maintainers:
> > +  - Li Jun <jun.li@nxp.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,ptn5110
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  port:
> > +    description: This port is used for usb role switch.
> > +    $ref: /schemas/graph.yaml#/properties/port
> 
> I don't think
> > +
> > +  connector:
> > +    type: object
> > +    $ref: /schemas/connector/usb-connector.yaml#
> > +    unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - connector
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/usb/pd.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ptn5110@50 {
> 
> tcpci or actually more popular: usb-typec
> 
> > +            compatible = "nxp,ptn5110";
> > +            reg = <0x50>;
> > +            interrupt-parent = <&gpio3>;
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski March 28, 2023, 10:43 a.m. UTC | #3 
On 28/03/2023 11:24, Jun Li wrote:
> Hi Krzysztof,
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Monday, March 27, 2023 3:41 PM
>> To: Peng Fan (OSS) <peng.fan@oss.nxp.com>; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org; Jun Li
>> <jun.li@nxp.com>
>> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Peng Fan <peng.fan@nxp.com>
>> Subject: Re: [PATCH] dt-bindings: usb: typec-tcpci: convert to DT schema
>> format
>>
>> On 23/03/2023 07:58, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> Convert the binding to DT schema format. The default speed is HS, so
>>> add a dummy port@0 in the example.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> ---
>>>
>>> V1:
>>>  The default speed is HS, so port@0 is not added for some device tree,
>>> however the usb-c-connector requires port@0. Not sure we should drop
>>> the required port@0 from usb-c-connector schema or add a dummy port@0
>>> for tcpci as what this patch does.
>>
>> imx8mq-librem5-devkit has full port@0 so just use similar approach.
>>
>>>
>>>  .../devicetree/bindings/usb/typec-tcpci.txt   | 49 ------------
>>>  .../devicetree/bindings/usb/typec-tcpci.yaml  | 80
>>> +++++++++++++++++++
>>>  2 files changed, 80 insertions(+), 49 deletions(-)  delete mode
>>> 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
>>>  create mode 100644
>>> Documentation/devicetree/bindings/usb/typec-tcpci.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
>>> b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
>>> deleted file mode 100644
>>> index 2082522b1c32..000000000000
>>> --- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
>>> +++ /dev/null
>>> @@ -1,49 +0,0 @@
>>> -TCPCI(Typec port cotroller interface) binding
>>> ----------------------------------------------
>>> -
>>> -Required properties:
>>> -- compatible:       should be set one of following:
>>> -		    - "nxp,ptn5110" for NXP USB PD TCPC PHY IC ptn5110.
>>> -
>>> -- reg:              the i2c slave address of typec port controller device.
>>> -- interrupt-parent: the phandle to the interrupt controller which provides
>>> -                    the interrupt.
>>> -- interrupts:       interrupt specification for tcpci alert.
>>> -
>>> -Required sub-node:
>>> -- connector: The "usb-c-connector" attached to the tcpci chip, the
>>> bindings
>>> -  of connector node are specified in
>>> -  Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> -
>>> -Example:
>>> -
>>> -ptn5110@50 {
>>> -	compatible = "nxp,ptn5110";
>>> -	reg = <0x50>;
>>> -	interrupt-parent = <&gpio3>;
>>> -	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>>> -
>>> -	usb_con: connector {
>>> -		compatible = "usb-c-connector";
>>> -		label = "USB-C";
>>> -		data-role = "dual";
>>> -		power-role = "dual";
>>> -		try-power-role = "sink";
>>> -		source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
>>> -		sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
>>> -			     PDO_VAR(5000, 12000, 2000)>;
>>> -		op-sink-microwatt = <10000000>;
>>> -
>>> -		ports {
>>> -			#address-cells = <1>;
>>> -			#size-cells = <0>;
>>> -
>>> -			port@1 {
>>> -				reg = <1>;
>>> -				usb_con_ss: endpoint {
>>> -					remote-endpoint = <&usb3_data_ss>;
>>> -				};
>>> -			};
>>> -		};
>>> -	};
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
>>> b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
>>> new file mode 100644
>>> index 000000000000..067d3b032e3d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
>>
>> Is this a binding for PTN5110 or for generic tcpci? Looks like the first,
>> thus name should be rather device specific, so nxp,ptn5110.
>> Specially that there are other tcpci chips in separate bindings.
> 
> This binding doc is target for generic tcpci, ptn5110 is the one


Does this mean that TCPCI requires every device to have exactly one
interrupt (no 0, no 2, exactly 1), no supplies and no additional GPIOs
(like reset GPIO)?

Because this is what this binding is saying.

> fully compliance with tcpci spec, if change it to be only specific
> to nxp,ptn5110, my understanding is then other chips need duplicate
> a binding doc even common tcpci binding and driver is enough for them.

Depends. Usually we have common schema used by actual device schemas. If
TCPCI-compliant device cannot have additional properties, then this one
here looks fine.

One more thing - typec-tcpci is a bit redundant "tc" means typec, so
basicaly you said "typec-typec-pci". This shouold be then just typec-pci
or tcpci.

Best regards,
Krzysztof
Jun Li March 29, 2023, 7:16 a.m. UTC | #4 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, March 28, 2023 6:43 PM
> To: Jun Li <jun.li@nxp.com>; Peng Fan (OSS) <peng.fan@oss.nxp.com>;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; Peng Fan <peng.fan@nxp.com>
> Subject: Re: [PATCH] dt-bindings: usb: typec-tcpci: convert to DT schema
> format
> 
> On 28/03/2023 11:24, Jun Li wrote:
> > Hi Krzysztof,
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Monday, March 27, 2023 3:41 PM
> >> To: Peng Fan (OSS) <peng.fan@oss.nxp.com>; robh+dt@kernel.org;
> >> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org; Jun Li
> >> <jun.li@nxp.com>
> >> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; Peng Fan <peng.fan@nxp.com>
> >> Subject: Re: [PATCH] dt-bindings: usb: typec-tcpci: convert to DT
> >> schema format
> >>
> >> On 23/03/2023 07:58, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> Convert the binding to DT schema format. The default speed is HS, so
> >>> add a dummy port@0 in the example.
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> ---
> >>>
> >>> V1:
> >>>  The default speed is HS, so port@0 is not added for some device
> >>> tree, however the usb-c-connector requires port@0. Not sure we
> >>> should drop the required port@0 from usb-c-connector schema or add a
> >>> dummy port@0 for tcpci as what this patch does.
> >>
> >> imx8mq-librem5-devkit has full port@0 so just use similar approach.
> >>
> >>>
> >>>  .../devicetree/bindings/usb/typec-tcpci.txt   | 49 ------------
> >>>  .../devicetree/bindings/usb/typec-tcpci.yaml  | 80
> >>> +++++++++++++++++++
> >>>  2 files changed, 80 insertions(+), 49 deletions(-)  delete mode
> >>> 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> >>> b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> >>> deleted file mode 100644
> >>> index 2082522b1c32..000000000000
> >>> --- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> >>> +++ /dev/null
> >>> @@ -1,49 +0,0 @@
> >>> -TCPCI(Typec port cotroller interface) binding
> >>> ----------------------------------------------
> >>> -
> >>> -Required properties:
> >>> -- compatible:       should be set one of following:
> >>> -		    - "nxp,ptn5110" for NXP USB PD TCPC PHY IC ptn5110.
> >>> -
> >>> -- reg:              the i2c slave address of typec port controller device.
> >>> -- interrupt-parent: the phandle to the interrupt controller which
> provides
> >>> -                    the interrupt.
> >>> -- interrupts:       interrupt specification for tcpci alert.
> >>> -
> >>> -Required sub-node:
> >>> -- connector: The "usb-c-connector" attached to the tcpci chip, the
> >>> bindings
> >>> -  of connector node are specified in
> >>> -  Documentation/devicetree/bindings/connector/usb-connector.yaml
> >>> -
> >>> -Example:
> >>> -
> >>> -ptn5110@50 {
> >>> -	compatible = "nxp,ptn5110";
> >>> -	reg = <0x50>;
> >>> -	interrupt-parent = <&gpio3>;
> >>> -	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> >>> -
> >>> -	usb_con: connector {
> >>> -		compatible = "usb-c-connector";
> >>> -		label = "USB-C";
> >>> -		data-role = "dual";
> >>> -		power-role = "dual";
> >>> -		try-power-role = "sink";
> >>> -		source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
> >>> -		sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
> >>> -			     PDO_VAR(5000, 12000, 2000)>;
> >>> -		op-sink-microwatt = <10000000>;
> >>> -
> >>> -		ports {
> >>> -			#address-cells = <1>;
> >>> -			#size-cells = <0>;
> >>> -
> >>> -			port@1 {
> >>> -				reg = <1>;
> >>> -				usb_con_ss: endpoint {
> >>> -					remote-endpoint = <&usb3_data_ss>;
> >>> -				};
> >>> -			};
> >>> -		};
> >>> -	};
> >>> -};
> >>> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> >>> b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> >>> new file mode 100644
> >>> index 000000000000..067d3b032e3d
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
> >>
> >> Is this a binding for PTN5110 or for generic tcpci? Looks like the
> >> first, thus name should be rather device specific, so nxp,ptn5110.
> >> Specially that there are other tcpci chips in separate bindings.
> >
> > This binding doc is target for generic tcpci, ptn5110 is the one
> 
> 
> Does this mean that TCPCI requires every device to have exactly one interrupt
> (no 0, no 2, exactly 1), 

Per spec below, TCPCI does need interrupt for alert, but no limit on the number.

"The TCPC uses I2C to communicate with the TCPM. The TCPC is an I2C slave
with Alert# signal for requesting attention."

> no supplies and no additional GPIOs (like reset
> GPIO)?

Those are out of scope of spec, I think it can be added as optional.

> 
> Because this is what this binding is saying.
> 
> > fully compliance with tcpci spec, if change it to be only specific to
> > nxp,ptn5110, my understanding is then other chips need duplicate a
> > binding doc even common tcpci binding and driver is enough for them.
> 
> Depends. Usually we have common schema used by actual device schemas. If
> TCPCI-compliant device cannot have additional properties,

My understanding was TCPCI-compliant device can have additional optional
properties.

> then this one here
> looks fine.
> 
> One more thing - typec-tcpci is a bit redundant "tc" means typec, so basicaly
> you said "typec-typec-pci". This shouold be then just typec-pci or tcpci.

Makes sense, "tcpci.yaml" looks better for me.

Thanks
Li Jun
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski March 29, 2023, 7:24 a.m. UTC | #5 
On 29/03/2023 09:16, Jun Li wrote:
> 
> Per spec below, TCPCI does need interrupt for alert, but no limit on the number.
> 
> "The TCPC uses I2C to communicate with the TCPM. The TCPC is an I2C slave
> with Alert# signal for requesting attention."
> 
>> no supplies and no additional GPIOs (like reset
>> GPIO)?
> 
> Those are out of scope of spec, I think it can be added as optional.
> 
>>
>> Because this is what this binding is saying.
>>
>>> fully compliance with tcpci spec, if change it to be only specific to
>>> nxp,ptn5110, my understanding is then other chips need duplicate a
>>> binding doc even common tcpci binding and driver is enough for them.
>>
>> Depends. Usually we have common schema used by actual device schemas. If
>> TCPCI-compliant device cannot have additional properties,
> 
> My understanding was TCPCI-compliant device can have additional optional
> properties.

Then I propose to rename it to device-matching name (nxp,ptn5110.yaml).
Except interrupt and connector, there is nothing here which describes
actual standard or common class. These can easily be moved to shared
binding later. I can easily imagine TCPCI devices with a bit different
interface and different properties.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
deleted file mode 100644
index 2082522b1c32..000000000000
--- a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
+++ /dev/null
@@ -1,49 +0,0 @@ 
-TCPCI(Typec port cotroller interface) binding
----------------------------------------------
-
-Required properties:
-- compatible:       should be set one of following:
-		    - "nxp,ptn5110" for NXP USB PD TCPC PHY IC ptn5110.
-
-- reg:              the i2c slave address of typec port controller device.
-- interrupt-parent: the phandle to the interrupt controller which provides
-                    the interrupt.
-- interrupts:       interrupt specification for tcpci alert.
-
-Required sub-node:
-- connector: The "usb-c-connector" attached to the tcpci chip, the bindings
-  of connector node are specified in
-  Documentation/devicetree/bindings/connector/usb-connector.yaml
-
-Example:
-
-ptn5110@50 {
-	compatible = "nxp,ptn5110";
-	reg = <0x50>;
-	interrupt-parent = <&gpio3>;
-	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
-
-	usb_con: connector {
-		compatible = "usb-c-connector";
-		label = "USB-C";
-		data-role = "dual";
-		power-role = "dual";
-		try-power-role = "sink";
-		source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
-		sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
-			     PDO_VAR(5000, 12000, 2000)>;
-		op-sink-microwatt = <10000000>;
-
-		ports {
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			port@1 {
-				reg = <1>;
-				usb_con_ss: endpoint {
-					remote-endpoint = <&usb3_data_ss>;
-				};
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.yaml b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
new file mode 100644
index 000000000000..067d3b032e3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-tcpci.yaml
@@ -0,0 +1,80 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/typec-tcpci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TCPCI(Typec port cotroller interface)
+
+maintainers:
+  - Li Jun <jun.li@nxp.com>
+
+properties:
+  compatible:
+    const: nxp,ptn5110
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  port:
+    description: This port is used for usb role switch.
+    $ref: /schemas/graph.yaml#/properties/port
+
+  connector:
+    type: object
+    $ref: /schemas/connector/usb-connector.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/usb/pd.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ptn5110@50 {
+            compatible = "nxp,ptn5110";
+            reg = <0x50>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+            usb_con: connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+                data-role = "dual";
+                power-role = "dual";
+                try-power-role = "sink";
+                source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)>;
+                sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM) PDO_VAR(5000, 12000, 2000)>;
+                op-sink-microwatt = <10000000>;
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                       reg = <0>;
+                    };
+
+                    port@1 {
+                       reg = <1>;
+                       usb_con_ss: endpoint {
+                           remote-endpoint = <&usb3_data_ss>;
+                       };
+                    };
+                };
+            };
+        };
+    };