[PATCH 0/2] usb: typec: mux: GPIO-based SBU mux

[PATCH 0/2] usb: typec: mux: GPIO-based SBU mux

[Bjorn Andersson]

A design found in various Qualcomm boards is to use a USB switch, controlled
through a pair of GPIO lines to connect, disconnect and switch the orientation
of the SBU lines in USB Type-C applications.

This series introduces a generic GPIO-driver for handling these designs.

Bjorn Andersson (2):
  dt-bindings: usb: Introduce GPIO-based SBU mux
  usb: typec: mux: Introduce GPIO-based SBU mux

 .../devicetree/bindings/usb/gpio-sbu-mux.yaml |  77 ++++++++
 drivers/usb/typec/mux/Kconfig                 |   6 +
 drivers/usb/typec/mux/Makefile                |   1 +
 drivers/usb/typec/mux/gpio-sbu-mux.c          | 171 ++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
 create mode 100644 drivers/usb/typec/mux/gpio-sbu-mux.c

-- 
2.35.1

[PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

Introduce a binding for GPIO-based mux hardware used for connecting,
disconnecting and switching orientation of the SBU lines in USB Type-C
applications.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml

diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
new file mode 100644
index 000000000000..7d8aca40c7ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: GPIO-based SBU mux
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description:
+  In USB Type-C applications the SBU lines needs to be connected, disconnected
+  and swapped depending on the altmode and orientation. This binding describes
+  a family of hardware which perform this based on GPIO controls.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - onnn,fsusb43l10x
+          - pericom,pi3usb102
+      - const: gpio-sbu-mux
+
+  enable-gpios:
+    description: Switch enable GPIO
+
+  select-gpios:
+    description: Orientation select
+
+  vcc-supply:
+    description: power supply
+
+  mode-switch:
+    description: Flag the port as possible handle of altmode switching
+    type: boolean
+
+  orientation-switch:
+    description: Flag the port as possible handler of orientation switching
+    type: boolean
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      A port node to link the SBU mux to a TypeC controller for the purpose of
+      handling altmode muxing and orientation switching.
+
+required:
+  - compatible
+  - enable-gpios
+  - select-gpios
+  - mode-switch
+  - orientation-switch
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    sbu-mux {
+      compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+      enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
+      select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
+
+      mode-switch;
+      orientation-switch;
+
+      port {
+        endpoint {
+          remote-endpoint = <&pmic_glink_dp0_sbu>;
+        };
+      };
+    };
+...
-- 
2.35.1

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Krzysztof Kozlowski]

On 10/08/2022 23:47, Bjorn Andersson wrote:
> Introduce a binding for GPIO-based mux hardware used for connecting,
> disconnecting and switching orientation of the SBU lines in USB Type-C
> applications.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> new file mode 100644
> index 000000000000..7d8aca40c7ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: GPIO-based SBU mux
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +description:
> +  In USB Type-C applications the SBU lines needs to be connected, disconnected
> +  and swapped depending on the altmode and orientation. This binding describes
> +  a family of hardware which perform this based on GPIO controls.

+Cc few folks.

This looks familiar to:

https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c

Rob and Stephen had several concerns about that approach.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Rob Herring]

On Thu, Aug 11, 2022 at 12:14:48PM +0300, Krzysztof Kozlowski wrote:
> On 10/08/2022 23:47, Bjorn Andersson wrote:
> > Introduce a binding for GPIO-based mux hardware used for connecting,
> > disconnecting and switching orientation of the SBU lines in USB Type-C
> > applications.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > new file mode 100644
> > index 000000000000..7d8aca40c7ca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: GPIO-based SBU mux
> > +
> > +maintainers:
> > +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> > +
> > +description:
> > +  In USB Type-C applications the SBU lines needs to be connected, disconnected
> > +  and swapped depending on the altmode and orientation. This binding describes
> > +  a family of hardware which perform this based on GPIO controls.
> 
> +Cc few folks.
> 
> This looks familiar to:
> 
> https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c
> 
> Rob and Stephen had several concerns about that approach.

My overall concern is a bunch of one-off bindings with no one thinking 
about a variety of USB-C h/w. I need h/w diagrams and corresponding 
bindings. The key part being more than 1. I'm not all that familiar with 
the former to help on the bindings.

Rob

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Sun 14 Aug 16:01 CDT 2022, Rob Herring wrote:

> On Thu, Aug 11, 2022 at 12:14:48PM +0300, Krzysztof Kozlowski wrote:
> > On 10/08/2022 23:47, Bjorn Andersson wrote:
> > > Introduce a binding for GPIO-based mux hardware used for connecting,
> > > disconnecting and switching orientation of the SBU lines in USB Type-C
> > > applications.
> > > 
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > >  .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++
> > >  1 file changed, 77 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > > new file mode 100644
> > > index 000000000000..7d8aca40c7ca
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> > > @@ -0,0 +1,77 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: GPIO-based SBU mux
> > > +
> > > +maintainers:
> > > +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> > > +
> > > +description:
> > > +  In USB Type-C applications the SBU lines needs to be connected, disconnected
> > > +  and swapped depending on the altmode and orientation. This binding describes
> > > +  a family of hardware which perform this based on GPIO controls.
> > 
> > +Cc few folks.
> > 
> > This looks familiar to:
> > 
> > https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c
> > 
> > Rob and Stephen had several concerns about that approach.
> 
> My overall concern is a bunch of one-off bindings with no one thinking 
> about a variety of USB-C h/w. I need h/w diagrams and corresponding 
> bindings. The key part being more than 1. I'm not all that familiar with 
> the former to help on the bindings.
> 

This is the setup that we're dealing with:

                     +------------- - -
 USB connector       | SoC
 +-+                 |
 | |                 |   +-----+
 |*|<------- HS -----|-->| HS  |
 |*|<------- HS -----|-->| phy |<-+   +--------+
 | |                 |   +-----+   \->|        |
 | |                 |                |  dwc3  |
 | |                 |   +-----+   /->|        |
 |*|<------- SS -----|-->|     |<-+   +--------+
 |*|<------- SS -----|-->| QMP |
 |*|<------- SS -----|-->| phy |
 |*|<------- SS -----|-->|     |<-\   +------------+
 | |                 |   +-----+   \->|            |
 | |                 |                |     DP     |
 | |    +-----+      |                | controller |
 |*|<-->| SBU |<-----|--------------->|            |
 |*|<-->| mux |<-----|--------------->|            |
 | |    +----+       |                +------------+
 +-+                 |
                     +------------- - -

The dwc3 controller is connected to the HS phy for HighSpeed signals and
QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for
DP-only, USB/DP combo or USB-only mode).

The DisplayPort controller is connected to the same QMP phy, for and is
muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only,
USB/DP combo or DP-only mode).

The SuperSpeed pins can be switched around within the QMP phy, to handle
the case where the USB Type-C cable is flipped around.


The AUX pins of the DP controller are connected to the SBU pins in the
connector. These signals needs to be disconnected while DP mode is not
negotiated with the remote. The DP controller does not support swapping
the two lines.
The disconnecting and swapping thereby needs to be performed by an
external entity. For which we have a few examples already, such as
fcs,fsa4480.

Lastly, in USB Power Delivery, the hot-plug signal found in a physical
DisplayPort or HDMI cable is communicated as a message. So the USB
Type-C controller must be able to pass this onto the DP controller.


I model the usb-c-connector as a child of the USB Type-C controller,
with the following representation of the connections:

connector {
  compatible = "usb-c-connector";

  ports {
    port@0 {
      reg = <0>;
      endpoint {
        remote-endpoint = <&dwc3>;
      };
    };

    port@1 {
      reg = <1>;
      endpoint@0 {
        reg = <0>;
        remote-endpoint = <&qmp_phy>;
      };
      endpoint@1 {
        reg = <1>;
        remote-endpoint = <&dp_controller>;
    };

    port@2 {
      reg = <2>;
      endpoint {
        remote-endpoint = <&sbu_mux>;
      };
    };
  };
};

This allows the USB Type-C controller to:
1) Perform USB role switching in the dwc3 on port@0
2) Orientation and muxing of the SuperSpeed lines in the QMP phy on
   port@1:0, implement a drm_bridge for signalling HPD back to the DP
   controller on port@1:1
3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines
   in the SBU mux on port@2.

The SBU mux in several of these designs is a component that takes a
power supply and two GPIOs, for enabling/disabling the connection and
flip the switch (which is used to swap the lines).

I hope this helps with the bigger picture.

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Stephen Boyd]

Quoting Bjorn Andersson (2022-08-17 16:00:05)
>
> This is the setup that we're dealing with:
>
>                      +------------- - -
>  USB connector       | SoC
>  +-+                 |
>  | |                 |   +-----+
>  |*|<------- HS -----|-->| HS  |
>  |*|<------- HS -----|-->| phy |<-+   +--------+
>  | |                 |   +-----+   \->|        |
>  | |                 |                |  dwc3  |
>  | |                 |   +-----+   /->|        |
>  |*|<------- SS -----|-->|     |<-+   +--------+
>  |*|<------- SS -----|-->| QMP |
>  |*|<------- SS -----|-->| phy |
>  |*|<------- SS -----|-->|     |<-\   +------------+
>  | |                 |   +-----+   \->|            |
>  | |                 |                |     DP     |
>  | |    +-----+      |                | controller |
>  |*|<-->| SBU |<-----|--------------->|            |
>  |*|<-->| mux |<-----|--------------->|            |
>  | |    +----+       |                +------------+
>  +-+                 |
>                      +------------- - -
>
> The dwc3 controller is connected to the HS phy for HighSpeed signals and
> QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for
> DP-only, USB/DP combo or USB-only mode).
>
> The DisplayPort controller is connected to the same QMP phy, for and is
> muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only,
> USB/DP combo or DP-only mode).
>
> The SuperSpeed pins can be switched around within the QMP phy, to handle
> the case where the USB Type-C cable is flipped around.
>
>
> The AUX pins of the DP controller are connected to the SBU pins in the
> connector. These signals needs to be disconnected while DP mode is not
> negotiated with the remote. The DP controller does not support swapping
> the two lines.
> The disconnecting and swapping thereby needs to be performed by an
> external entity. For which we have a few examples already, such as
> fcs,fsa4480.

By swapping you mean making SBU1 in the usb-c-connector be SBU2 and SBU2
in the usb-c-connector be SBU1 to the DP controller, right? For the case
when the cable is flipped.

>
> Lastly, in USB Power Delivery, the hot-plug signal found in a physical
> DisplayPort or HDMI cable is communicated as a message. So the USB
> Type-C controller must be able to pass this onto the DP controller.
>
>
> I model the usb-c-connector as a child of the USB Type-C controller,
> with the following representation of the connections:


>
> connector {
>   compatible = "usb-c-connector";
>
>   ports {
>     port@0 {
>       reg = <0>;
>       endpoint {
>         remote-endpoint = <&dwc3>;
>       };
>     };
>
>     port@1 {
>       reg = <1>;
>       endpoint@0 {
>         reg = <0>;
>         remote-endpoint = <&qmp_phy>;
>       };
>       endpoint@1 {
>         reg = <1>;
>         remote-endpoint = <&dp_controller>;
>     };

I'd expect the QMP phy to physically be the only thing connected on the
board. That matches the block diagram above. Inside the SoC the SS lines
will be muxed through the QMP phy to the DP controller or the USB
controller. Therefore, in the binding I'd expect there to be a single
port@1 for the connector:

	port@1 {
		reg = <1>;
		endpoint@0 {
			reg = <0>;
			remote-endpoint = <&qmp_phy>;
		};
	};

Somewhere above we would want 'data-lanes' to indicate how many SS lanes
are connected between the connector and the phy and if any of those
lanes need to be remapped in the phy.

	port@1 {
		reg = <1>;
		endpoint@0 {
			reg = <0>;
			remote-endpoint = <&qmp_phy>;
			data-lanes = <0 1 2 3>;
		};
	};

This would be the block diagram configuration, but it doesn't seem
right.

Other designs only connect two lanes to the qmp phy and the other two
connect to a USB hub. That's where it gets interesting because we don't
know how to represent that. Do we make two endpoints in the
usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS
RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we
make two endpoints then do we need to have two endpoints all the time
even though in this 4 SS line design it is redundant?

	port@1 {
		reg = <1>;
		endpoint@0 { // Represents RX1/TX1
			reg = <0>;
			remote-endpoint = <&qmp_phy_lanes01>;
		};
		endpoint@1 { // Represents RX2/TX2
			reg = <1>;
			remote-endpoint = <&qmp_phy_lanes23>;
		};
	};

I think we may need to always have two endpoints in the usb-c-connector
because data-lanes alone can't always handle it for us. Especially when
you consider the hub and DP phy designs.

	port@1 {
		reg = <1>;
		endpoint@0 { // Represents RX1/TX1
			reg = <0>;
			remote-endpoint = <&usb3_hub_portN>;
		};
		endpoint@1 { // Represents RX2/TX2
			reg = <1>;
			remote-endpoint = <&qmp_phy_lanes23>;
		};
	};

The remote-endpoint phandle is different, so data-lanes can't save us. I
suspect putting data-lanes in the usb-c-connector is really just
nonsense too, because the usb-c-connector is a dumb devicenode and it
can't actively change lane routing. The endpoint that's connected should
have the data-lanes property if for example, RX2 is connected to the
phy's TX2 pins and TX2 is connected to the phy's RX2 pins. Then the
driver for that endpoint can change the lane mapping at runtime to
present the proper data on the right pins in the connector.

>
>     port@2 {
>       reg = <2>;
>       endpoint {
>         remote-endpoint = <&sbu_mux>;
>       };
>     };
>   };
> };
>
> This allows the USB Type-C controller to:
> 1) Perform USB role switching in the dwc3 on port@0
> 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on
>    port@1:0, implement a drm_bridge for signalling HPD back to the DP
>    controller on port@1:1

We may need to have a port connection from the DP controller to the QMP
phy. But given that the DP controller already has a 'phys' phandle to
the QMP phy I think the DP controller driver could try to link to a drm
bridge created in the phy driver that mainly handles the HPD signaling
and any lane muxing/routing that needs to happen when DP pin
configuration is present.

> 3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines
>    in the SBU mux on port@2.
>
> The SBU mux in several of these designs is a component that takes a
> power supply and two GPIOs, for enabling/disabling the connection and
> flip the switch (which is used to swap the lines).

The SBU mux sounds OK to me. I don't think the SBU lines can be split
up, they're a pair that has to go together, just like CC lines and SS
pairs are in the typec spec. Of course, in DP spec we can have a single
DP lane which corresponds to a single RX or TX differential pair, but
with typec that isn't possible, we always have RX and TX together.

This actually brings up one final point. On the QMP phy we can reorder
the lanes willy nilly from what I recall, so that RX1 could be RX2 and
TX1 could be TX2. In such a design, we would have two ports in the qmp
phy to connect to the two TX/RX pairs in the connector, but then we
would need to tell the QMP phy that the lanes are all shifted.

	qmp_phy {
		ports {
			port@0 {
				reg = <0>;
				endpoint@0 {
					reg = <0>;
					remote-endpoint = <&usb_c_txrx1>;
					data-lanes = <1 2>
				};
				endpoint@1 {
					reg = <1>;
					remote-endpoint = <&usb_c_txrx2>;
					data-lanes = <3 0>;
				};
			};
		};
	};

This would do that for us, but when all four lanes are connected from
the qmp phy directly to the connector we could just as easily have done
it with one endpoint.

	qmp_phy {
		ports {
			port@0 {
				reg = <0>;
				endpoint@0 {
					reg = <0>;
					remote-endpoint = <&usb_c_ss>;
					data-lanes = <1 2 3 0>
				};
			};
		};
	};

So should we explicitly have two endpoints in the usb-c-connector for
the two pairs all the time, or should we represent that via data-lanes
and only split up the connector's endpoint if we need to connect the
usb-c-connector to two different endpoints?

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Prashant Malani]

> This would do that for us, but when all four lanes are connected from
> the qmp phy directly to the connector we could just as easily have done
> it with one endpoint.
>
>         qmp_phy {
>                 ports {
>                         port@0 {
>                                 reg = <0>;
>                                 endpoint@0 {
>                                         reg = <0>;
>                                         remote-endpoint = <&usb_c_ss>;
>                                         data-lanes = <1 2 3 0>
>                                 };
>                         };
>                 };
>         };
>
> So should we explicitly have two endpoints in the usb-c-connector for
> the two pairs all the time, or should we represent that via data-lanes
> and only split up the connector's endpoint if we need to connect the
> usb-c-connector to two different endpoints?

I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
to be compatible (without introducing `data-lanes`, at least) with all
the various
combinations on the remote side, if that remote side is a DRM bridge with DP
output capability (like it6505 or anx7625).
That type of DRM bridge supports 1, 2 or 4 lane DP connections.

So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
That should support every conceivable configuration and bridge/PHY hardware.
and also allows a way to specify any lane remapping (similar to what
"data lanes" does)
if that is required.
Then we are consistent with what an endpoint represents, regardless of whether
the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector  (2
or 4 lane) on its output side.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Stephen Boyd]

Quoting Prashant Malani (2022-08-19 13:14:25)
> > This would do that for us, but when all four lanes are connected from
> > the qmp phy directly to the connector we could just as easily have done
> > it with one endpoint.
> >
> >         qmp_phy {
> >                 ports {
> >                         port@0 {
> >                                 reg = <0>;
> >                                 endpoint@0 {
> >                                         reg = <0>;
> >                                         remote-endpoint = <&usb_c_ss>;
> >                                         data-lanes = <1 2 3 0>
> >                                 };
> >                         };
> >                 };
> >         };
> >
> > So should we explicitly have two endpoints in the usb-c-connector for
> > the two pairs all the time, or should we represent that via data-lanes
> > and only split up the connector's endpoint if we need to connect the
> > usb-c-connector to two different endpoints?
>
> I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
> to be compatible (without introducing `data-lanes`, at least) with all
> the various
> combinations on the remote side, if that remote side is a DRM bridge with DP
> output capability (like it6505 or anx7625).
> That type of DRM bridge supports 1, 2 or 4 lane DP connections.

Why can't the remote side that's a pure DP bridge (it6505) bundle
however many lanes it wants into one endpoint? If it's a pure DP bridge
we should design the bridge binding to have up to 4 endpoints, but
sometimes 2 or 1 and then overlay data-lanes onto that binding so that
we can tell the driver how to remap the lanes if it can. If the hardware
can't support remapping lanes then data-lanes shouldn't be in the
binding.

>
> So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
> That should support every conceivable configuration and bridge/PHY hardware.
> and also allows a way to specify any lane remapping (similar to what
> "data lanes" does)
> if that is required.
> Then we are consistent with what an endpoint represents, regardless of whether
> the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector  (2
> or 4 lane) on its output side.

I'd like to think in terms of the usb-c-connector, where the DP altmode
doesn't support one lane of DP and USB is only carried across two SS
lanes. Essentially, two SS lanes are always together, hence the idea
that we should have two endpoints in the SS port@1. In the simple case
above it seems we can get away with only one endpoint in the SS port@1
which is probably fine? I just don't know how that is represented in the
schema, but I suspect making another endpoint optional in the SS port@1
is the way to go.

Will there ever be a time when all 4 usb-c-connector remote-endpoint
phandles point to endpoints that are child nodes of different ports
(i.e. different qmp_phy nodes) with a 4 endpoint schema? I don't think
that is possible, so 4 endpoints is flexible but also verbose. It also
means we would have to walk the endpoints to figure out lane remapping,
wheres we can simply find the endpoint in the DP bridge ports and look
at data-lanes directly.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote:

> Quoting Prashant Malani (2022-08-19 13:14:25)
> > > This would do that for us, but when all four lanes are connected from
> > > the qmp phy directly to the connector we could just as easily have done
> > > it with one endpoint.
> > >
> > >         qmp_phy {
> > >                 ports {
> > >                         port@0 {
> > >                                 reg = <0>;
> > >                                 endpoint@0 {
> > >                                         reg = <0>;
> > >                                         remote-endpoint = <&usb_c_ss>;
> > >                                         data-lanes = <1 2 3 0>
> > >                                 };
> > >                         };
> > >                 };
> > >         };
> > >
> > > So should we explicitly have two endpoints in the usb-c-connector for
> > > the two pairs all the time, or should we represent that via data-lanes
> > > and only split up the connector's endpoint if we need to connect the
> > > usb-c-connector to two different endpoints?
> >
> > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
> > to be compatible (without introducing `data-lanes`, at least) with all
> > the various
> > combinations on the remote side, if that remote side is a DRM bridge with DP
> > output capability (like it6505 or anx7625).
> > That type of DRM bridge supports 1, 2 or 4 lane DP connections.
> 
> Why can't the remote side that's a pure DP bridge (it6505) bundle
> however many lanes it wants into one endpoint? If it's a pure DP bridge
> we should design the bridge binding to have up to 4 endpoints, but
> sometimes 2 or 1 and then overlay data-lanes onto that binding so that
> we can tell the driver how to remap the lanes if it can. If the hardware
> can't support remapping lanes then data-lanes shouldn't be in the
> binding.
> 

I don't see why we would want to further complicate the of_graph by
representing each signal pair between two fixed endpoint as individual
endpoints.

Let's describe the connections between the components, not the signals.

> >
> > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
> > That should support every conceivable configuration and bridge/PHY hardware.
> > and also allows a way to specify any lane remapping (similar to what
> > "data lanes" does)
> > if that is required.
> > Then we are consistent with what an endpoint represents, regardless of whether
> > the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector  (2
> > or 4 lane) on its output side.
> 
> I'd like to think in terms of the usb-c-connector, where the DP altmode
> doesn't support one lane of DP and USB is only carried across two SS
> lanes. Essentially, two SS lanes are always together, hence the idea
> that we should have two endpoints in the SS port@1. In the simple case
> above it seems we can get away with only one endpoint in the SS port@1
> which is probably fine? I just don't know how that is represented in the
> schema, but I suspect making another endpoint optional in the SS port@1
> is the way to go.
> 
> Will there ever be a time when all 4 usb-c-connector remote-endpoint
> phandles point to endpoints that are child nodes of different ports
> (i.e. different qmp_phy nodes) with a 4 endpoint schema? I don't think
> that is possible, so 4 endpoints is flexible but also verbose. It also
> means we would have to walk the endpoints to figure out lane remapping,
> wheres we can simply find the endpoint in the DP bridge ports and look
> at data-lanes directly.

The existing implementation provides the interfaces usb_role_switch,
usb_typec_mux and usb_typec_switch. These works based on the concept
that the USB Type-C controller will request the endpoints connected to
the usb-c-connector about changes such as "switch to host mode", "switch
to 2+2 USB/DP combo" and "switch orientation to reverse". We use this
same operations to inform any endpoint at any port about these events
and they all react accordingly.

Perhaps I'm misunderstanding your suggestion, but if you start
representing each individual lane in the SuperSpeed interface I believe
you would have to just abandon this interface and replace it with
something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1
and give me DP on port@1/endpoint@2 and port@1/endpoint@3".

I presume that such an interface would work, but I'm afraid I don't see
the merits of it and we would have to replace the Linux implementation.

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Prashant Malani]

On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote:
>
> > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
> > > to be compatible (without introducing `data-lanes`, at least) with all
> > > the various
> > > combinations on the remote side, if that remote side is a DRM bridge with DP
> > > output capability (like it6505 or anx7625).
> > > That type of DRM bridge supports 1, 2 or 4 lane DP connections.
> >
> > Why can't the remote side that's a pure DP bridge (it6505) bundle
> > however many lanes it wants into one endpoint? If it's a pure DP bridge
> > we should design the bridge binding to have up to 4 endpoints, but
> > sometimes 2 or 1 and then overlay data-lanes onto that binding so that
> > we can tell the driver how to remap the lanes if it can. If the hardware
> > can't support remapping lanes then data-lanes shouldn't be in the
> > binding.

2 endpoints sounds fine to me. The overloading of the bridge-side endpoint
to mean different things depending on what it's connected to seemed odd to
me, but if that is acceptable for the bridge binding, then great.

> The existing implementation provides the interfaces usb_role_switch,
> usb_typec_mux and usb_typec_switch. These works based on the concept
> that the USB Type-C controller will request the endpoints connected to
> the usb-c-connector about changes such as "switch to host mode", "switch
> to 2+2 USB/DP combo" and "switch orientation to reverse". We use this
> same operations to inform any endpoint at any port about these events
> and they all react accordingly.

Right, but that implementation/assumption doesn't work so well when you
have 2 Type-C ports which might route to the same bridge (2 lane from each).
The other 2 lanes from the other endpoints can go to (say) a USB HUB.

>
> Perhaps I'm misunderstanding your suggestion, but if you start
> representing each individual lane in the SuperSpeed interface I believe
> you would have to just abandon this interface and replace it with
> something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1
> and give me DP on port@1/endpoint@2 and port@1/endpoint@3".

I don't think that is necessary. The switch driver can register the switches (
and it can find out which end-points map to the same usb-c-connector).

From the port driver, the port driver just needs to tell each switch
registered for it's port that "I want
DP Pin assignment C/ DP Pin assignment D / Plain USB3.x" and the
switch driver(s) can figure out what to output on its pins (since
the Type-C binding will specify ep0 = A2-A3 (TX1), ep1 = B10-B11 , etc)

orientation-switch can tell the switch if the signals need to be swapped around.

The above notwithstanding, it sounds like the 2-ep approach has more support
than 4 ep-approach, so this specific example is moot.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Fri 19 Aug 17:18 CDT 2022, Prashant Malani wrote:

> On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote:
> >
> > > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
> > > > to be compatible (without introducing `data-lanes`, at least) with all
> > > > the various
> > > > combinations on the remote side, if that remote side is a DRM bridge with DP
> > > > output capability (like it6505 or anx7625).
> > > > That type of DRM bridge supports 1, 2 or 4 lane DP connections.
> > >
> > > Why can't the remote side that's a pure DP bridge (it6505) bundle
> > > however many lanes it wants into one endpoint? If it's a pure DP bridge
> > > we should design the bridge binding to have up to 4 endpoints, but
> > > sometimes 2 or 1 and then overlay data-lanes onto that binding so that
> > > we can tell the driver how to remap the lanes if it can. If the hardware
> > > can't support remapping lanes then data-lanes shouldn't be in the
> > > binding.
> 
> 2 endpoints sounds fine to me. The overloading of the bridge-side endpoint
> to mean different things depending on what it's connected to seemed odd to
> me, but if that is acceptable for the bridge binding, then great.
> 
> > The existing implementation provides the interfaces usb_role_switch,
> > usb_typec_mux and usb_typec_switch. These works based on the concept
> > that the USB Type-C controller will request the endpoints connected to
> > the usb-c-connector about changes such as "switch to host mode", "switch
> > to 2+2 USB/DP combo" and "switch orientation to reverse". We use this
> > same operations to inform any endpoint at any port about these events
> > and they all react accordingly.
> 
> Right, but that implementation/assumption doesn't work so well when you
> have 2 Type-C ports which might route to the same bridge (2 lane from each).
> The other 2 lanes from the other endpoints can go to (say) a USB HUB.
> 

Are you saying that two of your SS-lanes in connector A are connected to
directly to the QMP PHY at the same time as two SS-lanes from connector
B are connected to the same two pads on the QMP PHY - without any
mux etc inbetween?

I.e. that there are a set of pins in connector A which is directly
connected to a set of pins in connector B?


I was under the impression that in your hardware there's some component
muxing the single DP output to one of the connectors. If so there should
be no graph-link directly connecting the two usb-c-connectors and the
one QMP PHY.

Is this not the case?

> >
> > Perhaps I'm misunderstanding your suggestion, but if you start
> > representing each individual lane in the SuperSpeed interface I believe
> > you would have to just abandon this interface and replace it with
> > something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1
> > and give me DP on port@1/endpoint@2 and port@1/endpoint@3".
> 
> I don't think that is necessary. The switch driver can register the switches (
> and it can find out which end-points map to the same usb-c-connector).
> 
> From the port driver, the port driver just needs to tell each switch
> registered for it's port that "I want
> DP Pin assignment C/ DP Pin assignment D / Plain USB3.x" and the
> switch driver(s) can figure out what to output on its pins (since
> the Type-C binding will specify ep0 = A2-A3 (TX1), ep1 = B10-B11 , etc)
> 
> orientation-switch can tell the switch if the signals need to be swapped around.
> 

In a typical Qualcomm design the QMP PHY is directly connected to the
usb-c-connector and as such it is the component that implements
usb_typec_mux and usb_typec_switch.

Regards,
Bjorn

> The above notwithstanding, it sounds like the 2-ep approach has more support
> than 4 ep-approach, so this specific example is moot.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Prashant Malani]

On Fri, Aug 19, 2022 at 9:04 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Fri 19 Aug 17:18 CDT 2022, Prashant Malani wrote:
>
> > On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
>
> Are you saying that two of your SS-lanes in connector A are connected to
> directly to the QMP PHY at the same time as two SS-lanes from connector
> B are connected to the same two pads on the QMP PHY - without any
> mux etc inbetween?
>
> I.e. that there are a set of pins in connector A which is directly
> connected to a set of pins in connector B?
>
>
> I was under the impression that in your hardware there's some component
> muxing the single DP output to one of the connectors. If so there should
> be no graph-link directly connecting the two usb-c-connectors and the
> one QMP PHY.
>
> Is this not the case?

I can't speak to the QMP PHY specifically (since I'm not using that hardware),
but your impression is right.
There is a component (anx7625) muxing the single DP output to the 2
usb-c-connectors
(specifically, 2 lanes each from the 2 usb-c-connectors).
The other 2 lanes (from the 2 USB-C-connectors) go to a USB3 hub; hence the need
for 2 endpoints for each usb-c-connector).

So,  the anx7625 should register the mode switches and it needs the
graph connections
from 2 usb-c-connectors

BR,

-Prashant

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Fri 19 Aug 15:14 CDT 2022, Prashant Malani wrote:

> > This would do that for us, but when all four lanes are connected from
> > the qmp phy directly to the connector we could just as easily have done
> > it with one endpoint.
> >
> >         qmp_phy {
> >                 ports {
> >                         port@0 {
> >                                 reg = <0>;
> >                                 endpoint@0 {
> >                                         reg = <0>;
> >                                         remote-endpoint = <&usb_c_ss>;
> >                                         data-lanes = <1 2 3 0>
> >                                 };
> >                         };
> >                 };
> >         };
> >
> > So should we explicitly have two endpoints in the usb-c-connector for
> > the two pairs all the time, or should we represent that via data-lanes
> > and only split up the connector's endpoint if we need to connect the
> > usb-c-connector to two different endpoints?
> 
> I like 2 endpoints to represent the usb-c-connector, but that doesn't seem
> to be compatible (without introducing `data-lanes`, at least) with all
> the various
> combinations on the remote side, if that remote side is a DRM bridge with DP
> output capability (like it6505 or anx7625).
> That type of DRM bridge supports 1, 2 or 4 lane DP connections.
> 

You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to
your usb-c-connector at the same time as you physically connect 0, 2 or
4 lanes of USB from a USB PHY.

You must either have another component inbetween, or you will connect
some predefined subset of those signals to each output.

In the case where you have a mux of some sort inbetween, that would be
the thing that the usb-c-connector's port@1/endpoint references.

In the case that you hardwire 2 SS lanes to USB and 2 to the DP
hardware, you could specify port@1 with two endpoints and the Type-C
controller would be able to signal both when to turn on/off their
signals. But you wouldn't be able to do orientation switching.

> So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
> That should support every conceivable configuration and bridge/PHY hardware.
> and also allows a way to specify any lane remapping (similar to what
> "data lanes" does)
> if that is required.

Wouldn't that prevent you from handling orientation switching, given
that the graph is static?

> Then we are consistent with what an endpoint represents, regardless of whether
> the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector  (2
> or 4 lane) on its output side.

We can represent that perfectly fine with the proposed bindings.
In the USB Type-C case I have:

dp-controller {
    phys = <&qmp>;

    ports {
       dp_hpd: port@1 {
            endpoint = <&port_1_endpoint_1>;
        };
    };
};

qmp: qmp {
    port {
        qmp_out: endpoint {
            remote-endpoint = <&port_1_endpoint_0>;
        };
    };
};

connector {
    compatible = "usb-c-connector";
    ports {
        port@1 {
            port_1_endpoint_0: endpoint@0 {
                remote-endpoint = <&qmp_out>;
            };
            port_1_endpoint_1: endpoint@1 {
                remote-endpoint = <&dp_hpd>;
            };
        };
    };
};

The dp-controller binding is defined to have the output on port@1 and by
implementing a drm_bridge in the controller backing the connector it
will find that. The controller can use the links to inform the QMP about
muxing and orientation switching.

In the case of DP we have:

dp-controller {
    phys = <&dp_phy>;

    ports {
       dp_hpd: port@1 {
            endpoint = <&dp_connector>;
        };
    };
};

dp_phy: dp-phy {
    compatible = "qcom,dp-phy";
};

connector {
    compatible = "dp-connector";
    port {
        dp_connector: endpoint@0 {
            remote-endpoint = <&dp_hpd>;
        };
    };
};


The link between the dp_phy and the dp connector could be expressed
further, but this is a binding that already exists...

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Prashant Malani]

On Fri, Aug 19, 2022 at 3:01 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
>
> You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to
> your usb-c-connector at the same time as you physically connect 0, 2 or
> 4 lanes of USB from a USB PHY.

I apologize in case I'm misunderstanding, but why is that not possible?
anx7625 allows that configuration (2 lane DP + 2 lane USB going to
a single USB-C-connector)

Since the discussion is to support various conceivable hardware configurations
That same anx7625 can support 1 1-lane DP (or 2 1-lane DPs), and 1
2-lane Type-C output.
The cross-point switch allows for that level of configuration.

> > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
> > That should support every conceivable configuration and bridge/PHY hardware.
> > and also allows a way to specify any lane remapping (similar to what
> > "data lanes" does)
> > if that is required.
>
> Wouldn't that prevent you from handling orientation switching, given
> that the graph is static?

It depends. If the end-points from the usb-c-connector
go to the same switch, then it should allow orientation switching
(anx7625 allows
this). The port driver would just tell the orientation switch(es) attached to
it that we are in NORMAL or REVERSE orientation.

The graph is static, since the hardware line routing between components
doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin
X1,X2 on the switch hardware), but that is what the switch is for.
Note that in this case, the expectation is that
the switch driver only registers 1 switch (it can figure out that all
4 endpoints
go to the same Type-C port).

The limitation to orientation switching would depend on how the
hardware is routed.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Fri 19 Aug 17:55 CDT 2022, Prashant Malani wrote:

> On Fri, Aug 19, 2022 at 3:01 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> >
> > You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to
> > your usb-c-connector at the same time as you physically connect 0, 2 or
> > 4 lanes of USB from a USB PHY.
> 
> I apologize in case I'm misunderstanding, but why is that not possible?
> anx7625 allows that configuration (2 lane DP + 2 lane USB going to
> a single USB-C-connector)
> 

Right, but you can not connect 4 lanes DP from one chip at the same time
that you connect 4 lanes USB from another chip.

> Since the discussion is to support various conceivable hardware configurations
> That same anx7625 can support 1 1-lane DP (or 2 1-lane DPs), and 1
> 2-lane Type-C output.
> The cross-point switch allows for that level of configuration.
> 

We're talking about the static configuration here, where you describe
which component are connected together. We can not dynamically switch
the Devicetree representation around to match what the Type-C controller
negotiates.

> > > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1?
> > > That should support every conceivable configuration and bridge/PHY hardware.
> > > and also allows a way to specify any lane remapping (similar to what
> > > "data lanes" does)
> > > if that is required.
> >
> > Wouldn't that prevent you from handling orientation switching, given
> > that the graph is static?
> 
> It depends. If the end-points from the usb-c-connector
> go to the same switch, then it should allow orientation switching
> (anx7625 allows
> this). The port driver would just tell the orientation switch(es) attached to
> it that we are in NORMAL or REVERSE orientation.
> 

But why do you need to express the relationship between these 2
components with > 1 link in the graph?

> The graph is static, since the hardware line routing between components
> doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin
> X1,X2 on the switch hardware), but that is what the switch is for.
> Note that in this case, the expectation is that
> the switch driver only registers 1 switch (it can figure out that all
> 4 endpoints
> go to the same Type-C port).
> 

Why do we need to express this with 4 endpoints and then implement code
to discover that the 4 endpoints points to the same remote? Why not just
describe the logical relationship between the two components in one
endpoint reference?

> The limitation to orientation switching would depend on how the
> hardware is routed.

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Prashant Malani]

On Fri, Aug 19, 2022 at 9:09 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
>
> We're talking about the static configuration here, where you describe
> which component are connected together. We can not dynamically switch
> the Devicetree representation around to match what the Type-C controller
> negotiates.

Yes, but we don't need to switch the device tree representation at all.
The pin routing/connections from the connector (not the cable or the partner),
to the muxing hardware (QMP phy or anx7625) remains fixed always
The port driver tells what orientation the peripheral is connected with,
and the muxing/orientation hardware routes the signals according to that.

>
> But why do you need to express the relationship between these 2
> components with > 1 link in the graph?
>
> > The graph is static, since the hardware line routing between components
> > doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin
> > X1,X2 on the switch hardware), but that is what the switch is for.
> > Note that in this case, the expectation is that
> > the switch driver only registers 1 switch (it can figure out that all
> > 4 endpoints
> > go to the same Type-C port).
> >
>
> Why do we need to express this with 4 endpoints and then implement code
> to discover that the 4 endpoints points to the same remote? Why not just
> describe the logical relationship between the two components in one
> endpoint reference?

The issue I see is with the "supplier" side of that graph relationship
(i.e the DRM bridge side).
Since the bridge can be directly connected to a DP panel, the
endpoints can (technically)
represent a single DP lane. So, using 4 end-points for the
usb-c-connector port@1 gives
us something which is compatible with the bridge side endpoints too
(regardless of what
the bridge is connected to on the "output" side).
Reading the discussion, I agree 4 lanes is over-specifying, and 2
endpoints is probably
enough (especially if we can use data-lanes on the bridge side
to define the number of lanes if needed for DP panel connections).

BR,

-Prashant

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Tue, Aug 23, 2022 at 11:54:53AM -0700, Prashant Malani wrote:
> On Fri, Aug 19, 2022 at 9:09 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> >
> > We're talking about the static configuration here, where you describe
> > which component are connected together. We can not dynamically switch
> > the Devicetree representation around to match what the Type-C controller
> > negotiates.
> 
> Yes, but we don't need to switch the device tree representation at all.
> The pin routing/connections from the connector (not the cable or the partner),
> to the muxing hardware (QMP phy or anx7625) remains fixed always
> The port driver tells what orientation the peripheral is connected with,
> and the muxing/orientation hardware routes the signals according to that.
> 
> >
> > But why do you need to express the relationship between these 2
> > components with > 1 link in the graph?
> >
> > > The graph is static, since the hardware line routing between components
> > > doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin
> > > X1,X2 on the switch hardware), but that is what the switch is for.
> > > Note that in this case, the expectation is that
> > > the switch driver only registers 1 switch (it can figure out that all
> > > 4 endpoints
> > > go to the same Type-C port).
> > >
> >
> > Why do we need to express this with 4 endpoints and then implement code
> > to discover that the 4 endpoints points to the same remote? Why not just
> > describe the logical relationship between the two components in one
> > endpoint reference?
> 
> The issue I see is with the "supplier" side of that graph relationship
> (i.e the DRM bridge side).
> Since the bridge can be directly connected to a DP panel, the
> endpoints can (technically)
> represent a single DP lane. So, using 4 end-points for the
> usb-c-connector port@1 gives
> us something which is compatible with the bridge side endpoints too
> (regardless of what
> the bridge is connected to on the "output" side).
> Reading the discussion, I agree 4 lanes is over-specifying, and 2
> endpoints is probably
> enough (especially if we can use data-lanes on the bridge side
> to define the number of lanes if needed for DP panel connections).
> 

I'm sorry, but the part I don't understand is what you gain from
representing each physical line in your connection with a
remote-endpoint pair?

What I propose is that you tie the two pieces together with a single
reference. If you need to express the number of data-lanes we have
several places where this is described separately, using the
"data-lanes" property.


With this model, if you have a 1:1 connection you have a single
remote-endpoint pair, if you have a 1:N connection, then you would have
N remote-endpoint pairs.

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2022-08-17 16:00:05)
> >
> > This is the setup that we're dealing with:
> >
> >                      +------------- - -
> >  USB connector       | SoC
> >  +-+                 |
> >  | |                 |   +-----+
> >  |*|<------- HS -----|-->| HS  |
> >  |*|<------- HS -----|-->| phy |<-+   +--------+
> >  | |                 |   +-----+   \->|        |
> >  | |                 |                |  dwc3  |
> >  | |                 |   +-----+   /->|        |
> >  |*|<------- SS -----|-->|     |<-+   +--------+
> >  |*|<------- SS -----|-->| QMP |
> >  |*|<------- SS -----|-->| phy |
> >  |*|<------- SS -----|-->|     |<-\   +------------+
> >  | |                 |   +-----+   \->|            |
> >  | |                 |                |     DP     |
> >  | |    +-----+      |                | controller |
> >  |*|<-->| SBU |<-----|--------------->|            |
> >  |*|<-->| mux |<-----|--------------->|            |
> >  | |    +----+       |                +------------+
> >  +-+                 |
> >                      +------------- - -
> >
> > The dwc3 controller is connected to the HS phy for HighSpeed signals and
> > QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for
> > DP-only, USB/DP combo or USB-only mode).
> >
> > The DisplayPort controller is connected to the same QMP phy, for and is
> > muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only,
> > USB/DP combo or DP-only mode).
> >
> > The SuperSpeed pins can be switched around within the QMP phy, to handle
> > the case where the USB Type-C cable is flipped around.
> >
> >
> > The AUX pins of the DP controller are connected to the SBU pins in the
> > connector. These signals needs to be disconnected while DP mode is not
> > negotiated with the remote. The DP controller does not support swapping
> > the two lines.
> > The disconnecting and swapping thereby needs to be performed by an
> > external entity. For which we have a few examples already, such as
> > fcs,fsa4480.
> 
> By swapping you mean making SBU1 in the usb-c-connector be SBU2 and SBU2
> in the usb-c-connector be SBU1 to the DP controller, right? For the case
> when the cable is flipped.
> 

Correct.

> >
> > Lastly, in USB Power Delivery, the hot-plug signal found in a physical
> > DisplayPort or HDMI cable is communicated as a message. So the USB
> > Type-C controller must be able to pass this onto the DP controller.
> >
> >
> > I model the usb-c-connector as a child of the USB Type-C controller,
> > with the following representation of the connections:
> 
> 
> >
> > connector {
> >   compatible = "usb-c-connector";
> >
> >   ports {
> >     port@0 {
> >       reg = <0>;
> >       endpoint {
> >         remote-endpoint = <&dwc3>;
> >       };
> >     };
> >
> >     port@1 {
> >       reg = <1>;
> >       endpoint@0 {
> >         reg = <0>;
> >         remote-endpoint = <&qmp_phy>;
> >       };
> >       endpoint@1 {
> >         reg = <1>;
> >         remote-endpoint = <&dp_controller>;
> >     };
> 
> I'd expect the QMP phy to physically be the only thing connected on the
> board. That matches the block diagram above. Inside the SoC the SS lines
> will be muxed through the QMP phy to the DP controller or the USB
> controller. Therefore, in the binding I'd expect there to be a single
> port@1 for the connector:
> 
> 	port@1 {
> 		reg = <1>;
> 		endpoint@0 {
> 			reg = <0>;
> 			remote-endpoint = <&qmp_phy>;
> 		};
> 	};
> 

That is correct, the 4 SS pairs in the USB connector are connected to
the QMP PHY pads.


The second endpoint in port@1 comes from my RFC where I suggested adding
a 4th port to the usb-c-connector for connecting the usb-c-connector to
the DP controller for passing the virtual HPD signal. Rob suggested that
this indication relates to the SS pins and wanted this to be part of
port@1. But it's not actually a definition of any electrical connection.

> Somewhere above we would want 'data-lanes' to indicate how many SS lanes
> are connected between the connector and the phy and if any of those
> lanes need to be remapped in the phy.
> 
> 	port@1 {
> 		reg = <1>;
> 		endpoint@0 {
> 			reg = <0>;
> 			remote-endpoint = <&qmp_phy>;
> 			data-lanes = <0 1 2 3>;
> 		};
> 	};
> 
> This would be the block diagram configuration, but it doesn't seem
> right.
> 

The QMP phy will always be in control of the 4 lanes. The question
though is what those 4 lanes are allocated for, because depending on the
USB PD negotiation is might be 2 lanes DP + 2 lanes USB, or 4 lanes of
either function.

There are designs where we can only do 2 lanes DP, which today is
described in the DP controller...

> Other designs only connect two lanes to the qmp phy and the other two
> connect to a USB hub. That's where it gets interesting because we don't
> know how to represent that. Do we make two endpoints in the
> usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS
> RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we
> make two endpoints then do we need to have two endpoints all the time
> even though in this 4 SS line design it is redundant?
> 
> 	port@1 {
> 		reg = <1>;
> 		endpoint@0 { // Represents RX1/TX1
> 			reg = <0>;
> 			remote-endpoint = <&qmp_phy_lanes01>;
> 		};
> 		endpoint@1 { // Represents RX2/TX2
> 			reg = <1>;
> 			remote-endpoint = <&qmp_phy_lanes23>;
> 		};
> 	};
> 

So on the other side of that PHY we would have a multi-port USB
controller, or two USB controllers? Either way, this seems like a proper
representation of the two different ports, but not something we can do
with the QMP.

> I think we may need to always have two endpoints in the usb-c-connector
> because data-lanes alone can't always handle it for us. Especially when
> you consider the hub and DP phy designs.
> 
> 	port@1 {
> 		reg = <1>;
> 		endpoint@0 { // Represents RX1/TX1
> 			reg = <0>;
> 			remote-endpoint = <&usb3_hub_portN>;
> 		};
> 		endpoint@1 { // Represents RX2/TX2
> 			reg = <1>;
> 			remote-endpoint = <&qmp_phy_lanes23>;
> 		};
> 	};
> 
> The remote-endpoint phandle is different, so data-lanes can't save us. I
> suspect putting data-lanes in the usb-c-connector is really just
> nonsense too, because the usb-c-connector is a dumb devicenode and it
> can't actively change lane routing. The endpoint that's connected should
> have the data-lanes property if for example, RX2 is connected to the
> phy's TX2 pins and TX2 is connected to the phy's RX2 pins. Then the
> driver for that endpoint can change the lane mapping at runtime to
> present the proper data on the right pins in the connector.
> 

The QMP phy has certain ability to swap the signals around, so it's
conceivable that a data-lanes property in the outgoing port definition
could be used to reorder the SS lanes...

But it would be unrelated to the USB vs DP selection in my view.

All we want here is a connection between the usb-c-connector and the QMP
phy, such that the usb-c-connector's Type-C controller can inform the
QMP what has been negotiated.

> >
> >     port@2 {
> >       reg = <2>;
> >       endpoint {
> >         remote-endpoint = <&sbu_mux>;
> >       };
> >     };
> >   };
> > };
> >
> > This allows the USB Type-C controller to:
> > 1) Perform USB role switching in the dwc3 on port@0
> > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on
> >    port@1:0, implement a drm_bridge for signalling HPD back to the DP
> >    controller on port@1:1
> 
> We may need to have a port connection from the DP controller to the QMP
> phy. But given that the DP controller already has a 'phys' phandle to
> the QMP phy I think the DP controller driver could try to link to a drm
> bridge created in the phy driver that mainly handles the HPD signaling
> and any lane muxing/routing that needs to happen when DP pin
> configuration is present.
> 

The QMP has no knowledge of HPD signalling in Type-C, it's strictly a
virtual thing living in the Type-C controller. The Type-C controller
will request mux changes from the QMP and HPD signal changes as two
completely independent events.

Implementing a drm_bridge in the implementation backing the
usb-c-connector mimics e.g. dp-connector (implemented in
drivers/gpu/drm/bridge/display-connector.c) nicely.

Implementing the drm_bridge in the QMP phy means that we just add state
tracking for something that it doesn't know, hence we need another
mechanism to the Type-C controller to inform the phy that the HPD signal
has changed.


This is analog to the case you have today, where the QMP has no
knowledge of the GPIO pin that carries the HPD state in your design.

> > 3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines
> >    in the SBU mux on port@2.
> >
> > The SBU mux in several of these designs is a component that takes a
> > power supply and two GPIOs, for enabling/disabling the connection and
> > flip the switch (which is used to swap the lines).
> 
> The SBU mux sounds OK to me. I don't think the SBU lines can be split
> up, they're a pair that has to go together, just like CC lines and SS
> pairs are in the typec spec. Of course, in DP spec we can have a single
> DP lane which corresponds to a single RX or TX differential pair, but
> with typec that isn't possible, we always have RX and TX together.
> 
> This actually brings up one final point. On the QMP phy we can reorder
> the lanes willy nilly from what I recall, so that RX1 could be RX2 and
> TX1 could be TX2. In such a design, we would have two ports in the qmp
> phy to connect to the two TX/RX pairs in the connector, but then we
> would need to tell the QMP phy that the lanes are all shifted.
> 
> 	qmp_phy {
> 		ports {
> 			port@0 {
> 				reg = <0>;
> 				endpoint@0 {
> 					reg = <0>;
> 					remote-endpoint = <&usb_c_txrx1>;
> 					data-lanes = <1 2>
> 				};
> 				endpoint@1 {
> 					reg = <1>;
> 					remote-endpoint = <&usb_c_txrx2>;
> 					data-lanes = <3 0>;
> 				};
> 			};
> 		};
> 	};
> 
> This would do that for us, but when all four lanes are connected from
> the qmp phy directly to the connector we could just as easily have done
> it with one endpoint.
> 
> 	qmp_phy {
> 		ports {
> 			port@0 {
> 				reg = <0>;
> 				endpoint@0 {
> 					reg = <0>;
> 					remote-endpoint = <&usb_c_ss>;
> 					data-lanes = <1 2 3 0>
> 				};
> 			};
> 		};
> 	};
> 
> So should we explicitly have two endpoints in the usb-c-connector for
> the two pairs all the time, or should we represent that via data-lanes
> and only split up the connector's endpoint if we need to connect the
> usb-c-connector to two different endpoints?

I think the endpoint of port@1 should represent the set of signals
connected to the other side, in our case 1:1 with the QMP. I like the
idea of adding data-lanes to the QMP side in order to describe any
swapping of the pads, but I see that as a separate thing.

If you have a design where your usb-c-connector is wired to two
different PHYs and you have a Type-C controller that only negotiates the
2+2 mode, then I think it makes sense to represent that as two endpoint
of port@1 - but the QMP side would only reference one of these
endpoints.

Regards,
Bjorn

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Stephen Boyd]

I realize I've hijacked this thread to discuss the QMP binding :-/

Quoting Bjorn Andersson (2022-08-19 14:26:32)
> On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2022-08-17 16:00:05)
> > >
> > > This is the setup that we're dealing with:
> > >
> > >                      +------------- - -
> > >  USB connector       | SoC
> > >  +-+                 |
> > >  | |                 |   +-----+
> > >  |*|<------- HS -----|-->| HS  |
> > >  |*|<------- HS -----|-->| phy |<-+   +--------+
> > >  | |                 |   +-----+   \->|        |
> > >  | |                 |                |  dwc3  |
> > >  | |                 |   +-----+   /->|        |
> > >  |*|<------- SS -----|-->|     |<-+   +--------+
> > >  |*|<------- SS -----|-->| QMP |
> > >  |*|<------- SS -----|-->| phy |
> > >  |*|<------- SS -----|-->|     |<-\   +------------+
> > >  | |                 |   +-----+   \->|            |
> > >  | |                 |                |     DP     |
> > >  | |    +-----+      |                | controller |
> > >  |*|<-->| SBU |<-----|--------------->|            |
> > >  |*|<-->| mux |<-----|--------------->|            |
> > >  | |    +----+       |                +------------+
> > >  +-+                 |
> > >                      +------------- - -
> > >
[...]
> > I'd expect the QMP phy to physically be the only thing connected on the
> > board. That matches the block diagram above. Inside the SoC the SS lines
> > will be muxed through the QMP phy to the DP controller or the USB
> > controller. Therefore, in the binding I'd expect there to be a single
> > port@1 for the connector:
> >
> >       port@1 {
> >               reg = <1>;
> >               endpoint@0 {
> >                       reg = <0>;
> >                       remote-endpoint = <&qmp_phy>;
> >               };
> >       };
> >
>
> That is correct, the 4 SS pairs in the USB connector are connected to
> the QMP PHY pads.
>
>
> The second endpoint in port@1 comes from my RFC where I suggested adding
> a 4th port to the usb-c-connector for connecting the usb-c-connector to
> the DP controller for passing the virtual HPD signal. Rob suggested that
> this indication relates to the SS pins and wanted this to be part of
> port@1. But it's not actually a definition of any electrical connection.

I suspect this is the root of the debate. Should the binding be
describing logical connections between components or actual physical
connections? And should the connector binding have endpoints for
different altmodes, e.g. DP?

What do we do if thunderbolt support is yet another PHY that sits behind
the QMP phy? Do we need to make yet another endpoint in the
usb-c-connector to represent this connection? I hope not.

The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto
the SS lanes. Other altmodes that want to use the SS lanes would
similarly need to be routed through the QMP phy and muxed onto the lanes
when the altmode is used, e.g. thunderbolt. While it is certainly
convenient to have a "DP" endpoint in the usb-c-connector, I feel that
it is wrong, primarily because the DP phy has the QMP phy in between it
and the usb-c-connector, but also because DP is an altmode/virtual
construct built on top of the 4 lanes in the typec pinout.

We should look at the binding from the perspective of the connector and
figure out how the pinout can be mapped to the binding. That would allow
board designers to ignore the internal SoC details, and stay focused on
what is in the schematic, which is the qmp phy and the usb-c-connector
in this case. My understanding is that two SS lanes always have to go
together in the type-c spec, hence the two endpoints in the graph, but
if all the SS lanes are physically wired to the same PHY then we can
omit the second endpoint and use data-lanes if for example DP is handled
by a different phy.

>
> > Other designs only connect two lanes to the qmp phy and the other two
> > connect to a USB hub. That's where it gets interesting because we don't
> > know how to represent that. Do we make two endpoints in the
> > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS
> > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we
> > make two endpoints then do we need to have two endpoints all the time
> > even though in this 4 SS line design it is redundant?
> >
> >       port@1 {
> >               reg = <1>;
> >               endpoint@0 { // Represents RX1/TX1
> >                       reg = <0>;
> >                       remote-endpoint = <&qmp_phy_lanes01>;
> >               };
> >               endpoint@1 { // Represents RX2/TX2
> >                       reg = <1>;
> >                       remote-endpoint = <&qmp_phy_lanes23>;
> >               };
> >       };
> >
>
> So on the other side of that PHY we would have a multi-port USB
> controller, or two USB controllers?

I'm thinking of a single USB+DP PHY.

> Either way, this seems like a proper
> representation of the two different ports, but not something we can do
> with the QMP.

This example I gave is for the usb-c-connector, hence the
remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for
0+1 and 2+3. Sorry if that wasn't clear.

>
> The QMP phy has certain ability to swap the signals around, so it's
> conceivable that a data-lanes property in the outgoing port definition
> could be used to reorder the SS lanes...
>
> But it would be unrelated to the USB vs DP selection in my view.
>
> All we want here is a connection between the usb-c-connector and the QMP
> phy, such that the usb-c-connector's Type-C controller can inform the
> QMP what has been negotiated.

Ok. By Type-C controller you mean the typec manager? Is that all Linux
for you?

>
> > >
> > >     port@2 {
> > >       reg = <2>;
> > >       endpoint {
> > >         remote-endpoint = <&sbu_mux>;
> > >       };
> > >     };
> > >   };
> > > };
> > >
> > > This allows the USB Type-C controller to:
> > > 1) Perform USB role switching in the dwc3 on port@0
> > > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on
> > >    port@1:0, implement a drm_bridge for signalling HPD back to the DP
> > >    controller on port@1:1
> >
> > We may need to have a port connection from the DP controller to the QMP
> > phy. But given that the DP controller already has a 'phys' phandle to
> > the QMP phy I think the DP controller driver could try to link to a drm
> > bridge created in the phy driver that mainly handles the HPD signaling
> > and any lane muxing/routing that needs to happen when DP pin
> > configuration is present.
> >
>
> The QMP has no knowledge of HPD signalling in Type-C, it's strictly a
> virtual thing living in the Type-C controller. The Type-C controller
> will request mux changes from the QMP and HPD signal changes as two
> completely independent events.
>
> Implementing a drm_bridge in the implementation backing the
> usb-c-connector mimics e.g. dp-connector (implemented in
> drivers/gpu/drm/bridge/display-connector.c) nicely.
>
> Implementing the drm_bridge in the QMP phy means that we just add state
> tracking for something that it doesn't know, hence we need another
> mechanism to the Type-C controller to inform the phy that the HPD signal
> has changed.

Ok, so the idea is to make a drm bridge in the device registering the
usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for
you? I'm not really following along on this part.

On your design I believe the QMP phy is a mode-switch and an
orientation-switch. The orientation-switch is implemented as some bit in
the QMP registers to flip the SS lanes and the mode-switch is
implemented somehow that I don't really understand. Probably the QMP can
shut off USB for 4 lanes DP over the SS lanes? I recall some bit for the
different modes is in the QMP registers.

Or is the idea to make the USB (dwc3) and DP (msm_dp) controllers call
phy framework APIs to change the qmp mode (USB, DP, or USB+DP)?

>
>
> This is analog to the case you have today, where the QMP has no
> knowledge of the GPIO pin that carries the HPD state in your design.

Indeed, in my design the QMP configuration is "fixed" and two lanes are
dedicated for DP while another two lanes are dedicated to USB. The USB
lanes go to a USB hub and the hub ports are connected to two different
usb-c-connectors. The DP lanes go to another mux (similar to the SBU mux
logically) and the two lanes are muxed to one of the two
usb-c-connectors depending on what the typec manager decides. The HPD
signal bypasses QMP and goes directly to the DP controller (msm_dp) via
a GPIO. The HPD signal could just as easily be virtual like in your
design, but we use a GPIO for now.

For the QMP driver to figure this out it will need to be able to parse
the graph properties or we'll need more properties to describe the
configuration. I was hoping we could describe this solely through the
graph binding. We can probably do it by having reg numbered
ports/endpoints in the QMP's ports binding to represent the USB or DP
functionality (e.g. reg 0 is USB and reg 1 is DP) and then use
'data-lanes' to represent the number of lanes being used for that
functionality (and also if they're remapped). Someone needs to write out
all possible combinations and make sure the QMP binding can handle them
all. If the ports binding isn't present then the driver should default
to existing behavior (2 lanes DP, 2 lanes USB, normal orientation, no
lane remapping). When they do this they should also consider static
configurations that differ from the default, where the QMP doesn't flip
the lines and/or change modes. That would allow hardware engineers to
reroute lanes if that makes signal integrity better.

I expect that having the device registering the usb-c-connector make the
drm bridge would work on ChromeOS. We would have the cros-ec-typec
driver register the drm bridge(s) and notify HPD to the DP controller(s)
through the drm bridge instead of using the GPIO path. We'd have to
figure out how to express the connection to the dp controller in DT so
when it searches for the next bridge it can find the one made in
cros-ec-typec.

>
[...]
> >
> > So should we explicitly have two endpoints in the usb-c-connector for
> > the two pairs all the time, or should we represent that via data-lanes
> > and only split up the connector's endpoint if we need to connect the
> > usb-c-connector to two different endpoints?
>
> I think the endpoint of port@1 should represent the set of signals
> connected to the other side, in our case 1:1 with the QMP. I like the
> idea of adding data-lanes to the QMP side in order to describe any
> swapping of the pads, but I see that as a separate thing.
>
> If you have a design where your usb-c-connector is wired to two
> different PHYs and you have a Type-C controller that only negotiates the
> 2+2 mode, then I think it makes sense to represent that as two endpoint
> of port@1 - but the QMP side would only reference one of these
> endpoints.
>

Agreed. I think that means at most two endpoints are possible in port@1
in the usb-c-connector binding. We would only use the second endpoint if
we had two different PHYs that required it, otherwise only a single
endpoint.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Bjorn Andersson]

On Fri 19 Aug 22:51 CDT 2022, Stephen Boyd wrote:

> I realize I've hijacked this thread to discuss the QMP binding :-/
> 
> Quoting Bjorn Andersson (2022-08-19 14:26:32)
> > On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote:
> >
> > > Quoting Bjorn Andersson (2022-08-17 16:00:05)
> > > >
> > > > This is the setup that we're dealing with:
> > > >
> > > >                      +------------- - -
> > > >  USB connector       | SoC
> > > >  +-+                 |
> > > >  | |                 |   +-----+
> > > >  |*|<------- HS -----|-->| HS  |
> > > >  |*|<------- HS -----|-->| phy |<-+   +--------+
> > > >  | |                 |   +-----+   \->|        |
> > > >  | |                 |                |  dwc3  |
> > > >  | |                 |   +-----+   /->|        |
> > > >  |*|<------- SS -----|-->|     |<-+   +--------+
> > > >  |*|<------- SS -----|-->| QMP |
> > > >  |*|<------- SS -----|-->| phy |
> > > >  |*|<------- SS -----|-->|     |<-\   +------------+
> > > >  | |                 |   +-----+   \->|            |
> > > >  | |                 |                |     DP     |
> > > >  | |    +-----+      |                | controller |
> > > >  |*|<-->| SBU |<-----|--------------->|            |
> > > >  |*|<-->| mux |<-----|--------------->|            |
> > > >  | |    +----+       |                +------------+
> > > >  +-+                 |
> > > >                      +------------- - -
> > > >
> [...]
> > > I'd expect the QMP phy to physically be the only thing connected on the
> > > board. That matches the block diagram above. Inside the SoC the SS lines
> > > will be muxed through the QMP phy to the DP controller or the USB
> > > controller. Therefore, in the binding I'd expect there to be a single
> > > port@1 for the connector:
> > >
> > >       port@1 {
> > >               reg = <1>;
> > >               endpoint@0 {
> > >                       reg = <0>;
> > >                       remote-endpoint = <&qmp_phy>;
> > >               };
> > >       };
> > >
> >
> > That is correct, the 4 SS pairs in the USB connector are connected to
> > the QMP PHY pads.
> >
> >
> > The second endpoint in port@1 comes from my RFC where I suggested adding
> > a 4th port to the usb-c-connector for connecting the usb-c-connector to
> > the DP controller for passing the virtual HPD signal. Rob suggested that
> > this indication relates to the SS pins and wanted this to be part of
> > port@1. But it's not actually a definition of any electrical connection.
> 
> I suspect this is the root of the debate. Should the binding be
> describing logical connections between components or actual physical
> connections? And should the connector binding have endpoints for
> different altmodes, e.g. DP?
> 
> What do we do if thunderbolt support is yet another PHY that sits behind
> the QMP phy? Do we need to make yet another endpoint in the
> usb-c-connector to represent this connection? I hope not.
> 

It sounds like you would be comfortable with expressing the relationship
between the usb-c-connector and the QMP PHY in this design and then
express the relationship between the QMP and the thunderbolt separately,
forcing the QMP implementation to bridge any operations needed.

I've not looked enough at such design to argue for or against that, but
I can definitely see the merit of having the usb-c-connector graph be
linked to the component in the SoC that owns the pads - and not
everything behind that.

> The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto
> the SS lanes. Other altmodes that want to use the SS lanes would
> similarly need to be routed through the QMP phy and muxed onto the lanes
> when the altmode is used, e.g. thunderbolt. While it is certainly
> convenient to have a "DP" endpoint in the usb-c-connector, I feel that
> it is wrong, primarily because the DP phy has the QMP phy in between it
> and the usb-c-connector, but also because DP is an altmode/virtual
> construct built on top of the 4 lanes in the typec pinout.
> 
> We should look at the binding from the perspective of the connector and
> figure out how the pinout can be mapped to the binding. That would allow
> board designers to ignore the internal SoC details, and stay focused on
> what is in the schematic, which is the qmp phy and the usb-c-connector
> in this case. My understanding is that two SS lanes always have to go
> together in the type-c spec, hence the two endpoints in the graph, but
> if all the SS lanes are physically wired to the same PHY then we can
> omit the second endpoint and use data-lanes if for example DP is handled
> by a different phy.
> 

There is nothing in the schematics representing how the HPD signal comes
from the Type-C controller to the DP controller - but it is a M:N
relationship, so we must represent it in some way.

I suggested a new port for describing this virtual connection, Rob asked
for it to be a separate endpoint in port@1. I'm fine with either path.

But as Benson described to us, we do muxing of the signals in one
operation and we do HPD signalling in a completely separate operation -
from the Type-C controller's PoV. As such the QMP has nothing to do with
the HPD signal.

> >
> > > Other designs only connect two lanes to the qmp phy and the other two
> > > connect to a USB hub. That's where it gets interesting because we don't
> > > know how to represent that. Do we make two endpoints in the
> > > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS
> > > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we
> > > make two endpoints then do we need to have two endpoints all the time
> > > even though in this 4 SS line design it is redundant?
> > >
> > >       port@1 {
> > >               reg = <1>;
> > >               endpoint@0 { // Represents RX1/TX1
> > >                       reg = <0>;
> > >                       remote-endpoint = <&qmp_phy_lanes01>;
> > >               };
> > >               endpoint@1 { // Represents RX2/TX2
> > >                       reg = <1>;
> > >                       remote-endpoint = <&qmp_phy_lanes23>;
> > >               };
> > >       };
> > >
> >
> > So on the other side of that PHY we would have a multi-port USB
> > controller, or two USB controllers?
> 
> I'm thinking of a single USB+DP PHY.
> 
> > Either way, this seems like a proper
> > representation of the two different ports, but not something we can do
> > with the QMP.
> 
> This example I gave is for the usb-c-connector, hence the
> remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for
> 0+1 and 2+3. Sorry if that wasn't clear.
> 
> >
> > The QMP phy has certain ability to swap the signals around, so it's
> > conceivable that a data-lanes property in the outgoing port definition
> > could be used to reorder the SS lanes...
> >
> > But it would be unrelated to the USB vs DP selection in my view.
> >
> > All we want here is a connection between the usb-c-connector and the QMP
> > phy, such that the usb-c-connector's Type-C controller can inform the
> > QMP what has been negotiated.
> 
> Ok. By Type-C controller you mean the typec manager? Is that all Linux
> for you?
> 

I mean the entity that tells the remote-endpoints of the usb-c-connector
about the outcome of USB PD negotiations. This might be implemented
fully in Linux or partially in firmware.

But this something will be the thing that ultimately calls
typec_switch_set() et al.


Can you please elaborate on the operations you see that the typec
manager would perform on the remote-endpoint of endpoint@0 and
endpoint@1?

> >
> > > >
> > > >     port@2 {
> > > >       reg = <2>;
> > > >       endpoint {
> > > >         remote-endpoint = <&sbu_mux>;
> > > >       };
> > > >     };
> > > >   };
> > > > };
> > > >
> > > > This allows the USB Type-C controller to:
> > > > 1) Perform USB role switching in the dwc3 on port@0
> > > > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on
> > > >    port@1:0, implement a drm_bridge for signalling HPD back to the DP
> > > >    controller on port@1:1
> > >
> > > We may need to have a port connection from the DP controller to the QMP
> > > phy. But given that the DP controller already has a 'phys' phandle to
> > > the QMP phy I think the DP controller driver could try to link to a drm
> > > bridge created in the phy driver that mainly handles the HPD signaling
> > > and any lane muxing/routing that needs to happen when DP pin
> > > configuration is present.
> > >
> >
> > The QMP has no knowledge of HPD signalling in Type-C, it's strictly a
> > virtual thing living in the Type-C controller. The Type-C controller
> > will request mux changes from the QMP and HPD signal changes as two
> > completely independent events.
> >
> > Implementing a drm_bridge in the implementation backing the
> > usb-c-connector mimics e.g. dp-connector (implemented in
> > drivers/gpu/drm/bridge/display-connector.c) nicely.
> >
> > Implementing the drm_bridge in the QMP phy means that we just add state
> > tracking for something that it doesn't know, hence we need another
> > mechanism to the Type-C controller to inform the phy that the HPD signal
> > has changed.
> 
> Ok, so the idea is to make a drm bridge in the device registering the
> usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for
> you? I'm not really following along on this part.
> 

No, it's not a part of the QMP.

We want to use the graph from the usb-c-connector to signal the provider
of HS, SS and SBU-signals about changes related to the connector. As
such we associate the usb-c-connector with the Type-C
manager/controller.

Like described here, for a single usb-c-connector:
https://lore.kernel.org/all/20220818031512.319310-2-bjorn.andersson@linaro.org/

In this case, the pmic_glink firmware will send Linux messages which can
be directly translated to a set of typec_mux_set(), typec_switch_set()
and drm_bridge_hpd_notify() calls - with the graph defining which remote
components should receive these events.

> On your design I believe the QMP phy is a mode-switch and an
> orientation-switch. The orientation-switch is implemented as some bit in
> the QMP registers to flip the SS lanes and the mode-switch is
> implemented somehow that I don't really understand. Probably the QMP can
> shut off USB for 4 lanes DP over the SS lanes? I recall some bit for the
> different modes is in the QMP registers.
> 

Correct, typec_mux_set() passes the negotiated pin assignment, which the
QMP PHY can react to and reconfigure the muxing.

Similarly the QMP can implement typec_switch_set() to flip the bit to do
the orientation switching.

> Or is the idea to make the USB (dwc3) and DP (msm_dp) controllers call
> phy framework APIs to change the qmp mode (USB, DP, or USB+DP)?
> 

No.

> >
> >
> > This is analog to the case you have today, where the QMP has no
> > knowledge of the GPIO pin that carries the HPD state in your design.
> 
> Indeed, in my design the QMP configuration is "fixed" and two lanes are
> dedicated for DP while another two lanes are dedicated to USB. The USB
> lanes go to a USB hub and the hub ports are connected to two different
> usb-c-connectors. The DP lanes go to another mux (similar to the SBU mux
> logically) and the two lanes are muxed to one of the two
> usb-c-connectors depending on what the typec manager decides. The HPD
> signal bypasses QMP and goes directly to the DP controller (msm_dp) via
> a GPIO. The HPD signal could just as easily be virtual like in your
> design, but we use a GPIO for now.
> 
> For the QMP driver to figure this out it will need to be able to parse
> the graph properties or we'll need more properties to describe the
> configuration. I was hoping we could describe this solely through the
> graph binding. We can probably do it by having reg numbered
> ports/endpoints in the QMP's ports binding to represent the USB or DP
> functionality (e.g. reg 0 is USB and reg 1 is DP) and then use
> 'data-lanes' to represent the number of lanes being used for that
> functionality (and also if they're remapped). Someone needs to write out
> all possible combinations and make sure the QMP binding can handle them
> all. If the ports binding isn't present then the driver should default
> to existing behavior (2 lanes DP, 2 lanes USB, normal orientation, no
> lane remapping). When they do this they should also consider static
> configurations that differ from the default, where the QMP doesn't flip
> the lines and/or change modes. That would allow hardware engineers to
> reroute lanes if that makes signal integrity better.
> 
> I expect that having the device registering the usb-c-connector make the
> drm bridge would work on ChromeOS. We would have the cros-ec-typec
> driver register the drm bridge(s) and notify HPD to the DP controller(s)
> through the drm bridge instead of using the GPIO path. We'd have to
> figure out how to express the connection to the dp controller in DT so
> when it searches for the next bridge it can find the one made in
> cros-ec-typec.
> 

I don't think it makes sense in your design to register a drm_bridge per
usb-c-connector, because then you need to connect the one DP controller
to both the drm_bridges and you need to spill the mux-logic from the EC
into the DP controller as well.

If you put the muxing logic in entity that does the muxing and implement
a signle drm_bridge there you will mimic the current design nicely,
where there is a single connection (GPIO) between the EC and the DP
controller for propagating the HPD signal.

You could choose to model the two usb-c-connectors there somehow as
well, perhaps just as static entities directly in /, but that would then
be a question of how to describing the link between the EC and the two
connectors.

Similarly, describing the relationship between the QMP PHY and the mux
makes sense to me (or just not describe it at all, if you're not going
to invoke any of the muxing/switching operations on the PHY)

> >
> [...]
> > >
> > > So should we explicitly have two endpoints in the usb-c-connector for
> > > the two pairs all the time, or should we represent that via data-lanes
> > > and only split up the connector's endpoint if we need to connect the
> > > usb-c-connector to two different endpoints?
> >
> > I think the endpoint of port@1 should represent the set of signals
> > connected to the other side, in our case 1:1 with the QMP. I like the
> > idea of adding data-lanes to the QMP side in order to describe any
> > swapping of the pads, but I see that as a separate thing.
> >
> > If you have a design where your usb-c-connector is wired to two
> > different PHYs and you have a Type-C controller that only negotiates the
> > 2+2 mode, then I think it makes sense to represent that as two endpoint
> > of port@1 - but the QMP side would only reference one of these
> > endpoints.
> >
> 
> Agreed. I think that means at most two endpoints are possible in port@1
> in the usb-c-connector binding. We would only use the second endpoint if
> we had two different PHYs that required it, otherwise only a single
> endpoint.

Sure.

But I do need to have a link between a DP controller and something
representing each USB-C port.

By registering a drm_bridge associated with the usb-c-connector the DP
controller implementation and binding will look identical between the
dp-connector case and the usb-c-connector case.

But the two options I see is to either add it in port@1 as a separate
logical endpoint or to add a new logical port.

The alternative to this would be to have a separate of graph outside the
multiple connectors, where each port@N implements the drm_bridge for
connector@N - but I feel we're just making things overly complicated,
just to avoid adding a logical endpoint/port in the usb-c-connector.

Regards,
Bjorn.

Re: [PATCH 1/2] dt-bindings: usb: Introduce GPIO-based SBU mux

[Stephen Boyd]

Quoting Bjorn Andersson (2022-08-19 21:56:17)
> On Fri 19 Aug 22:51 CDT 2022, Stephen Boyd wrote:
>
> > I realize I've hijacked this thread to discuss the QMP binding :-/
> >
> > The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto
> > the SS lanes. Other altmodes that want to use the SS lanes would
> > similarly need to be routed through the QMP phy and muxed onto the lanes
> > when the altmode is used, e.g. thunderbolt. While it is certainly
> > convenient to have a "DP" endpoint in the usb-c-connector, I feel that
> > it is wrong, primarily because the DP phy has the QMP phy in between it
> > and the usb-c-connector, but also because DP is an altmode/virtual
> > construct built on top of the 4 lanes in the typec pinout.
> >
> > We should look at the binding from the perspective of the connector and
> > figure out how the pinout can be mapped to the binding. That would allow
> > board designers to ignore the internal SoC details, and stay focused on
> > what is in the schematic, which is the qmp phy and the usb-c-connector
> > in this case. My understanding is that two SS lanes always have to go
> > together in the type-c spec, hence the two endpoints in the graph, but
> > if all the SS lanes are physically wired to the same PHY then we can
> > omit the second endpoint and use data-lanes if for example DP is handled
> > by a different phy.
> >
>
> There is nothing in the schematics representing how the HPD signal comes
> from the Type-C controller to the DP controller - but it is a M:N
> relationship, so we must represent it in some way.
>
> I suggested a new port for describing this virtual connection, Rob asked
> for it to be a separate endpoint in port@1. I'm fine with either path.

I don't think we should be making a virtual connection. I'm not sure Rob
wants it to be a virtual connection either.

>
> But as Benson described to us, we do muxing of the signals in one
> operation and we do HPD signalling in a completely separate operation -
> from the Type-C controller's PoV. As such the QMP has nothing to do with
> the HPD signal.
>
> > >
> > > > Other designs only connect two lanes to the qmp phy and the other two
> > > > connect to a USB hub. That's where it gets interesting because we don't
> > > > know how to represent that. Do we make two endpoints in the
> > > > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS
> > > > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we
> > > > make two endpoints then do we need to have two endpoints all the time
> > > > even though in this 4 SS line design it is redundant?
> > > >
> > > >       port@1 {
> > > >               reg = <1>;
> > > >               endpoint@0 { // Represents RX1/TX1
> > > >                       reg = <0>;
> > > >                       remote-endpoint = <&qmp_phy_lanes01>;
> > > >               };
> > > >               endpoint@1 { // Represents RX2/TX2
> > > >                       reg = <1>;
> > > >                       remote-endpoint = <&qmp_phy_lanes23>;
> > > >               };
> > > >       };
> > > >
> > >
> > > So on the other side of that PHY we would have a multi-port USB
> > > controller, or two USB controllers?
> >
> > I'm thinking of a single USB+DP PHY.
> >
> > > Either way, this seems like a proper
> > > representation of the two different ports, but not something we can do
> > > with the QMP.
> >
> > This example I gave is for the usb-c-connector, hence the
> > remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for
> > 0+1 and 2+3. Sorry if that wasn't clear.
> >
> > >
> > > The QMP phy has certain ability to swap the signals around, so it's
> > > conceivable that a data-lanes property in the outgoing port definition
> > > could be used to reorder the SS lanes...
> > >
> > > But it would be unrelated to the USB vs DP selection in my view.
> > >
> > > All we want here is a connection between the usb-c-connector and the QMP
> > > phy, such that the usb-c-connector's Type-C controller can inform the
> > > QMP what has been negotiated.
> >
> > Ok. By Type-C controller you mean the typec manager? Is that all Linux
> > for you?
> >
>
> I mean the entity that tells the remote-endpoints of the usb-c-connector
> about the outcome of USB PD negotiations. This might be implemented
> fully in Linux or partially in firmware.
>
> But this something will be the thing that ultimately calls
> typec_switch_set() et al.
>
>
> Can you please elaborate on the operations you see that the typec
> manager would perform on the remote-endpoint of endpoint@0 and
> endpoint@1?

Sorry, which endpoints are we talking about here?

> >
> > Ok, so the idea is to make a drm bridge in the device registering the
> > usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for
> > you? I'm not really following along on this part.
> >
>
> No, it's not a part of the QMP.
>
> We want to use the graph from the usb-c-connector to signal the provider
> of HS, SS and SBU-signals about changes related to the connector. As
> such we associate the usb-c-connector with the Type-C
> manager/controller.
>
> Like described here, for a single usb-c-connector:
> https://lore.kernel.org/all/20220818031512.319310-2-bjorn.andersson@linaro.org/
>
> In this case, the pmic_glink firmware will send Linux messages which can
> be directly translated to a set of typec_mux_set(), typec_switch_set()
> and drm_bridge_hpd_notify() calls - with the graph defining which remote
> components should receive these events.

Ok, got it. The usb-c-connector is registered by the qcom,pmic-glink
driver. That is similar to the google,cros-ec-typec driver that we have
on chromeos.

>
> > >
> > >
> > > This is analog to the case you have today, where the QMP has no
> > > knowledge of the GPIO pin that carries the HPD state in your design.

Yes.

>
> I don't think it makes sense in your design to register a drm_bridge per
> usb-c-connector, because then you need to connect the one DP controller
> to both the drm_bridges and you need to spill the mux-logic from the EC
> into the DP controller as well.
>
> If you put the muxing logic in entity that does the muxing and implement
> a signle drm_bridge there you will mimic the current design nicely,
> where there is a single connection (GPIO) between the EC and the DP
> controller for propagating the HPD signal.

Yes, agreed. I believe implementing the drm_bridge in an EC driver is
the approach we will take. We will have to add a port binding to the EC
binding that accepts displayport as an input. Essentially the driver
will act as a DP connector that accepts the 2 DP lanes coming from QMP
and then muxes them onto one or the other usb-c-connector. The same
driver will need to support multiple DP input ports and USB inputs.

>
> You could choose to model the two usb-c-connectors there somehow as
> well, perhaps just as static entities directly in /, but that would then
> be a question of how to describing the link between the EC and the two
> connectors.
>
> Similarly, describing the relationship between the QMP PHY and the mux
> makes sense to me (or just not describe it at all, if you're not going
> to invoke any of the muxing/switching operations on the PHY)
>
> > >
> > [...]
> > > >
> > > > So should we explicitly have two endpoints in the usb-c-connector for
> > > > the two pairs all the time, or should we represent that via data-lanes
> > > > and only split up the connector's endpoint if we need to connect the
> > > > usb-c-connector to two different endpoints?
> > >
> > > I think the endpoint of port@1 should represent the set of signals
> > > connected to the other side, in our case 1:1 with the QMP. I like the
> > > idea of adding data-lanes to the QMP side in order to describe any
> > > swapping of the pads, but I see that as a separate thing.
> > >
> > > If you have a design where your usb-c-connector is wired to two
> > > different PHYs and you have a Type-C controller that only negotiates the
> > > 2+2 mode, then I think it makes sense to represent that as two endpoint
> > > of port@1 - but the QMP side would only reference one of these
> > > endpoints.
> > >
> >
> > Agreed. I think that means at most two endpoints are possible in port@1
> > in the usb-c-connector binding. We would only use the second endpoint if
> > we had two different PHYs that required it, otherwise only a single
> > endpoint.
>
> Sure.
>
> But I do need to have a link between a DP controller and something
> representing each USB-C port.

Do you? I think you need a link between the DP driver and the drm_bridge
driver that sends virtual hpd signals based on typec messages. That may
be the usb-c port driver, or it may be the driver implementing the
mode-switch. It doesn't need to be the usb-c-connector.

>
> By registering a drm_bridge associated with the usb-c-connector the DP
> controller implementation and binding will look identical between the
> dp-connector case and the usb-c-connector case.
>
> But the two options I see is to either add it in port@1 as a separate
> logical endpoint or to add a new logical port.
>
> The alternative to this would be to have a separate of graph outside the
> multiple connectors, where each port@N implements the drm_bridge for
> connector@N - but I feel we're just making things overly complicated,
> just to avoid adding a logical endpoint/port in the usb-c-connector.
>

In your case you have a 1:1 relationship between the QMP and the
usb-c-connector, so anything extra in the graph relationship between QMP
and the usb-c-connector looks like extra overly complicated binding. In
the chromeos trogdor case, we have a physical switch on the DP lanes to
steer DP from QMP to one of the two usb-c-connectors that are controlled
by the EC. Making a direct connection between DP and the
usb-c-connectors is impossible because DP has 1 graph output that can't
be connected to two usb-c-connectors at the same time.

The creation of a drm_bridge for a usb-c-connector doesn't work.
Connecting the QMP to the EC is a solution, or connecting the DP
controller to the EC is another solution, but the QMP path is preferred.

Here's why:

 (brace yourself for the wall of text!)

 1. QMP needs to know lane routing

We need to know which lanes coming out of QMP are for DP. Are they the
first two lanes (TX1/RX1) or the second two lanes (TX2/RX2), or all 4
lanes? The orientation bit inside QMP needs to be configured properly
too. In the 1:1 case you have this is not important until the DP pinconf
is assigned; connecting the DP controller directly to the
usb-c-connector in DT works because typec framework controls the
orientation switch inside QMP that's connected to the other port in
usb-c-connector. In the 1:N case the mapping is static, and we need a
way to express which lanes from QMP are for DP and which lanes from QMP
are for USB.

 2. DP lane remapping is controlled in the DP phy before orientation is
 controlled in the QMP phy

DP lanes coming out of the DP phy can be remapped however desired via a
register in the DP phy. The orientation control works on the TX1/RX1 and
TX2/RX2 pairs via QMP registers, so that two lanes DP and two lanes USB
can't put the two USB lanes on TX1/TX2 and the two DP lanes on RX1/RX2
when the orientation is flipped. If QMP has all four lanes wired to a DP
connector, i.e. USB is disabled, then we would use the data-lanes
property inside the DP phy's graph endpoint to figure out how to remap
the four DP lanes going to the QMP and out of the SoC to the connector.
The QMP orientation bit would need to be set to normal, so that
engineers can ignore QMP and how it flips the lanes it took from the DP
phy.

 3. QMP should have incoming and outgoing graph ports

The data-lanes property should only be inside graph endpoints, hence the
requirement to use a graph binding from the DP phy to the QMP node.
Sometimes the lanes from QMP are directly connected to a
usb-c-connector, hence the requirement to have a graph binding in QMP
that can connect to a usb-c-connector. Other times the QMP node would be
directly connected to a dp-connector. In that case we would also use a
graph binding to link the connector to QMP.

 4. Graph endpoints without a remote-endpoint property are bad style

In your design this wouldn't be the case, but in chromeos' case we would
connect the DP controller to the EC or one of the two usb-c-connectors
and then if we want to configure QMP lanes we would have to implement
the graph in QMP but omit the remote-endpoint property. That's because
we have 2 usb-c-connectors for 1 QMP. Having two endpoints in
usb-c-connector doesn't help us, because we can only connect QMP to one
of the two usb-c-connectors. We're unable to express that both
usb-c-connectors support DP from this one DP controller.

TL;DR: We have a verbose binding indeed, but it is required because
trogdor takes 2 DP lanes and muxes them to different usb-c-connectors.

To do that, we have to send the DP lanes to the thing that controls the
muxing (the EC). It's also complicated by the fact that the DP and USB
phy are split from the controllers, and put behind QMP (basically a
typec phy). I propose we make the graph binding for QMP have logically
numbered endpoints: 0 for USB+DP (most common), 1 for USB, and 2 for DP
and use data-lanes to physically map the pins while connecting the DP
and USB phys to QMP within the graph and using data-lanes again.

Here are some examples:

 - 4 lanes DP only, normal orientation of QMP, remapped DP lanes
  qmp {
    ports {
      qmp_dp_in: port@1 {
        reg = <1>;
        remote-endpoint = <&dp_phy_out>;
      };
      port@2 {
        reg = <2>;
        qmp_dp_out: endpoint@2 {
          reg = <2>;
          // data-lanes indicates how many lanes are used for DP
          // and if the lanes are flipped
          //
          // data-lanes = <0 1 2 3> == normal orientation (default)
          // data-lanes = <2 3 0 1> == flipped orientation
          // data-lanes = <0 1> == 2 lane DP, normal orientation
          // data-lanes = <2 3> == 2 lane DP, flipped orientation
          // data-lanes = <0/1/2/3> == 1 lane DP
          remote-endpoint = <&dp_connector>;
        };
      };
    };

    dp-phy {
      ports {
        dp_phy_out: port {
          remote-endpoint = <&qmp_dp_in>;
          data-lanes = <3 1 2 0>; // remap lanes
        };
      };
    };
  };

  dp-connector {
    compatible = "dp-connector";
    ports {
      dp_connector: port@0 {
        remote-endpoint = <&qmp_dp_out>;
      };
    };
  };

 - 2 lanes DP, 2 lanes USB fixed, flipped orientation of QMP
  qmp {
    ports {
      qmp_usb_in: port@0 {
        reg = <0>;
        remote-endpoint = <&usb_phy_out>;
      };
      qmp_dp_in: port@1 {
        reg = <1>;
        remote-endpoint = <&dp_phy_out>;
      };
      port@2 {
        reg = <2>;
        qmp_usb_out: endpoint@1 {
          reg = <1>;
          data-lanes = <2 3>; // SSTRX2 (flipped)
          remote-endpoint = <&usb_hub_in>;
        };
        qmp_dp_out: endpoint@2 {
          reg = <2>;
          data-lanes = <0 1>; // SSTRX1 (flipped)
          remote-endpoint = <&cros_ec_dp_in>;
        };
      };
    };

    dp-phy {
      ports {
        dp_phy_out: port {
          remote-endpoint = <&qmp_dp_in>;
          data-lanes = <0 1>; // DP phy fixed at two lanes, remap possible
        };
      };
    };

    usb-phy {
      ports {
        usb_phy_out: port {
          remote-endpoint = <&qmp_usb_in>;
          data-lanes = <0 1>; // remap lanes possibly? otherwise implicit
        };
      };
    };
  };

  usb-hub {
    compatible = "usb-hub";
    ports {
      usb_hub_in: port@0 {
        remote-endpoint = <&qmp_usb_out>;
      };
    };
  };

  ec {
    cros_ec_typec {
      ports {
        port@0 { // inputs
          reg = <1>;
          cros_ec_dp_in: endpoint@0 {
            reg = <0>;
            remote-endpoint = <&qmp_dp_out>;
          };
        };
      };

      usb-c0 {
        compatible = "usb-c-connector";
	// Do we care to connect this in the graph?
      };

      usb-c1 {
        compatible = "usb-c-connector";
      };
    };

 - 2/4 lanes DP, 2 lanes USB (i.e. USB+DP what you have)
  qmp {
    mode-switch;
    orientation-switch;
    ports {
      qmp_usb_in: port@0 {
        reg = <0>;
        remote-endpoint = <&usb_phy_out>;
      };
      qmp_dp_in: port@1 {
        reg = <1>;
        remote-endpoint = <&dp_phy_out>;
      };
      port@2 {
        reg = <2>;
        qmp_usb_dp_out: endpoint@0 {
          reg = <0>;
          // data-lanes indicates orientation if this
          // doesn't have an orientation-switch property
          //
          // data-lanes = <0 1 2 3> == normal (default)
          // data-lanes = <2 3 0 1> == flipped
          remote-endpoint = <&usb_c_connector_ss0>;
        };
      };
    };

    dp-phy {
      ports {
        dp_phy_out: port {
          remote-endpoint = <&qmp_dp_in>;
          // data-lanes can only be <0 1> or <1 0> and
          // orientation-switch can't be present in qmp
          // when data-lanes is here.
        };
      };
    };

    usb-phy {
      ports {
        usb_phy_out: port {
          remote-endpoint = <&qmp_usb_in>;
        };
      };
    };
  };

  glink {
    usb-c-connector {
      compatible = "usb-c-connector";
      ports {
        port@1 {
          reg = <1>;
          endpoint@0 {
            reg = <0>;
            remote-endpoint = <&qmp_usb_dp_out>;
          };
        };
      };
    };
  };

In your case (this last example) you don't need to have an extra graph
outside the connector in glink, because you have a direct connection
between QMP and the usb-c-connector. As I understand it, you want to add
another endpoint to usb-c-connector above, endpoint@1 in port@1, that
connects directly to the DP controller. It doesn't scale if we add
another altmode though, we'll have to add another virtual endpoint. We
can also see that DP is connected if we walk the graph to the dp phy.

------

Assuming everything is good above, the primary concern I'm left with is
how to find the drm_bridge from the DP controller driver. It would be
convenient to make a graph connection from the DP controller to the
mode-switch device. Then we could cut out the DP phy and the QMP part
and avoid walking the graph from DP phy to qmp to the next endpoint that
may or may not be a drm_bridge. In your case, the mode-switch is the
qmp, because it is used to control 2 or 4 lanes of DP. In chromeos' case
it's the EC. Either way, we're talking about a virtual link in the
binding, to make things simpler for the drm_bridge linkage code
devm_drm_of_get_bridge(). It would leave us with a parallel graph
connection from the DP controller node and the QMP node. I'm not excited
about this approach.

I wonder if drm can learn to walk from the 'phys' property to the graph
of the phy node and then search from there for a drm_bridge. Or if it's
simpler we can make a drm_bridge in the dp phy, qmp, and in
cros_ec/glink, where the dp phy and qmp would do nothing besides have a
drm_bridge_funcs::attach() function that knew which port in their graph
to search for the next bridge on. Then we could connect the port in the
DP controller's graph representing the DP output to the DP phy graph as
an input. Then the drm_bridge isn't entirely useless, just a small bit
of code to do the walk on attach.

The glink driver would make a drm_bridge for each usb-c-connector and
associate the connector of_node with the bridge. Similarly, with a cros
EC where the relationship is 1:1 we would make a drm_bridge for each
usb-c-connector and omit the graph binding outside the connector in
cros_ec_typec, directly connecting the graph from QMP to the connector.
This is because of_drm_find_bridge() looks for the parent of the graph
to find the bridge, and we would have two bridges in this case that need
different nodes.

[PATCH 2/2] usb: typec: mux: Introduce GPIO-based SBU mux

[Bjorn Andersson]

A design found in various Qualcomm-based boards is to use a USB switch,
controlled through a pair of GPIO lines to connect, disconnect and
switch the orientation of the SBU lines in USB Type-C applications.

This introduces a generic driver, which implements the typec_switch and
typec_mux interfaces to perform these operations.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/usb/typec/mux/Kconfig        |   6 +
 drivers/usb/typec/mux/Makefile       |   1 +
 drivers/usb/typec/mux/gpio-sbu-mux.c | 171 +++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 drivers/usb/typec/mux/gpio-sbu-mux.c

diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index 5eb2c17d72c1..c46fa4f9d3df 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -12,6 +12,12 @@ config TYPEC_MUX_FSA4480
 	  common USB Type-C connector.
 	  If compiled as a module, the module will be named fsa4480.
 
+config TYPEC_MUX_GPIO_SBU
+	tristate "Generic GPIO based SBU mux for USB Type-C applications"
+	help
+	  Say Y or M if your system uses a GPIO based mux for managing the
+	  connected state and the swapping of the SBU lines in a Type-C port.
+
 config TYPEC_MUX_PI3USB30532
 	tristate "Pericom PI3USB30532 Type-C cross switch driver"
 	depends on I2C
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
index e52a56c16bfb..dda67e19b58b 100644
--- a/drivers/usb/typec/mux/Makefile
+++ b/drivers/usb/typec/mux/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TYPEC_MUX_FSA4480)		+= fsa4480.o
+obj-$(CONFIG_TYPEC_MUX_GPIO_SBU)	+= gpio-sbu-mux.o
 obj-$(CONFIG_TYPEC_MUX_PI3USB30532)	+= pi3usb30532.o
 obj-$(CONFIG_TYPEC_MUX_INTEL_PMC)	+= intel_pmc_mux.o
diff --git a/drivers/usb/typec/mux/gpio-sbu-mux.c b/drivers/usb/typec/mux/gpio-sbu-mux.c
new file mode 100644
index 000000000000..35f7bd3029a9
--- /dev/null
+++ b/drivers/usb/typec/mux/gpio-sbu-mux.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ */
+
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
+
+struct gpio_sbu_mux {
+	struct gpio_desc *enable_gpio;
+	struct gpio_desc *select_gpio;
+
+	struct typec_switch_dev *sw;
+	struct typec_mux_dev *mux;
+
+	struct mutex lock; /* protect enabled and swapped */
+	bool enabled;
+	bool swapped;
+};
+
+static int gpio_sbu_switch_set(struct typec_switch_dev *sw,
+			       enum typec_orientation orientation)
+{
+	struct gpio_sbu_mux *sbu_mux = typec_switch_get_drvdata(sw);
+	bool enabled;
+	bool swapped;
+
+	mutex_lock(&sbu_mux->lock);
+
+	enabled = sbu_mux->enabled;
+
+	switch (orientation) {
+	case TYPEC_ORIENTATION_NONE:
+		enabled = false;
+		break;
+	case TYPEC_ORIENTATION_NORMAL:
+		swapped = false;
+		break;
+	case TYPEC_ORIENTATION_REVERSE:
+		swapped = true;
+		break;
+	}
+
+	if (enabled != sbu_mux->enabled)
+		gpiod_set_value(sbu_mux->enable_gpio, enabled);
+
+	if (swapped != sbu_mux->swapped)
+		gpiod_set_value(sbu_mux->select_gpio, swapped);
+
+	sbu_mux->enabled = enabled;
+	sbu_mux->swapped = swapped;
+
+	mutex_unlock(&sbu_mux->lock);
+
+	return 0;
+}
+
+static int gpio_sbu_mux_set(struct typec_mux_dev *mux,
+			    struct typec_mux_state *state)
+{
+	struct gpio_sbu_mux *sbu_mux = typec_mux_get_drvdata(mux);
+
+	mutex_lock(&sbu_mux->lock);
+
+	switch (state->mode) {
+	case TYPEC_STATE_SAFE:
+	case TYPEC_STATE_USB:
+		sbu_mux->enabled = false;
+		break;
+	case TYPEC_DP_STATE_C:
+	case TYPEC_DP_STATE_D:
+	case TYPEC_DP_STATE_E:
+		sbu_mux->enabled = true;
+		break;
+	default:
+		break;
+	}
+
+	gpiod_set_value(sbu_mux->enable_gpio, sbu_mux->enabled);
+
+	mutex_unlock(&sbu_mux->lock);
+
+	return 0;
+}
+
+static int gpio_sbu_mux_probe(struct platform_device *pdev)
+{
+	struct typec_switch_desc sw_desc = { };
+	struct typec_mux_desc mux_desc = { };
+	struct device *dev = &pdev->dev;
+	struct gpio_sbu_mux *sbu_mux;
+
+	sbu_mux = devm_kzalloc(dev, sizeof(*sbu_mux), GFP_KERNEL);
+	if (!sbu_mux)
+		return -ENOMEM;
+
+	mutex_init(&sbu_mux->lock);
+
+	sbu_mux->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(sbu_mux->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(sbu_mux->enable_gpio),
+				     "unable to acquire enable gpio\n");
+
+	sbu_mux->select_gpio = devm_gpiod_get(dev, "select", GPIOD_OUT_LOW);
+	if (IS_ERR(sbu_mux->select_gpio))
+		return dev_err_probe(dev, PTR_ERR(sbu_mux->select_gpio),
+				     "unable to acquire select gpio\n");
+
+	sw_desc.drvdata = sbu_mux;
+	sw_desc.fwnode = dev_fwnode(dev);
+	sw_desc.set = gpio_sbu_switch_set;
+
+	sbu_mux->sw = typec_switch_register(dev, &sw_desc);
+	if (IS_ERR(sbu_mux->sw))
+		return dev_err_probe(dev, PTR_ERR(sbu_mux->sw),
+				     "failed to register typec switch\n");
+
+	mux_desc.drvdata = sbu_mux;
+	mux_desc.fwnode = dev_fwnode(dev);
+	mux_desc.set = gpio_sbu_mux_set;
+
+	sbu_mux->mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(sbu_mux->mux)) {
+		typec_switch_unregister(sbu_mux->sw);
+		return dev_err_probe(dev, PTR_ERR(sbu_mux->mux),
+				     "failed to register typec mux\n");
+	}
+
+	platform_set_drvdata(pdev, sbu_mux);
+
+	return 0;
+}
+
+static int gpio_sbu_mux_remove(struct platform_device *pdev)
+{
+	struct gpio_sbu_mux *sbu_mux = platform_get_drvdata(pdev);
+
+	gpiod_set_value(sbu_mux->enable_gpio, 0);
+
+	typec_mux_unregister(sbu_mux->mux);
+	typec_switch_unregister(sbu_mux->sw);
+
+	return 0;
+}
+
+static const struct of_device_id gpio_sbu_mux_match[] = {
+	{ .compatible = "gpio-sbu-mux", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_sbu_mux_match);
+
+static struct platform_driver gpio_sbu_mux_driver = {
+	.probe = gpio_sbu_mux_probe,
+	.remove = gpio_sbu_mux_remove,
+	.driver = {
+		.name = "gpio_sbu_mux",
+		.of_match_table = gpio_sbu_mux_match,
+	},
+};
+module_platform_driver(gpio_sbu_mux_driver);
+
+MODULE_DESCRIPTION("GPIO based SBU mux driver");
+MODULE_LICENSE("GPL");
-- 
2.35.1