From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: lrg@ti.com, linux-kernel@vger.kernel.org,
m.szyprowski@samsung.com, Jaroslav Kysela <perex@perex.cz>,
Takashi Iwai <tiwai@suse.de>,
Samuel Ortiz <sameo@linux.intel.com>,
Steve Glendinning <steve.glendinning@smsc.com>,
Richard Purdie <rpurdie@rpsys.net>,
Timur Tabi <timur@freescale.com>,
Kyungmin Park <kyungmin.park@samsung.com>
Subject: Re: [PATCH/RFC] regulator: Reverse the disable sequence in regulator_bulk_disable()
Date: Wed, 25 Jan 2012 11:57:05 +0000 [thread overview]
Message-ID: <20120125115705.GF3687@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1327491338-18817-1-git-send-email-s.nawrocki@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 2425 bytes --]
On Wed, Jan 25, 2012 at 12:35:38PM +0100, Sylwester Nawrocki wrote:
> Often there is a need for disabling a set of regulators in order opposite
> to the enable order. Currently the function regulator_bulk_disable() walks
> list of regulators in same order as regulator_bulk_enable(). This may cause
> trouble, especially for devices with mixed analogue and digital circuits.
> So reverse the disabling sequence of regulator_bulk_disable().
> While at it, also correct the comment.
So, I've applied this since it shouldn't do any harm and probably is
more what we meant to do but note that the bulk APIs don't make any
guarantees about ordering - in particular when we do the enable we fire
off a bunch of threads to bring the regulators up in parallel so the
ordering really is going to be unreliable as it depends on the scheduler
and the rates at which the various regulators ramp. This is done so
that we can enable faster as we don't have to wait for each regulator to
ramp in series.
Whatever driver inspired you to submit this change is therefore probably
buggy or fragile at the minute - is it something that's in mainline or
next right now?
At some point I'd like to enhance things further so we can coalesce
register writes where multiple regulators have their enable bits in the
same register but that's a relatively large amount of work for a small
benefit unless we do something cute with regmap (and that is likely to
be too cute).
> The alternatives to directly modifying regulator_bulk_disable() could be:
> - re-implement it in modules that need the order reversed; it is not
> really helpful in practice since such code would have to be repeated
> in multiple modules;
> - create new function, e.g. regulator_bulk_disable_reversed() with the
> order reversed - not sure if it is not an overkill though;
The third option is that where devices really care about the power
sequencing they should explicitly write that in code and only use the
bulk APIs where they don't care. Typically this will mean either a few
sets of bulk supplies or a single set of bulk supplies and then some
number of individual supplies. An awful lot of devices don't have any
sequencing constraints at all, apparently including most of those using
the API at present.
BTW, your CC list here is *really* random - please think more about who
you're CCing, it looks like you've done something with get_maintainer.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2012-01-25 11:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-25 11:35 [PATCH/RFC] regulator: Reverse the disable sequence in regulator_bulk_disable() Sylwester Nawrocki
2012-01-25 11:57 ` Mark Brown [this message]
2012-01-25 13:35 ` Bill Gatliff
2012-01-25 13:44 ` Mark Brown
2012-01-25 17:20 ` Sylwester Nawrocki
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=20120125115705.GF3687@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lrg@ti.com \
--cc=m.szyprowski@samsung.com \
--cc=perex@perex.cz \
--cc=rpurdie@rpsys.net \
--cc=s.nawrocki@samsung.com \
--cc=sameo@linux.intel.com \
--cc=steve.glendinning@smsc.com \
--cc=timur@freescale.com \
--cc=tiwai@suse.de \
/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