Re: [PATCH RFC] iio: mxs-lradc: add scaling to enable divide_by_two operation

[Lars-Peter Clausen <lars@metafoo.de>]

On 07/04/2013 10:58 AM, Hector Palacios wrote:
> Dear Alexandre,
> 
> On 07/03/2013 06:54 PM, Alexandre Belloni wrote:
>> On 03/07/2013 18:39, Hector Palacios wrote:
>>> Dear Lars,
>>>
>>> On 07/03/2013 12:46 PM, Lars-Peter Clausen wrote:
>>>> The scale attribute reports the scale of the raw value. To gain the
>>>> value of
>>>> the result in mV you have to multiply the raw attribute with the scale
>>>> attribute. The scale attribute does not return the result of a
>>>> conversion with
>>>> a scale different applied.
>>>
>>> Thank you for the guidance, I misunderstood the whole thing.
>>>
>>>> I suggest you implement this in two steps. First implement the scale
>>>> property
>>>> for all channels. Use a LUT for each channel's fullscale value (in mV)
>>>> and
>>>> implement reading the scale like this.
>>>>
>>>> case IIO_CHAN_INFO_SCALE:
>>>> *val = lut[chan->channel];
>>>> *val2 = chan->scan_type.realbits;
>>>> return IIO_VAL_FRACTIONAL_LOG2;
>>>>
>>>> The next step is to make the scale modifiable for channels which
>>>> support this.
>>>
>>> I'm sorry, it's the first time I work with an iio driver and I'm afraid
>>> I don't understand this.
>>> Let's say the fullscale value for channel 0 is 1.85V by default (when
>>> divide_by_two is disabled) and 3.7V (when enabled).
>>>
>>> The channel's realbits is currently set to 18, because (although the ADC
>>> resolution is 12 bits) the device can read several samples and return an
>>> accumulated value in 18 bits.
>>>
>>> Now, considering the default case where 1.85V is the max ref voltage and
>>> considering that the driver only reads one sample during the raw_read
>>> operation (12 bits), what value should a read of 'scale' return?
>>>
>>>      scale = Vref_mv / ((1 << 12) - 1) = 1850 /4095 = 0.45177045 ?
>>>
>>
>> That's right. Although, I would think that all the values are on 18bits,
>> so you probably want to use chan->scan_type.realbits there.
> 
> The field of the register that stores the sample is 18 bits, but each sample
> is only 12 bits. The read_raw of a given channel will only read one sample
> and thus returns a value between 0 and 4095.
> If I use realbits here it will divide the Vref_mv by (1 << 18) and will give
> me a wrong scale value.
> Either we change the 'realbits' to 12 which is the real resolution of the
> ADC, or we leave it as 18 and use a 12 in this operation.

I'd change realbits to 12.

> 
>> Using IIO_VAL_FRACTIONAL_LOG2 allows you to avoid doing that computation
>> in your driver (sometimes, it is actually difficult to get it right
>> because you have to use 64 bits numbers).
>>
>> Also, it simplifies things a bit, you can then use something like:
>>
>> case IIO_CHAN_INFO_SCALE:
>> *val = lut[chan->channel];
>> *val2 = chan->scan_type.realbits - is_divide_by_two;
>> return IIO_VAL_FRACTIONAL_LOG2;
> 
> Yes, but not all channels have a by two divisor. Some have a by 4, so I'll
> need to work out a formula per channel divisor.

The by 4 divider is a fixed divider though. That's why you use a lookup
table for the default scale. E.g. for channels with a fixed by 4 divider the
lut would contain 1850 * 4, for channels with a fixed by 2 divider 1850 * 2
and so on.

> When using IIO_VAL_FRACTIONAL_LOG2, val must be given in mV?

Yes

> 
>>> That is, the value to multiply the raw value to get the voltage in mV?
>>> So that if reading the 'raw' returns say 425 and reading scale returns
>>> 0.45177 the measured voltage is 425 * 0.45177 = 192mV?
>>>
>>
>> Right.
>>
>>>
>>> Then let's say I have implemented a function to modify the scale (which
>>> should eventually allow me to enable/disable the by two divisor). What
>>> is the user supposed to write to the 'scale'? Is he supposed to supply a
>>> decimal number like 0.45177 * 2 = 0.903? This would be pretty odd.
>>> Should I create a table instead so that writing a 0 means divisor
>>> disabled and writing 1 means divisor enabled, for example?
>>>
>>
>> That is why you should provide an in_voltageX_scale_available file,
>> providing a list of valid values.
> 
> I see. Thank you very much.
> 
> Best regards,
> -- 
> Hector Palacios