From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: skannan@codeaurora.org
Cc: Liam Girdwood <lrg@slimlogic.co.uk>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: Call into regulator driver only when voltage min/max really changes.
Date: Sun, 12 Dec 2010 13:13:28 +0000 [thread overview]
Message-ID: <20101212131328.GF15189@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <d681667c87585c281d0330c030c97025.squirrel@codeaurora.org>
On Sun, Dec 12, 2010 at 04:41:04AM -0800, skannan@codeaurora.org wrote:
> > On Sun, Dec 12, 2010 at 02:55:40AM -0800, Saravana Kannan wrote:
> > Out of interest do we have any examples of consumers that do this
> > sufficiently often and/or in paths sufficiently performance critical for
> > it to be an issue? Sounds like there might be room for optimisation in
> > those consumers.
> Wouldn't it be better to optimize in one location (the core), instead of
> optimizing in several consumers?
It's certainly good to optimise in the core, which is why I acked your
patch, but it strikes me that if these consumers are regularly setting
the regulator voltage to the same thing over and over again they may
well also be doing whatever else goes along with setting the voltage
over and over again and there's room for optimisation there also.
> > The down side of doing this is that if the regulator state changes
> > underneath us we've now got no way of recovering from that situation.
> > This is something that's only partially supported by the API at the
> > minute but it's nice to have a story about how drivers can work with
> > this. I'll send a patch adding an explicit sync API.
> Would this be a problem irrespective of this optimization?
At the minute this can be handled at the consumer level since if the
consumer knows that the voltage got changed underneath it (and if it
cares about the voltage then it probably ought to know about this
otherwise something is going to get upset) it can reconfigure the
voltage by calling set_voltage() again.
> Btw, after sending this patch, I just realized there might be another
> "bug" in the existing code (not my changes). What's the meaning of the
> "voltage change" event? It appears to be sent even if the regulator driver
> fails to set the voltage. It would be correct if the event just means
> "someone called set_voltage", but that seems like an unlikely definition.
It's an indication to other consumers that they might want to reread the
voltage. Overnotification doesn't really hurt here, though it'd be nice
to avoid it.
> Let me know if I should send in a patch for that too.
It'd probably coollide with the update I'm doing for syncing voltages.
next prev parent reply other threads:[~2010-12-12 13:13 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-12 10:55 [PATCH] regulator: Call into regulator driver only when voltage min/max really changes Saravana Kannan
2010-12-12 12:18 ` Mark Brown
2010-12-12 12:41 ` skannan
2010-12-12 13:13 ` Mark Brown [this message]
2010-12-12 13:53 ` Mark Brown
2010-12-12 21:41 ` Saravana Kannan
2010-12-13 14:41 ` Liam Girdwood
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=20101212131328.GF15189@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lrg@slimlogic.co.uk \
--cc=skannan@codeaurora.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox