Messages in this thread | | | Date | Mon, 4 Feb 2019 17:35:23 +0000 | From | Charles Keepax <> | Subject | Re: [PATCH] regulator: lochnagar: Fix n_voltages setting for VDDCORE |
| |
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
| |