Hi Peter,

On Sun, Nov 28, 2021 at 10:17:50AM +0100, Peter Rosin wrote:

Hi!

On 2021-11-27 21:27, Liam Beguin wrote:

quoted

Hi Peter,

On Mon, Nov 22, 2021 at 01:53:44AM +0100, Peter Rosin wrote:

quoted

Hi Liam!

On 2021-11-15 04:43, Liam Beguin wrote:

quoted

Hi Jonathan, Peter,

snip

quoted

quoted

quoted

- keep IIO_VAL_FRACTIONAL scale when possible, if not default to fixed
  point

This is not what is going on. Patch 9/14 will convert all fractional
scales to fixed point. But I would really like if you in the "reduce
risk of integer overflow" patch (8/14) would hold true to the above
and keep the fractional scale when possible and only fall back to
the less precise fractional-log case if any of the multiplications
needed for an exact fractional scale causes overflow.

Thanks for looking at these patches again.

quoted

The v8 discussion concluded that this was a valid approach, right?

Yes, I remember you saying that you'd be more comfortable keeping the
IIO_VAL_FRACTIONAL.

quoted

I know you also said that the core exposes the scale with nano
precision in sysfs anyway, but that is not true for in-kernel
consumers. They have an easier time reading the "real" scale value
compared to going via the string representation of fixed point
returned from iio_format_value. At least the rescaler itself does so,
which means that chaining rescalers might suffer needless accuracy
degradation.

Agreed, that makes total sense.

If I'm not mistaken, the first condition in the case, if (!rem), will
return IIO_VAL_FRACTIONAL if the division is exact, keeping all the
precision. No?

Only if the resulting scale fits in nine decimals. That's never the
case if you have primes other than 2 and 5 in the denominator (after
eliminating gcd of course). Which mean that if you chain one rescaler
doing 1/3 and one doing 3/1, you would get a combined scale of
0.999999999 instead of 3/3 if we take the approach of these patches.

So, what I'm after is that - for IIO_VAL_FRACTIONAL - not take the
multiply-by-1e9 code path /unless/ the existing fractional approach
overflows in either numerator or denominator (or both).

Understood, I'll update based on this.

Side note: The same could be done for IIO_VAL_INT when the numerator
overflows (since the denominator cannot overflow), but I guess that
can be done later.

Agreed, I don't mind working on this later but I'd like to focus on
getting the current changes in first.

Thanks,
Liam

Cheers,
Peter