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 <pmalani@xxxxxxxxxxxx> wrote:
>
> 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:
> > On Wed, Jun 10, 2020 at 11:49 AM Prashant Malani <pmalani@xxxxxxxxxxxx> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Wed, Jun 10, 2020 at 9:53 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> > > >
> > > > > 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).
>
> >
> > > 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
>
