RE: [PATCH] dt-bindings: usb: typec-tcpci: convert to DT schema format

[Jun Li]

Hi Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Monday, March 27, 2023 3:41 PM
> To: Peng Fan (OSS) <peng.fan@xxxxxxxxxxx>; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Jun Li
> <jun.li@xxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Peng Fan <peng.fan@xxxxxxx>
> 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@xxxxxxx>
> >
> > 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@xxxxxxx>
> > ---
> >
> > 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@xxxxxxx>
> > +
> > +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