On 2021-07-09 21:30, Liam Beguin wrote:
> On Fri Jul 9, 2021 at 12:29 PM EDT, Peter Rosin wrote:
>>
>>
>> On 2021-07-06 18:09, Liam Beguin wrote:
>>> From: Liam Beguin <lvb@xxxxxxxxxx>
>>>
>>> Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
>>> Scale the integer part and the decimal parts individually and keep the
>>> original scaling type.
>>>
>>> Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx>
>>> ---
>>> drivers/iio/afe/iio-rescale.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
>>> index ba3bdcc69b16..1d0e24145d87 100644
>>> --- a/drivers/iio/afe/iio-rescale.c
>>> +++ b/drivers/iio/afe/iio-rescale.c
>>> @@ -89,7 +89,15 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
>>> do_div(tmp, 1000000000LL);
>>> *val = tmp;
>>> return ret;
>>> + case IIO_VAL_INT_PLUS_NANO:
>>> + case IIO_VAL_INT_PLUS_MICRO:
>>> + tmp = (s64)*val * rescale->numerator;
>>> + *val = div_s64(tmp, rescale->denominator);
>>> + tmp = (s64)*val2 * rescale->numerator;
>>> + *val2 = div_s64(tmp, rescale->denominator);
>>
>
> Hi Peter,
>
>> Hi!
>>
>> You are losing precision, and you are not mormalising after the
>> calculation.
>
> Can you elaborate a little on what you mean here?
>
> Do you mean that I should make sure that *val2, the PLUS_{NANO,MICRO}
> part, doesn't contain an integer part? And if so transfer that part back
> to *val?
Yes. On 32-bit, you will easily wrap, especially for PLUS_NANO. You'd
only need a scale factor of 10 or so and a fractional part above .5 to
hit the roof (10 * 500000000 > 2^32).
But I also mean that you are losing precision when you are scaling
the integer part and the fractional part separately. That deserves
at least a comment, but ideally it should be handled correctly.
>> I think it's better to not even attempt this given that the results can
>> be
>> really poor.
>
> Unfortunatelly, I'm kinda stuck with this as some of my ADC use these
> types.
Ok. Crap. :-)
Cheers,
Peter
