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

Rob Herring
2023-01-25T02:31:31Z

On Tue, Jan 24, 2023 at 11:04 AM Bjorn Andersson
<quic_bjorande@quicinc.com> wrote:
>
> On Tue, Jan 24, 2023 at 10:00:12AM -0600, Rob Herring wrote:
> > On Thu, Jan 19, 2023 at 11:40 AM Bjorn Andersson
> > <quic_bjorande@quicinc.com> wrote:
> > >
> > > On Thu, Jan 19, 2023 at 10:11:32AM -0600, Rob Herring wrote:
> > > > On Wed, Jan 18, 2023 at 10:08:11AM -0800, Bjorn Andersson wrote:
> > > > > On Tue, Jan 17, 2023 at 11:56:57AM -0600, Rob Herring wrote:
> > > > > > On Thu, Jan 12, 2023 at 08:11:14PM -0800, Bjorn Andersson wrote:
> > > > > > > From: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > > > > >
> > > > > > > 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>
> > > > > > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > > > > > ---
> > > >
> > > >
> > > > > > > + tcpm {
> > > > > > > + connector {
> > > > > > > + compatible = "usb-c-connector";
> > > > > > > +
> > > > > > > + ports {
> > > > > > > + #address-cells = <1>;
> > > > > > > + #size-cells = <0>;
> > > > > > > +
> > > > > > > + port@0 {
> > > > > > > + reg = <0>;
> > > > > > > + tcpm_hs_out: endpoint {
> > > > > > > + remote-endpoint = <&usb_hs_phy_in>;
> > > > > > > + };
> > > > > > > + };
> > > > > > > +
> > > > > > > + port@1 {
> > > > > > > + reg = <1>;
> > > > > > > + tcpm_ss_out: endpoint {
> > > > > > > + remote-endpoint = <&usb_ss_phy_in>;
> > > > > > > + };
> > > > > > > + };
> > > > > > > +
> > > > > > > + port@2 {
> > > > > > > + reg = <2>;
> > > > > > > + tcpm_sbu_out: endpoint {
> > > > > > > + remote-endpoint = <&sbu_mux_in>;
> > > > > > > + };
> > > > > > > + };
> > > > > > > + };
> > > > > > > + };
> > > > > > > + };
> > > > > > > +
> > > > > > > + 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 {
> > > > > > > + sbu_mux_in: endpoint {
> > > > > > > + remote-endpoint = <&tcpm_sbu_out>;
> > > > > > > + };
> > > > > >
> > > > > > Don't you need a connection to whatever drives SBU? Maybe your case is
> > > > > > fixed because the phy does the DP/USB muxing? But the binding needs to
> > > > > > support the worst case which I guess would be all the muxing/switching
> > > > > > is done by separate board level components.
> > > > > >
> > > > >
> > > > > Perhaps I'm misunderstanding your request, but I think this is the worst
> > > > > case you're talking about.
> > > > >
> > > > > &usb_ss_phy_in is a reference to the PHY, which does switching/muxing of
> > > > > the SuperSpeed lanes in the connector, but the PHY provides no control
> > > > > over the SBU signals.
> > > > >
> > > > > So this sbu-mux is a separate component between the SBU-pads on the SoC
> > > > > and the usb-c-connector, referenced through he &sbu_mux_in reference.
> > > > >
> > > > >
> > > > > So upon e.g. a orientation switch, the typec_switch_set() call the tcpm
> > > > > implementation will request orientation switching from port@1 and port@2
> > > > > (no orientation-switch on port@0/HS pins).
> > > >
> > > > 'port@2' is supposed to define the connection to what controls SBU. The
> > > > mux here switches the signals, but it doesn't control them.
> > >
> > > The SBU signals are driven by the SS PHY, on behalf of the DisplayPort
> > > controller. These signals are turned on/off as a result of the TCPM
> > > indicating the HPD state to the DisplayPort controller.
> > >
> > > There's a such not really a direct representation today of the entity
> > > that drives the SBU lines. It happens to be a sub-block in
> > > &usb_ss_phy_in, but I don't envision that we need/want any signaling
> > > between the TCPM and the SBU-"driver".
> > >
> > >
> > > I see that I missed that in the example above, your suggestion on how to
> > > model that relationship (TCPM - DP controller) was to add an additional
> > > endpoint in port@1. So that's the current design (but neither ports nor
> > > endpoints are significant from an implementation point of view).
> > >
> > > > The mux should sit in the middle, but the graph terminates at the mux.
> > > > You don't have a connection presumably because you know what the
> > > > connection.
> > >
> > > But do you suggest that the graph should reference the entity that
> > > drives the SBU signals?
> >
> > Yes, that was the original intent.
> >
>
> Directly from the connector, or just indirectly?
>
> > > What about the discrete mux?
> >
> > You mean the mux in this binding, right? That should be in the middle:
> >
> > DPaux --> SBUmux --> connector
> >
> > Maybe the SS phy is in there too.
> >
>
> The signal originally comes from the DP controller, the analog
> electronics lives in the SS phy, then the signal goes to the SBU mux and
> finally to the connector.
>
> > >
> > > > Perhaps because there is only 1 connector and controller.
> > > >
> > >
> > > There is one SBU mux, one DP controller and one SS PHY per
> > > usb-c-connector.
> > >
> > > > Suppose you have 2 connectors and 2 controllers which drive SBU
> > > > signals. Also assume that the SBU signals are completely independent
> > > > from what's driving the altmode SS signals. How would you describe that?
> > > >
> > >
> > > This is the setup we have on e.g. SC8280XP CRD; where the TCPM has two
> > > usb-c-connectors defined, each with their graph referencing the SS PHY,
> > > DP controller and respective sbu-mux.
> > >
> > > There's an incomplete example of this published at [1] (where the SS phy
> > > isn't represented yet - and hence there's no control over the SS lanes,
> > > nor is the HS lanes connected to the dwc3 for role switching).
> > >
> > > Perhaps I'm misunderstanding your concerns though?
> >
> > That looks like you can assume who drives SBU based on the DP
> > controller. Probably a safe assumption for DP (that DP-aux is part of
> > the DP controller), but I was more worried about if you can't assume
> > that relationship. Take HDMI for example where the DDC signals can
> > come from anywhere. They could be part of the HDMI bridge, a general
> > purpose I2C bus off the SoC, or bitbanged GPIOs. Though from what I've
> > read, HDMI Altmode is dead. I don't know if the need to describe the
> > SBU connection would apply to anything else.
> >
> > I guess this all boils down to whether the SBU mux should have a 2nd
> > optional port as the input for what drives it.
> >
>
> Are you saying that the connector should link with the mux and then the
> source of the signal should be daisy chained? Or that we should just
> link both of them as two separate endpoints from the connector?

The former. The data path of the signal in h/w should match in the DT
graph. The caveat being we don't normally show PHYs in that mix
because they are somewhat transparent. That's maybe becoming less true
with routing or muxing included in PHYs.

Rob