* [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability
@ 2022-12-14 3:14 carlos.song
2022-12-14 3:14 ` [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback carlos.song
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:14 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
In patch v3, the main work is fixing the bug that mask bits out of an
uninitialized variable. It caused by improper use of filed mask and
FIELD_PREP()/FIELD_GET(). Meanwhile give a further fix based patch v2
and make patches to optimize codes and clean up useless codes for a
good readability.
- Patch v2 [6/7] is split into three minimal fix patches:
iio: imu: fxos8700: fix incorrect ODR mode readback
iio: imu: fxos8700: improve readability by field mask and regmap_write
iio: imu: fxos8700: fix failed initialization ODR value assignment
- A new clean fix is proposed:
iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN
- A further fix based on PATCH v2 [7/7]:
iio: imu: fxos8700: fix MAGN sensor scale and unit
Carlos Song (5):
iio: imu: fxos8700: fix incorrect ODR mode readback
iio: imu: fxos8700: improve readability by field mask and regmap_write
iio: imu: fxos8700: fix failed initialization ODR mode assignment
iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN
iio: imu: fxos8700: fix MAGN sensor scale and unit
drivers/iio/imu/fxos8700_core.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
@ 2022-12-14 3:14 ` carlos.song
2022-12-23 16:55 ` Jonathan Cameron
2022-12-14 3:15 ` [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write carlos.song
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:14 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
The absence of a correct offset leads an incorrect ODR mode
readback after use a hexadecimal number to mark the value from
FXOS8700_CTRL_REG1.
Get ODR mode by field mask and FIELD_GET clearly and conveniently.
Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Changes for V3:
- Rework commit log
diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index 773f62203bf0..83ab7d0f79b3 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -10,6 +10,7 @@
#include <linux/regmap.h>
#include <linux/acpi.h>
#include <linux/bitops.h>
+#include <linux/bitfield.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -147,6 +148,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_GENMSK GENMASK(5, 3)
/* Bit definitions for FXOS8700_M_CTRL_REG1 */
#define FXOS8700_HMS_MASK GENMASK(1, 0)
@@ -524,7 +526,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
if (ret)
return ret;
- val &= FXOS8700_CTRL_ODR_MSK;
+ val = FIELD_GET(FXOS8700_CTRL_ODR_GENMSK, val);
for (i = 0; i < odr_num; i++)
if (val == fxos8700_odr[i].bits)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
2022-12-14 3:14 ` [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback carlos.song
@ 2022-12-14 3:15 ` carlos.song
2022-12-23 16:59 ` Jonathan Cameron
2022-12-14 3:15 ` [PATCH v3 3/5] iio: imu: fxos8700: fix failed initialization ODR mode assignment carlos.song
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:15 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
FXOS8700_CTRL_ODR_MSK is a hex digit mask and FXOS8700_CTRL_ODR_GENMSK
is a field mask. They have a similar function. And mixing regmap_write
and regmap_update_bits isn't good for readability.
Remove FXOS8700_CTRL_ODR_GENMSK and set FXOS8700_CTRL_ODR_MSK a field
mask definition with a synchronous change. Use regmap_write() instead
of regmap_update_bits() to update bits. They are good for readability.
Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Changes for V3:
- Remove FXOS8700_CTRL_ODR_GENMSK and set FXOS8700_CTRL_ODR_MSK a
field mask.
- Legal use of filed mask and FIELD_PREP() to select ODR mode
- Rework commit log
diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index 83ab7d0f79b3..a1af5d0fde5d 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -145,10 +145,9 @@
#define FXOS8700_NVM_DATA_BNK0 0xa7
/* Bit definitions for FXOS8700_CTRL_REG1 */
-#define FXOS8700_CTRL_ODR_MSK 0x38
#define FXOS8700_CTRL_ODR_MAX 0x00
#define FXOS8700_CTRL_ODR_MIN GENMASK(4, 3)
-#define FXOS8700_CTRL_ODR_GENMSK GENMASK(5, 3)
+#define FXOS8700_CTRL_ODR_MSK GENMASK(5, 3)
/* Bit definitions for FXOS8700_M_CTRL_REG1 */
#define FXOS8700_HMS_MASK GENMASK(1, 0)
@@ -510,10 +509,8 @@ static int fxos8700_set_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
if (i >= odr_num)
return -EINVAL;
- return regmap_update_bits(data->regmap,
- FXOS8700_CTRL_REG1,
- FXOS8700_CTRL_ODR_MSK + FXOS8700_ACTIVE,
- fxos8700_odr[i].bits << 3 | active_mode);
+ val = val | FIELD_PREP(FXOS8700_CTRL_ODR_MSK, fxos8700_odr[i].bits) | active_mode;
+ return regmap_write(data->regmap, FXOS8700_CTRL_REG1, val);
}
static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
@@ -526,7 +523,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
if (ret)
return ret;
- val = FIELD_GET(FXOS8700_CTRL_ODR_GENMSK, val);
+ val = FIELD_GET(FXOS8700_CTRL_ODR_MSK, val);
for (i = 0; i < odr_num; i++)
if (val == fxos8700_odr[i].bits)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/5] iio: imu: fxos8700: fix failed initialization ODR mode assignment
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
2022-12-14 3:14 ` [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback carlos.song
2022-12-14 3:15 ` [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write carlos.song
@ 2022-12-14 3:15 ` carlos.song
2022-12-14 3:15 ` [PATCH v3 4/5] iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN carlos.song
2022-12-14 3:15 ` [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit carlos.song
4 siblings, 0 replies; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:15 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
The absence of correct offset leads a failed initialization ODR mode
assignment.
Select MAX ODR mode as the initialization ODR mode by field mask and
FIELD_PREP.
Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Changes for V3:
- Legal use of FIELD_PREP() and field mask to select initialization
ODR mode
- Rework commit log
diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index a1af5d0fde5d..de4ced979226 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -611,6 +611,7 @@ static const struct iio_info fxos8700_info = {
static int fxos8700_chip_init(struct fxos8700_data *data, bool use_spi)
{
int ret;
+ int reg;
unsigned int val;
struct device *dev = regmap_get_device(data->regmap);
@@ -663,8 +664,11 @@ static int fxos8700_chip_init(struct fxos8700_data *data, bool use_spi)
return ret;
/* Max ODR (800Hz individual or 400Hz hybrid), active mode */
- return regmap_write(data->regmap, FXOS8700_CTRL_REG1,
- FXOS8700_CTRL_ODR_MAX | FXOS8700_ACTIVE);
+ ret = regmap_read(data->regmap, FXOS8700_CTRL_REG1, ®);
+ if (ret)
+ return ret;
+ reg = reg | FIELD_PREP(FXOS8700_CTRL_ODR_MSK, FXOS8700_CTRL_ODR_MAX) | FXOS8700_ACTIVE;
+ return regmap_write(data->regmap, FXOS8700_CTRL_REG1, reg);
}
static void fxos8700_chip_uninit(void *data)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/5] iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
` (2 preceding siblings ...)
2022-12-14 3:15 ` [PATCH v3 3/5] iio: imu: fxos8700: fix failed initialization ODR mode assignment carlos.song
@ 2022-12-14 3:15 ` carlos.song
2022-12-14 3:15 ` [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit carlos.song
4 siblings, 0 replies; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:15 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
FXOS8700_CTRL_ODR_MIN is not used but value is probably wrong.
Remove it for a good readability.
Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Changes for V3:
- Proposed a separate clean fix
diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index de4ced979226..7b370bd643a1 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -146,7 +146,6 @@
/* Bit definitions for FXOS8700_CTRL_REG1 */
#define FXOS8700_CTRL_ODR_MAX 0x00
-#define FXOS8700_CTRL_ODR_MIN GENMASK(4, 3)
#define FXOS8700_CTRL_ODR_MSK GENMASK(5, 3)
/* Bit definitions for FXOS8700_M_CTRL_REG1 */
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
` (3 preceding siblings ...)
2022-12-14 3:15 ` [PATCH v3 4/5] iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN carlos.song
@ 2022-12-14 3:15 ` carlos.song
2022-12-23 17:01 ` Jonathan Cameron
4 siblings, 1 reply; 10+ messages in thread
From: carlos.song @ 2022-12-14 3:15 UTC (permalink / raw)
To: jic23, lars
Cc: rjones, Jonathan.Cameron, haibo.chen, carlos.song, linux-imx, linux-iio
From: Carlos Song <carlos.song@nxp.com>
+/-1200uT is a MAGN sensor full measurement range. Magnetometer scale
is the magnetic sensitivity parameter. It is referenced as 0.1uT
according to datasheet and magnetometer channel unit is Gauss in
sysfs-bus-iio documentation. Gauss and uTesla unit conversion
relationship as follows: 0.1uT = 0.001Gs.
Set magnetometer scale and available magnetometer scale as fixed 0.001Gs.
Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Changes for V2:
- Modify the magnetometer sensitivity unit to be consistent with the
documentation as 0.001g
- Rework commit log
Changes for V3:
- Modify the magnetometer sensitivity unit "g" to standard unit "Gs"
- Check and confirm uscale value is correct. The readback of
MAGN scale is 0.001 Gs
- Rework commit log
diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
index 7b370bd643a1..8320a3b6f942 100644
--- a/drivers/iio/imu/fxos8700_core.c
+++ b/drivers/iio/imu/fxos8700_core.c
@@ -351,7 +351,7 @@ static int fxos8700_set_scale(struct fxos8700_data *data,
struct device *dev = regmap_get_device(data->regmap);
if (t == FXOS8700_MAGN) {
- dev_err(dev, "Magnetometer scale is locked at 1200uT\n");
+ dev_err(dev, "Magnetometer scale is locked at 0.001Gs\n");
return -EINVAL;
}
@@ -396,7 +396,7 @@ static int fxos8700_get_scale(struct fxos8700_data *data,
static const int scale_num = ARRAY_SIZE(fxos8700_accel_scale);
if (t == FXOS8700_MAGN) {
- *uscale = 1200; /* Magnetometer is locked at 1200uT */
+ *uscale = 1000; /* Magnetometer is locked at 0.001Gs */
return 0;
}
@@ -587,7 +587,7 @@ static IIO_CONST_ATTR(in_accel_sampling_frequency_available,
static IIO_CONST_ATTR(in_magn_sampling_frequency_available,
"1.5625 6.25 12.5 50 100 200 400 800");
static IIO_CONST_ATTR(in_accel_scale_available, "0.000244 0.000488 0.000976");
-static IIO_CONST_ATTR(in_magn_scale_available, "0.000001200");
+static IIO_CONST_ATTR(in_magn_scale_available, "0.001000");
static struct attribute *fxos8700_attrs[] = {
&iio_const_attr_in_accel_sampling_frequency_available.dev_attr.attr,
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback
2022-12-14 3:14 ` [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback carlos.song
@ 2022-12-23 16:55 ` Jonathan Cameron
2022-12-23 16:58 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2022-12-23 16:55 UTC (permalink / raw)
To: carlos.song
Cc: lars, rjones, Jonathan.Cameron, haibo.chen, linux-imx, linux-iio
On Wed, 14 Dec 2022 11:14:59 +0800
carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> The absence of a correct offset leads an incorrect ODR mode
> readback after use a hexadecimal number to mark the value from
> FXOS8700_CTRL_REG1.
>
> Get ODR mode by field mask and FIELD_GET clearly and conveniently.
>
> Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
>
You need a
---
above the change log to avoid git picking it up when I apply the patch.
I've fixed that up whilst applying.
Applied to the fixes-togreg branch of iio.git and marked for stable.
> Changes for V3:
> - Rework commit log
>
> diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
> index 773f62203bf0..83ab7d0f79b3 100644
> --- a/drivers/iio/imu/fxos8700_core.c
> +++ b/drivers/iio/imu/fxos8700_core.c
> @@ -10,6 +10,7 @@
> #include <linux/regmap.h>
> #include <linux/acpi.h>
> #include <linux/bitops.h>
> +#include <linux/bitfield.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> @@ -147,6 +148,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_GENMSK GENMASK(5, 3)
>
> /* Bit definitions for FXOS8700_M_CTRL_REG1 */
> #define FXOS8700_HMS_MASK GENMASK(1, 0)
> @@ -524,7 +526,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
> if (ret)
> return ret;
>
> - val &= FXOS8700_CTRL_ODR_MSK;
> + val = FIELD_GET(FXOS8700_CTRL_ODR_GENMSK, val);
>
> for (i = 0; i < odr_num; i++)
> if (val == fxos8700_odr[i].bits)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback
2022-12-23 16:55 ` Jonathan Cameron
@ 2022-12-23 16:58 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2022-12-23 16:58 UTC (permalink / raw)
To: carlos.song
Cc: lars, rjones, Jonathan.Cameron, haibo.chen, linux-imx, linux-iio
On Fri, 23 Dec 2022 16:55:29 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 14 Dec 2022 11:14:59 +0800
> carlos.song@nxp.com wrote:
>
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > The absence of a correct offset leads an incorrect ODR mode
> > readback after use a hexadecimal number to mark the value from
> > FXOS8700_CTRL_REG1.
> >
> > Get ODR mode by field mask and FIELD_GET clearly and conveniently.
> >
> > Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> >
> You need a
> ---
>
> above the change log to avoid git picking it up when I apply the patch.
> I've fixed that up whilst applying.
>
> Applied to the fixes-togreg branch of iio.git and marked for stable.
Dropped again because this makes no sense wrt to split of what is
going on in patch 2.
>
> > Changes for V3:
> > - Rework commit log
> >
> > diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
> > index 773f62203bf0..83ab7d0f79b3 100644
> > --- a/drivers/iio/imu/fxos8700_core.c
> > +++ b/drivers/iio/imu/fxos8700_core.c
> > @@ -10,6 +10,7 @@
> > #include <linux/regmap.h>
> > #include <linux/acpi.h>
> > #include <linux/bitops.h>
> > +#include <linux/bitfield.h>
> >
> > #include <linux/iio/iio.h>
> > #include <linux/iio/sysfs.h>
> > @@ -147,6 +148,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_GENMSK GENMASK(5, 3)
> >
> > /* Bit definitions for FXOS8700_M_CTRL_REG1 */
> > #define FXOS8700_HMS_MASK GENMASK(1, 0)
> > @@ -524,7 +526,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
> > if (ret)
> > return ret;
> >
> > - val &= FXOS8700_CTRL_ODR_MSK;
> > + val = FIELD_GET(FXOS8700_CTRL_ODR_GENMSK, val);
> >
> > for (i = 0; i < odr_num; i++)
> > if (val == fxos8700_odr[i].bits)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write
2022-12-14 3:15 ` [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write carlos.song
@ 2022-12-23 16:59 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2022-12-23 16:59 UTC (permalink / raw)
To: carlos.song
Cc: lars, rjones, Jonathan.Cameron, haibo.chen, linux-imx, linux-iio
On Wed, 14 Dec 2022 11:15:00 +0800
carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> FXOS8700_CTRL_ODR_MSK is a hex digit mask and FXOS8700_CTRL_ODR_GENMSK
> is a field mask. They have a similar function. And mixing regmap_write
> and regmap_update_bits isn't good for readability.
>
> Remove FXOS8700_CTRL_ODR_GENMSK and set FXOS8700_CTRL_ODR_MSK a field
> mask definition with a synchronous change. Use regmap_write() instead
> of regmap_update_bits() to update bits. They are good for readability.
>
> Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
Needs a comment on why this deserves a fixes tag.
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
>
---
needed here.
> Changes for V3:
> - Remove FXOS8700_CTRL_ODR_GENMSK and set FXOS8700_CTRL_ODR_MSK a
> field mask.
> - Legal use of filed mask and FIELD_PREP() to select ODR mode
> - Rework commit log
>
> diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
> index 83ab7d0f79b3..a1af5d0fde5d 100644
> --- a/drivers/iio/imu/fxos8700_core.c
> +++ b/drivers/iio/imu/fxos8700_core.c
> @@ -145,10 +145,9 @@
> #define FXOS8700_NVM_DATA_BNK0 0xa7
>
> /* Bit definitions for FXOS8700_CTRL_REG1 */
> -#define FXOS8700_CTRL_ODR_MSK 0x38
> #define FXOS8700_CTRL_ODR_MAX 0x00
> #define FXOS8700_CTRL_ODR_MIN GENMASK(4, 3)
> -#define FXOS8700_CTRL_ODR_GENMSK GENMASK(5, 3)
> +#define FXOS8700_CTRL_ODR_MSK GENMASK(5, 3)
Why jump through this loop in patch 1 and then this? Just use
ODR_MSK in that fix in the first place. Doesn't matter for
purposes of a fix that it is in a less than ideal form.
>
> /* Bit definitions for FXOS8700_M_CTRL_REG1 */
> #define FXOS8700_HMS_MASK GENMASK(1, 0)
> @@ -510,10 +509,8 @@ static int fxos8700_set_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
> if (i >= odr_num)
> return -EINVAL;
>
> - return regmap_update_bits(data->regmap,
> - FXOS8700_CTRL_REG1,
> - FXOS8700_CTRL_ODR_MSK + FXOS8700_ACTIVE,
> - fxos8700_odr[i].bits << 3 | active_mode);
> + val = val | FIELD_PREP(FXOS8700_CTRL_ODR_MSK, fxos8700_odr[i].bits) | active_mode;
> + return regmap_write(data->regmap, FXOS8700_CTRL_REG1, val);
> }
>
> static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
> @@ -526,7 +523,7 @@ static int fxos8700_get_odr(struct fxos8700_data *data, enum fxos8700_sensor t,
> if (ret)
> return ret;
>
> - val = FIELD_GET(FXOS8700_CTRL_ODR_GENMSK, val);
> + val = FIELD_GET(FXOS8700_CTRL_ODR_MSK, val);
>
> for (i = 0; i < odr_num; i++)
> if (val == fxos8700_odr[i].bits)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit
2022-12-14 3:15 ` [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit carlos.song
@ 2022-12-23 17:01 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2022-12-23 17:01 UTC (permalink / raw)
To: carlos.song
Cc: lars, rjones, Jonathan.Cameron, haibo.chen, linux-imx, linux-iio
On Wed, 14 Dec 2022 11:15:03 +0800
carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> +/-1200uT is a MAGN sensor full measurement range. Magnetometer scale
> is the magnetic sensitivity parameter. It is referenced as 0.1uT
> according to datasheet and magnetometer channel unit is Gauss in
> sysfs-bus-iio documentation. Gauss and uTesla unit conversion
> relationship as follows: 0.1uT = 0.001Gs.
>
> Set magnetometer scale and available magnetometer scale as fixed 0.001Gs.
>
> Fixes: 84e5ddd5c46e ("iio: imu: Add support for the FXOS8700 IMU")
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
>
---
as for all the other patches.
3,4,5 look fine to me other than that. So I'll pick them up once
the patch 1/2 split is sorted out.
Jonathan
> Changes for V2:
> - Modify the magnetometer sensitivity unit to be consistent with the
> documentation as 0.001g
> - Rework commit log
> Changes for V3:
> - Modify the magnetometer sensitivity unit "g" to standard unit "Gs"
> - Check and confirm uscale value is correct. The readback of
> MAGN scale is 0.001 Gs
> - Rework commit log
>
> diff --git a/drivers/iio/imu/fxos8700_core.c b/drivers/iio/imu/fxos8700_core.c
> index 7b370bd643a1..8320a3b6f942 100644
> --- a/drivers/iio/imu/fxos8700_core.c
> +++ b/drivers/iio/imu/fxos8700_core.c
> @@ -351,7 +351,7 @@ static int fxos8700_set_scale(struct fxos8700_data *data,
> struct device *dev = regmap_get_device(data->regmap);
>
> if (t == FXOS8700_MAGN) {
> - dev_err(dev, "Magnetometer scale is locked at 1200uT\n");
> + dev_err(dev, "Magnetometer scale is locked at 0.001Gs\n");
> return -EINVAL;
> }
>
> @@ -396,7 +396,7 @@ static int fxos8700_get_scale(struct fxos8700_data *data,
> static const int scale_num = ARRAY_SIZE(fxos8700_accel_scale);
>
> if (t == FXOS8700_MAGN) {
> - *uscale = 1200; /* Magnetometer is locked at 1200uT */
> + *uscale = 1000; /* Magnetometer is locked at 0.001Gs */
> return 0;
> }
>
> @@ -587,7 +587,7 @@ static IIO_CONST_ATTR(in_accel_sampling_frequency_available,
> static IIO_CONST_ATTR(in_magn_sampling_frequency_available,
> "1.5625 6.25 12.5 50 100 200 400 800");
> static IIO_CONST_ATTR(in_accel_scale_available, "0.000244 0.000488 0.000976");
> -static IIO_CONST_ATTR(in_magn_scale_available, "0.000001200");
> +static IIO_CONST_ATTR(in_magn_scale_available, "0.001000");
>
> static struct attribute *fxos8700_attrs[] = {
> &iio_const_attr_in_accel_sampling_frequency_available.dev_attr.attr,
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-12-23 16:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-14 3:14 [PATCH v3 0/5] iio: imu: fxos8700: fix bugs about ODR and changes for a good readability carlos.song
2022-12-14 3:14 ` [PATCH v3 1/5] iio: imu: fxos8700: fix incorrect ODR mode readback carlos.song
2022-12-23 16:55 ` Jonathan Cameron
2022-12-23 16:58 ` Jonathan Cameron
2022-12-14 3:15 ` [PATCH v3 2/5] iio: imu: fxos8700: improve readability by field mask and regmap_write carlos.song
2022-12-23 16:59 ` Jonathan Cameron
2022-12-14 3:15 ` [PATCH v3 3/5] iio: imu: fxos8700: fix failed initialization ODR mode assignment carlos.song
2022-12-14 3:15 ` [PATCH v3 4/5] iio: imu: fxos8700: remove definition FXOS8700_CTRL_ODR_MIN carlos.song
2022-12-14 3:15 ` [PATCH v3 5/5] iio: imu: fxos8700: fix MAGN sensor scale and unit carlos.song
2022-12-23 17:01 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.