RE: [PATCH v3 1/5] dt-bindings: usb: convert ti,hd3ss3220 bindings to json-schema

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Wesley Cheng,

Thanks for the feedback.

> Subject: Re: [PATCH v3 1/5] dt-bindings: usb: convert ti,hd3ss3220 bindings to
> json-schema
>
>
>
> On 8/24/2020 7:10 AM, Biju Das wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Convert ti,hd3ss3220.txt to YAML. Updated the binding documentation as
> > graph bindings of this device model Super Speed (SS) data bus to the
> > Super Speed (SS) capable connector.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v2->v3: Replaced Tabs with spaces in the example section.
> > v1->v2 : No change
> > Ref: https://patchwork.kernel.org/patch/11669423/
> > ---
> >  .../devicetree/bindings/usb/ti,hd3ss3220.txt  | 38 ---------
> > .../devicetree/bindings/usb/ti,hd3ss3220.yaml | 81
> +++++++++++++++++++
> >  2 files changed, 81 insertions(+), 38 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > deleted file mode 100644
> > index 2bd21b22ce95..000000000000
> > --- a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > +++ /dev/null
> > @@ -1,38 +0,0 @@
> > -TI HD3SS3220 TypeC DRP Port Controller.
> > -
> > -Required properties:
> > - - compatible: Must be "ti,hd3ss3220".
> > - - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> > - - interrupts: An interrupt specifier.
> > -
> > -Required sub-node:
> > - - connector: The "usb-c-connector" attached to the hd3ss3220 chip. The
> > -   bindings of the connector node are specified in:
> > -
> > -Documentation/devicetree/bindings/connector/usb-connector.yaml
> > -
> > -Example:
> > -hd3ss3220@47 {
> > -compatible = "ti,hd3ss3220";
> > -reg = <0x47>;
> > -interrupt-parent = <&gpio6>;
> > -interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> > -
> > -connector {
> > -compatible = "usb-c-connector";
> > -label = "USB-C";
> > -data-role = "dual";
> > -
> > -ports {
> > -#address-cells = <1>;
> > -#size-cells = <0>;
> > -
> > -port@1 {
> > -reg = <1>;
> > -hd3ss3220_ep: endpoint {
> > -remote-endpoint =
> <&usb3_role_switch>;
> > -};
> > -};
> > -};
> > -};
> > -};
> > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> > b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> > new file mode 100644
> > index 000000000000..750a099529c0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.yaml
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/ti,hd3ss3220.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI HD3SS3220 TypeC DRP Port Controller
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |-
> > +  HD3SS3220 is a USB SuperSpeed (SS) 2:1 mux with DRP port
> > +controller. The device provides Channel
> > +  Configuration (CC) logic and 5V VCONN sourcing for ecosystems
> > +implementing USB Type-C. The
> > +  HD3SS3220 can be configured as a Downstream Facing Port (DFP),
> > +Upstream Facing Port (UFP) or a
> > +  Dual Role Port (DRP) making it ideal for any application.
> > +
> > +properties:
> > +  compatible:
> > +   const: ti,hd3ss3220
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  ports:
> > +    description: OF graph bindings (specified in bindings/graph.txt) that
> model
> > +      SS data bus to the SS capable connector.
> > +    type: object
> > +    properties:
> > +      port@0:
> > +        type: object
> > +        description: Super Speed (SS) capable connector.
> > +
> > +      port@1:
> > +        type: object
> > +        description: Super Speed (SS) data bus.
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        hd3ss3220@47 {
> > +                compatible = "ti,hd3ss3220";
> > +                reg = <0x47>;
> > +                interrupt-parent = <&gpio6>;
> > +                interrupts = <3>;
> > +
> > +                ports {
> > +                        #address-cells = <1>;
> > +                        #size-cells = <0>;
> > +                        port@0 {
> > +                                reg = <0>;
> > +                                hd3ss3220_in_ep: endpoint {
> > +                                        remote-endpoint = <&ss_ep>;
> > +                                };
>
> Hi Biju,
>
> I'm involved in a separate thread[1] and I just wanted to get your inputs on
> how you understood the port and endpoint assignments for the USB type C
> connectors in different designs.
>
> Since the hd3ss3220 does the type C lane detect and has an integrated lane
> select mux, what is the ss_ep being used for?

In our case, Type C Connector --> HS lines directly connected to SoC, where as the SS lines connected  to HD3SS3220 Mux and output of mux is Connected to SoC.

This mux driver detects state change on the CC lines, reading the orientation/role and communicating this information to SoC for assigning roles using USB role switch framework.

Case 1:-  Model connector node as the child node of the mux.

The bindings for this case [1] is not complaint to typec connector bindings, since it is missing port@0 node.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt?h=next-20200917

typec Connector bindings requires port@0 should be HS and port@1 should be SS as per the discussion [2],  so I need to remodel connector node as separate one.
[2] https://patchwork.kernel.org/patch/11669423/

Case 2:-  Model connector node as a separate one based on the hardware design

Type c connector port@0  --> SoC  (hs ep)
Type c connector  port@1 --> connected to USB3.0 Mux -->  SoC( for usb3 role switch)

Here the input of the Mux is connected to TypeC connector and Output of the mux is connected to the SoC, which is inline with hardware design.
The full example of this model here [3]
[3] https://patchwork.kernel.org/patch/11733263/

In both our designs, looks like, type c connector connected to SS MUX and output of SS Mux is Connected to SoC for role switch. Am I missing some thing?
So may be we need to update the connector bindings for this use case. What do you think?

Cheers,
Biju





Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647