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:

quoted

From: Liam Beguin <redacted>

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 <redacted>
---
 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