Re: [PATCH 1/2] twl6030: regulator: Fix vsel calculations in set/get voltage apis

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Thu, Feb 18, 2010 at 12:02:49PM +0530, Nayak, Rajendra wrote:

> > This looks wrong - this code is inside a loop over vsel->voltage
> > mappings, if that mapping is wrong and needs to be ignored (which is
> > what this code is doing) then we shouldn't be looking at it at all.

> Not sure if I understood your comment completely, but here's how I think
> it will work.

> The for loop runs through all the valid voltages supported by the regulator
> and and when it finds the first in-range value programs the VOLTAGE register.
> For twl4030 the VOLTAGE register can be programmed with just the index
> of the table since the mapping was something like

> 1800mV = 0x0
> 2800mV = 0x1
> 2900mV = 0x2
> 3000mV = 0x3

> Now for twl6030 I just have added an additional vsel calulation part after
> the in-range value is found, since using the table index directly as vsel
> would not work.

Right, but this means that the code is looking at one table of values to
find a voltage it likes and then picking the actual selector from
another set of values.  This means that depending on the actual
differences it may either pick a value which isn't valid for the TWL6030
or omit values which the TWL6030 can actually support.  Either way from
a code inspection point of view the code doesn't look good due to the
mismatch between the two sets of selectors.

The TWL6030 case should be doing something more like other regulator
drivers with straightforward mappings between selector and voltage (most
of them) and not using the TWL4030 value table.