Re: [RFC PATCHv2] usb: USB Type-C Connector Class

From: Guenter Roeck
Date: Fri Jun 03 2016 - 09:52:10 EST

On 06/03/2016 06:21 AM, Heikki Krogerus wrote:

On Thu, Jun 02, 2016 at 09:12:19AM -0700, Guenter Roeck wrote:
On Thu, Jun 02, 2016 at 01:18:53PM +0300, Heikki Krogerus wrote:
On Wed, Jun 01, 2016 at 04:29:26PM -0700, Guenter Roeck wrote:
On Wed, Jun 01, 2016 at 11:26:09AM +0200, Oliver Neukum wrote:
On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote:
Just noticed that the "active" file is for now read only, but it needs
to be changed to writable. That file will of course provide means for
the userspace to Exit and Enter modes. But please note that the
responsibility of the dependencies between the modes, say, if a plug
needs to be in one mode or the other in order for the partner to enter
some specific mode, will fall on the Alternate Mode specific drivers
once we have the altmode bus. I remember there were concerns about
this in the original thread.

There's one thing we haven't touched upon yet. And I cannot really find
an answer in the spec.

What do we do if we return from S4 or S3? I think we need to restore
the ALternate Mode because our display may be running over that
Alternate Mode.
If we want to support USB persist we also need to restore data role
after S4.

I don't have an answer ... but another interesting question.

How do we distinguish between alternate modes supported by a host vs.
alternate modes supported by a sink ? typec_capability includes a pointer
to alternate modes supportedf by the connector, but it is not clear if
those are alternate modes supported as host, or alternate modes supported
as device, or alternate modes supported by both.

This doesn't matter much if only a fixed role is supported, but it does matter
for dual role ports. A laptop will typically only support DisplayPort as host,
for example.

The DP alternate mode spec actually separates the display role from
Type-C role. A laptop most likely would only support the modes for
display host roles, but if the port was DRP port then it would still
do so in both Type-C roles.

So basically, even if the display was Type-C host, it would still work
as a display when attached to the laptop.

Any idea ?

I'm actually not sure this is a problem.

Yes, this was a bad example, since the DisplayPort mode vdo includes a flag
indicating if the port supports source, sink, or both.

I meant that in case of DP alternate mode, there should not be a

Let's use a different example:
Google devices (such as power adapters) have mode '1' for firmware upgrades.
Obviously hosts will support that, but what should the host advertise if it
is configured as sink ?

Maybe this is just my personal confusion, and there is no real problem.
It might as well be that the Google mode VDO _should_ include a flag
indicating if the port supports updating the partner, and/or if it supports
being updated. For now I'll just assume that this is the case.

Well, do you think we can rely on always being able to get this detail
from VDO?

Not really. Like in the Google case, one end will implement sending the firmware,
the other end will implement receiving it and writing it to flash (or whatever).
Which is which isn't currently made visible to user space. I suspect that
other "intelligent" devices like Apple's multi-function adapter do the same,
though obviously I don't really know what the two VDO modes do on the apple

I'll have to have some ABI to user space for the alternate mode, for example
to send the firmware file to the kernel. I am not there yet, though, so I
don't really know exactly how that will look like. Most likely it is going to be
added sysfs attributes in the mode device (eg usbc0.svid:18d1/mode0/firmware).

Something else, which goes back into the symlink question. If I create the
alternate mode devices before calling typec_register_port(), the devices won't
have a parent and don't show up in the class directory. You previously solved
that with the symlink. I am trying to solve it in my current code by calling
typec_register_altmodes() from typec_register_port() - primarily because I
don't really want to duplicate all the device creation code in my driver.

In my test case, this gives me

Side note: I didn't provide a description/name for the modes, because that
would result in something like usbc0.DisplayPort/ instead of usbc0.svid:ff01/,
and I prefer a consistent ABI. Since this _is_ part of the ABI, would it make
sense to standardize on names for modes in sysfs ? For example, how should
a "Display Port" mode directory be named ? It doesn't sound good if I
use "usbc0.svid:ff01", someone else uses "usbc0.DisplayPort", and yet
someone else uses "usbc0.displayport".

Also, do we at some point need to standardize the ABI for the standard
alternate modes such as DisplayPort (if there are any - again I am not
there yet) ?

in addition to

(when connecting the Apple adapter), which is exactly what I would expect to see.

Is this sensible ? Do we have a reason for expecting the alternate mode
_devices_ to be created (without parent) when calling typec_register_port() ?

So if you would prefer that the class code takes care of creating the
alternate modes when typec_register_port() is called, I'm fine with
that too. Let's make it so.

Sounds good to me. Many other subsystems do the same, ie create the subsystem
device(s) during registration with the subsystem, so this is in line with other
kernel code.

Should I send you a follow-up patch on top of yours ?