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

[Bjorn Andersson]

On Tue, Jan 31, 2023 at 01:44:05PM -0600, Rob Herring wrote:
> On Mon, Jan 30, 2023 at 01:42:14PM -0800, Bjorn Andersson wrote:
> > On Mon, Jan 30, 2023 at 10:48:13AM -0600, Rob Herring wrote:
> > > On Wed, Jan 25, 2023 at 03:40:13PM -0800, Bjorn Andersson wrote:
> > > > On Tue, Jan 24, 2023 at 08:31:13PM -0600, Rob Herring wrote:
> > > > > On Tue, Jan 24, 2023 at 11:04 AM Bjorn Andersson
> > > > > <quic_bjorande@xxxxxxxxxxx> wrote:
[..]
> > This is the design we have in a range of different boards:
> 
> *This* is what I need for every Type-C binding.
> 

Glad you like it.

> > 
> >                      +------------- - -
> >  USB connector       | SoC
> >  +-+                 |   +--------+    +-------+
> >  | |                 |   |        |    |       |
> >  |*|<------- HS -----|-->| HS phy |<-->| (EUD) |<--+
> >  | |                 |   |        |    |       |   |   +--------+
> >  | |                 |   +--------+    +-------+   +-->|        |
> >  | |                 |                                 |  dwc3  |
> >  | |                 |   +--------+        /---------->|        |
> >  | |   +----------+  |   |        |<------/            +--------+
> >  |*|<--|(redriver)|<-|-->| SS phy |
> >  | |   +----------+  |   |        |<-\   +------------+
> >  | |                 |   +--------+   \->|            |
> >  | |                 |                   |     DP     |
> >  | |     +-----+     |                   | controller |
> >  |*|<--->| SBU |<----|------------------>|            |
> >  | |     | mux |     |                   |            |
> >  | |     +----+      |                   +------------+
> >  +-+                 |
> >                      +------------- - -
> 
> Where's the TCPM?
> 

The TCPM here becomes the implementation behind one more more
USB connectors.

> 
> > The EUD and redriver are only found/used in some designs.  My proposed
> > representation of this (without those) is:
> 
> I'd assume a redriver is mostly transparent to s/w?
> 

There are both cases. But per our discussion (summarized below), each
entity in the graph should represent the actual signal path.  So in the
case where it need to be represented in the signal path, the
implementation would have to deal with it being the port@1
remote-endpoint.

> 
> > 
> > /soc {
> >     usb-controller {
> >         dwc3 {
> >             port {
> >                 dwc0-out: endpoint {
> >                     remote-endpoint = <&connector0_hs>;
> >             };
> >         };
> >     };
> > 
> >     ss_phy: phy {
> >         port {
> >             ss_phy_out: endpoint {
> >                 remote-endpoint = <&connector0_ss>;
> >             };
> >         };
> >     };
> > 
> >     display-subsystem {
> >         displayport-controller {
> >             phys = <&ss_phy>;
> >             ports {
> >                 port@1 {
> >                     reg = <1>;
> >                     dp0_out: endpoint {
> >                         remote-endpoint = <&connector0_hpd>;
> >                     };
> >                 };
> >             };
> >         };
> >     };
> > };
> > 
> > usb0-sbu-mux {
> >     compatible = "gpio-sbu-mux";
> > 
> >     port {
> >         sbu0_out: endpoint {
> >             remote-endpoint = <&connector_sbu>;
> >         };
> >     };
> > };
> > 
> > tcpm {
> >     connector@0 {
> >         compatible = "usb-c-connector";
> >         reg = <0>;
> >         ports {
> >             port@0 {
> >                 reg = <0>;
> >                 connector0_hs: endpoint {
> >                     remote-endpoint = <&dwc0_out>;
> >                 };
> >             };
> > 
> >             port@1 {
> >                 reg = <1>;
> >                 connector0_ss: endpoint@0 {
> >                     remote-endpoint = <&ss_phy_out>;
> >                 };
> >                 connector0_hpd: endpoint@1 {
> >                     remote-endpoint = <&dp0_out>;
> >                 };
> 
> This just looks wrong to me because one connection is the output of the 
> phy's mux and one is the input. The USB SS connection is implicit, but I 
> think it should be explicit from dwc3 to ss_phy. It would need an output 
> port and 2 input ports. I want to say we already have bindings doing 
> this.
> 

Right, endpoint@0 represents the actual signal path, while endpoint@1
represents the display signal source or HPD destination.

It does look weird, but that's what we agreed upon in a previous
iteration.

> >             };
> > 
> >             port@2 {
> >                 reg = <2>;
> >                     connector_sbu: endpoint {
> >                         remote-endpoint = <&sbu0_out>;
> >                 };
> >             };
> >         };
> >     };
> > };
> > 
[..]
> > 
> > /dp-connector {
> >     compatible = "dp-connector";
> > 
> >     port {
> >         connector: endpoint {
> >             remote-endpoint = <&dp_out>;
> >         };
> >     };
> > };
> > 
> > /soc {
> >     display-subsystem {
> >         displayport-controller {
> >             phys = <&some_dp_phy>;
> >             ports {
> >                 port@1 {
> >                     reg = <1>;
> >                     dp_out: endpoint {
> >                         remote-endpoint = <&connector>;
> >                     };
> >                 };
> >             };
> >         };
> >     };
> > };
> > 
> > As you said previously, it doesn't make sense to represent the phy in this
> > graph. We just define the output of the controller as port@1 and link that to
> > the connector.
> 
> What I said (or meant) was we don't represent phys which are just 
> providing the electrical interface. Your 'phy' is also a mux/switch, so 
> it does make sense to represent it in the graph.
> 

Attempting to summarize our lengthy discussion on IRC.

The output port of the display block represents the signal path.

In the even that the associated phy is merely a dumb D/A converter, the
next logical entity on that path is the connector, such as the
dp-connector example above.

If, on the other hand, the phy, or any other component, on the signal path
is doing more than just electrical conversion, it should be represented
in the DT and linked using the of-graph.

As such, in the case where the phy is involved in e.g. orientation
switching, the output (port@1 in the Qualcomm DP binding) of the
display block should be tied to the phy, and then the phy should be
connected to the next entity on the path (e.g. the usb-c-connector).

In both cases, the phys property can be seen as representing the
"control interface", and the graph is used to represent the signal path.

> > 
> > So what is the output of the dp controller in the USB case - if we're not
> > representing that as the HPD link directly between the connector and the
> > controller?
> 
> The answer lies in your block diagram... 
> 

So, each (active) component in the diagram should be represented in the
Devicetree and the links between them should be represented by
of-graphs.

> The question I think is whether we could standardize the mux/switch 
> graph ports. Say 'port@0' is the output to type-C connector port@1, 
> and port@[1-9] are altmode connections to USB/DP/TB. If we did that, 
> then generic code can walk the graph from a controller to the connector. 
> We only need to know that port@0 goes to the connector.

In the display bindings today, we use port@0 for in and port@1 for
out, but it doesn't some universal.

> However, that assumes there is only 1 entity in the middle and I don't
> know if that holds true.
> 

We've seen examples of redrivers or the ChromeOS switch, where this
doesn't hold. In the latter we have two outputs (one being active at a
time)...

Regards,
Bjorn