diff mbox series

[v5,1/9] dt-bindings: usb: Add Type-C switch binding

Message ID 20220622173605.1168416-2-pmalani@chromium.org
State New
Headers show
Series usb: typec: Introduce typec-switch binding | expand

Commit Message

Prashant Malani June 22, 2022, 5:34 p.m. UTC
Introduce a binding which represents a component that can control the
routing of USB Type-C data lines as well as address data line
orientation (based on CC lines' orientation).

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes since v4:
- Added Reviewed-by tags.
- Patch moved to 1/9 position (since Patch v4 1/7 and 2/7 were
  applied to usb-next)

Changes since v3:
- No changes.

Changes since v2:
- Added Reviewed-by and Tested-by tags.

Changes since v1:
- Removed "items" from compatible.
- Fixed indentation in example.

 .../devicetree/bindings/usb/typec-switch.yaml | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml

Comments

Rob Herring June 29, 2022, 5:58 p.m. UTC | #1
On Wed, Jun 29, 2022 at 9:01 AM Pin-yen Lin <treapking@chromium.org> wrote:
>
> On Wed, Jun 29, 2022 at 10:33 PM Pin-yen Lin <treapking@chromium.org> wrote:
> >
> > On Wed, Jun 29, 2022 at 2:23 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Jun 27, 2022 at 02:43:39PM -0700, Prashant Malani wrote:
> > > > Hello Rob,
> > > >
> > > > On Mon, Jun 27, 2022 at 2:04 PM Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Wed, Jun 22, 2022 at 05:34:30PM +0000, Prashant Malani wrote:
> > > > > > Introduce a binding which represents a component that can control the
> > > > > > routing of USB Type-C data lines as well as address data line
> > > > > > orientation (based on CC lines' orientation).
> > > > > >
> > > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > > > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > > > > Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > > > > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > > > > ---
> > > > > >
> > > > > > Changes since v4:
> > > > > > - Added Reviewed-by tags.
> > > > > > - Patch moved to 1/9 position (since Patch v4 1/7 and 2/7 were
> > > > > >   applied to usb-next)
> > > > > >
> > > > > > Changes since v3:
> > > > > > - No changes.
> > > > > >
> > > > > > Changes since v2:
> > > > > > - Added Reviewed-by and Tested-by tags.
> > > > > >
> > > > > > Changes since v1:
> > > > > > - Removed "items" from compatible.
> > > > > > - Fixed indentation in example.
> > > > > >
> > > > > >  .../devicetree/bindings/usb/typec-switch.yaml | 74 +++++++++++++++++++
> > > > > >  1 file changed, 74 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/usb/typec-switch.yaml b/Documentation/devicetree/bindings/usb/typec-switch.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..78b0190c8543
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
> > > > > > @@ -0,0 +1,74 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/usb/typec-switch.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: USB Type-C Switch
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Prashant Malani <pmalani@chromium.org>
> > > > > > +
> > > > > > +description:
> > > > > > +  A USB Type-C switch represents a component which routes USB Type-C data
> > > > > > +  lines to various protocol host controllers (e.g USB, VESA DisplayPort,
> > > > > > +  Thunderbolt etc.) depending on which mode the Type-C port, port partner
> > > > > > +  and cable are operating in. It can also modify lane routing based on
> > > > > > +  the orientation of a connected Type-C peripheral.
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    const: typec-switch
> > > > > > +
> > > > > > +  mode-switch:
> > > > > > +    type: boolean
> > > > > > +    description: Specify that this switch can handle alternate mode switching.
> > > > > > +
> > > > > > +  orientation-switch:
> > > > > > +    type: boolean
> > > > > > +    description: Specify that this switch can handle orientation switching.
> > > > > > +
> > > > > > +  ports:
> > > > > > +    $ref: /schemas/graph.yaml#/properties/ports
> > > > > > +    description: OF graph binding modelling data lines to the Type-C switch.
> > > > > > +
> > > > > > +    properties:
> > > > > > +      port@0:
> > > > > > +        $ref: /schemas/graph.yaml#/properties/port
> > > > > > +        description: Link between the switch and a Type-C connector.
> > > > > > +
> > > > > > +    required:
> > > > > > +      - port@0
> > > > > > +
> > > > > > +required:
> > > > > > +  - compatible
> > > > > > +  - ports
> > > > > > +
> > > > > > +anyOf:
> > > > > > +  - required:
> > > > > > +      - mode-switch
> > > > > > +  - required:
> > > > > > +      - orientation-switch
> > > > > > +
> > > > > > +additionalProperties: true
> > > > > > +
> > > > > > +examples:
> > > > > > +  - |
> > > > > > +    drm-bridge {
> > > > > > +        usb-switch {
> > > > > > +            compatible = "typec-switch";
> > > > >
> > > > > Unless this child is supposed to represent what the parent output is
> > > > > connected to, this is just wrong as, at least for the it6505 chip, it
> > > > > doesn't know anything about Type-C functionality. The bridge is
> > > > > just a protocol converter AFAICT.
> > > >
> > > > I'll let Pin-Yen comment on the specifics of the it6505 chip.
> > >
> > > We're all waiting...
> >
> > Yes it6505 is just a protocol converter. But in our use case, the output DP
> > lines are connected to the Type-C ports and the chip has to know which
> > port has DP Alt mode enabled. Does this justify a child node here?
> >
> > Does it make more sense if we we eliminate the usb-switch node here
> > and list the ports in the top level?

In the it6505 node? No, the it6505 h/w knows nothing about Type-C
switching so neither should its binding.

What device controls the switching in this case? Again, block diagrams
please if you want advice on what the binding should look like.

> > > > > If the child node represents what the output is connected to (like a
> > > > > bus), then yes that is a pattern we have used.
> > > >
> > > > For the anx7625 case, the child node does represent what the output is connected
> > > > to (the usb-c-connector via the switch). Does that not qualify? Or do you mean
> > > > the child node should be a usb-c-connector itself?
> > > >
> > > > > For example, a panel
> > > > > represented as child node of a display controller. However, that only
> > > > > works for simple cases, and is a pattern we have gotten away from in
> > > > > favor of using the graph binding.
> > > >
> > > > The child node will still use a OF graph binding to connect to the
> > > > usb-c-connector.
> > > > Is that insufficient to consider a child node usage here?
> > > > By "using the graph binding", do you mean "only use the top-level ports" ?
> > > > I'm trying to clarify this, so that it will inform future versions and patches.
> > >
> > > What I want to see is block diagrams of possible h/w with different
> > > scenarios and then what the binding looks like in those cases. The
> > > switching/muxing could be in the SoC, a bridge chip, a Type C
> > > controller, a standalone mux chip, or ????. If you want a somewhat
> > > genericish binding, then you need to consider all of these.
>
> Then, is it suitable to put the switch binding into the drivers own bindings
> (i.e., the bindings for it6505 and anx7625), and introduce some helper
> functions to eliminate the duplication in the code?

Only for h/w devices that have switch h/w.

> Though we will have two similar bindings in two drivers with this approach.

While the anx7625 driver having Type-C awareness makes sense given it
has a switch and a type-C controller, that doesn't make sense for
it6505 (and every other bridge driver that's just providing DP
output).

Rob
Stephen Boyd June 29, 2022, 9:58 p.m. UTC | #2
Quoting Rob Herring (2022-06-29 10:58:52)
> On Wed, Jun 29, 2022 at 9:01 AM Pin-yen Lin <treapking@chromium.org> wrote:
> > >
> > > Yes it6505 is just a protocol converter. But in our use case, the output DP
> > > lines are connected to the Type-C ports and the chip has to know which
> > > port has DP Alt mode enabled. Does this justify a child node here?
> > >
> > > Does it make more sense if we we eliminate the usb-switch node here
> > > and list the ports in the top level?
>
> In the it6505 node? No, the it6505 h/w knows nothing about Type-C
> switching so neither should its binding.
>
> What device controls the switching in this case? Again, block diagrams
> please if you want advice on what the binding should look like.

My understanding is there are 4 DP lanes on it6505 and two lanes are
connected to one usb-c-connector and the other two lanes are connected
to a different usb-c-connector. The IT6505 driver will send DP out on
the associated two DP lanes depending on which usb-c-connector has DP
pins assigned by the typec manager.

        					     +-------+
        					     |       |
          +--------+                            /----+ usb-c |
          | IT6505 |                           / /---+       |
          |        +------------ lane 0 ------/ /    |       |
          |        +------------ lane 1 -------/     +-------+
 DPI -----+        |
          |        |                                 +-------+
          |        |                                 |       |
          |        +------------ lane 2 -------------+ usb-c |
          |        +------------ lane 3 -------------+       |
          |        |                                 |       |
          +--------+                                 +-------+

The bridge is a mux that steers DP to one or the other usb-c-connector
based on what the typec manager decides.

I would expect this to be described with the existing port binding in
the it6505 node. The binding would need to be extended to describe the
output side.

        bridge@5c {
            compatible = "ite,it6505";
	    ...

            ports {
                #address-cells = <1>;
                #size-cells = <0>;

                port@0 {
                    reg = <0>;
                    it6505_in: endpoint {
                        remote-endpoint = <&dpi_out>;
                    };
                };

                port@1 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <1>;

                    it6505_out_lanes_01: endpoint@0 {
                        reg = <0>
                        data-lanes = <0 1>;
                        remote-endpoint = <&typec0>;
                    };

                    it6505_out_lanes_23: endpoint@1 {
                        reg = <1>
                        data-lanes = <2 3>;
                        remote-endpoint = <&typec1>;
                    };
                };
            };
        };

        usb-c-connector {
            compatible = "usb-c-connector";
            ....
            ports {
                #address-cells = <1>;
                #size-cells = <0>;

                port@1 {
                    reg = <1>;
                    typec0: endpoint {
                        remote-endpoint = <&it6505_out_lanes_01>;
                    };
                };
            };
        };

Note: port@1 in usb-c-connector above is for superspeed lines, which
technically DP reuses. But we can also shove USB3 superspeed lines over
the other two superspeed pins in the usb-c-connector pinout. Probably
port@1 should have two endpoints so we can connect usb superspeed lines
there in addition to DP lines.

Another use case would be to have the IT6505 send 4 lanes of DP to a
dp-connector. Or send one lane of DP to 4 dp-connectors? Sounds awful but
possible if this bridge can drive one lane DP out on any DP output lane.

        bridge@5c {
            compatible = "ite,it6505";
	    ...

            ports {
                #address-cells = <1>;
                #size-cells = <0>;

                port@0 {
                    reg = <0>;
                    it6505_in: endpoint {
                        remote-endpoint = <&dpi_out>;
                    };
                };

                port@1 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <1>;

                    it6505_out_lane_0: endpoint@0 {
                        reg = <0>
                        data-lanes = <0>;
                        remote-endpoint = <&dp0>;
                    };

                    it6505_out_lane_1: endpoint@1 {
                        reg = <1>
                        data-lanes = <1>;
                        remote-endpoint = <&dp1>;
                    };

                    it6505_out_lane_2: endpoint@2 {
                        reg = <2>
                        data-lanes = <2>;
                        remote-endpoint = <&dp1>;
                    };

                    it6505_out_lane_3: endpoint@3 {
                        reg = <3>
                        data-lanes = <3>;
                        remote-endpoint = <&dp1>;
                    };
                };
            };
        };


Or we could send 4 lanes of DP to a typec redriver that's controlled by
firmware outside of the kernel where the redriver takes in 4 lanes DP
and USB3 and muxes USB or USB+DP or just DP.

        bridge@5c {
            compatible = "ite,it6505";
	    ...

            ports {
                #address-cells = <1>;
                #size-cells = <0>;

                port@0 {
                    reg = <0>;
                    it6505_in: endpoint {
                        remote-endpoint = <&dpi_out>;
                    };
                };

                port@1 {
                    #address-cells = <1>;
                    #size-cells = <0>;
                    reg = <1>;

                    it6505_out: endpoint@0 {
                        reg = <0>
                        data-lanes = <0 1 2 3>;
                        remote-endpoint = <&cros_ec_typec_dp_in>;
                    };
                };
            };
        };

	cros_ec@0 {
	    compatible = "google,cros-ec-spi";
	    ...

	    cros_ec_typec: typec {
	         compatible = "google,cros-ec-typec";
		 ports {
                     #address-cells = <1>;
                     #size-cells = <0>;

                     port@0 {
		         cros_ec_typec_dp_in: endpoint {
			     remote-endpoint = <&it6505_out>;
			 };
                     };
		 };
	    };
	};

In this case we would want to have cros_ec_typec describe some sort of
input graph binding to accept DP. I imagine the EC as a dp-bridge in
this scenario, where it takes in DP and muxes it along with USB onto a
usb-c-connector. If the EC is managing multiple usb-c-connectors then
the typec framework may need help determining which port DP is going to,
especially in the case that two DP bridges are used and they're both
sent to the EC so that the EC can mux them onto a usb-c-connector along
with USB.

(I can draw more block diagrams if you prefer)

>
> > > > > > If the child node represents what the output is connected to (like a
> > > > > > bus), then yes that is a pattern we have used.
> > > > >
> > > > > For the anx7625 case, the child node does represent what the output is connected
> > > > > to (the usb-c-connector via the switch). Does that not qualify? Or do you mean
> > > > > the child node should be a usb-c-connector itself?
> > > > >
> > > > > > For example, a panel
> > > > > > represented as child node of a display controller. However, that only
> > > > > > works for simple cases, and is a pattern we have gotten away from in
> > > > > > favor of using the graph binding.
> > > > >
> > > > > The child node will still use a OF graph binding to connect to the
> > > > > usb-c-connector.
> > > > > Is that insufficient to consider a child node usage here?
> > > > > By "using the graph binding", do you mean "only use the top-level ports" ?
> > > > > I'm trying to clarify this, so that it will inform future versions and patches.
> > > >
> > > > What I want to see is block diagrams of possible h/w with different
> > > > scenarios and then what the binding looks like in those cases. The
> > > > switching/muxing could be in the SoC, a bridge chip, a Type C
> > > > controller, a standalone mux chip, or ????. If you want a somewhat
> > > > genericish binding, then you need to consider all of these.
> >
> > Then, is it suitable to put the switch binding into the drivers own bindings
> > (i.e., the bindings for it6505 and anx7625), and introduce some helper
> > functions to eliminate the duplication in the code?
>
> Only for h/w devices that have switch h/w.
>
> > Though we will have two similar bindings in two drivers with this approach.
>
> While the anx7625 driver having Type-C awareness makes sense given it
> has a switch and a type-C controller, that doesn't make sense for
> it6505 (and every other bridge driver that's just providing DP
> output).
>

I don't see the benefit to making a genericish binding for typec
switches, even if the hardware has typec awareness like anx7625. It
looks like the graph binding can already handle what we need. By putting
it in the top-level ports node we have one way to describe the
input/output of the device instead of describing it in the top-level in
the display connector case and the child typec switch node in the usb c
connector case.

I think the difficulty comes from the combinatorial explosion of
possible configurations. As evidenced here, hardware engineers can take
a DP bridge and use it as a DP mux as long as the bridge has lane
control. Or they can take a device like anx7625 and ignore the USB
aspect and use the internal crosspoint switch as a DP mux. The anx7625
part could be a MIPI-to-DP display bridge plus mux that is connected to
two dp-connectors, in which case typec isn't even involved, but we could
mux between two dp connectors.

Also, the typec framework would like to simply walk the graph from the
usb-c-connector looking for nodes that have 'mode-switch' or
'orientation-switch' properties and treat those devices as the typec
switches for the connector. This means that we have to add these typec
properties like 'mode-switch' to something like the IT6505 bridge
binding, which is a little awkward. I wonder if those properties aren't
really required. Would it be sufficient if the framework could walk the
graph and look for registered typec switches in the kernel that have a
matching of_node?
Prashant Malani June 29, 2022, 10:55 p.m. UTC | #3
On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > What device controls the switching in this case? Again, block diagrams
> > please if you want advice on what the binding should look like.
>
> My understanding is there are 4 DP lanes on it6505 and two lanes are
> connected to one usb-c-connector and the other two lanes are connected
> to a different usb-c-connector. The IT6505 driver will send DP out on
> the associated two DP lanes depending on which usb-c-connector has DP
> pins assigned by the typec manager.
>
>                                                      +-------+
>                                                      |       |
>           +--------+                            /----+ usb-c |
>           | IT6505 |                           / /---+       |
>           |        +------------ lane 0 ------/ /    |       |
>           |        +------------ lane 1 -------/     +-------+
>  DPI -----+        |
>           |        |                                 +-------+
>           |        |                                 |       |
>           |        +------------ lane 2 -------------+ usb-c |
>           |        +------------ lane 3 -------------+       |
>           |        |                                 |       |
>           +--------+                                 +-------+
>
> The bridge is a mux that steers DP to one or the other usb-c-connector
> based on what the typec manager decides.
>
> I would expect this to be described with the existing port binding in
> the it6505 node. The binding would need to be extended to describe the
> output side.
>
>         bridge@5c {
>             compatible = "ite,it6505";

We'll need a top level "mode-switch" property here.
>             ...
>
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@0 {
>                     reg = <0>;
>                     it6505_in: endpoint {
>                         remote-endpoint = <&dpi_out>;
>                     };
>                 };
>
>                 port@1 {
>                     #address-cells = <1>;
>                     #size-cells = <0>;
>                     reg = <1>;
>
>                     it6505_out_lanes_01: endpoint@0 {
>                         reg = <0>
>                         data-lanes = <0 1>;
>                         remote-endpoint = <&typec0>;
>                     };
>
>                     it6505_out_lanes_23: endpoint@1 {
>                         reg = <1>
>                         data-lanes = <2 3>;
>                         remote-endpoint = <&typec1>;
>                     };
>                 };
>             };
>         };
>
>         usb-c-connector {
>             compatible = "usb-c-connector";
>             ....
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@1 {
>                     reg = <1>;
>                     typec0: endpoint {
>                         remote-endpoint = <&it6505_out_lanes_01>;
>                     };
>                 };
>             };
>         };

We can adopt this binding, but from what I gathered in this thread, that
shouldn't be done, because IT6505 isn't meant to be aware of Type-C
connections at all.

>
> I don't see the benefit to making a genericish binding for typec
> switches, even if the hardware has typec awareness like anx7625. It
> looks like the graph binding can already handle what we need. By putting
> it in the top-level ports node we have one way to describe the
> input/output of the device instead of describing it in the top-level in
> the display connector case and the child typec switch node in the usb c
> connector case.

Ack, I'll drop the generic binding for future revisions.

>
> I think the difficulty comes from the combinatorial explosion of
> possible configurations. As evidenced here, hardware engineers can take
> a DP bridge and use it as a DP mux as long as the bridge has lane
> control. Or they can take a device like anx7625 and ignore the USB
> aspect and use the internal crosspoint switch as a DP mux. The anx7625
> part could be a MIPI-to-DP display bridge plus mux that is connected to
> two dp-connectors, in which case typec isn't even involved, but we could
> mux between two dp connectors.

Each containing a single DP lane, right?
I think that will not be a valid configuration, since there is only 1 HPD
pin (so it's assuming both DP lanes go to the same DP sink).

But yes, your larger point is valid: h/w engineers can repurpose these
bridges in ways the datasheet doesn't originally anticipate.

>
> Also, the typec framework would like to simply walk the graph from the
> usb-c-connector looking for nodes that have 'mode-switch' or
> 'orientation-switch' properties and treat those devices as the typec
> switches for the connector. This means that we have to add these typec
> properties like 'mode-switch' to something like the IT6505 bridge
> binding, which is a little awkward. I wonder if those properties aren't
> really required. Would it be sufficient if the framework could walk the
> graph and look for registered typec switches in the kernel that have a
> matching of_node?

My interpretation of the current mode-switch search code [1] is that
a top level property of "mode-switch" is required.

[1] https://elixir.bootlin.com/linux/v5.19-rc4/source/drivers/usb/typec/mux.c#L347
Stephen Boyd June 29, 2022, 11:55 p.m. UTC | #4
Quoting Prashant Malani (2022-06-29 15:55:10)
> On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > My understanding is there are 4 DP lanes on it6505 and two lanes are
> > connected to one usb-c-connector and the other two lanes are connected
> > to a different usb-c-connector. The IT6505 driver will send DP out on
> > the associated two DP lanes depending on which usb-c-connector has DP
> > pins assigned by the typec manager.
[...]
>
> We can adopt this binding, but from what I gathered in this thread, that
> shouldn't be done, because IT6505 isn't meant to be aware of Type-C
> connections at all.

How will the driver know which usb-c-connector to route DP to without
making the binding aware of typec connections?

> >
> > I think the difficulty comes from the combinatorial explosion of
> > possible configurations. As evidenced here, hardware engineers can take
> > a DP bridge and use it as a DP mux as long as the bridge has lane
> > control. Or they can take a device like anx7625 and ignore the USB
> > aspect and use the internal crosspoint switch as a DP mux. The anx7625
> > part could be a MIPI-to-DP display bridge plus mux that is connected to
> > two dp-connectors, in which case typec isn't even involved, but we could
> > mux between two dp connectors.
>
> Each containing a single DP lane, right?

Yes.

> I think that will not be a valid configuration, since there is only 1 HPD
> pin (so it's assuming both DP lanes go to the same DP sink).

HPD can be signalled out of band, or not at all (no-hpd). I suspect it's
valid to ignore/disconnect the HPD pin here and start/stop DP when, for
example, the HPD pin toggles within a dp-connector. HPD could be
signaled directly to the kernel via an out of band gpio going from the
dp-connector to the SoC. In this case HPD for each dp-connector could be
a different gpio and the driver may be required to arbitrate between the
two dp-connectors with some 'first to signal wins' logic or something.

>
> But yes, your larger point is valid: h/w engineers can repurpose these
> bridges in ways the datasheet doesn't originally anticipate.

Yeah, I'm also trying to say that devices with typec logic may not be
used for typec purposes.

>
> >
> > Also, the typec framework would like to simply walk the graph from the
> > usb-c-connector looking for nodes that have 'mode-switch' or
> > 'orientation-switch' properties and treat those devices as the typec
> > switches for the connector. This means that we have to add these typec
> > properties like 'mode-switch' to something like the IT6505 bridge
> > binding, which is a little awkward. I wonder if those properties aren't
> > really required. Would it be sufficient if the framework could walk the
> > graph and look for registered typec switches in the kernel that have a
> > matching of_node?
>
> My interpretation of the current mode-switch search code [1] is that
> a top level property of "mode-switch" is required.

Yeah that's how it is right now, but does it have to stay that way?
Could the code search the graph and look for a matching node that's
registered with the typec framework? The goal is to avoid adding typec
properties like 'mode-switch' to bindings like bridge converters that
aren't expected to work with typec. Hopefully the binding can be written
with the output pins in mind and what modes on those pins the hardware
supports (e.g. two lanes on anx7625 can't be split apart whereas on
it6505 each pair can be used directly).
Prashant Malani June 30, 2022, 5:10 p.m. UTC | #5
(CC+ Bjorn)

On Wed, Jun 29, 2022 at 4:55 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Prashant Malani (2022-06-29 15:55:10)
> > On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > My understanding is there are 4 DP lanes on it6505 and two lanes are
> > > connected to one usb-c-connector and the other two lanes are connected
> > > to a different usb-c-connector. The IT6505 driver will send DP out on
> > > the associated two DP lanes depending on which usb-c-connector has DP
> > > pins assigned by the typec manager.
> [...]
> >
> > We can adopt this binding, but from what I gathered in this thread, that
> > shouldn't be done, because IT6505 isn't meant to be aware of Type-C
> > connections at all.
>
> How will the driver know which usb-c-connector to route DP to without
> making the binding aware of typec connections?

I agree with you; I'm saying my interpretation of the comments of this
thread are that it's not the intended usage of the it6505 part, so the driver
shouldn't be updated to support that.

>
> HPD can be signalled out of band, or not at all (no-hpd). I suspect it's
> valid to ignore/disconnect the HPD pin here and start/stop DP when, for
> example, the HPD pin toggles within a dp-connector. HPD could be
> signaled directly to the kernel via an out of band gpio going from the
> dp-connector to the SoC. In this case HPD for each dp-connector could be
> a different gpio and the driver may be required to arbitrate between the
> two dp-connectors with some 'first to signal wins' logic or something.

Sure, it's possible. I just didn't see anything in the anx7625 datasheet
to suggest it supported 2x1-lane DP outputs.

For that matter I don't think even it6505 supports > 1 DP sink (based
on my reading of the datasheet), but I don't have too much experience
with these parts.


> > My interpretation of the current mode-switch search code [1] is that
> > a top level property of "mode-switch" is required.
>
> Yeah that's how it is right now, but does it have to stay that way?
> Could the code search the graph and look for a matching node that's
> registered with the typec framework?

I'll have to get back to you on that after reading the code a bit more.
Maybe Heikki or Bjorn have some comments about it.
The ACPI Type-C ports do require a device handle labelled "mode-switch"
which points to the switch device.
Rob Herring July 12, 2022, 5:45 p.m. UTC | #6
On Thu, Jun 30, 2022 at 10:10:32AM -0700, Prashant Malani wrote:
> (CC+ Bjorn)
> 
> On Wed, Jun 29, 2022 at 4:55 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Prashant Malani (2022-06-29 15:55:10)
> > > On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > >
> > > > My understanding is there are 4 DP lanes on it6505 and two lanes are
> > > > connected to one usb-c-connector and the other two lanes are connected
> > > > to a different usb-c-connector. The IT6505 driver will send DP out on
> > > > the associated two DP lanes depending on which usb-c-connector has DP
> > > > pins assigned by the typec manager.
> > [...]
> > >
> > > We can adopt this binding, but from what I gathered in this thread, that
> > > shouldn't be done, because IT6505 isn't meant to be aware of Type-C
> > > connections at all.
> >
> > How will the driver know which usb-c-connector to route DP to without
> > making the binding aware of typec connections?
> 
> I agree with you; I'm saying my interpretation of the comments of this
> thread are that it's not the intended usage of the it6505 part, so the driver
> shouldn't be updated to support that.

That's not the right interpretation. There should not be some Type-C 
specific child mux/switch node because the device has no such h/w within 
it. Assuming all the possibilities Stephen outlined are valid, it's 
clear this lane selection has nothing to do with Type-C. It does have an 
output port for its DP output already and using that to describe the 
connection to DP connector(s) and/or Type-C connector(s) should be 
handled.

Whether the driver is type-C aware is a separate question from the 
binding. I would think the driver just needs to be told (or it can ask) 
which endpoint should be active and it just enables output on the
corresponding lanes for that endpoint. I'm not sure if all DP bridge 
chips have the same flexibility on their output lanes, but I would 
assume many do and we don't want to be duplicating the same code to 
handle that in every bridge driver.

Rob
Prashant Malani July 13, 2022, 9:58 p.m. UTC | #7
On Tue, Jul 12, 2022 at 10:45 AM Rob Herring <robh@kernel.org> wrote:
>
> > I agree with you; I'm saying my interpretation of the comments of this
> > thread are that it's not the intended usage of the it6505 part, so the driver
> > shouldn't be updated to support that.
>
> That's not the right interpretation. There should not be some Type-C
> specific child mux/switch node because the device has no such h/w within
> it. Assuming all the possibilities Stephen outlined are valid, it's
> clear this lane selection has nothing to do with Type-C. It does have an
> output port for its DP output already and using that to describe the
> connection to DP connector(s) and/or Type-C connector(s) should be
> handled.

Got it. Thanks for the clarification.

>
> Whether the driver is type-C aware is a separate question from the
> binding. I would think the driver just needs to be told (or it can ask)
> which endpoint should be active and it just enables output on the
> corresponding lanes for that endpoint.

Is it acceptable to tag the end-points with a "mode-switch" /
"orientation-switch"
property? Something like the following:

```
        dp-bridge@5c {
            compatible = "ite,it6505";
            ...
            port {
                #adderss-cells = <1>;
                #size-cells = <0>;

                ite_typec0: endpoint@0 {
                    reg = <0>;
                    mode-switch;
                    remote-endpoint = <&typec_connector0>;
                };
                ite_typec1: endpoint@1 {
                    reg = <1>;
                    mode-switch;
                    remote-endpoint = <&typec_connector1>;
                };
            };
        };
```
Or should the DRM bridge device binding not have those properties in
the end-points either?
The reasons those are required are:
- The Type-C matching code looks for the "mode-switch" identifier in
the fwnode while performing the switch matching [1]
- While we can look up whether the remote-endpoint is a
"usb-c-connector" in the bridge driver the
"mode-switch"/"orientation-switch" property tells the bridge driver
whether to register just a mode-switch, an orientation switch or both.

Best regards,

- Prashant

[1] https://elixir.bootlin.com/linux/v5.19-rc6/source/drivers/usb/typec/mux.c#L347
Prashant Malani Sept. 16, 2022, 6:21 p.m. UTC | #8
Hi folks,

On Fri, Sep 2, 2022 at 12:41 AM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Rob,
>
> On Jul 12 11:45, Rob Herring wrote:
> >
> > That's not the right interpretation. There should not be some Type-C
> > specific child mux/switch node because the device has no such h/w within
> > it. Assuming all the possibilities Stephen outlined are valid, it's
> > clear this lane selection has nothing to do with Type-C. It does have an
> > output port for its DP output already and using that to describe the
> > connection to DP connector(s) and/or Type-C connector(s) should be
> > handled.
> > Rob
>
> Below I've listed the proposal binding (for the Type-C connector) along
> with 2 sample hardware diagrams and corresponding DT.

Any thoughts about this proposal?

>
> The updated binding in usb-c-connector would be as follows:
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index ae515651fc6b..a043b09cb8ec 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -183,6 +183,30 @@ properties:
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: Super Speed (SS), present in SS capable connectors.
> +        properties:
> +          '#address-cells':
> +            const: 1
> +
> +          '#size-cells':
> +            const: 0
> +
> +        patternProperties:
> +          "^endpoint@[0-1]$":
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description:
> +              Endpoints for the two SS lanes. endpoint@0 refers to SSTRX1 (A2,A3,B10,B11)
> +              and endpoint@1 refers to SSTRX2 (B2,B3,A10,A11).
> +            additionalProperties: false
> +
> +              properties:
> +                reg:
> +                  maxItems: 1
> +
> +                remote-endpoint: true
> +
> +              required:
> +                - reg
> +                - remote-endpoint
>
>        port@2:
>          $ref: /schemas/graph.yaml#/properties/port
>
> Here are 2 examples of how that would look on some existing hardware:
>
> Example 1. 2 usb-c-connectors connecting to 1 drm bridge / DP switch:
>
> Here is the diagram we are using on the MTK platform:
>
>                  SOC
>         +---------------------+                                              C0
>         |                     |            +----------+       2 lane      +--------+
>         |                     |            |          +---------/---------+ SSTRX1 |
>         |                     |            |          |                   |        |
>         |    MIPI DPI         |            |          |  2 lane           |        |
>         |                     +------------+ ANX 7625 +---/-----+    +----+ SSTRX2 |
>         |                     |            |          |         |    |    +--------+
>         |                     |            +----------+         |    |
>         +---------------------+                                 |    |
>         |                     |            +----------+ 2 lane  |    |       C1
>         |                     |            |          +----/----C----+    +--------+
>         |    USB3 HC          |   2 lane   |          |         |         | SSTRX1 |
>         |                     +-----/------+ USB3 HUB |         +---------+        |
>         |  (host controller)  |            |          |       2 lane      |        |
>         |                     |            |          +---------/---------+ SSTRX2 |
>         +---------------------+            |          |                   |        |
>                                            +----------+                   +--------+
>
> Some platforms use it6505, so that can be swapped in for anx7625
> without any change to the rest of the hardware diagram.
>
> From the above, we can see that it is helpful to describe the
> Type-C SS lines as 2 endpoints:
> - 1 for SSTX1+SSRX1 (A2,A3 + B10,B11)
> - 1 for SSTX2+SSRX2 (B2,B3 + A10, A11)
>
> A device tree for this would look as follows:
>
> // Type-C port driver
> ec {
>     ...
>     cros_ec_typec {
>         ...
>         usb-c0 {
>             compatible = "usb-c-connector";
>             ports {
>                 hs : port@0 {
>                     ...
>                 };
>                 ss: port@1 {
>                     reg = <1>;
>                     c0_sstrx1: endpoint@0 {
>                         reg = <0>;
>                         remote-endpoint = <&anx7625_out0>;
>                     };
>                     c0_sstrx2: endpoint@0 {
>                         reg = <0>;
>                         remote-endpoint = <&usb3hub_out0>;
>                     };
>                 };
>                 sbu : port@2 {
>                     ...
>                 };
>             };
>         };
>         usb-c1 {
>             compatible = "usb-c-connector";
>             ports {
>                 hs : port@0 {
>                     ...
>                 };
>                 ss: port@1 {
>                     reg = <1>;
>                     c1_sstrx1: endpoint@0 {
>                         reg = <0>;
>                         remote-endpoint = <&anx7625_out1>;
>                     };
>                     c1_sstrx2: endpoint@0 {
>                         reg = <0>;
>                         remote-endpoint = <&usb3hub_out1>;
>                     };
>                 };
>                 sbu : port@2 {
>                     ...
>                 };
>             };
>         };
>     };
> };
>
> // DRM bridge / Type-C mode switch
> anx_bridge: anx7625@58 {
>     compatible = "analogix,anx7625";
>     reg = <0x58>;
>     ...
>     // Input from DP controller
>     port@0 {
>         reg = <0>;
>         ...
>     };
>
>     // Output to Type-C connector / DP panel
>     port@1 {
>         reg = <1>;
>
>         anx7625_out0: endpoint@0 {
>             reg = <0>;
>             mode-switch;
>             remote-endpoint = <&c0_sstrx1>;
>         };
>         anx7625_out1: endpoint@1 {
>             reg = <1>;
>             mode-switch;
>             remote-endpoint = <&c1_sstrx1>;
>         };
>     };
> };
>
> // USB3 hub
> usb3hub: foo_hub {
>     ...
>     ports@0 {
>          // End point connected to USB3 host controller on SOC.
>     };
>     port@1 {
>         reg = <1>;
>
>         foo_hub_out0: endpoint@0 {
>             reg = <0>;
>             mode-switch; ---> See c.) later
>             remote-endpoint = <&c0_sstrx2>;
>         };
>         foo_hub_out1: endpoint@1 {
>             reg = <1>;
>             mode-switch;
>             remote-endpoint = <&c1_sstrx2>;
>         };
>     };
> };
>
> Notes:
> - On the Chrome OS platform, the USB3 Hub is controlled by
> the EC, so we don't really need to describe that connection,
> but I've added a minimal one here just to show how the graph
> connection would work if the HUB was controlled by the SoC.
> - The above assumes that other hardware is controlling orientation.
> We can add "orientation-switch" drivers along the graph path
> if there is other hardware which controls orientation.
>
> Example 2: 1 USB-C connector connected to 1 drm-bridge/ mode-switch
>
> I've tried to use Bjorn's example [1], but I might have made
> some mistakes since I don't have access to the schematic.
>
>
>                   SoC
>   +------------------------------------------+
>   |                                          |
>   |  +---------------+                       |
>   |  |               |                       |
>   |  |  DP ctrllr    |       +---------+     |                 C0
>   |  |               +-------+         |     |   2 lane     +----------+
>   |  +---------------+       |  QMP    +-----+-----/--------+ SSTRX1   |
>   |                          |  PHY    |     |              |          |
>   |  +-------------+  2 lane |         |     |   2 lane     |          |
>   |  |             +----/----+         +-----+-----/--------+ SSTRX2   |
>   |  |    dwc3     |         +---------+     |              |          |
>   |  |             |                         |              |          |
>   |  |             |         +---------+     |              |          |
>   |  |             +---------+ HS PHY  |     |   HS lanes   |          |
>   |  +-------------+         |         +-----+----/---------+ D +/-    |
>   |                          |         |     |              +----------+
>   |                          +---------+     |
>   |                                          |
>   +------------------------------------------+
>
> The DT would look something like this (borrowing from Stephen's example [2]):
>
> qmp {
>     mode-switch; ----> See b.) later.
>     orientation-switch;
>     ports {
>         qmp_usb_in: port@0 {
>             reg = <0>;
>             remote-endpoint = <&usb3_phy_out>;
>         };
>         qmp_dp_in: port@1 {
>             reg = <1>;
>             remote-endpoint = <&dp_phy_out>;
>         };
>         port@2 {
>             reg = <2>;
>             qmp_usb_dp_out0: endpoint@0 {
>                 reg = <0>;
>                 remote-endpoint = <&c0_sstrx1>;
>             };
>             qmp_usb_dp_out1: endpoint@1 {
>                 reg = <1>;
>                 remote-endpoint = <&c0_sstrx2>;
>             };
>         };
> };
>
> dp-phy {
>     ports {
>         dp_phy_out: port {
>             remote-endpoint = <&qmp_dp_in>;
>         };
>     };
> };
>
> dwc3: usb-phy {
>     ports {
>         usb3_phy_out: port@0 {
>             reg = <0>;
>             remote-endpoint = <&qmp_usb_in>;
>         };
>     };
> };
>
> glink {
>     c0: usb-c-connector {
>         compatible = "usb-c-connector";
>         ports {
>             hs: port@0 {
>                 reg = <0>;
>                 endpoint@0 {
>                     reg = <0>;
>                     remote-endpoint = <&hs_phy_out>;
>                 };
>             };
>
>             ss: port@1 {
>                 reg = <1>;
>                 c0_sstrx1: endpoint@0 {
>                     reg = <0>;
>                     remote-endpoint = <&qmp_usb_dp_out0>;
>                 };
>                 c0_sstrx2: endpoint@1 {
>                     reg = <1>;
>                     remote-endpoint = <&qmp_usb_dp_out1>;
>                 };
>             };
>         };
>     };
> };
>
> Notes:
> a. This proposal doesn't deal with the DRM bridge HPD forwarding; I
> believe that is covered by Stephen's example/proposal in [2], and
> can be addressed separately. That said, this binding is compatible
> with the proposal in [2], that is, make the "mode-switch" driver a
> drm-bridge and forward the HPD info to the upstream DRM-bridge (DP controller).
> The driver implementing "mode-switch" will be able to do that, since
> it gets DP status/attention VDOS with HPD info from the Type-C port driver.
> b. If both SSTRX pairs from a connector are routed to the same
> hardware block (example 2) then the device would keep "mode-switch"
> as a top level property (and the fwnode associated with "mode-switch"
> is the drm-bridge device).
> c. If SSTRX pairs from 2 connectors are routed to the same
> hardware block (example 1), then each end-point which is connected to
> the USB-C connector will have a "mode-switch" property in its end-point.
> There will be 2 mode switches registered here, and the fwnode for each
> "mode-switch" is the end-point node.
>
> b.) and c.) can be handled by Type C mux registration and matching
> code. We already have 3 mux devs for each mux [3].
>
> For the single mode-switch case, mux_dev[1] will just refer to the top-level
> mode-switch registered by the DRM bridge / switch driver (example 1).
> For the 2 mode-switch case, typec_mux_dev[1] will have 2 child
> typec_mux_dev's, each of which represents the mode-switches
> registered by the DRM bridge / switch driver. Introducing this
> indirection means the port driver / alternate mode driver don't
> need to care about how the connectors are routed; the framework
> will just call the mux_set() function on the mux_dev() or its
> children if it has any.
>
> The benefit of this approach is existing bindings (which just
> assume 1 endpoint from usb-c-connector/port@1) should continue to
> work without any changes.
>
> Why don't we use data lanes for the usb-c-connector
> endpoints? I guess we could, but I am not a fan of adding the
> extra data-lane parsing logic to the Type-C framework (I
> don't think drivers need that level of detail from the connector
> binding). And even then, we will still need an extra end-point
> if the lanes of the USB-C connector are routed to different hardware blocks.
>
> The Type-C connector spec doesn't specify any alternate modes
> with < 1 SSTRX pair, so the most we can ever have (short of a
> major change to the spec) is 2 SSTRX end points for a
> connector each being routed to different hardware blocks.
> Codifying these as endpoint@0 and endpoint@1 in the usb-c-connector
> binding seems to line up nicely with this detail of the spec.
>
> Thanks,
>
> -Prashant
>
> [1] https://lore.kernel.org/linux-usb/Yv1y9Wjp16CstJvK@baldur/
> [2] https://lore.kernel.org/linux-usb/CAE-0n52-QVeUVCB1qZzPbYyrb1drrbJf6H2DEEW9bOE6mh7egw@mail.gmail.com/
> [3] https://elixir.bootlin.com/linux/v6.0-rc3/source/drivers/usb/typec/mux.c#L259
Pin-yen Lin Oct. 3, 2022, 3:42 a.m. UTC | #9
Hi all,

Are there any thoughts or comments about this proposal?

On Sat, Sep 17, 2022 at 2:22 AM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi folks,
>
> On Fri, Sep 2, 2022 at 12:41 AM Prashant Malani <pmalani@chromium.org> wrote:
> >
> > Hi Rob,
> >
> > On Jul 12 11:45, Rob Herring wrote:
> > >
> > > That's not the right interpretation. There should not be some Type-C
> > > specific child mux/switch node because the device has no such h/w within
> > > it. Assuming all the possibilities Stephen outlined are valid, it's
> > > clear this lane selection has nothing to do with Type-C. It does have an
> > > output port for its DP output already and using that to describe the
> > > connection to DP connector(s) and/or Type-C connector(s) should be
> > > handled.
> > > Rob
> >
> > Below I've listed the proposal binding (for the Type-C connector) along
> > with 2 sample hardware diagrams and corresponding DT.
>
> Any thoughts about this proposal?
>
> >
> > The updated binding in usb-c-connector would be as follows:
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index ae515651fc6b..a043b09cb8ec 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -183,6 +183,30 @@ properties:
> >        port@1:
> >          $ref: /schemas/graph.yaml#/properties/port
> >          description: Super Speed (SS), present in SS capable connectors.
> > +        properties:
> > +          '#address-cells':
> > +            const: 1
> > +
> > +          '#size-cells':
> > +            const: 0
> > +
> > +        patternProperties:
> > +          "^endpoint@[0-1]$":
> > +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > +            description:
> > +              Endpoints for the two SS lanes. endpoint@0 refers to SSTRX1 (A2,A3,B10,B11)
> > +              and endpoint@1 refers to SSTRX2 (B2,B3,A10,A11).
> > +            additionalProperties: false
> > +
> > +              properties:
> > +                reg:
> > +                  maxItems: 1
> > +
> > +                remote-endpoint: true
> > +
> > +              required:
> > +                - reg
> > +                - remote-endpoint
> >
> >        port@2:
> >          $ref: /schemas/graph.yaml#/properties/port
> >
> > Here are 2 examples of how that would look on some existing hardware:
> >
> > Example 1. 2 usb-c-connectors connecting to 1 drm bridge / DP switch:
> >
> > Here is the diagram we are using on the MTK platform:
> >
> >                  SOC
> >         +---------------------+                                              C0
> >         |                     |            +----------+       2 lane      +--------+
> >         |                     |            |          +---------/---------+ SSTRX1 |
> >         |                     |            |          |                   |        |
> >         |    MIPI DPI         |            |          |  2 lane           |        |
> >         |                     +------------+ ANX 7625 +---/-----+    +----+ SSTRX2 |
> >         |                     |            |          |         |    |    +--------+
> >         |                     |            +----------+         |    |
> >         +---------------------+                                 |    |
> >         |                     |            +----------+ 2 lane  |    |       C1
> >         |                     |            |          +----/----C----+    +--------+
> >         |    USB3 HC          |   2 lane   |          |         |         | SSTRX1 |
> >         |                     +-----/------+ USB3 HUB |         +---------+        |
> >         |  (host controller)  |            |          |       2 lane      |        |
> >         |                     |            |          +---------/---------+ SSTRX2 |
> >         +---------------------+            |          |                   |        |
> >                                            +----------+                   +--------+
> >
> > Some platforms use it6505, so that can be swapped in for anx7625
> > without any change to the rest of the hardware diagram.
> >
> > From the above, we can see that it is helpful to describe the
> > Type-C SS lines as 2 endpoints:
> > - 1 for SSTX1+SSRX1 (A2,A3 + B10,B11)
> > - 1 for SSTX2+SSRX2 (B2,B3 + A10, A11)
> >
> > A device tree for this would look as follows:
> >
> > // Type-C port driver
> > ec {
> >     ...
> >     cros_ec_typec {
> >         ...
> >         usb-c0 {
> >             compatible = "usb-c-connector";
> >             ports {
> >                 hs : port@0 {
> >                     ...
> >                 };
> >                 ss: port@1 {
> >                     reg = <1>;
> >                     c0_sstrx1: endpoint@0 {
> >                         reg = <0>;
> >                         remote-endpoint = <&anx7625_out0>;
> >                     };
> >                     c0_sstrx2: endpoint@0 {
> >                         reg = <0>;
> >                         remote-endpoint = <&usb3hub_out0>;
> >                     };
> >                 };
> >                 sbu : port@2 {
> >                     ...
> >                 };
> >             };
> >         };
> >         usb-c1 {
> >             compatible = "usb-c-connector";
> >             ports {
> >                 hs : port@0 {
> >                     ...
> >                 };
> >                 ss: port@1 {
> >                     reg = <1>;
> >                     c1_sstrx1: endpoint@0 {
> >                         reg = <0>;
> >                         remote-endpoint = <&anx7625_out1>;
> >                     };
> >                     c1_sstrx2: endpoint@0 {
> >                         reg = <0>;
> >                         remote-endpoint = <&usb3hub_out1>;
> >                     };
> >                 };
> >                 sbu : port@2 {
> >                     ...
> >                 };
> >             };
> >         };
> >     };
> > };
> >
> > // DRM bridge / Type-C mode switch
> > anx_bridge: anx7625@58 {
> >     compatible = "analogix,anx7625";
> >     reg = <0x58>;
> >     ...
> >     // Input from DP controller
> >     port@0 {
> >         reg = <0>;
> >         ...
> >     };
> >
> >     // Output to Type-C connector / DP panel
> >     port@1 {
> >         reg = <1>;
> >
> >         anx7625_out0: endpoint@0 {
> >             reg = <0>;
> >             mode-switch;
> >             remote-endpoint = <&c0_sstrx1>;
> >         };
> >         anx7625_out1: endpoint@1 {
> >             reg = <1>;
> >             mode-switch;
> >             remote-endpoint = <&c1_sstrx1>;
> >         };
> >     };
> > };
> >
> > // USB3 hub
> > usb3hub: foo_hub {
> >     ...
> >     ports@0 {
> >          // End point connected to USB3 host controller on SOC.
> >     };
> >     port@1 {
> >         reg = <1>;
> >
> >         foo_hub_out0: endpoint@0 {
> >             reg = <0>;
> >             mode-switch; ---> See c.) later
> >             remote-endpoint = <&c0_sstrx2>;
> >         };
> >         foo_hub_out1: endpoint@1 {
> >             reg = <1>;
> >             mode-switch;
> >             remote-endpoint = <&c1_sstrx2>;
> >         };
> >     };
> > };
> >
> > Notes:
> > - On the Chrome OS platform, the USB3 Hub is controlled by
> > the EC, so we don't really need to describe that connection,
> > but I've added a minimal one here just to show how the graph
> > connection would work if the HUB was controlled by the SoC.
> > - The above assumes that other hardware is controlling orientation.
> > We can add "orientation-switch" drivers along the graph path
> > if there is other hardware which controls orientation.
> >
> > Example 2: 1 USB-C connector connected to 1 drm-bridge/ mode-switch
> >
> > I've tried to use Bjorn's example [1], but I might have made
> > some mistakes since I don't have access to the schematic.
> >
> >
> >                   SoC
> >   +------------------------------------------+
> >   |                                          |
> >   |  +---------------+                       |
> >   |  |               |                       |
> >   |  |  DP ctrllr    |       +---------+     |                 C0
> >   |  |               +-------+         |     |   2 lane     +----------+
> >   |  +---------------+       |  QMP    +-----+-----/--------+ SSTRX1   |
> >   |                          |  PHY    |     |              |          |
> >   |  +-------------+  2 lane |         |     |   2 lane     |          |
> >   |  |             +----/----+         +-----+-----/--------+ SSTRX2   |
> >   |  |    dwc3     |         +---------+     |              |          |
> >   |  |             |                         |              |          |
> >   |  |             |         +---------+     |              |          |
> >   |  |             +---------+ HS PHY  |     |   HS lanes   |          |
> >   |  +-------------+         |         +-----+----/---------+ D +/-    |
> >   |                          |         |     |              +----------+
> >   |                          +---------+     |
> >   |                                          |
> >   +------------------------------------------+
> >
> > The DT would look something like this (borrowing from Stephen's example [2]):
> >
> > qmp {
> >     mode-switch; ----> See b.) later.
> >     orientation-switch;
> >     ports {
> >         qmp_usb_in: port@0 {
> >             reg = <0>;
> >             remote-endpoint = <&usb3_phy_out>;
> >         };
> >         qmp_dp_in: port@1 {
> >             reg = <1>;
> >             remote-endpoint = <&dp_phy_out>;
> >         };
> >         port@2 {
> >             reg = <2>;
> >             qmp_usb_dp_out0: endpoint@0 {
> >                 reg = <0>;
> >                 remote-endpoint = <&c0_sstrx1>;
> >             };
> >             qmp_usb_dp_out1: endpoint@1 {
> >                 reg = <1>;
> >                 remote-endpoint = <&c0_sstrx2>;
> >             };
> >         };
> > };
> >
> > dp-phy {
> >     ports {
> >         dp_phy_out: port {
> >             remote-endpoint = <&qmp_dp_in>;
> >         };
> >     };
> > };
> >
> > dwc3: usb-phy {
> >     ports {
> >         usb3_phy_out: port@0 {
> >             reg = <0>;
> >             remote-endpoint = <&qmp_usb_in>;
> >         };
> >     };
> > };
> >
> > glink {
> >     c0: usb-c-connector {
> >         compatible = "usb-c-connector";
> >         ports {
> >             hs: port@0 {
> >                 reg = <0>;
> >                 endpoint@0 {
> >                     reg = <0>;
> >                     remote-endpoint = <&hs_phy_out>;
> >                 };
> >             };
> >
> >             ss: port@1 {
> >                 reg = <1>;
> >                 c0_sstrx1: endpoint@0 {
> >                     reg = <0>;
> >                     remote-endpoint = <&qmp_usb_dp_out0>;
> >                 };
> >                 c0_sstrx2: endpoint@1 {
> >                     reg = <1>;
> >                     remote-endpoint = <&qmp_usb_dp_out1>;
> >                 };
> >             };
> >         };
> >     };
> > };
> >
> > Notes:
> > a. This proposal doesn't deal with the DRM bridge HPD forwarding; I
> > believe that is covered by Stephen's example/proposal in [2], and
> > can be addressed separately. That said, this binding is compatible
> > with the proposal in [2], that is, make the "mode-switch" driver a
> > drm-bridge and forward the HPD info to the upstream DRM-bridge (DP controller).
> > The driver implementing "mode-switch" will be able to do that, since
> > it gets DP status/attention VDOS with HPD info from the Type-C port driver.
> > b. If both SSTRX pairs from a connector are routed to the same
> > hardware block (example 2) then the device would keep "mode-switch"
> > as a top level property (and the fwnode associated with "mode-switch"
> > is the drm-bridge device).
> > c. If SSTRX pairs from 2 connectors are routed to the same
> > hardware block (example 1), then each end-point which is connected to
> > the USB-C connector will have a "mode-switch" property in its end-point.
> > There will be 2 mode switches registered here, and the fwnode for each
> > "mode-switch" is the end-point node.
> >
> > b.) and c.) can be handled by Type C mux registration and matching
> > code. We already have 3 mux devs for each mux [3].
> >
> > For the single mode-switch case, mux_dev[1] will just refer to the top-level
> > mode-switch registered by the DRM bridge / switch driver (example 1).
> > For the 2 mode-switch case, typec_mux_dev[1] will have 2 child
> > typec_mux_dev's, each of which represents the mode-switches
> > registered by the DRM bridge / switch driver. Introducing this
> > indirection means the port driver / alternate mode driver don't
> > need to care about how the connectors are routed; the framework
> > will just call the mux_set() function on the mux_dev() or its
> > children if it has any.
> >
> > The benefit of this approach is existing bindings (which just
> > assume 1 endpoint from usb-c-connector/port@1) should continue to
> > work without any changes.
> >
> > Why don't we use data lanes for the usb-c-connector
> > endpoints? I guess we could, but I am not a fan of adding the
> > extra data-lane parsing logic to the Type-C framework (I
> > don't think drivers need that level of detail from the connector
> > binding). And even then, we will still need an extra end-point
> > if the lanes of the USB-C connector are routed to different hardware blocks.
> >
> > The Type-C connector spec doesn't specify any alternate modes
> > with < 1 SSTRX pair, so the most we can ever have (short of a
> > major change to the spec) is 2 SSTRX end points for a
> > connector each being routed to different hardware blocks.
> > Codifying these as endpoint@0 and endpoint@1 in the usb-c-connector
> > binding seems to line up nicely with this detail of the spec.
> >
> > Thanks,
> >
> > -Prashant
> >
> > [1] https://lore.kernel.org/linux-usb/Yv1y9Wjp16CstJvK@baldur/
> > [2] https://lore.kernel.org/linux-usb/CAE-0n52-QVeUVCB1qZzPbYyrb1drrbJf6H2DEEW9bOE6mh7egw@mail.gmail.com/
> > [3] https://elixir.bootlin.com/linux/v6.0-rc3/source/drivers/usb/typec/mux.c#L259
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/typec-switch.yaml b/Documentation/devicetree/bindings/usb/typec-switch.yaml
new file mode 100644
index 000000000000..78b0190c8543
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
@@ -0,0 +1,74 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/typec-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: USB Type-C Switch
+
+maintainers:
+  - Prashant Malani <pmalani@chromium.org>
+
+description:
+  A USB Type-C switch represents a component which routes USB Type-C data
+  lines to various protocol host controllers (e.g USB, VESA DisplayPort,
+  Thunderbolt etc.) depending on which mode the Type-C port, port partner
+  and cable are operating in. It can also modify lane routing based on
+  the orientation of a connected Type-C peripheral.
+
+properties:
+  compatible:
+    const: typec-switch
+
+  mode-switch:
+    type: boolean
+    description: Specify that this switch can handle alternate mode switching.
+
+  orientation-switch:
+    type: boolean
+    description: Specify that this switch can handle orientation switching.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    description: OF graph binding modelling data lines to the Type-C switch.
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Link between the switch and a Type-C connector.
+
+    required:
+      - port@0
+
+required:
+  - compatible
+  - ports
+
+anyOf:
+  - required:
+      - mode-switch
+  - required:
+      - orientation-switch
+
+additionalProperties: true
+
+examples:
+  - |
+    drm-bridge {
+        usb-switch {
+            compatible = "typec-switch";
+            mode-switch;
+            orientation-switch;
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    anx_ep: endpoint {
+                        remote-endpoint = <&typec_controller>;
+                    };
+                };
+            };
+        };
+    };