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

[Guenter Roeck]

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.

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.

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
    /sys/class/type-c/usbc0/
	usbc0.svid:18d1
	usbc0.svid:18d1/mode0
	usbc0.svid:18d1/mode0/vdo
	usbc0.svid:18d1/mode0/description
	usbc0.svid:18d1/mode0/active
	...
	usbc0.svid:ff01
	usbc0.svid:ff01/mode0/vdo
	usbc0.svid:ff01/mode0/description
	usbc0.svid:ff01/mode0/active

in addition to
    /sys/class/type-c/usbc0/
	usbc0-partner/usbc0-partner.svid:05ac
	usbc0-partner/usbc0-partner.svid:05ac/mode0
	usbc0-partner/usbc0-partner.svid:05ac/mode0/vdo
	usbc0-partner/usbc0-partner.svid:05ac/mode0/description
	usbc0-partner/usbc0-partner.svid:05ac/mode0/active
	usbc0-partner/usbc0-partner.svid:05ac/mode1
	usbc0-partner/usbc0-partner.svid:05ac/mode1/vdo
	usbc0-partner/usbc0-partner.svid:05ac/mode1/description
	usbc0-partner/usbc0-partner.svid:05ac/mode1/active
	...
	usbc0-partner/usbc0-partner.svid:ff01
	usbc0-partner/usbc0-partner.svid:ff01/mode0
	usbc0-partner/usbc0-partner.svid:ff01/mode0/vdo
	usbc0-partner/usbc0-partner.svid:ff01/mode0/description
	usbc0-partner/usbc0-partner.svid:ff01/mode0/active

(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() ?

Thanks,
Guenter