Hi Rob,

Thought I'd check in again to see if you've had a chance to look at
this proposal.

Since Type C connector class framework assumes the existing
"{mode,orientation,data-role}-switch" bindings for non-DT platforms
already, as I see it, we can either:

1. Implement a different handling for DT platforms which utilizes port
end-points and update the Type C connector class framework to parse
those accordingly; this is what the above proposal suggests. It
reserves some end-points for the "switches" that the Type C connector
class framework expects and just follows the OF graph till it finds
the various switches. Other schemas that use usb-connector.yaml schema
can add more end-points as their use case deems needed, as long as
they're not the reserved ones.

<or>

2. Let various schemas that use usb-connector.schema add their own
bindings according to their requirements (in the example of
cros-ec-typec, it is adding the "*-switch" nodes directly under each
connector instead of using OF graph so that Type C connector class
framework can detect the switches, but there other examples for other
use cases).

I'm fine with either, but since this thread is now nearly 3 months
old, it would be nice to arrive at a decision.

Best regards,

On Mon, Jun 29, 2020 at 1:41 PM Prashant Malani [off-list ref] wrote:

Hi Rob,

Just following up on this. Would the below example align better with
OF graph requirements?

Example begins at <example_start>, but in summary:
- port@1 (Superspeed) of usb-c-connector will have 3 endpoints (0 =
goes to mode switch, 1 = goes to orientation switch, 2 = goes to data
role switch)
- port@2 (SBU) of usb-c-connector will have 2 endpoints (0 = goes to
mode switch, 1 = goes to orientation switch)
-These end points can go through arbitrarily long paths (including
retimers) as long as they end up at the following devices:
    a. device with compatible string "typec-mode-switch" for endpoint 0.
    b. device with compatible string "typec-orientation-switch" for endpoint 1.
    c. device with compatible string "typec-data-role-switch" for endpoint 2.
- Connector class framework will perform the traversal from
usb-c-connector port endpoints to the "*-switch" devices.

Best regards,

On Fri, Jun 12, 2020 at 10:34 AM Prashant Malani [off-list ref] wrote:

quoted

Hi Rob,

Thanks as always for your help in reviewing this proposal!

Kindly see inline

(Trimming text);
On Thu, Jun 11, 2020 at 02:00:47PM -0600, Rob Herring wrote:

quoted

On Wed, Jun 10, 2020 at 11:49 AM Prashant Malani [off-list ref] wrote:

quoted

Hi Rob,

On Wed, Jun 10, 2020 at 9:53 AM Rob Herring [off-list ref] wrote:

quoted

quoted

On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote:

I think the updated example handles this grouping (port@1 going to a
"SS mux") although as you said it should probably be a group of muxes,
but I think the example illustrates the point. Is that assessment
correct?

Yes, but let's stop calling it a mux. It's a "USB Type C signal routing blob".

Ack.

Let's go with "-switch" ? That's what the connector class uses and it
conveys the meaning (unless that is a reserved keyword in DT).

quoted

quoted

Would this block the addition of the "*-switch" properties? IIUC the
two are related but not dependent on each other.

The *-switch properties are phandles which the Type C connector class
framework expects (and uses to get handles to those switches).
These would point to the "mux" or "group of mux" abstractions as noted earlier.

You don't need them though. Walk the graph. You get the connector
port@1 remote endpoint and then get its parent.

I see; would it be something along the lines of this? (DT example
follows; search for "example_end" to jump to bottom):

<example_start>

connector@0 {
    compatible = "usb-c-connector";
    reg = <0>;
    power-role = "dual";
    data-role = "dual";
    try-power-role = "source";
    ....
    ports {
        #address-cells = <1>;
        #size-cells = <0>;

        port@0 {
            reg = <0>;
            usb_con_hs: endpoint {
                remote-endpoint = <&foo_usb_hs_controller>;
            };
        };

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

            usb_con0_ss_mode: endpoint@0 {
                reg = <0>
                remote-endpoint = <&mode_switch_ss_in>;
            };

            usb_con0_ss_orientation: endpoint@1 {
                        reg = <1>
                        remote-endpoint = <&orientation_switch_ss_in>;
            };

            usb_con0_ss_data_role: endpoint@2 {
                        reg = <2>
                        remote-endpoint = <&data_role_switch_in>;
            };
        };

        port@2 {
            reg = <2>;
            #address-cells = <1>;
            #size-cells = <0>;
            usb_con0_sbu_mode: endpoint@0 {
                        reg = <0>
                        remote-endpoint = <&mode_switch_sbu_in>;
            };
            usb_con0_sbu_orientation: endpoint@1 {
                        reg = <1>
                        remote-endpoint = <&orientation_switch_sbu_in>;
            };
        };
    };
};

mode_switch {
    compatible = "typec-mode-switch";
    mux-controls = <&mode_mux_controller>;
    mux-control-names = "mode";
    #address-cells = <1>;
    #size-cells = <0>;

    port@0 {
        reg = <0>;
        mode_switch_ss_in: endpoint {
            remote-endpoint = <&usb_con0_ss_mode>
        };
    };

    port@1 {
        reg = <1>;
        mode_switch_out_usb3: endpoint {
            remote-endpoint = <&usb3_0_ep>
        };
    };

    port@2 {
        reg = <2>;
        mode_switch_out_dp: endpoint {
            remote-endpoint = <&dp0_out_ep>
        };
    };

    port@3 {
        reg = <3>;
        mode_switch_sbu_in: endpoint {
            remote-endpoint = <&usb_con0_sbu_mode>
        };
    };
    // ... other ports similarly defined.
};

orientation_switch {
    compatible = "typec-orientation-switch";
    mux-controls = <&orientation_mux_controller>;
    mux-control-names = "orientation";
    #address-cells = <1>;
    #size-cells = <0>;

    port@0 {
        reg = <0>;
        orientation_switch_ss_in: endpoint {
            remote-endpoint = <&usb_con0_ss_orientation>
        };
    };

    port@1
        reg = <1>;
        orientation_switch_sbu_in: endpoint {
            remote-endpoint = <&usb_con0_sbu_orientation>
        };
    };
    // ... other ports similarly defined.
};

data_role_switch {
    compatible = "typec-data-role-switch";
    mux-controls = <&data_role_switch_controller>;
    mux-control-names = "data_role";

    port {
        data_role_switch_in: endpoint {
            remote-endpoint = <&usb_con0_ss_data_role>
        };
    };
};

<example_end>

Would this be conformant to OF graph and usb-connector bindings
requirements? We'll certainly send out a format PATCH/RFC series for
this, but I was hoping to gauge whether we're thinking along the right lines.

So, in effect this would mean:
- New bindings(and compatible strings) to be added for:
  typec-{orientation,data-role,mode}-switch.
- Handling in Type C connector class to parse switches from OF graph.
- Handling in Type C connector class for distinct switches for port@1
  (SS lines) and port@2 (SBU lines).

The only thing I'm confused about is how we can define these switch
remote-endpoint bindings in usb-connector.yaml; the port can have an
remote-endpoint, but can we specify what the parent of the remote-endpoint
should have as a compatible string? Or do we not need to?

Best regards,

-Prashant