Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 4 Feb 2019 17:35:23 +0000
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
Message-ID: <20190204173523.GM3837@imbe.wolfsonmicro.main>
References: <20190129030156.2024-1-axel.lin@ingics.com>
 <20190129095438.GJ3837@imbe.wolfsonmicro.main>
 <20190129180547.GI10524@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20190129180547.GI10524@sirena.org.uk>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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