Re: [PATCH v4 1/5] gadget: Introduce the notifier functions

[Felipe Balbi]

On Fri, Oct 02, 2015 at 07:49:09PM +0100, Mark Brown wrote:
> On Fri, Oct 02, 2015 at 12:23:11PM -0500, Felipe Balbi wrote:
> > On Fri, Oct 02, 2015 at 05:47:43PM +0100, Mark Brown wrote:
> 
> > > Can you elaborate on what the difficulties are and how punting to
> > > userspace helps?  If anything I'd expect punting to userspace to make
> 
> > the difficulty was mainly making sure all involved parties talk the same
> > language. I mean, you plug cable and detect charger (this is usually done by the
> > PMIC or by a BQ-type device - probably done at drivers/power_supply).
> 
> > First difficulty comes right here. Power_supply notifies that we're attached to
> > a charger (sometimes it can't differentiate a host/hub charger from a wall
> > charger), a few ms later you get a notification from the gadget that it got
> > enumerated with a 500mA configuration. Depending on the order of things, your
> > load will be configured either to 2A (maximum host/hub charger current) or
> > 500mA. Yes, this can be mitigated :-)
> 
> > Focussing on this alone, you can have as much as 4 different subsystems
> > involved, all throwing raw_notifiers at each other. Now each of these subsystems
> > need to maintain their own state machine about what notification we have
> > received and what are the valid next states.
> 
> OK, so part of the goal here was to outsource all the state machines to
> some generic code - what you're describing here is definitely a problem
> and the idea was to provide a central place that has the logic and makes
> the decisions about what we should be doing.  If we don't have that it's
> going to get messy.
> 
> > I would rather have userspace be the single place getting notifications because
> > then we solve these details at a single location.
> 
> No argument on the place, making that place be userspace I'm less sold
> on.

fair enough. This would be an entire new entity, though, and users (chargers,
battery chips, USB controllers, USB PHYs, etc) should blindly notify this entity
and no care about what happened before or what are possible next states.

As long as we can guarantee that, then I'd be okay adding this to the kernel.

> > > Things more difficult, if nothing else it means we need to get whatever
> > > userspace component deployed in all relevant userspace stacks with
> > > appropriate configuration in order to do things.
> 
> > but that will be a thing for the kernel too. We will have to deploy this new
> > kernel component in all relevant stacks with appropriate configuration in order
> > to do things :-) No changes there.
> 
> Sure, but it's one project which is developed entirely in the open -
> that's a lot more manageable.

We can have a fully open source userland daemon too. Much like systemd, bluez,
neard, and many, many others. Why wouldn't that be a thing ?

> > > We do punt a lot of configuration to userspace for audio because there
> > > are substantial device specific policy elements in the configuration and
> > > it's a far from painless experience getting the code and configuration
> > > deployed where people need it, definitely a not great for users.
> 
> > it's the same for this situation. We will have a ton of device specific
> > configuration, specially with power delivery class, billboard class, the
> > alternate modes and so on. If we already do this part in userland, adding all
> > those extras will be, IMO, far simpler.
> 
> Can you elaborate a bit?  As far as I can tell all those are still in
> the generic USB space rather than the sort of device specifics I'm
> thinking of.

let's consider the billboard class, for example. With the new Type-C connector,
the extra CC pins get added and power delivery messaging goes on top of
that. Billboard class gives us the opportunity to send a power delivery request
to mux the data pins on the TypeC connector to something completely non-USB.

So we could run NATIVE PCIe (single lane though) on top of this. Native
DisplayPort, Native Thunderbolt (which the Fruit company announced will switch
to type-C, so that will be a thing), native VGA... the list goes on and on.

At that point, this is not entirely USB realm anymore :-)

Similar applies to power delivery charging profile themselves. Not all profiles
are mandatory. Some are optional and that will be completely device/board
specific. How an OEM implements that in the PCB is also completely
board-specific :-) Some might have a dumb IC hardcoding some messages, some
might have something largely more flexible.

> The things we've got with audio are a combination of tuning to taste and
> (even more importantly) very different ideas about what you want to do
> with an audio subsystem that influence how you set things up in a given
> use case.

the same thing will happen with Type-C and power delivery. There won't be tuning
to taste, of course :-) But there will be "very different ideas what what you
want to do with" your charging methodology.

> > > I think that where we can do it the bit where work out how the various
> > > components are connected and aggregate their functionality together is
> > > definitely a kernel job.  It means that userspace doesn't need to worry
> > > about implementation details of the particular system and instead gets a
> > > consistent, standard view of the device (in this case a USB port and how
> > > much power we can draw from it).
> 
> > Actually, I was thinking about it in a more granular fashion. Kernel exposes a
> > charger/power_supply, a few regulators, a UDC view and this single userspace
> > daemon figures out how to tie those together.
> 
> That sounds worrying - it means that new hardware support needs
> supporting in every userspace (it seems inevitable that there will be at
> least some reimplementations because that's fun) which gets old really
> fast, and every userspace needs to understand every topology.

very true, but it's also true for a kernel solution.

It seems like you want it in the kernel because of it being convenient :-)

> > The view here isn't really a USB port, it's just a source of power. But how much
> > power we can draw from it depends on, possibly, a ton of different chips and
> > different notifications.
> 
> Right, yes - it's the power input/output associated with the USB port.
> The USB port is just the physical thing users can see so it's a good way
> to label it.  That could mean that we ought to move the aggregation bit
> into the power supply code of course, but then a lot of the decisions
> are going to be specific to USB.

in a sense, yes. But they can happen at a layer which knows nothing (or very
little) about USB. Here's an example:

Not everybody follows USB Battery Charging class. Some chargers don't short
D+/D- to signify they are a wall charger. Some will use different resistance
values on the ID pin to tell you how much current you can draw from them.

From a PMIC (or something else) point of view, all it needs is a tiny current
source and a an ADC, then it reports the ADC value to the upper layer. This
really doesn't even need to know that it's using an ID pin, or whatever.

> > > For the policy stuff we do want to have userspace be able to control
> > > things but we need to think about how to do that - for example does the
> > > entire flow need to be in userspace, or can we just expose settings
> > > which userspace can manage?
> 
> > considering the amount of different designs that are already out there and all
> > the extras that are going to show up due to type-c, I think we'd be better off
> > exposing as much to userspace as possible.
> 
> How different are the end goals of these designs really going to be
> though - that's more of the question for me?  Part of what the kernel is
> doing is hiding implementation details from userspace.  I'd expect
> userspace to be interested in things like the maximum current allowed in
> or out in a given mode rather than the details of how that is accomplished.

okay, this is a fair point :-) I just don't see, as of now at least, how we can
get to that in a way that's somewhat future-proof. It seems like we will
add more and more notifications until we can't maintain this anymore.

Say, we get a "certifiable" USB BC1.2 solution in the kernel. Current systems
are happy, we drop yet another out-of-tree set of patches from AOSP, every holds
hands and sings Kumbaya.

In another year or so, power delivery will be *the* thing. Now all the work done
assuming BC1.2 is gone, outdated. Adding power delivery brings an entire new set
of requirements. A new protocol stack (yes! it runs on CC pins, remember ?), new
messages, etc etc...

With Type-C there's no port type anymore. What we used to refer as Host/Hub
Charger, Standard Port, Charging Port, Wall Charger, etc, now becomes a single
type (let's call it type-c for short) which can provide profiles. These profiles
are negotiated using that new stack I pointed out above, not by checking
resistance on data lines or ID pin.

If we're not careful, it will be really difficult to make power delivery fit
into this.

Add to that the fact that the same power delivery pipe (the CC pins) can be used
to tell the other end of remux the pins to e.g. PCIe, and s**t just hit the fan
:-)

In short, whatever we do, needs to consider coping with incoming requests from
userspace at a minimum because userspace will need control over the port
alternate modes. I mean, don't get me wrong, these is all uber-cool to me, but
it'll be a pain to make a flexible/generic solution :-)

> > > I'm not sure that the NFC model is working well for everyone - it's OK
> > > if you happen to be running Android but if you aren't you're often left
> > > sitting there working out what to do with an Android HAL.  We can do the
> 
> > NFC works pretty well for neard, afaict. And without android.
> 
> Ah, that looks better than it was now - it looks like they're providing
> a reasonable hardware abstraction for messaging.  Still, there's other
> examples of this model that are less successful where the kernel
> interface isn't generic.
> 
> > The real difficulty here is coming up with an interface which we're agreeing to
> > supporting for the next 30 years. Whatever that is, as soon as it gets exposed
> > to userland, we will have to support it.
> 
> To me that sounds like an argument for hiding things :)

the problem with hiding, though, is that once something new comes about, it
might not fit our current abstraction and it might be big PITA to support "old"
and new systems.

> > And this another reason I think a more granular approach is better, as it allows
> > us to easily add more pieces as they come along.
> 
> OTOH it's more work for the system as a whole.

it's more work outside the kernel, yes. The total amount of work (kernel +
userspace) will be the same, regardless if you hide it in the kernel or in userspace.

-- 
balbi

Attachment: signature.asc
Description: PGP signature