Message-ID: <20160623120055.GC27263@kuha.fi.intel.com>
Date:	Thu, 23 Jun 2016 15:00:55 +0300
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	Oliver Neukum <oneukum@...e.com>
Cc:	Felipe Balbi <felipe.balbi@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Guenter Roeck <linux@...ck-us.net>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCHv3 1/2] usb: USB Type-C connector class

Hi Oliver,

On Thu, Jun 23, 2016 at 10:38:58AM +0200, Oliver Neukum wrote:
> On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
> > On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
> 
> > No it's not. DRP means a port that can operate as _either_ Source
> > (host) or Sink (device), but not at the same time..
> 
> Yes, but it is unclear what you will be after a connection
> and that's the point.

Which is a fact that we can do nothing about. The role after
connection with DRP ports will be dictated by the partner or selected
randomly in case the partner is also DRP. We can prefer a role, but
that in the end guarantees nothing. So if the role that we end up with
after connection (seen in current_data_role) does not satisfy us, all
we can do is try to swap it.

I'm not sure what is your point here.

> > > And you can be able to become a host and be able to become a device.
> > > But not at the same time. These ports are switchable.
> > > 
> > > The current API cannot express the difference.
> > 
> > I think you have misunderstood something. The only case where the port
> > can be dual-role is if it's set to be DRP. Otherwise it's Source only
> > OR Sink only.
> > 
> > The "Role Supported" bits only tell us how we can program for example
> > the ROLE_CONTROL registers. I guess the "Roles Supported" bits in
> > DEVICE_CAPABILITIES are not explained properly, so let's go over them
> > here:
> > 
> > 000b = Source _or_ Sink only
> > 001b = Source only
> > 010b = Sink only
> > 011b = Sink only with support for autonomously detected accessory modes
> > 100b = DRP only, and this I believe mean we can not program the port
> >         to be Sink only or Source only
> 
> I think so, too.
> 
> > 101b = Source only OR Sink only OR DRP, plus ability to detect
> >         accessories and I guess also cables autonomously
> > 110b = Source only OR Sink only OR DRP
> > 
> > So where the spec lists "Source, Sink", it actually should have said
> > "Source only OR Sink only".
> > 
> > But you still have only the following options for a port:
> > 1) Source only (host)
> > 2) Sink only (device)
> > 3) DRP (device, host)
> 
> Yes, so you can map "000b = Source _or_ Sink only" to host or device
> depending on the current setting. But then you lose the information
> that it can be changed. 

No it can't. The idea with the Roles Supported bits is for the driver
to be able to select the most appropriate role that fits the abilities
of the platform.

The configuration of the port after probing the port controller will
never change. If you have initially configured the port to be Sink
only (so device), it most likely means your platform can not act as
Source even if the port controller would.

And if you want to change the configuration of the port, for example
if your platform is capable of supporting Source and Sink modes, but
your port controller is not capable of supporting DRP (which would be
pretty messed up situation) but instead forces you to choose between
Sink and Source, you would in practice in any case have to unregister,
reconfigure and register the port again.

But in most cases the platform will not support all the capabilities
the port controller will be capable of. If for example on your
platform you have only USB host controller, it just means you will
have to have port controller that returns either 000b, 001b, 101b or
110b in the supported roles bits. Otherwise it will no be usable on
your platform.

> So we throw away information.
> 
> And you map "100b = DRP only" and "101b" and "110b" to host, device

No I don't. If our platform can only support Sink mode, value "100b"
will not work and can not be ever registered, and values "101b" and
"110b" will report "device" in supported_data_roles.

And it is not the class that defines the capabilities of a port.
They are defined by the drivers that register the ports.

> which again drops information.

There is no use in knowing details about the port controller
capabilities like if a port could be configured to be Source or Sink
only instead of just DRP from the typec class point of view. Those
details are port controller specific, and completely out side the
scope of the class driver. Not all USB Type-C PHYs will be port
controllers and not all ports registered with the class will even have
a PHY to deal with. This means we will not even always be able to read
the same kinds of details of the port like we are with port
controllers.

So if you want to get the capabilities of the port controller in use,
the port controller driver will have to expose them to user space, not
the class.


Br,

-- 
heikki

Powered by blists - more mailing lists