| Message ID | 20210530005917.20953-4-liambeguin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | iio: afe: add temperature rescaling support | expand |
Hi! Thanks for the patches. However, things have recently changed under your feet. Can you please adjust to https://patchwork.kernel.org/project/linux-iio/list/?series=484153 https://lore.kernel.org/linux-iio/20210518190201.26657c49@jic23-huawei/T/#m0de421cc9f6bc10bfa2622d65be750aaced3810c and resend? On 2021-05-30 02:59, Liam Beguin wrote: > From: Liam Beguin <lvb@xiphos.com> > > Make use of the IIO core to compute the source channel's processed > value. This reduces duplication and will facilitate the addition of > offsets in the iio-rescale driver. Cheers, Peter
Hi Peter, On Mon May 31, 2021 at 3:09 AM EDT, Peter Rosin wrote: > Hi! > > Thanks for the patches. However, things have recently changed under your > feet. > Can you please adjust to > > https://patchwork.kernel.org/project/linux-iio/list/?series=484153 > https://lore.kernel.org/linux-iio/20210518190201.26657c49@jic23-huawei/T/#m0de421cc9f6bc10bfa2622d65be750aaced3810c > > and resend? Thanks for pointing those out. I'll rebase on the latest -rc and resend. Liam > > On 2021-05-30 02:59, Liam Beguin wrote: > > From: Liam Beguin <lvb@xiphos.com> > > > > Make use of the IIO core to compute the source channel's processed > > value. This reduces duplication and will facilitate the addition of > > offsets in the iio-rescale driver. > > Cheers, > Peter
On Sat, 29 May 2021 20:59:11 -0400 Liam Beguin <liambeguin@gmail.com> wrote: > From: Liam Beguin <lvb@xiphos.com> > > Make use of the IIO core to compute the source channel's processed > value. This reduces duplication and will facilitate the addition of > offsets in the iio-rescale driver. Fairly sure you just dropped a lot or precision if the devices do provide a scale. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/iio/afe/iio-rescale.c | 37 ++++++++++------------------------- > 1 file changed, 10 insertions(+), 27 deletions(-) > > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c > index e42ea2b1707d..4d0813b274d1 100644 > --- a/drivers/iio/afe/iio-rescale.c > +++ b/drivers/iio/afe/iio-rescale.c > @@ -38,36 +38,20 @@ static int rescale_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct rescale *rescale = iio_priv(indio_dev); > - unsigned long long tmp; > int ret; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > - return iio_read_channel_raw(rescale->source, val); > + ret = iio_read_channel_processed(rescale->source, val); iio_read_channel_processed() provides a IIO_VAL_INT as you can see. Now that can be heavily truncated. In some cases it is always 0 (e.g. device reading very small currents only). To maintain that scaling we deliberately combine it with the scaling from the afe, hopefully maintaining that precision because the scale value has much higher degree of precision. We could probably do this better than currently with a more complex conversion function. It might seem like a good idea to fix up iio_read_channel_processed to return IIO_VAL_INT_PLUS_MICRO or similar, but potentially that would still throw away precision, for example if the scale is expressed as IIO_VAL_FRACTIONAL to a high degree of precision. Jonathan > > + return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > - ret = iio_read_channel_scale(rescale->source, val, val2); > - switch (ret) { > - case IIO_VAL_FRACTIONAL: > - *val *= rescale->numerator; > - *val2 *= rescale->denominator; > - return ret; > - case IIO_VAL_INT: > - *val *= rescale->numerator; > - if (rescale->denominator == 1) > - return ret; > - *val2 = rescale->denominator; > - return IIO_VAL_FRACTIONAL; > - case IIO_VAL_FRACTIONAL_LOG2: > - tmp = *val * 1000000000LL; > - do_div(tmp, rescale->denominator); > - tmp *= rescale->numerator; > - do_div(tmp, 1000000000LL); > - *val = tmp; > - return ret; > - default: > - return -EOPNOTSUPP; > - } > + *val = rescale->numerator; > + if (rescale->denominator == 1) > + return IIO_VAL_INT; > + *val2 = rescale->denominator; > + > + return IIO_VAL_FRACTIONAL; > default: > return -EINVAL; > } > @@ -130,9 +114,8 @@ static int rescale_configure_channel(struct device *dev, > chan->ext_info = rescale->ext_info; > chan->type = rescale->cfg->type; > > - if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) || > - !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) { > - dev_err(dev, "source channel does not support raw/scale\n"); > + if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW)) { > + dev_err(dev, "source channel does not support raw\n"); > return -EINVAL; > } >
diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c index e42ea2b1707d..4d0813b274d1 100644 --- a/drivers/iio/afe/iio-rescale.c +++ b/drivers/iio/afe/iio-rescale.c @@ -38,36 +38,20 @@ static int rescale_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct rescale *rescale = iio_priv(indio_dev); - unsigned long long tmp; int ret; switch (mask) { case IIO_CHAN_INFO_RAW: - return iio_read_channel_raw(rescale->source, val); + ret = iio_read_channel_processed(rescale->source, val); + return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: - ret = iio_read_channel_scale(rescale->source, val, val2); - switch (ret) { - case IIO_VAL_FRACTIONAL: - *val *= rescale->numerator; - *val2 *= rescale->denominator; - return ret; - case IIO_VAL_INT: - *val *= rescale->numerator; - if (rescale->denominator == 1) - return ret; - *val2 = rescale->denominator; - return IIO_VAL_FRACTIONAL; - case IIO_VAL_FRACTIONAL_LOG2: - tmp = *val * 1000000000LL; - do_div(tmp, rescale->denominator); - tmp *= rescale->numerator; - do_div(tmp, 1000000000LL); - *val = tmp; - return ret; - default: - return -EOPNOTSUPP; - } + *val = rescale->numerator; + if (rescale->denominator == 1) + return IIO_VAL_INT; + *val2 = rescale->denominator; + + return IIO_VAL_FRACTIONAL; default: return -EINVAL; } @@ -130,9 +114,8 @@ static int rescale_configure_channel(struct device *dev, chan->ext_info = rescale->ext_info; chan->type = rescale->cfg->type; - if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) || - !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) { - dev_err(dev, "source channel does not support raw/scale\n"); + if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW)) { + dev_err(dev, "source channel does not support raw\n"); return -EINVAL; }