Message-ID: <158284127336.4688.623067902277673206@swboyd.mtv.corp.google.com>
Date:   Thu, 27 Feb 2020 14:07:53 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Prashant Malani <pmalani@...omium.org>,
        linux-kernel@...r.kernel.org, enric.balletbo@...labora.com,
        bleung@...omium.org, devicetree@...r.kernel.org,
        Guenter Roeck <groeck@...omium.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v3 1/4] dt-bindings: Add cros-ec Type C port driver

Quoting Heikki Krogerus (2020-02-27 08:38:25)
> Hi Stephen,
> 
> On Thu, Feb 27, 2020 at 12:41:13AM -0800, Stephen Boyd wrote:
> > > +examples:
> > > +  - |+
> > > +    cros_ec: ec@0 {
> > > +      compatible = "google,cros-ec-spi";
> > > +
> > > +      typec {
> > > +        compatible = "google,cros-ec-typec";
> > > +
> > > +        usb_con: connector {
> > > +          compatible = "usb-c-connector";
> > > +          port-number = <0>;
> > > +          power-role = "dual";
> > > +          data-role = "dual";
> > > +          try-power-role = "source";
> > > +        };
> > 
> > I thought that perhaps this would be done with the OF graph APIs instead
> > of being a child of the ec node. I don't see how the usb connector is
> > anything besides a child of the top-level root node because it's
> > typically on the board. We put board level components at the root.
> 
> No.
> 
> The above follows the usb-connector bindings, so it is correct:
> Documentation/devicetree/bindings/connector/usb-connector.txt
> 
> So the connector is always a child of the "CC controller" with the USB
> Type-C connectors, which in this case is the EC (from operating systems
> perspective). The "CC controller" controls connectors, and it doesn't
> actually do anything else. So placing the connectors under the
> "connector controller" is also logically correct.

Ah ok I see. The graph binding is for describing the data path, not the
control path. Makes sense. 

> 
> > Yes, the connector is intimately involved with the EC here, but I would
> > think that we would have an OF graph connection from the USB controller
> > on the SoC to the USB connector, traversing through anything that may be
> > in that path, such as a USB hub. Maybe the connector node itself can
> > point to the EC type-c controller with some property like
> 
> I think your idea here is that there should be only a single node for
> each connector that is then linked with every component that it is
> physically connected to (right?), but please note that that is not
> enough. Every component attached to the connector must have its own
> child node that represents the "port" that is physically connected to
> the USB Type-C connector.
> 
> So for example, the USB controller nodes have child nodes for every
> USB2 port as well as for every USB3 port. Similarly, the GPU
> controllers have child node for every DisplayPort, etc. And I believe
> that is already how it has been done in DT (and also in ACPI).

It looks like perhaps you're conflating ports in USB spec with the OF
graph port? I want there to be one node per type-c connector that I can
physically see on the device. Is that not sufficient?

Are there any examples of the type-c connector in DT? I see some
NXP/Freescale boards and one Renesas board so far. Maybe there are other
discussions I can read up on?

> 
> Those "port" nodes then just need to be linked with the "connector"
> node. I think for that the idea was to use OF graph, but I'm really
> sceptical about that. The problem is that with the USB Type-C
> connectors we have to be able to identify the connections, i.e. which
> endpoint is the USB2 port, which is the DisplayPort and so on, and OF
> graph does not give any means to do that on its own. We will have to
> rely on separate device properties in order to do the identification.
> Currently it is not documented anywhere which property should be used
> for that.

I hope that this patch series can document this. Why can't that work by
having multiple OF graph ports for USB2 port, DisplayPort, USB3 port,
etc? The data path goes to the connector and we can attach more
information to each port node to describe what type of endpoint is there
like a DisplayPort capable type-c connector for example.

> 
> For ACPI we are going to propose that with every type of connection,
> there should be a device property that returns a reference to the
> appropriate port. That way there are no problems identifying the
> connections. All we need to do is to define the property names for
> every type of connection. "usb2-port" for the USB2 or high speed port,
> "usb3-port" for USB3, etc.
> 

That sounds like something we should figure out now for DT firmwares
too. For this particular binding, I don't know if we need to do anything
besides figure out how to represent multiple connectors underneath the
EC node. The other properties seem fairly generic and so I'd expect this
series to migrate
Documentation/devicetree/bindings/connector/usb-connector.txt to YAML
and refine the binding with anything necessary, like a 'reg' property to
allow multiple ports to exist underneath the "CC controller".

Powered by blists - more mailing lists