Re: [PATCH v9 00/14] iio: afe: add temperature rescaling support

Liam Beguin
2021-11-30T18:52:08Z

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:
> > Hi Peter,
> >
> > On Mon, Nov 22, 2021 at 01:53:44AM +0100, Peter Rosin wrote:
> >> Hi Liam!
> >>
> >> On 2021-11-15 04:43, Liam Beguin wrote:
> >>> Hi Jonathan, Peter,
>
> snip
>
> >>> - 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.
> >
> >> 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.
> >
> >> 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