Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Tue, Feb 09, 2016 at 10:20:45AM -0800, Greg KH wrote:
> On Tue, Feb 09, 2016 at 07:01:21PM +0200, Heikki Krogerus wrote:
> > The purpose of this class is to provide unified interface
> > for user space to get the status and basic information about
> > USB Type-C Connectors in the system, control data role
> > swapping, and when USB PD is available, also power role
> > swapping and Altenate Modes.
> > 
> > The class will export the following interfaces for every
> > USB Type-C Connector in the system to sysfs:
> > 
> > 1. connected - Connection status of the connector
> > 2. alternate_mode - The current Alternate Mode
> > 3. alternate_modes - Lists all Alternate Modes the connector supports
> > 4. partner_alt_modes - Lists partner's Alternate Modes when connected
> > 5. partner_type - Can be USB, Charger, Alt Mode or Accessory
> > 6. data_role - The current data role, host or device
> > 7. data_roles - Data roles supported by the connector
> > 8. power_role - Connector's current power role, source or sink
> > 9. power_roles - Power roles supported by the connector
> > 10. power_operation_mode - The current power level in use
> > 11. usb_pd - yes if the connector supports USB PD.
> > 12. audio_accessory - yes if the connector supports Audio Accessory
> > 13. debug_accessory - yes if the connector supports Debug Accessory
> 
> You forgot to document these sysfs files in Documenataion/ABI :(

Man, it's almost like I enjoy making these stupid mistakes :(

> 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. 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.

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.

> > 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.

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.

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 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.

I hope I was able to explain myself.


Thanks,

-- 
heikki