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,

> Subject: RE: [PATCH v3 1/5] dt-bindings: usb: convert ti,hd3ss3220 bindings to
> json-schema
>
> 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=n
> ext-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?

FYI, I have posted v4 with changes in usb connector bindings.

[1] https://patchwork.kernel.org/patch/11787677/
[2] https://patchwork.kernel.org/patch/11787679/

Regards,
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