diff mbox series

iio: adxrs290: fix data signedness

Message ID 20211111121915.173616-1-nuno.sa@analog.com (mailing list archive)
State Superseded
Headers show
Series iio: adxrs290: fix data signedness | expand

Commit Message

Nuno Sa Nov. 11, 2021, 12:19 p.m. UTC 
From: Kister Genesis Jimenez <kister.jimenez@analog.com>

Properly sign-extend the rate and temperature data.

Fixes: 2c8920fff1457 ("iio: gyro: Add driver support for ADXRS290")
Signed-off-by: KJimenez <kister.jimenez@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/gyro/adxrs290.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko Nov. 11, 2021, 12:49 p.m. UTC | #1 
On Thu, Nov 11, 2021 at 2:19 PM Nuno Sá <nuno.sa@analog.com> wrote:
>
> From: Kister Genesis Jimenez <kister.jimenez@analog.com>
>
> Properly sign-extend the rate and temperature data.

...

>         /* extract lower 12 bits temperature reading */
> -       *val = temp & 0x0FFF;
> +       *val = sign_extend32(temp & 0x0FFF, 11);

What role does the ' & 0x0FFF' part play now?
Isn't it simply a dup (redundant) piece?
Nuno Sa Nov. 11, 2021, 1:50 p.m. UTC | #2 
> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Thursday, November 11, 2021 1:49 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>
> Cc: linux-iio <linux-iio@vger.kernel.org>; Jonathan Cameron
> <jic23@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Nishant Malpani <nish.malpani25@gmail.com>;
> Jimenez, Kister <Kister.Jimenez@analog.com>
> Subject: Re: [PATCH] iio: adxrs290: fix data signedness
> 
> [External]
> 
> On Thu, Nov 11, 2021 at 2:19 PM Nuno Sá <nuno.sa@analog.com>
> wrote:
> >
> > From: Kister Genesis Jimenez <kister.jimenez@analog.com>
> >
> > Properly sign-extend the rate and temperature data.
> 
> ...
> 
> >         /* extract lower 12 bits temperature reading */
> > -       *val = temp & 0x0FFF;
> > +       *val = sign_extend32(temp & 0x0FFF, 11);
> 
> What role does the ' & 0x0FFF' part play now?
> Isn't it simply a dup (redundant) piece?

Oops, you're right. It serves no purpose as we shift out the bits
that we do not care. My fault, I just acted as a bot...

- Nuno Sá
diff mbox series

Patch

diff --git a/drivers/iio/gyro/adxrs290.c b/drivers/iio/gyro/adxrs290.c
index 3e0734ddafe3..5954fcf52370 100644
--- a/drivers/iio/gyro/adxrs290.c
+++ b/drivers/iio/gyro/adxrs290.c
@@ -7,6 +7,7 @@ 
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -124,7 +125,7 @@  static int adxrs290_get_rate_data(struct iio_dev *indio_dev, const u8 cmd, int *
 		goto err_unlock;
 	}
 
-	*val = temp;
+	*val = sign_extend32(temp, 15);
 
 err_unlock:
 	mutex_unlock(&st->lock);
@@ -146,7 +147,7 @@  static int adxrs290_get_temp_data(struct iio_dev *indio_dev, int *val)
 	}
 
 	/* extract lower 12 bits temperature reading */
-	*val = temp & 0x0FFF;
+	*val = sign_extend32(temp & 0x0FFF, 11);
 
 err_unlock:
 	mutex_unlock(&st->lock);