From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751578AbcBKOI5 (ORCPT ); Thu, 11 Feb 2016 09:08:57 -0500 Received: from mga14.intel.com ([192.55.52.115]:27260 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750980AbcBKOIy (ORCPT ); Thu, 11 Feb 2016 09:08:54 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,431,1449561600"; d="scan'208";a="900810265" Date: Thu, 11 Feb 2016 16:07:20 +0200 From: Heikki Krogerus To: Greg KH Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Mathias Nyman , Felipe Balbi Subject: Re: [PATCH 1/3] usb: USB Type-C Connector Class Message-ID: <20160211140720.GB32213@kuha.fi.intel.com> References: <1455037283-106479-1-git-send-email-heikki.krogerus@linux.intel.com> <1455037283-106479-2-git-send-email-heikki.krogerus@linux.intel.com> <20160209182045.GB31787@kroah.com> <20160210103840.GC5270@kuha.fi.intel.com> <20160210172619.GB28335@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160210172619.GB28335@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 10, 2016 at 09:26:19AM -0800, Greg KH wrote: > 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"? Because the alternative is that every platform will have a custom way of doing these things.. With the single interface I mean the class. If we don't present USB Type-C ports to the userspace from a unified interface, how can anybody create userspace software to support control of USB Type-C ports? Or can we accept that we will always need platform specific user space software if we want to control the USB Type-C ports? In case you are still skeptical about why userspace would need any interaction with the USB Type-C ports, think about the following use case: You plug what you think is something that will charge your phone, nothing is printed on the screen - we don't now get any details about the parter - but we must be charging, right? After a while you realize that you are not charging, instead the thing you plugged in is actually draining you phone of power? This will happen if the partner happens to be for example.. 1) Accessory 2) Alternate Mode that we enter successfully, so no Billboard Device. 3) Both our device and the partner are DRP and we end up being the host initially. Nothing bad has happened in this example, but it's horrible user experience. > > 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? I'm in contact with some Android developers, but Android is a bit frustrating. I know the absolute minimum about the world of Android, but the way it looks to me is that everybody is doing their own thing. The expectations about things also vary from company to company. Maybe there is a forum somewhere that I'm not aware of? I'm pretty sure ChromeOS is going to be different. I'll try to make some contacts there. > > > > 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? Well, in case of that fusb30x driver, there definitely is no conflict if there really is no interaction with the userspace. They would just never register with this class. But there really should never be any conflict between USB Type-C and USB PD. For USB Type-C connectors USB PD is just an optional "library" that can have the following functions and nothing else: 1) DR_Swap - Data Role swap 2) PR_Swap - Power Role swap 2) VCONN_Swap - VCONN Source swap 3) Discover SVIDs - Alternate Modes 3) Discover Modes - The actual modes under SVID 4) Enter Mode 4) Exit Mode So those functions will always be the same. From Type-C point of view it really makes no difference how they have actually been implemented (in software, PD controller, firmware, etc). > > 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'm fairly certain that we can make the Type-C part good. It's so straight forward. I'm more worried about USB PD. I'm a bit skeptical about whether it will even be possible to create a stack that works for every type of USB PD thingy out there. For cases where there is just the PHY like fusb30x, the PD stack will be made completely in software. That we can (maybe) handle. Also cases where we have a complex USB PD controller that handles all layers starting from protocol, we will be able to handle, as with them the stack is not needed at all. Even the simple USB PD controllers that take care of protocol layer (completely) but leave the policy layers to the software, we will be able to handle. The problem comes from the USB PD controller and PHYs that fall somewhere in-between. The ones that implement "partial" protocol layer and "partial" policy engine, or consist of several components, or.. ..I have to stop there. I just vomited in my mouth a little. I fear USB PD can end up being a little bit like USB Charging (Felipe can tell more about that) except many times worse! Maybe we need to start thinking about the ground rules when somebody adds the PD stack, like you can use only complete parts of stack and never try to mix your oddities into it, otherwise you have to implement everything from scratch for your particular platform, etc. Perhaps things are not as bad as I fear, but let's just make sure Type-C is always separate from 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. > > 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 :( I think there must have been some misunderstanding. We really have to interact with the userspace with USB Type-C ports. Thanks, -- heikki