diff mbox series

[v10,03/14] iio: inkern: make a best effort on offset calculation

Message ID 20211219223953.16074-4-liambeguin@gmail.com (mailing list archive)
State Superseded
Headers show
Series iio: afe: add temperature rescaling support | expand

Commit Message

Liam Beguin Dec. 19, 2021, 10:39 p.m. UTC 
From: Liam Beguin <lvb@xiphos.com>

iio_convert_raw_to_processed_unlocked() assumes the offset is an
integer. Make a best effort to get a valid offset value for fractional
cases without breaking implicit truncations.

Fixes: 48e44ce0f881 ("iio:inkern: Add function to read the processed value")
Signed-off-by: Liam Beguin <lvb@xiphos.com>
---
 drivers/iio/inkern.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Comments

Andy Shevchenko Dec. 21, 2021, 5:42 a.m. UTC | #1 
On Mon, Dec 20, 2021 at 11:17 AM Liam Beguin <liambeguin@gmail.com> wrote:
>
> From: Liam Beguin <lvb@xiphos.com>
>
> iio_convert_raw_to_processed_unlocked() assumes the offset is an
> integer. Make a best effort to get a valid offset value for fractional
> cases without breaking implicit truncations.

...

> +                       offset_val /= (1 << offset_val2);

Besides potentially being problematic (if arg is 31 for the left
shift) why not simply

_val >>= _val2;

?
Liam Beguin Dec. 21, 2021, 5:50 p.m. UTC | #2 
Hi Andy,

On Tue, Dec 21, 2021 at 07:42:31AM +0200, Andy Shevchenko wrote:
> On Mon, Dec 20, 2021 at 11:17 AM Liam Beguin <liambeguin@gmail.com> wrote:
> >
> > From: Liam Beguin <lvb@xiphos.com>
> >
> > iio_convert_raw_to_processed_unlocked() assumes the offset is an
> > integer. Make a best effort to get a valid offset value for fractional
> > cases without breaking implicit truncations.
> 
> ...
> 
> > +                       offset_val /= (1 << offset_val2);
> 
> Besides potentially being problematic (if arg is 31 for the left
> shift) why not simply
> 
> _val >>= _val2;

That is safer, and more elegant! I'll update.

Thanks,
Liam

> --
> With Best Regards,
> Andy Shevchenko
diff mbox series

Patch

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index dbe13fad3cbb..aff6d2d3447c 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -595,13 +595,35 @@  EXPORT_SYMBOL_GPL(iio_read_channel_average_raw);
 static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 	int raw, int *processed, unsigned int scale)
 {
-	int scale_type, scale_val, scale_val2, offset;
+	int scale_type, scale_val, scale_val2;
+	int offset_type, offset_val, offset_val2;
 	s64 raw64 = raw;
-	int ret;
 
-	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
-	if (ret >= 0)
-		raw64 += offset;
+	offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
+				       IIO_CHAN_INFO_OFFSET);
+	if (offset_type >= 0) {
+		switch (offset_type) {
+		case IIO_VAL_INT:
+			break;
+		case IIO_VAL_INT_PLUS_MICRO:
+		case IIO_VAL_INT_PLUS_NANO:
+			/*
+			 * Both IIO_VAL_INT_PLUS_MICRO and IIO_VAL_INT_PLUS_NANO
+			 * implicitely truncate the offset to it's integer form.
+			 */
+			break;
+		case IIO_VAL_FRACTIONAL:
+			offset_val /= offset_val2;
+			break;
+		case IIO_VAL_FRACTIONAL_LOG2:
+			offset_val /= (1 << offset_val2);
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		raw64 += offset_val;
+	}
 
 	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
 					IIO_CHAN_INFO_SCALE);