On Fri Jul 23, 2021 at 5:16 PM EDT, Peter Rosin wrote: > On 2021-07-21 05:06, Liam Beguin wrote: > > From: Liam Beguin <lvb@xxxxxxxxxx> > > > > Some ADCs use IIO_VAL_INT_PLUS_{NANO,MICRO} scale types. > > Add support for these to allow using the iio-rescaler with them. > > > > Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx> > > --- > > drivers/iio/afe/iio-rescale.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c > > index d0669fd8eac5..2b73047365cc 100644 > > --- a/drivers/iio/afe/iio-rescale.c > > +++ b/drivers/iio/afe/iio-rescale.c > > @@ -41,6 +41,20 @@ int rescale_process_scale(struct rescale *rescale, int scale_type, > > do_div(tmp, 1000000000LL); > > *val = tmp; > > return scale_type; > > + case IIO_VAL_INT_PLUS_NANO: > > + tmp = ((s64)*val * 1000000000LL + *val2) * rescale->numerator; > > + tmp = div_s64(tmp, rescale->denominator); > > + > > + *val = div_s64(tmp, 1000000000LL); > > + *val2 = tmp - *val * 1000000000LL; > > + return scale_type; Hi Peter, > > Hi! > > My objection from v5 still stands. Did you forget or did you simply send > the > wrong patch? Apologies, again I didn't mean to make it seem like I ignored your comments. I tried your suggestion, but had issues when *val2 would overflow into the integer part. Even though what I has was more prone to integer overflow with the first multiplication, I thought it was still a valid solution as it passed the tests. > > Untested suggestion, this time handling negative values and > canonicalizing any > overflow from the fraction calculation. > > neg = *val < 0 || *val2 < 0; > tmp = (s64)abs(*val) * rescale->numerator; > rem = do_div(tmp, rescale->denominator); > *val = tmp; > tmp = rem * 1000000000LL + (s64)abs(*val2) * rescale->numerator; > do_div(tmp, rescale->denominator); > *val2 = do_div(tmp, 1000000000LL); > *val += tmp; > if (neg) { > if (*val < 0) > *val = -*val; > else > *val2 = -*val; I'll look into this suggestion. > } > > > + case IIO_VAL_INT_PLUS_MICRO: > > + tmp = ((s64)*val * 1000000LL + *val2) * rescale->numerator; > > + tmp = div_s64(tmp, rescale->denominator); > > + > > + *val = div_s64(tmp, 1000000); > > Why do you not have the LL suffix here? Doesnt' LL make it into a 64 bit integer? I left it out because the second parameter of div_s64() should be s32. Thanks, Liam > > Cheers, > Peter > > > + *val2 = tmp - *val * 1000000; > > + return scale_type; > > default: > > return -EOPNOTSUPP; > > } > >