diff mbox series

[3/4] iio: imu: fxos8700: fix ODR offset error

Message ID 20221202103538.2218925-4-carlos.song@nxp.com (mailing list archive)
State Changes Requested
Headers show
Series iio: imu: fxos8700: fix few bug in reading raw data and configuring register | expand

Commit Message

Carlos Song Dec. 2, 2022, 10:35 a.m. UTC
Correct offset of ODR configure is needed when configure the register
and read ODR data from the register.

Give the correct offset to value when configuring ODR bit and
reading ODR data from CTRL_REG1 register.

Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Fixes: 058f2a09e645 ("iio: imu: fxos8700: fix CTRL_REG1 register configuration error")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/iio/imu/fxos8700_core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Jonathan Cameron Dec. 4, 2022, 3:15 p.m. UTC | #1
On Fri,  2 Dec 2022 18:35:37 +0800
Carlos Song <carlos.song@nxp.com> wrote:

> Correct offset of ODR configure is needed when configure the register
> and read ODR data from the register.
> 
> Give the correct offset to value when configuring ODR bit and
> reading ODR data from CTRL_REG1 register.
> 
> Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
> Fixes: 058f2a09e645 ("iio: imu: fxos8700: fix CTRL_REG1 register configuration error")
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Minor suggestion inline.

Btw, I'm not particularly keen on internal review tags.
I know the policy varies by company, but it it isn't too much of a problem, I'd
prefer to have seen Haibo Chen's review on list as that then gives me some
way to build trust in Haibo's reviews over the long run!

If not possible, then that's fine.

Jonathan

> ---
>  drivers/iio/imu/fxos8700_core.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
> index 60c08519d8af..27e3bd61d054 100644
> --- a/drivers/iio/imu/fxos8700_core.c
> +++ b/drivers/iio/imu/fxos8700_core.c
> @@ -147,6 +147,7 @@
>  #define FXOS8700_CTRL_ODR_MSK       0x38
>  #define FXOS8700_CTRL_ODR_MAX       0x00
>  #define FXOS8700_CTRL_ODR_MIN       GENMASK(4, 3)
> +#define FXOS8700_CTRL_ODR_OFFSET    3
>  
>  /* Bit definitions for FXOS8700_M_CTRL_REG1 */
>  #define FXOS8700_HMS_MASK           GENMASK(1, 0)
> @@ -498,8 +499,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
>  	if (ret)
>  		return ret;
>  
> -	val &= FXOS8700_CTRL_ODR_MSK;
> -
> +	val = (val & FXOS8700_CTRL_ODR_MSK) >> FXOS8700_CTRL_ODR_OFFSET;

FIELD_GET() / FIELD_PREP() preferred as it avoids need to separately define
an offset.

>  	for (i = 0; i < odr_num; i++)
>  		if (val == fxos8700_odr[i].bits)
>  			break;
> @@ -636,7 +636,8 @@ static int fxos8700_chip_init(struct fxos8700_data *data, bool use_spi)
>  
>  	/* Max ODR (800Hz individual or 400Hz hybrid), active mode */
>  	return regmap_write(data->regmap, FXOS8700_CTRL_REG1,
> -			   FXOS8700_CTRL_ODR_MAX | FXOS8700_ACTIVE);
> +			   FXOS8700_CTRL_ODR_MAX << FXOS8700_CTRL_ODR_OFFSET |
> +			   FXOS8700_ACTIVE);
>  }
>  
>  static void fxos8700_chip_uninit(void *data)
diff mbox series

Patch

diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index 60c08519d8af..27e3bd61d054 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -147,6 +147,7 @@ 
 #define FXOS8700_CTRL_ODR_MSK       0x38
 #define FXOS8700_CTRL_ODR_MAX       0x00
 #define FXOS8700_CTRL_ODR_MIN       GENMASK(4, 3)
+#define FXOS8700_CTRL_ODR_OFFSET    3
 
 /* Bit definitions for FXOS8700_M_CTRL_REG1 */
 #define FXOS8700_HMS_MASK           GENMASK(1, 0)
@@ -498,8 +499,7 @@  static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
 	if (ret)
 		return ret;
 
-	val &= FXOS8700_CTRL_ODR_MSK;
-
+	val = (val & FXOS8700_CTRL_ODR_MSK) >> FXOS8700_CTRL_ODR_OFFSET;
 	for (i = 0; i < odr_num; i++)
 		if (val == fxos8700_odr[i].bits)
 			break;
@@ -636,7 +636,8 @@  static int fxos8700_chip_init(struct fxos8700_data *data, bool use_spi)
 
 	/* Max ODR (800Hz individual or 400Hz hybrid), active mode */
 	return regmap_write(data->regmap, FXOS8700_CTRL_REG1,
-			   FXOS8700_CTRL_ODR_MAX | FXOS8700_ACTIVE);
+			   FXOS8700_CTRL_ODR_MAX << FXOS8700_CTRL_ODR_OFFSET |
+			   FXOS8700_ACTIVE);
 }
 
 static void fxos8700_chip_uninit(void *data)