Message-ID: <20160211140720.GB32213@kuha.fi.intel.com>
Date:	Thu, 11 Feb 2016 16:07:20 +0200
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Felipe Balbi <balbi@...nel.org>
Subject: Re: [PATCH 1/3] usb: USB Type-C Connector Class

On Wed, Feb 10, 2016 at 09:26:19AM -0800, Greg KH wrote:
> On Wed, Feb 10, 2016 at 12:38:40PM +0200, Heikki Krogerus wrote:
> > > And what is userspace going to do with these files?  Why does it care?
> > 
> > The OS policy regarding USB Type-C ports needs to come from user
> > space.
> 
> What drives this "need"?
> 
> > The user must be allowed to select the USB data role, and when
> > USB PD is supported, also the power role, and at the same time we need
> > to export all the relevant information about the USB Type-C ports to
> > the user space, like connection status, the type of partner etc. And
> > all that from a single interface.
> 
> Again, what drives this "need" to be in a "single interface"?

Because the alternative is that every platform will have a custom way
of doing these things.. With the single interface I mean the class.
If we don't present USB Type-C ports to the userspace from a unified
interface, how can anybody create userspace software to support
control of USB Type-C ports? Or can we accept that we will always need
platform specific user space software if we want to control the USB
Type-C ports?

In case you are still skeptical about why userspace would need any
interaction with the USB Type-C ports, think about the following use
case: You plug what you think is something that will charge your
phone, nothing is printed on the screen - we don't now get any details
about the parter - but we must be charging, right? After a while you
realize that you are not charging, instead the thing you plugged in is
actually draining you phone of power? This will happen if the partner
happens to be for example..
1) Accessory
2) Alternate Mode that we enter successfully, so no Billboard Device.
3) Both our device and the partner are DRP and we end up being the
   host initially.

Nothing bad has happened in this example, but it's horrible user
experience.

> > I'm pretty sure that this is exactly what distributions like Ubuntu,
> > RedHat etc. want, an I know for fact that Chrome OS and Android will
> > expect the user to be in control over the roles and get that
> > information about the ports one way or the other.
> 
> Given that ChromeOS and Android already do this type of thing today, why
> not work with those developers to ensure that this really is the
> interface they want / expect?

I'm in contact with some Android developers, but Android is a bit
frustrating. I know the absolute minimum about the world of Android,
but the way it looks to me is that everybody is doing their own thing.
The expectations about things also vary from company to company.
Maybe there is a forum somewhere that I'm not aware of?

I'm pretty sure ChromeOS is going to be different. I'll try to make
some contacts there.

> > > > The data_role, power_role and alternate_mode are also
> > > > writable and can be used for executing role swapping and
> > > > entering modes. When USB PD is not supported by the
> > > > connector or partner, power_role will reflect the value of
> > > > the data_role, and is not swappable independently.
> > > 
> > > How does this implementation differ from those in other drivers that we
> > > have seen, but not submitted for merging?  I'm referring to the code
> > > from Fairchild for their USB Type C driver:
> > > 	https://github.com/gregkh/fusb30x
> > > and the driver that is in the latest Nexus 6 Android release (don't have
> > > the link to the android kernel tree at the moment sorry, but it's public
> > > and I think Linaro is working on cleaning it up...)
> > 
> > That would be USB PD stack and driver for fusb30x USB Type-C/PD PHYs.
> > It's the second USB PD stack I've seen (and sadly also second driver
> > for fusb30x), but I just know there are more.
> 
> Oh, there's more than just 2 drivers for that fusb30x chip floating
> around.  My repo is not the latest version and it's a truly horrid piece
> of code, never to be run on any hardware you actually care about power
> as it doesn't care.
> 
> > My class is just about exporting control of USB Type-C ports to the
> > user space, and note, USB Type-C *not* USB PD. I don't thing that my
> > little class and the USB PD stack, where ever it ends up coming from,
> > conflict with each other.
> 
> But we need to ensure that it doesn't conflict, and given that you are
> already using the same directory those stacks are using, perhaps you can
> look at them to see that you aren't duplicating any work?

Well, in case of that fusb30x driver, there definitely is no conflict
if there really is no interaction with the userspace. They would just
never register with this class.

But there really should never be any conflict between USB Type-C and
USB PD. For USB Type-C connectors USB PD is just an optional "library"
that can have the following functions and nothing else:

1) DR_Swap - Data Role swap
2) PR_Swap - Power Role swap
2) VCONN_Swap - VCONN Source swap
3) Discover SVIDs - Alternate Modes
3) Discover Modes - The actual modes under SVID
4) Enter Mode
4) Exit Mode

So those functions will always be the same. From Type-C point of view
it really makes no difference how they have actually been implemented
(in software, PD controller, firmware, etc).

> > The only difference is that I'm clearly separating USB Type-C from USB
> > PD (and actually everything else), which is the correct thing to do.
> > USB Type-C is not the same thing as USB PD. USB Type-C does not always
> > come with USB PD.
> 
> I agree, keeping them separate seems good, but I worry when you have to
> do both how that is going to look.

I'm fairly certain that we can make the Type-C part good. It's so
straight forward. I'm more worried about USB PD.

I'm a bit skeptical about whether it will even be possible to create a
stack that works for every type of USB PD thingy out there. For cases
where there is just the PHY like fusb30x, the PD stack will be made
completely in software. That we can (maybe) handle. Also cases where
we have a complex USB PD controller that handles all layers starting
from protocol, we will be able to handle, as with them the stack is not
needed at all. Even the simple USB PD controllers that take care of
protocol layer (completely) but leave the policy layers to the
software, we will be able to handle. The problem comes from the USB PD
controller and PHYs that fall somewhere in-between. The ones that
implement "partial" protocol layer and "partial" policy engine, or
consist of several components, or..

..I have to stop there. I just vomited in my mouth a little. I fear
USB PD can end up being a little bit like USB Charging (Felipe can
tell more about that) except many times worse! Maybe we need to start
thinking about the ground rules when somebody adds the PD stack, like
you can use only complete parts of stack and never try to mix your
oddities into it, otherwise you have to implement everything from
scratch for your particular platform, etc.

Perhaps things are not as bad as I fear, but let's just make sure
Type-C is always separate from USB PD!

> > I did not go through that code, but I'm guessing the guys have for
> > example exported similar role swapping controls to user space from
> > some part of their stack. So those guys would just need to register
> > their fusb30x with this class, let the class take care of exporting
> > those controls and probable continue using their USB PD stack exactly
> > like they have done before.
> 
> the fusb30x code does it all within kernel space, no userspace
> interactions needed due to timing requirements (so they say).  I'm not
> saying that this is a good idea / design, just that I'm getting
> conflicting requirements from different camps at the moment and it's
> really hard to sort it all out :(

I think there must have been some misunderstanding. We really have to
interact with the userspace with USB Type-C ports.


Thanks,

-- 
heikki

Powered by blists - more mailing lists