[1/4] dt-bindings: usb-connector: Add support for Type-C alternate-modes

This commit adds the minimum bindings required to allow describing which
altmodes a port supports. Currently this is limited to just specifying:

1. The svid, which is the id of the altmode, e.g. displayport altmode has
a svid of 0xff01.

2. The vdo, a 32 bit integer, typically used as a bitmask describing the
capabilities of the altmode, the bits in the vdo are specified in the
specification of the altmode, the dt-binding simply refers to the
specification as that is the canonical source of the meaning of the bits.

Later on we may want to extend the binding with extra properties specific
to some altmode, but for now this is sufficient to e.g. hook up
displayport alternate-mode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note I hope I got the yaml correct, this is my first time writing a
dt-binding in the new yaml style. I did run:
make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/connector/usb-connector.yaml
and that was happy.
---
 .../bindings/connector/usb-connector.yaml     | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

On Tue, Jul 14, 2020 at 01:36:14PM +0200, Hans de Goede wrote:
> This commit adds the minimum bindings required to allow describing which
> altmodes a port supports. Currently this is limited to just specifying:
> 
> 1. The svid, which is the id of the altmode, e.g. displayport altmode has
> a svid of 0xff01.
> 
> 2. The vdo, a 32 bit integer, typically used as a bitmask describing the
> capabilities of the altmode, the bits in the vdo are specified in the
> specification of the altmode, the dt-binding simply refers to the
> specification as that is the canonical source of the meaning of the bits.

What if this information should be derived from information already in 
DT (or would be there if alt mode connections are described)?

> 
> Later on we may want to extend the binding with extra properties specific
> to some altmode, but for now this is sufficient to e.g. hook up
> displayport alternate-mode.

I don't think this is sufficient as it doesn't describe how alternate 
modes are connected to various components. This has been discussed some 
here[1] with the CrOS folks. Maybe this is orthogonal, IDK, but I really 
need something that is somewhat complete and not sprinkle a few new 
properties at a time.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note I hope I got the yaml correct, this is my first time writing a
> dt-binding in the new yaml style. I did run:
> make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/connector/usb-connector.yaml
> and that was happy.

That aspect of it looks fine.

Rob

[1] https://lkml.org/lkml/2020/4/22/1819

HI Rob,

On Mon, Jul 20, 2020 at 7:26 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jul 14, 2020 at 01:36:14PM +0200, Hans de Goede wrote:
> > This commit adds the minimum bindings required to allow describing which
> > altmodes a port supports. Currently this is limited to just specifying:
> >
> > 1. The svid, which is the id of the altmode, e.g. displayport altmode has
> > a svid of 0xff01.
> >
> > 2. The vdo, a 32 bit integer, typically used as a bitmask describing the
> > capabilities of the altmode, the bits in the vdo are specified in the
> > specification of the altmode, the dt-binding simply refers to the
> > specification as that is the canonical source of the meaning of the bits.
>
> What if this information should be derived from information already in
> DT (or would be there if alt mode connections are described)?
>
> >
> > Later on we may want to extend the binding with extra properties specific
> > to some altmode, but for now this is sufficient to e.g. hook up
> > displayport alternate-mode.
>
> I don't think this is sufficient as it doesn't describe how alternate
> modes are connected to various components. This has been discussed some
> here[1] with the CrOS folks. Maybe this is orthogonal, IDK, but I really
> need something that is somewhat complete and not sprinkle a few new
> properties at a time.

Just thought I'd add a link[2] to some amendments which were made to
the "switch"
proposal in the thread you added a link to (it has many responses, so
not sure if my
responses got lost in the thread). I also wrote a short summary[3].

Please ignore these if you've already taken a look at them :)

Best regards,

-Prashant

[2] https://lkml.org/lkml/2020/6/12/602
[3] https://lkml.org/lkml/2020/7/10/234
>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > Note I hope I got the yaml correct, this is my first time writing a
> > dt-binding in the new yaml style. I did run:
> > make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/connector/usb-connector.yaml
> > and that was happy.
>
> That aspect of it looks fine.
>
> Rob
>
> [1] https://lkml.org/lkml/2020/4/22/1819
>

Hi,

On 7/21/20 4:26 AM, Rob Herring wrote:
> On Tue, Jul 14, 2020 at 01:36:14PM +0200, Hans de Goede wrote:
>> This commit adds the minimum bindings required to allow describing which
>> altmodes a port supports. Currently this is limited to just specifying:
>>
>> 1. The svid, which is the id of the altmode, e.g. displayport altmode has
>> a svid of 0xff01.
>>
>> 2. The vdo, a 32 bit integer, typically used as a bitmask describing the
>> capabilities of the altmode, the bits in the vdo are specified in the
>> specification of the altmode, the dt-binding simply refers to the
>> specification as that is the canonical source of the meaning of the bits.
> 
> What if this information should be derived from information already in
> DT (or would be there if alt mode connections are described)?
> 
>>
>> Later on we may want to extend the binding with extra properties specific
>> to some altmode, but for now this is sufficient to e.g. hook up
>> displayport alternate-mode.
> 
> I don't think this is sufficient as it doesn't describe how alternate
> modes are connected to various components. This has been discussed some
> here[1] with the CrOS folks. Maybe this is orthogonal, IDK, but I really
> need something that is somewhat complete and not sprinkle a few new
> properties at a time.

Right, but that is an orthogonal problem, this is telling the Type-C
controller which modes it is allowed to negotiate and which capabilties
(altmode specific, stored in the vdo) it should advertise.

I agree that if the connector is connected to a mux and how that mux is then
connected to the SoC, or if the SoC has a multi-mode phy also needs to be
specified in some cases. But that is mostly a separate problem.
One thing which we will want to add to this part of the bindings when that
other part is in place is a link to the endpoint *after* the mux, that is
after the mode- and role-switch in Prashant's example here:
https://lkml.org/lkml/2020/6/12/602

The Type-C controller may receive out-of-band messages related to the
altmode (through USB-PD messages) which need to be communicated to
the endpoint, so in the case of display-port altmode, the dp0_out_ep
from Prashant's example. Note the link/object reference I'm suggesting
here deliberately skips the mux, since the oob messages need to be
send through the endpoint without the mux being involved since they are
oob after all.

Specifically there is no pin on the Type-C connector for the display-port
hotplug-detect pin, so hot(un)plug is signaled through altmode specific
USB-PD messages.

Note that this binding and the 2 patches implementing it for x86
devices (*), are already useful / functional. The user just needs to
manually run "xrandr" to force the video-output driver to manually
recheck for new/changed monitors, just like an old VGA ports without
load detection.

I haven't fully figured out how to wire up the hotplug signal in the
kernel yet, which is why the link to the DP endpoint is not yet part of
the bindings.

*) Using sw-fw-nodes to pass the info from a drivers/platform/x86/
driver to the Type-C controller code which uses fw_nodes to get this info

So since this is x86 only for now; and AFAIK you don't want to take bindings
upstream until there is an actual DT user anyways, my main goal of including
this was to see if we are at least on the right way with this. With x86 it
is all in the kernel, so if the binding changes a bit we can easily adjust the
drivers/platform/x86/ code generating the nodes at the same time as we
update the Type-C controller code to implement the final binding. But it
would be good to know that we are at least going in the right direction.

BTW note that making the binding look like this was proposed by Heikki,
the Type-C subsys maintainer, I ended up implementing this because Heikki
did no have the time for it.

Regards,

Hans





> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Note I hope I got the yaml correct, this is my first time writing a
>> dt-binding in the new yaml style. I did run:
>> make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/connector/usb-connector.yaml
>> and that was happy.
> 
> That aspect of it looks fine.
> 
> Rob
> 
> [1] https://lkml.org/lkml/2020/4/22/1819
>

Hi Rob,

Restarting this thread, since I think we can re-use Patch 1/4, and I
dind't want some of the context to be lost by starting a new thread.

On Wed, Jul 22, 2020 at 09:18:02AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 7/21/20 4:26 AM, Rob Herring wrote:
> > On Tue, Jul 14, 2020 at 01:36:14PM +0200, Hans de Goede wrote:
> > > This commit adds the minimum bindings required to allow describing which
> > > altmodes a port supports. Currently this is limited to just specifying:
> > > 
> > > 1. The svid, which is the id of the altmode, e.g. displayport altmode has
> > > a svid of 0xff01.
> > > 
> > > 2. The vdo, a 32 bit integer, typically used as a bitmask describing the
> > > capabilities of the altmode, the bits in the vdo are specified in the
> > > specification of the altmode, the dt-binding simply refers to the
> > > specification as that is the canonical source of the meaning of the bits.
> > 
> > What if this information should be derived from information already in
> > DT (or would be there if alt mode connections are described)?
> > 
> > > 
> > > Later on we may want to extend the binding with extra properties specific
> > > to some altmode, but for now this is sufficient to e.g. hook up
> > > displayport alternate-mode.
> > 
> > I don't think this is sufficient as it doesn't describe how alternate
> > modes are connected to various components. This has been discussed some
> > here[1] with the CrOS folks. Maybe this is orthogonal, IDK, but I really
> > need something that is somewhat complete and not sprinkle a few new
> > properties at a time.
> 
> Right, but that is an orthogonal problem, this is telling the Type-C
> controller which modes it is allowed to negotiate and which capabilties
> (altmode specific, stored in the vdo) it should advertise.
> 

I concur. There is value in listing the alternate modes supported by a
connector in the connector bindings. PD negotiation (which may include
alternate mode entry) is something which is handled
by the port driver / TCPM itself, so this sounds like a reasonable place
to explicitly describe this information rather than derive it from OF
graph.

While it is important to describe how the connector is routed through the
switches and eventually to the various controllers (DP, xHCI, USB4 etc.),
it doesn't sound like we should make the Type C port driver rely on those
graph connections to derive what alternate modes to support.

FWIW, I did provide a more fleshed out example of how the OF graph
connections from port to various PHYs would work a while back, but
didn't get much feedback on it [1]

> BTW note that making the binding look like this was proposed by Heikki,
> the Type-C subsys maintainer, I ended up implementing this because Heikki
> did no have the time for it.

If the binding itself looks fine, then I'd request we move forward with
including it in the usb-connector bindings rather than stalling on the
OF graph switch bindings.
Heikki had mentioned [2] that we can adjust the ACPI bindings to accommodate
device tree requirements, and it looks like the current implementation is already in
the mainline connector class code [3], just the binding is missing.

I would be happy to re-upload this patch, with follow on patches which:
- Add the altmodes node to a Chrome OS device tree file
- Update the cros-ec-typec port driver to call the function introduced in [3].

I've tested this locally and it works fine.

[1]:
https://lore.kernel.org/lkml/CACeCKacUa1-ttBmKS_Q_xZCsArgGWkB4s9eG0c5Lc5RHa1W35Q@mail.gmail.com/
[2]:
https://lore.kernel.org/linux-usb/Ya8vxq+%2FP%2FWG4kHo@kuha.fi.intel.com/
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b458a4c5d7302947556e12c83cfe4da769665d0

Would be good to hear your thoughts on the above.

Thanks,

-Prashant