On Wed, Jun 10, 2020 at 10:53:45AM -0600, Rob Herring wrote: > On Wed, Jun 10, 2020 at 9:34 AM Heikki Krogerus > <heikki.krogerus@xxxxxxxxxxxxxxx> wrote: > > > > On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote: > > > Hi Rob, > > > > > > Thanks again for the comments and feedback. Kindly see responses inline: > > > > > > (Trimming unrelated text from thread): > > > > > > On Tue, Jun 09, 2020 at 02:30:11PM -0600, Rob Herring wrote: > > > > On Fri, May 29, 2020 at 5:30 PM Prashant Malani <pmalani@xxxxxxxxxxxx> wrote: > > > > > > > > > > Nodes truncated and unrelated fields omitted in the interest of brevity: > > > > > > > > > > // Chrome OS EC Type C Port Manager. > > > > > typec { > > > > > compatible = "google,cros-ec-typec"; > > > > > #address-cells = <1>; > > > > > #size-cells = <0>; > > > > > > > > > > connector@0 { > > > > > compatible = "usb-c-connector"; > > > > > reg = <0>; > > > > > power-role = "dual"; > > > > > data-role = "dual"; > > > > > try-power-role = "source"; > > > > > mode-switch = <&foo_mux>; > > > > > // Other switches can point to the same mux. > > > > > .... > > > > > > > > The connector is supposed to have 'ports' for USB2, USB3, and Aux > > > > unless the parent is the USB controller. > > > Understood; so, coupled with Heikki's explanation (see below for where > > > I've pasted it), would it be something like so? (adding inline to the connector > > > node definition): > > > > > > 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>; > > > usb_con0_ss: endpoint@0 { > > > remote-endpoint = <&mode_mux_in>; > > > }; > > > }; > > > > > > port@2 { > > > reg = <2>; > > > usb_con_sbu: endpoint { > > > remote-endpoint = <&foo_dp_aux>; > > > }; > > > }; > > > }; > > > > The pins that can be reassigned can in practice go anywhere. We can't > > group them in any way. What do we do for example when the two sideband > > pins go to different locations? > > The sideband pins from the connector go to multiple places or the > sideband signal from a controller go to multiple connectors? Either > way, that's solved with multiple endpoints. In the former case, port@2 > would have multiple endpoints with all the possible connections. The > general model of the graph is each port is a separate data channel and > multiple endpoints are either a mux or fanout depending on the data > direction. No, that's not what I'm trying to ask here. Bad example, sorry. I'm trying to understand why is it necessary to slit the connector into three separate interfaces? There does not seem to be anything in the kernel that would benefit from that. Why isn't the connector described as a single interface in devicetree? My concern with the three separate interfaces is that they may force us to know in kernel which of the three interfaces are association with a mode, and actually not just the mode, but the possible the pin configurations of the mode. That is something that we may end up having to hard code into the drivers, even though it does not provide any useful information to us, and that would not be OK. Right now they also allow making assumptions regarding the alternate modes since there are no "bindings" for those, for example, if these OF graph ports have an endpoint to the DP, it means DP alt mode is supported. But that is of course not true. DisplayPort is used also with other alternate modes. We must never make any assumptions based on those interfaces. So again, why do we have them? Either I'm missing something, or the devicetree description of the Type-C connectors really is way too complex, way too "low level", causing us potential problems without providing anything that we could actually ever use in the operating system. thanks, -- heikki