lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 2 Sep 2022 07:41:07 +0000
From:   Prashant Malani <pmalani@...omium.org>
To:     Rob Herring <robh@...nel.org>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        Pin-yen Lin <treapking@...omium.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        Benson Leung <bleung@...omium.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Nícolas F . R . A . Prado 
        <nfraprado@...labora.com>, Allen Chen <allen.chen@....com.tw>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>, devicetree@...r.kernel.org,
        dri-devel@...ts.freedesktop.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Jonas Karlman <jonas@...boo.se>,
        José Expósito <jose.exposito89@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Maxime Ripard <maxime@...no.tech>,
        Neil Armstrong <narmstrong@...libre.com>,
        Robert Foss <robert.foss@...aro.org>,
        Sam Ravnborg <sam@...nborg.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Xin Ji <xji@...logixsemi.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding

Hi Rob,

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

Below I've listed the proposal binding (for the Type-C connector) along
with 2 sample hardware diagrams and corresponding DT.

The updated binding in usb-c-connector would be as follows:

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index ae515651fc6b..a043b09cb8ec 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -183,6 +183,30 @@ properties:
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: Super Speed (SS), present in SS capable connectors.
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
+        patternProperties:
+          "^endpoint@[0-1]$":
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description:
+              Endpoints for the two SS lanes. endpoint@0 refers to SSTRX1 (A2,A3,B10,B11)
+              and endpoint@1 refers to SSTRX2 (B2,B3,A10,A11).
+            additionalProperties: false
+
+              properties:
+                reg:
+                  maxItems: 1
+
+                remote-endpoint: true
+
+              required:
+                - reg
+                - remote-endpoint

       port@2:
         $ref: /schemas/graph.yaml#/properties/port

Here are 2 examples of how that would look on some existing hardware:

Example 1. 2 usb-c-connectors connecting to 1 drm bridge / DP switch:

Here is the diagram we are using on the MTK platform:

                 SOC
        +---------------------+                                              C0
        |                     |            +----------+       2 lane      +--------+
        |                     |            |          +---------/---------+ SSTRX1 |
        |                     |            |          |                   |        |
        |    MIPI DPI         |            |          |  2 lane           |        |
        |                     +------------+ ANX 7625 +---/-----+    +----+ SSTRX2 |
        |                     |            |          |         |    |    +--------+
        |                     |            +----------+         |    |
        +---------------------+                                 |    |
        |                     |            +----------+ 2 lane  |    |       C1
        |                     |            |          +----/----C----+    +--------+
        |    USB3 HC          |   2 lane   |          |         |         | SSTRX1 |
        |                     +-----/------+ USB3 HUB |         +---------+        |
        |  (host controller)  |            |          |       2 lane      |        |
        |                     |            |          +---------/---------+ SSTRX2 |
        +---------------------+            |          |                   |        |
                                           +----------+                   +--------+

Some platforms use it6505, so that can be swapped in for anx7625
without any change to the rest of the hardware diagram.

>From the above, we can see that it is helpful to describe the
Type-C SS lines as 2 endpoints:
- 1 for SSTX1+SSRX1 (A2,A3 + B10,B11)
- 1 for SSTX2+SSRX2 (B2,B3 + A10, A11)

A device tree for this would look as follows:

// Type-C port driver
ec {
    ...
    cros_ec_typec {
        ...
        usb-c0 {
            compatible = "usb-c-connector";
            ports {
                hs : port@0 {
                    ...
                };
                ss: port@1 {
                    reg = <1>;
                    c0_sstrx1: endpoint@0 {
                        reg = <0>;
                        remote-endpoint = <&anx7625_out0>;
                    };
                    c0_sstrx2: endpoint@0 {
                        reg = <0>;
                        remote-endpoint = <&usb3hub_out0>;
                    };
                };
                sbu : port@2 {
                    ...
                };
            };
        };
        usb-c1 {
            compatible = "usb-c-connector";
            ports {
                hs : port@0 {
                    ...
                };
                ss: port@1 {
                    reg = <1>;
                    c1_sstrx1: endpoint@0 {
                        reg = <0>;
                        remote-endpoint = <&anx7625_out1>;
                    };
                    c1_sstrx2: endpoint@0 {
                        reg = <0>;
                        remote-endpoint = <&usb3hub_out1>;
                    };
                };
                sbu : port@2 {
                    ...
                };
            };
        };
    };
};

// DRM bridge / Type-C mode switch
anx_bridge: anx7625@58 {
    compatible = "analogix,anx7625";
    reg = <0x58>;
    ...
    // Input from DP controller
    port@0 {
        reg = <0>;
        ...
    };

    // Output to Type-C connector / DP panel
    port@1 {
        reg = <1>;

        anx7625_out0: endpoint@0 {
            reg = <0>;
            mode-switch;
            remote-endpoint = <&c0_sstrx1>;
        };
        anx7625_out1: endpoint@1 {
            reg = <1>;
            mode-switch;
            remote-endpoint = <&c1_sstrx1>;
        };
    };
};

// USB3 hub
usb3hub: foo_hub {
    ...
    ports@0 {
         // End point connected to USB3 host controller on SOC.
    };
    port@1 {
        reg = <1>;

        foo_hub_out0: endpoint@0 {
            reg = <0>;
            mode-switch; ---> See c.) later
            remote-endpoint = <&c0_sstrx2>;
        };
        foo_hub_out1: endpoint@1 {
            reg = <1>;
            mode-switch;
            remote-endpoint = <&c1_sstrx2>;
        };
    };
};

Notes:
- On the Chrome OS platform, the USB3 Hub is controlled by
the EC, so we don't really need to describe that connection,
but I've added a minimal one here just to show how the graph
connection would work if the HUB was controlled by the SoC.
- The above assumes that other hardware is controlling orientation.
We can add "orientation-switch" drivers along the graph path
if there is other hardware which controls orientation.

Example 2: 1 USB-C connector connected to 1 drm-bridge/ mode-switch

I've tried to use Bjorn's example [1], but I might have made
some mistakes since I don't have access to the schematic.


                  SoC
  +------------------------------------------+
  |                                          |
  |  +---------------+                       |
  |  |               |                       |
  |  |  DP ctrllr    |       +---------+     |                 C0
  |  |               +-------+         |     |   2 lane     +----------+
  |  +---------------+       |  QMP    +-----+-----/--------+ SSTRX1   |
  |                          |  PHY    |     |              |          |
  |  +-------------+  2 lane |         |     |   2 lane     |          |
  |  |             +----/----+         +-----+-----/--------+ SSTRX2   |
  |  |    dwc3     |         +---------+     |              |          |
  |  |             |                         |              |          |
  |  |             |         +---------+     |              |          |
  |  |             +---------+ HS PHY  |     |   HS lanes   |          |
  |  +-------------+         |         +-----+----/---------+ D +/-    |
  |                          |         |     |              +----------+
  |                          +---------+     |
  |                                          |
  +------------------------------------------+

The DT would look something like this (borrowing from Stephen's example [2]):

qmp {
    mode-switch; ----> See b.) later.
    orientation-switch;
    ports {
        qmp_usb_in: port@0 {
            reg = <0>;
            remote-endpoint = <&usb3_phy_out>;
        };
        qmp_dp_in: port@1 {
            reg = <1>;
            remote-endpoint = <&dp_phy_out>;
        };
        port@2 {
            reg = <2>;
            qmp_usb_dp_out0: endpoint@0 {
                reg = <0>;
                remote-endpoint = <&c0_sstrx1>;
            };
            qmp_usb_dp_out1: endpoint@1 {
                reg = <1>;
                remote-endpoint = <&c0_sstrx2>;
            };
        };
};

dp-phy {
    ports {
        dp_phy_out: port {
            remote-endpoint = <&qmp_dp_in>;
        };
    };
};

dwc3: usb-phy {
    ports {
        usb3_phy_out: port@0 {
            reg = <0>;
            remote-endpoint = <&qmp_usb_in>;
        };
    };
};

glink {
    c0: usb-c-connector {
        compatible = "usb-c-connector";
        ports {
            hs: port@0 {
                reg = <0>;
                endpoint@0 {
                    reg = <0>;
                    remote-endpoint = <&hs_phy_out>;
                };
            };

            ss: port@1 {
                reg = <1>;
                c0_sstrx1: endpoint@0 {
                    reg = <0>;
                    remote-endpoint = <&qmp_usb_dp_out0>;
                };
                c0_sstrx2: endpoint@1 {
                    reg = <1>;
                    remote-endpoint = <&qmp_usb_dp_out1>;
                };
            };
        };
    };
};

Notes:
a. This proposal doesn't deal with the DRM bridge HPD forwarding; I
believe that is covered by Stephen's example/proposal in [2], and
can be addressed separately. That said, this binding is compatible
with the proposal in [2], that is, make the "mode-switch" driver a
drm-bridge and forward the HPD info to the upstream DRM-bridge (DP controller).
The driver implementing "mode-switch" will be able to do that, since
it gets DP status/attention VDOS with HPD info from the Type-C port driver.
b. If both SSTRX pairs from a connector are routed to the same
hardware block (example 2) then the device would keep "mode-switch"
as a top level property (and the fwnode associated with "mode-switch"
is the drm-bridge device).
c. If SSTRX pairs from 2 connectors are routed to the same
hardware block (example 1), then each end-point which is connected to
the USB-C connector will have a "mode-switch" property in its end-point.
There will be 2 mode switches registered here, and the fwnode for each
"mode-switch" is the end-point node.

b.) and c.) can be handled by Type C mux registration and matching
code. We already have 3 mux devs for each mux [3].

For the single mode-switch case, mux_dev[1] will just refer to the top-level
mode-switch registered by the DRM bridge / switch driver (example 1).
For the 2 mode-switch case, typec_mux_dev[1] will have 2 child
typec_mux_dev's, each of which represents the mode-switches
registered by the DRM bridge / switch driver. Introducing this
indirection means the port driver / alternate mode driver don't
need to care about how the connectors are routed; the framework
will just call the mux_set() function on the mux_dev() or its
children if it has any.

The benefit of this approach is existing bindings (which just
assume 1 endpoint from usb-c-connector/port@1) should continue to
work without any changes.

Why don't we use data lanes for the usb-c-connector
endpoints? I guess we could, but I am not a fan of adding the
extra data-lane parsing logic to the Type-C framework (I
don't think drivers need that level of detail from the connector
binding). And even then, we will still need an extra end-point
if the lanes of the USB-C connector are routed to different hardware blocks.

The Type-C connector spec doesn't specify any alternate modes
with < 1 SSTRX pair, so the most we can ever have (short of a
major change to the spec) is 2 SSTRX end points for a
connector each being routed to different hardware blocks.
Codifying these as endpoint@0 and endpoint@1 in the usb-c-connector
binding seems to line up nicely with this detail of the spec.

Thanks,

-Prashant

[1] https://lore.kernel.org/linux-usb/Yv1y9Wjp16CstJvK@baldur/
[2] https://lore.kernel.org/linux-usb/CAE-0n52-QVeUVCB1qZzPbYyrb1drrbJf6H2DEEW9bOE6mh7egw@mail.gmail.com/
[3] https://elixir.bootlin.com/linux/v6.0-rc3/source/drivers/usb/typec/mux.c#L259

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ