lkml.org 
[lkml]   [2020]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props
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@chromium.org> wrote:
> >
> > Hi Rob,
> >
> > On Wed, Jun 10, 2020 at 9:53 AM Rob Herring <robh@kernel.org> 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

\
 
 \ /
  Last update: 2020-06-12 19:34    [W:0.123 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site