Date:	Thu, 23 Jun 2016 06:29:11 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Oliver Neukum <oneukum@...e.com>
Cc:	Felipe Balbi <felipe.balbi@...ux.intel.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCHv3 1/2] usb: USB Type-C connector class

On 06/23/2016 05:00 AM, Heikki Krogerus wrote:
> 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.
>
Agreed.

Guenter

Powered by blists - more mailing lists