All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Felipe Balbi <balbi@ti.com>, Baolin Wang <baolin.wang@linaro.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	sre@kernel.org, dbaryshkov@gmail.com, dwmw2@infradead.org,
	peter.chen@freescale.com, stern@rowland.harvard.edu,
	r.baldyga@samsung.com, sojka@merica.cz,
	yoshihiro.shimoda.uh@renesas.com, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, sameo@linux.intel.com,
	lee.jones@linaro.org, ckeepax@opensource.wolfsonmicro.com,
	patches@opensource.wolfsonmicro.com, linux-pm@vger.kernel.org,
	device-mainlining@lists.linuxfoundation.org
Subject: Re: [PATCH v4 1/5] gadget: Introduce the notifier functions
Date: Thu, 1 Oct 2015 12:58:49 -0500	[thread overview]
Message-ID: <20151001175849.GH4469@saruman.tx.rr.com> (raw)
In-Reply-To: <20151001174308.GL12635@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]

Hi,

On Thu, Oct 01, 2015 at 06:43:08PM +0100, Mark Brown wrote:
> On Thu, Oct 01, 2015 at 12:29:32PM -0500, Felipe Balbi wrote:
> 
> > Frankly, I wanted all of this to be decided in userland with the
> > kernel just providing notification and basic safety checks (we don't
> > want to allow a bogus userspace daemon frying anybody's devices).
> 
> What's the advantage of pushing this to userspace?  By the time we
> provide enough discoverability to enable userspace to configure itself
> it seems like we'd have enough information to do the job anyway.

you're going to be dealing with a setup where each vendor does one thing
differently. Some will have it all in the SoC part of a single IP (dwc3 can be
configured that way), some will push it out to companion IC, some might even use
some mostly discrete setup and so on...

We're gonna be dealing with a decision that involves information from multiple
subsystems (USB, regulator, hwmon, power supply to name a few).

We tried doing it all in the kernel back in N800, N810 and N900/N9 days and it's
just plain difficult. To make matters worse, N900 had two USB PHYs, one for
actual runtime use and another just for detecting the charger type on the other
end.

On top of all that, we still need to figure out what to do when a particular
configuration gets chosen, and what to do when the bus goes into the different
suspend levels.

It's going to be a lot of policy getting added to the kernel. On top of all
that, when Type C and power delivery is finally a thing, there will an entire
new stack for USB C commands (using the Type C CC pins or modulated on VBUS for
legacy connectors) which we will have to add to the kernel. And different
devices will support different charging profiles (there are at least 6 of those,
IIRC).

With all these different things going on, it's best to do what e.g. NFC folks
did. Just a small set of hooks in the kernel, but actual implementation done by
a userspace daemon. This makes it even easier for middleware development since
we can provide a higher level API for middleware to talk to the charging daemon.

Another benefit is that this charging daemon can also give hints to power
management policy that e.g. we're charging at 10W or 100W and we can e.g. switch
to performance governor safely even though battery is rather low.

Anyway, there's really a lot that needs to happen and shuving it all in the
kernel is, IMHO, the wrong decision. I would be much happier with minimal safety
requirements in the kernel and let a userspace daemon (which we should certainly
provide a reference implementation) figure out what to do. This might be better
for things like Chromebooks and Android phones too which might make completely
different decisions given a certain charging profile.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Felipe Balbi <balbi@ti.com>, Baolin Wang <baolin.wang@linaro.org>,
	Greg KH <gregkh@linuxfoundation.org>, <sre@kernel.org>,
	<dbaryshkov@gmail.com>, <dwmw2@infradead.org>,
	<peter.chen@freescale.com>, <stern@rowland.harvard.edu>,
	<r.baldyga@samsung.com>, <sojka@merica.cz>,
	<yoshihiro.shimoda.uh@renesas.com>, <linux-usb@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <sameo@linux.intel.com>,
	<lee.jones@linaro.org>, <ckeepax@opensource.wolfsonmicro.com>,
	<patches@opensource.wolfsonmicro.com>, <linux-pm@vger.kernel.org>,
	<device-mainlining@lists.linuxfoundation.org>
Subject: Re: [PATCH v4 1/5] gadget: Introduce the notifier functions
Date: Thu, 1 Oct 2015 12:58:49 -0500	[thread overview]
Message-ID: <20151001175849.GH4469@saruman.tx.rr.com> (raw)
In-Reply-To: <20151001174308.GL12635@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]

Hi,

On Thu, Oct 01, 2015 at 06:43:08PM +0100, Mark Brown wrote:
> On Thu, Oct 01, 2015 at 12:29:32PM -0500, Felipe Balbi wrote:
> 
> > Frankly, I wanted all of this to be decided in userland with the
> > kernel just providing notification and basic safety checks (we don't
> > want to allow a bogus userspace daemon frying anybody's devices).
> 
> What's the advantage of pushing this to userspace?  By the time we
> provide enough discoverability to enable userspace to configure itself
> it seems like we'd have enough information to do the job anyway.

you're going to be dealing with a setup where each vendor does one thing
differently. Some will have it all in the SoC part of a single IP (dwc3 can be
configured that way), some will push it out to companion IC, some might even use
some mostly discrete setup and so on...

We're gonna be dealing with a decision that involves information from multiple
subsystems (USB, regulator, hwmon, power supply to name a few).

We tried doing it all in the kernel back in N800, N810 and N900/N9 days and it's
just plain difficult. To make matters worse, N900 had two USB PHYs, one for
actual runtime use and another just for detecting the charger type on the other
end.

On top of all that, we still need to figure out what to do when a particular
configuration gets chosen, and what to do when the bus goes into the different
suspend levels.

It's going to be a lot of policy getting added to the kernel. On top of all
that, when Type C and power delivery is finally a thing, there will an entire
new stack for USB C commands (using the Type C CC pins or modulated on VBUS for
legacy connectors) which we will have to add to the kernel. And different
devices will support different charging profiles (there are at least 6 of those,
IIRC).

With all these different things going on, it's best to do what e.g. NFC folks
did. Just a small set of hooks in the kernel, but actual implementation done by
a userspace daemon. This makes it even easier for middleware development since
we can provide a higher level API for middleware to talk to the charging daemon.

Another benefit is that this charging daemon can also give hints to power
management policy that e.g. we're charging at 10W or 100W and we can e.g. switch
to performance governor safely even though battery is rather low.

Anyway, there's really a lot that needs to happen and shuving it all in the
kernel is, IMHO, the wrong decision. I would be much happier with minimal safety
requirements in the kernel and let a userspace daemon (which we should certainly
provide a reference implementation) figure out what to do. This might be better
for things like Chromebooks and Android phones too which might make completely
different decisions given a certain charging profile.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2015-10-01 17:59 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-24 17:39 [PATCH v4 0/5] Introduce usb charger framework to deal with the usb gadget power negotation Baolin Wang
2015-09-24 17:39 ` [PATCH v4 1/5] gadget: Introduce the notifier functions Baolin Wang
2015-10-01 17:29   ` Felipe Balbi
2015-10-01 17:29     ` Felipe Balbi
2015-10-01 17:43     ` Mark Brown
2015-10-01 17:58       ` Felipe Balbi [this message]
2015-10-01 17:58         ` Felipe Balbi
2015-10-01 18:01         ` Felipe Balbi
2015-10-01 18:01           ` Felipe Balbi
2015-10-02 16:47         ` Mark Brown
2015-10-02 17:23           ` Felipe Balbi
2015-10-02 17:23             ` Felipe Balbi
2015-10-02 18:49             ` Mark Brown
     [not found]               ` <20151002184909.GC12635-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-10-02 19:11                 ` Felipe Balbi
2015-10-02 19:11                   ` Felipe Balbi
2015-10-04 22:55                   ` Mark Brown
2015-10-05 15:15                     ` Felipe Balbi
2015-10-05 15:15                       ` Felipe Balbi
2015-10-05 16:18                       ` Mark Brown
     [not found]                         ` <20151005161833.GZ12635-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-10-05 16:29                           ` Felipe Balbi
2015-10-05 16:29                             ` Felipe Balbi
2015-10-07 16:44             ` [Device-mainlining] " Bjorn Andersson
2015-10-08 15:51         ` Pavel Machek
2015-10-02  5:41     ` Greg KH
2015-10-02 17:27       ` Felipe Balbi
2015-10-02 17:27         ` Felipe Balbi
2015-10-08 15:50     ` Pavel Machek
2015-10-09 21:17       ` Felipe Balbi
2015-10-09 21:17         ` Felipe Balbi
2015-10-12 16:56         ` Mark Brown
     [not found]         ` <87oag7u4go.fsf-HgARHv6XitJaoMGHk7MhZQC/G2K4zDHf@public.gmane.org>
2016-04-09 16:01           ` Pavel Machek
2016-04-09 16:01             ` Pavel Machek
2015-09-24 17:39 ` [PATCH v4 2/5] gadget: Introduce the usb charger framework Baolin Wang
2015-09-24 17:39 ` [PATCH v4 3/5] gadget: Support for " Baolin Wang
2015-09-24 17:39 ` [PATCH v4 4/5] gadget: Integrate with the usb gadget supporting for usb charger Baolin Wang
2015-09-24 17:39 ` [PATCH v4 5/5] power: wm831x_power: Support USB charger current limit management Baolin Wang
  -- strict thread matches above, loose matches on Subject: below --
2015-08-19  9:13 [PATCH v4 0/5] Introduce usb charger framework to deal with the usb gadget power negotation Baolin Wang
     [not found] ` <cover.1439974219.git.baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-08-19  9:13   ` [PATCH v4 1/5] gadget: Introduce the notifier functions Baolin Wang
2015-08-19  9:13     ` Baolin Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151001175849.GH4469@saruman.tx.rr.com \
    --to=balbi@ti.com \
    --cc=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.wolfsonmicro.com \
    --cc=dbaryshkov@gmail.com \
    --cc=device-mainlining@lists.linuxfoundation.org \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=patches@opensource.wolfsonmicro.com \
    --cc=peter.chen@freescale.com \
    --cc=r.baldyga@samsung.com \
    --cc=sameo@linux.intel.com \
    --cc=sojka@merica.cz \
    --cc=sre@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.