| Message ID | 20210530005917.20953-3-liambeguin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | iio: afe: add temperature rescaling support | expand |
Hi! On 2021-05-30 02:59, Liam Beguin wrote: > From: Liam Beguin <lvb@xiphos.com> > > iio_convert_raw_to_processed_unlocked() assumes the offset is an > integer. Make that clear to the consumer by returning an error when an > unsupported offset type is detected. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/iio/inkern.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > index 4b6a8e11116a..dede4536d499 100644 > --- a/drivers/iio/inkern.c > +++ b/drivers/iio/inkern.c > @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan, > int ret; > > ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET); > - if (ret >= 0) > + if (ret == IIO_VAL_INT) { > raw64 += offset; > + } else if (ret >= 0) { > + dev_err(&chan->indio_dev->dev, "unsupported offset type"); > + return -EINVAL; > + } > > scale_type = iio_channel_read(chan, &scale_val, &scale_val2, > IIO_CHAN_INFO_SCALE); > This breaks the implicit truncation that happens for drivers that have offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO} Implicit truncation might be more appropriate than an error? However, to error out on fractional offsets etc still seem appropriate, but there are corner cases where the existing code did the right thing. E.g. a denominator of one or a fractional-log2 of zero, but a big denominator and a smaller numerator would also just result in a relatively harmless truncation. I don't know if it's really right to just break that? Cheers, Peter
Hi Peter, On Mon May 31, 2021 at 5:45 AM EDT, Peter Rosin wrote: > Hi! > > On 2021-05-30 02:59, Liam Beguin wrote: > > From: Liam Beguin <lvb@xiphos.com> > > > > iio_convert_raw_to_processed_unlocked() assumes the offset is an > > integer. Make that clear to the consumer by returning an error when an > > unsupported offset type is detected. > > > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > > --- > > drivers/iio/inkern.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c > > index 4b6a8e11116a..dede4536d499 100644 > > --- a/drivers/iio/inkern.c > > +++ b/drivers/iio/inkern.c > > @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan, > > int ret; > > > > ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET); > > - if (ret >= 0) > > + if (ret == IIO_VAL_INT) { > > raw64 += offset; > > + } else if (ret >= 0) { > > + dev_err(&chan->indio_dev->dev, "unsupported offset type"); > > + return -EINVAL; > > + } > > > > scale_type = iio_channel_read(chan, &scale_val, &scale_val2, > > IIO_CHAN_INFO_SCALE); > > > > This breaks the implicit truncation that happens for drivers that have > offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO} > > Implicit truncation might be more appropriate than an error? > > However, to error out on fractional offsets etc still seem appropriate, > but > there are corner cases where the existing code did the right thing. E.g. > a denominator of one or a fractional-log2 of zero, but a big denominator > and > a smaller numerator would also just result in a relatively harmless > truncation. > > I don't know if it's really right to just break that? Apologies for missing these. You're right that this change shouldn't break what used to work implicitly. I'll rework this. Liam > > Cheers, > Peter
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c index 4b6a8e11116a..dede4536d499 100644 --- a/drivers/iio/inkern.c +++ b/drivers/iio/inkern.c @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan, int ret; ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET); - if (ret >= 0) + if (ret == IIO_VAL_INT) { raw64 += offset; + } else if (ret >= 0) { + dev_err(&chan->indio_dev->dev, "unsupported offset type"); + return -EINVAL; + } scale_type = iio_channel_read(chan, &scale_val, &scale_val2, IIO_CHAN_INFO_SCALE);