From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Mark Brown <broonie@kernel.org>
Cc: Axel Lin <axel.lin@ingics.com>,
Liam Girdwood <lgirdwood@gmail.com>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] regulator: lochnagar: Fix n_voltages setting for VDDCORE
Date: Mon, 4 Feb 2019 17:35:23 +0000 [thread overview]
Message-ID: <20190204173523.GM3837@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <20190129180547.GI10524@sirena.org.uk>
On Tue, Jan 29, 2019 at 06:05:47PM +0000, Mark Brown wrote:
> On Tue, Jan 29, 2019 at 09:54:38AM +0000, Charles Keepax wrote:
>
> > Hmm... I think this might need a little more investigation, it
> > does indeed fail at present (amazing that it works upto exactly 1.2V
> > which is common the usecase) but this feels more like a core
> > problem. There are indeed only 57 valid voltage settings as the
> > range is 0x8 - 0x41, but it seems that presently the core treats
> > this less as n_voltages and more like max_selector. Should we
> > perhaps be teaching the core to understand the min_selector
> > instead?
>
> We've not had a need for this before but that seems like a reasonable
> addition.
Ok been looking at this last couple of days and I am not so sure
it was a good idea after all.
There are basically two options:
1) Update the usages of selector so it is consistently an index
into the available voltages.
2) Update the handling of n_voltages so that it is converted to
max_selector when used.
Both of these really seem to end up with code that is more
complex than the current system. You end up needing to always
specify or calculate and store the minimum selector and maybe
the maximum one. This needs handling across the various different
schemes for specifying the voltage ranges for the regulators.
So I am adding my ack to the patch, and think we should really
just apply Axel's fix.
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
One thought that did cross my mind, would we be tempted to rename
n_voltages to max_selector? It would be more consistent with the
current usage, although its a horrible big change that doesn't
really make any functional difference.
Thanks,
Charles
prev parent reply other threads:[~2019-02-04 17:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-29 3:01 Axel Lin
2019-01-29 9:54 ` Charles Keepax
2019-01-29 18:05 ` Mark Brown
2019-02-04 17:35 ` Charles Keepax [this message]
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=20190204173523.GM3837@imbe.wolfsonmicro.main \
--to=ckeepax@opensource.cirrus.com \
--cc=axel.lin@ingics.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.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
Powered by JetHome