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 <pmalani@xxxxxxxxxxxx> 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 <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
> >
