Messages in this thread | | | Date | Tue, 27 Jul 2021 20:21:48 -0400 | Subject | Re: [PATCH v6 05/13] iio: afe: rescale: add INT_PLUS_{MICRO,NANO} support | From | "Liam Beguin" <> |
| |
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@xiphos.com> > > > > 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@xiphos.com> > > --- > > 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; > > } > >
| |