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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160210172619.GB28335@kroah.com>
Date:	Wed, 10 Feb 2016 09:26:19 -0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
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 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"?

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

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

> 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 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 :(

thanks,

greg k-h