Re: [PATCHv3 1/2] usb: USB Type-C connector class

[Guenter Roeck]

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