Thread (33 messages) 33 messages, 5 authors, 2020-07-17

Re: [PATCH 1/2] dt-bindings: chrome: Add cros-ec-typec mux props

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Date: 2020-06-12 12:46:41
Also in: lkml

On Wed, Jun 10, 2020 at 10:53:45AM -0600, Rob Herring wrote:
On Wed, Jun 10, 2020 at 9:34 AM Heikki Krogerus
[off-list ref] wrote:
quoted
On Tue, Jun 09, 2020 at 04:57:40PM -0700, Prashant Malani wrote:
quoted
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:
quoted
On Fri, May 29, 2020 at 5:30 PM Prashant Malani [off-list ref] wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help