* [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 17:11 ` Conor Dooley
2026-10-02 11:54 ` [PATCH v5 2/6] iio: imu: inv_icm42607: Simplify IIO channel macros Kanak Shilledar
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
TDK Invensense, ICM-42670-P is a 3-axis accelerometer. The device can
support I2C, SPI and I3C. This device has very similar properties as
that of the icm42607/p. Add this device to the existing dt-binding and
update the description to match the devices.
Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
index 9e22b603d47fb..c79eb0a1a890e 100644
--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
@@ -11,10 +11,10 @@ maintainers:
description: |
6-axis MotionTracking device that combines a 3-axis gyroscope and a 3-axis
- accelerometer.
+ accelerometer. Some devices only have a 3-axis accelerometer.
It has a configurable host interface that supports I3C, I2C and SPI serial
- communication, features a 2kB FIFO and 2 programmable interrupts with
+ communication, features up to 2.25kB FIFO and 2 programmable interrupts with
ultra-low-power wake-on-motion support to minimize system power consumption.
Other industry-leading features include InvenSense on-chip APEX Motion
@@ -28,6 +28,7 @@ properties:
compatible:
oneOf:
- enum:
+ - invensense,icm42370p
- invensense,icm42600
- invensense,icm42602
- invensense,icm42605
@@ -81,6 +82,7 @@ allOf:
compatible:
contains:
enum:
+ - invensense,icm42370p
- invensense,icm42600
- invensense,icm42602
- invensense,icm42605
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P
2026-10-02 11:54 ` [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P Kanak Shilledar
@ 2026-10-02 17:11 ` Conor Dooley
2026-10-02 17:11 ` Conor Dooley
0 siblings, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2026-10-02 17:11 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]
On Fri, Oct 02, 2026 at 01:54:25PM +0200, Kanak Shilledar wrote:
> TDK Invensense, ICM-42670-P is a 3-axis accelerometer. The device can
> support I2C, SPI and I3C. This device has very similar properties as
> that of the icm42607/p. Add this device to the existing dt-binding and
> update the description to match the devices.
Reading this it's not clear to me why a fallback cannot be used.
You say "similar properties", but what I see below is slotting into an
existing enum, without differing restrictions.
Thanks,
Conor.
>
> Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
> Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
> ---
> Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 9e22b603d47fb..c79eb0a1a890e 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -11,10 +11,10 @@ maintainers:
>
> description: |
> 6-axis MotionTracking device that combines a 3-axis gyroscope and a 3-axis
> - accelerometer.
> + accelerometer. Some devices only have a 3-axis accelerometer.
>
> It has a configurable host interface that supports I3C, I2C and SPI serial
> - communication, features a 2kB FIFO and 2 programmable interrupts with
> + communication, features up to 2.25kB FIFO and 2 programmable interrupts with
> ultra-low-power wake-on-motion support to minimize system power consumption.
>
> Other industry-leading features include InvenSense on-chip APEX Motion
> @@ -28,6 +28,7 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - invensense,icm42370p
> - invensense,icm42600
> - invensense,icm42602
> - invensense,icm42605
> @@ -81,6 +82,7 @@ allOf:
> compatible:
> contains:
> enum:
> + - invensense,icm42370p
> - invensense,icm42600
> - invensense,icm42602
> - invensense,icm42605
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P
2026-10-02 17:11 ` Conor Dooley
@ 2026-10-02 17:11 ` Conor Dooley
0 siblings, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2026-10-02 17:11 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2616 bytes --]
On Fri, Oct 02, 2026 at 06:11:00PM +0100, Conor Dooley wrote:
> On Fri, Oct 02, 2026 at 01:54:25PM +0200, Kanak Shilledar wrote:
> > TDK Invensense, ICM-42670-P is a 3-axis accelerometer. The device can
> > support I2C, SPI and I3C. This device has very similar properties as
> > that of the icm42607/p. Add this device to the existing dt-binding and
> > update the description to match the devices.
>
> Reading this it's not clear to me why a fallback cannot be used.
>
> You say "similar properties", but what I see below is slotting into an
> existing enum, without differing restrictions.
The lack of a gyroscope is probably why? Please note that here.
pw-bot: changes-requested
>
> Thanks,
> Conor.
>
> >
> > Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
> > Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
> > ---
> > Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> > index 9e22b603d47fb..c79eb0a1a890e 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> > +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> > @@ -11,10 +11,10 @@ maintainers:
> >
> > description: |
> > 6-axis MotionTracking device that combines a 3-axis gyroscope and a 3-axis
> > - accelerometer.
> > + accelerometer. Some devices only have a 3-axis accelerometer.
> >
> > It has a configurable host interface that supports I3C, I2C and SPI serial
> > - communication, features a 2kB FIFO and 2 programmable interrupts with
> > + communication, features up to 2.25kB FIFO and 2 programmable interrupts with
> > ultra-low-power wake-on-motion support to minimize system power consumption.
> >
> > Other industry-leading features include InvenSense on-chip APEX Motion
> > @@ -28,6 +28,7 @@ properties:
> > compatible:
> > oneOf:
> > - enum:
> > + - invensense,icm42370p
> > - invensense,icm42600
> > - invensense,icm42602
> > - invensense,icm42605
> > @@ -81,6 +82,7 @@ allOf:
> > compatible:
> > contains:
> > enum:
> > + - invensense,icm42370p
> > - invensense,icm42600
> > - invensense,icm42602
> > - invensense,icm42605
> >
> > --
> > 2.43.0
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 2/6] iio: imu: inv_icm42607: Simplify IIO channel macros
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
2026-10-02 11:54 ` [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 11:54 ` [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info Kanak Shilledar
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
The INV_ICM42607_ACCEL_CHAN and INV_ICM42607_GYRO_CHAN macro had a third
parameter of _ext_info, drop it and just point the .ext_info field to
the respective inv_icm42607_*_ext_infos. This reduces the repetitive
calling of inv_icm42607_*_ext_infos struct in the IIO channel spec
struct and makes the code more easy to follow.
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c | 13 +++++--------
drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c | 13 +++++--------
2 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
index 0b3f035c2da0c..9a3ace3e7fcf9 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
@@ -17,7 +17,7 @@
#include "inv_icm42607.h"
#include "inv_icm42607_temp.h"
-#define INV_ICM42607_ACCEL_CHAN(_modifier, _index, _ext_info) \
+#define INV_ICM42607_ACCEL_CHAN(_modifier, _index) \
{ \
.type = IIO_ACCEL, \
.modified = 1, \
@@ -34,7 +34,7 @@
.storagebits = 16, \
.endianness = IIO_BE, \
}, \
- .ext_info = _ext_info, \
+ .ext_info = inv_icm42607_accel_ext_infos, \
}
enum inv_icm42607_accel_scan {
@@ -50,12 +50,9 @@ static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = {
};
static const struct iio_chan_spec inv_icm42607_accel_channels[] = {
- INV_ICM42607_ACCEL_CHAN(IIO_MOD_X, INV_ICM42607_ACCEL_SCAN_X,
- inv_icm42607_accel_ext_infos),
- INV_ICM42607_ACCEL_CHAN(IIO_MOD_Y, INV_ICM42607_ACCEL_SCAN_Y,
- inv_icm42607_accel_ext_infos),
- INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z,
- inv_icm42607_accel_ext_infos),
+ INV_ICM42607_ACCEL_CHAN(IIO_MOD_X, INV_ICM42607_ACCEL_SCAN_X),
+ INV_ICM42607_ACCEL_CHAN(IIO_MOD_Y, INV_ICM42607_ACCEL_SCAN_Y),
+ INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z),
INV_ICM42607_TEMP_CHAN(INV_ICM42607_ACCEL_SCAN_TEMP),
};
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
index 5b4683c2dd1e4..a3bad6fbc1981 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
@@ -17,7 +17,7 @@
#include "inv_icm42607.h"
#include "inv_icm42607_temp.h"
-#define INV_ICM42607_GYRO_CHAN(_modifier, _index, _ext_info) \
+#define INV_ICM42607_GYRO_CHAN(_modifier, _index) \
{ \
.type = IIO_ANGL_VEL, \
.modified = 1, \
@@ -34,7 +34,7 @@
.storagebits = 16, \
.endianness = IIO_BE, \
}, \
- .ext_info = _ext_info, \
+ .ext_info = inv_icm42607_gyro_ext_infos, \
}
enum inv_icm42607_gyro_scan {
@@ -50,12 +50,9 @@ static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
};
static const struct iio_chan_spec inv_icm42607_gyro_channels[] = {
- INV_ICM42607_GYRO_CHAN(IIO_MOD_X, INV_ICM42607_GYRO_SCAN_X,
- inv_icm42607_gyro_ext_infos),
- INV_ICM42607_GYRO_CHAN(IIO_MOD_Y, INV_ICM42607_GYRO_SCAN_Y,
- inv_icm42607_gyro_ext_infos),
- INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z,
- inv_icm42607_gyro_ext_infos),
+ INV_ICM42607_GYRO_CHAN(IIO_MOD_X, INV_ICM42607_GYRO_SCAN_X),
+ INV_ICM42607_GYRO_CHAN(IIO_MOD_Y, INV_ICM42607_GYRO_SCAN_Y),
+ INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z),
INV_ICM42607_TEMP_CHAN(INV_ICM42607_GYRO_SCAN_TEMP),
};
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
2026-10-02 11:54 ` [PATCH v5 1/6] dt-bindings: iio: imu: icm42600: Add ICM-42670-P Kanak Shilledar
2026-10-02 11:54 ` [PATCH v5 2/6] iio: imu: inv_icm42607: Simplify IIO channel macros Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 13:03 ` Andy Shevchenko
2026-10-02 11:54 ` [PATCH v5 4/6] iio: imu: inv_icm42607: Add support for ICM-42370-P Kanak Shilledar
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
Update the chip_info struct with a new `has_gyro` property to support,
devices which do not have gyro functionality. This is a precursor to the
next commit which adds support for the Invensense, ICM-42370-P. It is
similar to the existing device except it only has accelerometer. Check
all operations related to gyro with the boolean.
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
drivers/iio/imu/inv_icm42607/inv_icm42607.h | 1 +
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 41 +++++++++++++++---------
2 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
index 4d51b0da1aa16..e4075288247b8 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
@@ -130,6 +130,7 @@ struct inv_icm42607_hw {
const char *name;
const struct inv_icm42607_conf *conf;
u8 whoami;
+ bool has_gyro;
};
/**
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 190e998f7b8ef..d974cba66e1be 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -96,6 +96,7 @@ const struct inv_icm42607_hw inv_icm42607_hw_data = {
.whoami = INV_ICM42607_WHOAMI,
.name = "icm42607",
.conf = &inv_icm42607_default_conf,
+ .has_gyro = true,
};
EXPORT_SYMBOL_NS_GPL(inv_icm42607_hw_data, "IIO_ICM42607");
@@ -103,6 +104,7 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
.whoami = INV_ICM42607P_WHOAMI,
.name = "icm42607p",
.conf = &inv_icm42607_default_conf,
+ .has_gyro = true,
};
EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
@@ -417,17 +419,21 @@ static int inv_icm42607_set_init_conf(struct inv_icm42607_state *st,
unsigned int val;
int ret;
- val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, conf->gyro.mode);
- val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode);
+ val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode);
+ if (st->hw->has_gyro)
+ val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, conf->gyro.mode);
+
ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
if (ret)
return ret;
- val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK, conf->gyro.fs);
- val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK, conf->gyro.odr);
- ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
- if (ret)
- return ret;
+ if (st->hw->has_gyro) {
+ val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK, conf->gyro.fs);
+ val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK, conf->gyro.odr);
+ ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+ if (ret)
+ return ret;
+ }
val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG0_FS_SEL_MASK, conf->accel.fs);
val |= FIELD_PREP(INV_ICM42607_ACCEL_CONFIG0_ODR_MASK, conf->accel.odr);
@@ -435,11 +441,13 @@ static int inv_icm42607_set_init_conf(struct inv_icm42607_state *st,
if (ret)
return ret;
- val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, conf->gyro.filter);
- ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
- INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
- if (ret)
- return ret;
+ if (st->hw->has_gyro) {
+ val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, conf->gyro.filter);
+ ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+ INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+ if (ret)
+ return ret;
+ }
val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK, conf->accel.filter);
ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
@@ -635,10 +643,11 @@ int inv_icm42607_core_probe(struct regmap *regmap,
if (IS_ERR(st->indio_accel))
return PTR_ERR(st->indio_accel);
- /* Initialize IIO device for Gyro */
- st->indio_gyro = inv_icm42607_gyro_init(st);
- if (IS_ERR(st->indio_gyro))
- return PTR_ERR(st->indio_gyro);
+ if (st->hw->has_gyro) {
+ st->indio_gyro = inv_icm42607_gyro_init(st);
+ if (IS_ERR(st->indio_gyro))
+ return PTR_ERR(st->indio_gyro);
+ }
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info
2026-10-02 11:54 ` [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info Kanak Shilledar
@ 2026-10-02 13:03 ` Andy Shevchenko
0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2026-10-02 13:03 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
On Fri, Oct 02, 2026 at 01:54:27PM +0200, Kanak Shilledar wrote:
> Update the chip_info struct with a new `has_gyro` property to support,
> devices which do not have gyro functionality. This is a precursor to the
> next commit which adds support for the Invensense, ICM-42370-P. It is
> similar to the existing device except it only has accelerometer. Check
> all operations related to gyro with the boolean.
...
> int ret;
>
> - val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, conf->gyro.mode);
> - val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode);
> + val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode);
> + if (st->hw->has_gyro)
> + val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, conf->gyro.mode);
Personally I found if-else slightly better as one needs no detour to understand
the whole value.
if (st->hw->has_gyro)
val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode) |
FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, conf->gyro.mode);
else
val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, conf->accel.mode);
> ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> if (ret)
> return ret;
...
> + if (st->hw->has_gyro) {
> + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK, conf->gyro.fs);
> + val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK, conf->gyro.odr);
No need to have two assignments, do it at once.
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 4/6] iio: imu: inv_icm42607: Add support for ICM-42370-P
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
` (2 preceding siblings ...)
2026-10-02 11:54 ` [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 13:04 ` Andy Shevchenko
2026-10-02 11:54 ` [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access Kanak Shilledar
2026-10-02 11:54 ` [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support Kanak Shilledar
5 siblings, 1 reply; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
Add support for the Invensense ICM-42370-P MEMS MotionTracking 3-axis
accelerometer with built-in temperature sensor. This device is almost
identical to the existing Invensense ICM-42607-P IMU, but lacks
gyroscope. The device supports I2C, SPI and I3C. Implement only I2C
support for now, and provide support for raw sensor reads via sysfs.
There is also a built-in temperature sensor but it can not be turned off
similar to the ICM-42607.
While at it, change so dev_err_probe() is returned
when the WHO_AM_I read fails.
Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
drivers/iio/imu/inv_icm42607/inv_icm42607.h | 2 ++
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 24 ++++++++++++++++++++++--
drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c | 6 ++++++
3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
index e4075288247b8..ca5f59eb436c0 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
@@ -367,6 +367,7 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_REG_FIFO_DATA 0x3F
#define INV_ICM42607_REG_WHOAMI 0x75
+#define INV_ICM42370P_WHOAMI 0x0D
#define INV_ICM42607P_WHOAMI 0x60
#define INV_ICM42607_WHOAMI 0x67
@@ -393,6 +394,7 @@ typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
extern const struct regmap_config inv_icm42607_regmap_config;
extern const struct inv_icm42607_hw inv_icm42607_hw_data;
extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
+extern const struct inv_icm42607_hw inv_icm42370p_hw_data;
extern const struct dev_pm_ops inv_icm42607_pm_ops;
const struct iio_mount_matrix *
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index d974cba66e1be..87b1499a3689f 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -92,6 +92,22 @@ static const struct inv_icm42607_conf inv_icm42607_default_conf = {
},
};
+static const struct inv_icm42607_conf inv_icm42370_default_conf = {
+ .accel = {
+ .mode = INV_ICM42607_SENSOR_MODE_OFF,
+ .fs = INV_ICM42607_ACCEL_FS_4G,
+ .odr = INV_ICM42607_ODR_100HZ,
+ .filter = INV_ICM42607_FILTER_BW_25HZ,
+ },
+};
+
+const struct inv_icm42607_hw inv_icm42370p_hw_data = {
+ .whoami = INV_ICM42370P_WHOAMI,
+ .name = "icm42370p",
+ .conf = &inv_icm42370_default_conf,
+};
+EXPORT_SYMBOL_NS_GPL(inv_icm42370p_hw_data, "IIO_ICM42607");
+
const struct inv_icm42607_hw inv_icm42607_hw_data = {
.whoami = INV_ICM42607_WHOAMI,
.name = "icm42607",
@@ -476,7 +492,7 @@ static int inv_icm42607_setup(struct inv_icm42607_state *st,
ret = regmap_read(st->map, INV_ICM42607_REG_WHOAMI, &val);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "WHOAMI read failed\n");
/* Warn, but don't fail. */
if (val != st->hw->whoami)
@@ -638,7 +654,11 @@ int inv_icm42607_core_probe(struct regmap *regmap,
pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(dev);
- /* Initialize IIO device for Accel */
+ /*
+ * Invensense, ICM42607 and ICM42607P both have accelerometer
+ * and gyroscope functionality. Whereas, Invensense, ICM42370P
+ * has only accelerometer.
+ */
st->indio_accel = inv_icm42607_accel_init(st);
if (IS_ERR(st->indio_accel))
return PTR_ERR(st->indio_accel);
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
index e903106af84a8..cfafba2e3e68e 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
@@ -63,6 +63,9 @@ static const struct i2c_device_id inv_icm42607_id[] = {
}, {
.name = "icm42607p",
.driver_data = (kernel_ulong_t)&inv_icm42607p_hw_data,
+ }, {
+ .name = "icm42370p",
+ .driver_data = (kernel_ulong_t)&inv_icm42370p_hw_data,
},
{ }
};
@@ -75,6 +78,9 @@ static const struct of_device_id inv_icm42607_of_matches[] = {
}, {
.compatible = "invensense,icm42607p",
.data = &inv_icm42607p_hw_data,
+ }, {
+ .compatible = "invensense,icm42370p",
+ .data = &inv_icm42370p_hw_data,
},
{ }
};
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 4/6] iio: imu: inv_icm42607: Add support for ICM-42370-P
2026-10-02 11:54 ` [PATCH v5 4/6] iio: imu: inv_icm42607: Add support for ICM-42370-P Kanak Shilledar
@ 2026-10-02 13:04 ` Andy Shevchenko
0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2026-10-02 13:04 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
On Fri, Oct 02, 2026 at 01:54:28PM +0200, Kanak Shilledar wrote:
> Add support for the Invensense ICM-42370-P MEMS MotionTracking 3-axis
> accelerometer with built-in temperature sensor. This device is almost
> identical to the existing Invensense ICM-42607-P IMU, but lacks
> gyroscope. The device supports I2C, SPI and I3C. Implement only I2C
> support for now, and provide support for raw sensor reads via sysfs.
> There is also a built-in temperature sensor but it can not be turned off
> similar to the ICM-42607.
>
> While at it, change so dev_err_probe() is returned
> when the WHO_AM_I read fails.
...
> }, {
> .name = "icm42607p",
> .driver_data = (kernel_ulong_t)&inv_icm42607p_hw_data,
> + }, {
> + .name = "icm42370p",
> + .driver_data = (kernel_ulong_t)&inv_icm42370p_hw_data,
> },
> { }
...
> }, {
> .compatible = "invensense,icm42607p",
> .data = &inv_icm42607p_hw_data,
> + }, {
> + .compatible = "invensense,icm42370p",
> + .data = &inv_icm42370p_hw_data,
> },
> { }
Keep the ID tables ordered by name/compatible.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
` (3 preceding siblings ...)
2026-10-02 11:54 ` [PATCH v5 4/6] iio: imu: inv_icm42607: Add support for ICM-42370-P Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 13:12 ` Andy Shevchenko
2026-10-02 11:54 ` [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support Kanak Shilledar
5 siblings, 1 reply; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
The device supports indirect register access to different banks. A
specific routine needs to be followed when accessing the registers in
another bank as documented in the datasheet (section 13). This is
required for accessing registers configured via the user and
implementing buffer support. The implementation is inspired from the
icm45600 driver. The banks are defined based on their initial bank
access code which are written to the BLK_SEL_* regs. However, there is a
variation for MREG1 as User Bank 0 and MREG1 both share the same 0x00,
so User Bank 1 has 0x00 and MREG1 has 0x01. When MREG1 register is
accessed, then BLK_SEL_* is written with 0x00 instead of 0x01. All the
register access goes through the 16 bit virtual regmap layered on top of
the 8bit bus regmap. Bank id in the upper byte and the register address
in the lower byte. Bank 0 accesses are forwarded to the bus regmap with
the bank field stripped, while MREG accesses are forwarded to bank
switching sequence.
Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
Datasheet: https://www.lcsc.com/product-detail/C5129967.html
Assisted-by: LLM
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
LLM was used to implement the mreg back switching support.
---
drivers/iio/imu/inv_icm42607/inv_icm42607.h | 167 +++++++++------
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 248 +++++++++++++++++++++--
drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c | 5 +
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c | 5 +
4 files changed, 344 insertions(+), 81 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
index ca5f59eb436c0..896265b9a179f 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
@@ -167,31 +167,59 @@ struct inv_icm42607_sensor_state {
int filter;
};
-/* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
+/*
+ * Virtual register addresses: bank id in the upper byte, register address in
+ * the lower byte. Bank 0 is directly addressable on the bus. MREG banks are
+ * reached through the BLK_SEL/MADDR/M indirect access window, one byte per
+ * transaction (datasheet section 13, no burst support).
+ *
+ * MREG1 programs BLK_SEL = 0x00, which collides with the direct bank, so it
+ * gets the distinct virtual id 0x01. MREG2 and MREG3 virtual ids match their
+ * BLK_SEL values.
+ */
+#define INV_ICM42607_REG_BANK_MASK GENMASK(15, 8)
+#define INV_ICM42607_REG_ADDR_MASK GENMASK(7, 0)
+
+#define INV_ICM42607_BANK0 0x00
+#define INV_ICM42607_MREG1 0x01
+#define INV_ICM42607_MREG2 0x28
+#define INV_ICM42607_MREG3 0x50
+
+/* BLK_SEL value written to the device for each virtual bank id. */
+#define INV_ICM42607_MREG1_BLK_SEL 0x00
+#define INV_ICM42607_MREG2_BLK_SEL 0x28
+#define INV_ICM42607_MREG3_BLK_SEL 0x50
+
+/*
+ * Datasheet section 13, Accessing MREGx Registers: no bus access for 10us
+ * after programming the indirect access window.
+ */
+#define INV_ICM42607_MREG_ACCESS_DELAY_US 10
/* Register Map for User Bank 0 */
-#define INV_ICM42607_REG_MCLK_RDY 0x00
+#define INV_ICM42607_REG_MCLK_RDY 0x0000
+#define INV_ICM42607_MCLK_RDY_BIT BIT(3)
-#define INV_ICM42607_REG_DEVICE_CONFIG 0x01
+#define INV_ICM42607_REG_DEVICE_CONFIG 0x0001
#define INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE BIT(2)
#define INV_ICM42607_DEVICE_CONFIG_SPI_MODE BIT(0)
-#define INV_ICM42607_REG_SIGNAL_PATH_RESET 0x02
+#define INV_ICM42607_REG_SIGNAL_PATH_RESET 0x0002
#define INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET BIT(4)
#define INV_ICM42607_SIGNAL_PATH_RESET_FIFO_FLUSH BIT(2)
-#define INV_ICM42607_REG_DRIVE_CONFIG1 0x03
+#define INV_ICM42607_REG_DRIVE_CONFIG1 0x0003
#define INV_ICM42607_DRIVE_CONFIG1_I3C_DDR_MASK GENMASK(5, 3)
#define INV_ICM42607_DRIVE_CONFIG1_I3C_SDR_MASK GENMASK(2, 0)
-#define INV_ICM42607_REG_DRIVE_CONFIG2 0x04
+#define INV_ICM42607_REG_DRIVE_CONFIG2 0x0004
#define INV_ICM42607_DRIVE_CONFIG2_I2C_MASK GENMASK(5, 3)
#define INV_ICM42607_DRIVE_CONFIG2_ALL_MASK GENMASK(2, 0)
-#define INV_ICM42607_REG_DRIVE_CONFIG3 0x05
+#define INV_ICM42607_REG_DRIVE_CONFIG3 0x0005
#define INV_ICM42607_DRIVE_CONFIG3_SPI_MASK GENMASK(2, 0)
-#define INV_ICM42607_REG_INT_CONFIG 0x06
+#define INV_ICM42607_REG_INT_CONFIG 0x0006
#define INV_ICM42607_INT_CONFIG_INT2_LATCHED BIT(5)
#define INV_ICM42607_INT_CONFIG_INT2_PUSH_PULL BIT(4)
#define INV_ICM42607_INT_CONFIG_INT2_ACTIVE_HIGH BIT(3)
@@ -202,81 +230,81 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INT_CONFIG_INT1_ACTIVE_LOW 0x00
/* all sensor data are 16 bits (2 registers wide) in big-endian */
-#define INV_ICM42607_REG_TEMP_DATA1 0x09
-#define INV_ICM42607_REG_TEMP_DATA0 0x0A
-#define INV_ICM42607_REG_ACCEL_DATA_X1 0x0B
-#define INV_ICM42607_REG_ACCEL_DATA_X0 0x0C
-#define INV_ICM42607_REG_ACCEL_DATA_Y1 0x0D
-#define INV_ICM42607_REG_ACCEL_DATA_Y0 0x0E
-#define INV_ICM42607_REG_ACCEL_DATA_Z1 0x0F
-#define INV_ICM42607_REG_ACCEL_DATA_Z0 0x10
-#define INV_ICM42607_REG_GYRO_DATA_X1 0x11
-#define INV_ICM42607_REG_GYRO_DATA_X0 0x12
-#define INV_ICM42607_REG_GYRO_DATA_Y1 0x13
-#define INV_ICM42607_REG_GYRO_DATA_Y0 0x14
-#define INV_ICM42607_REG_GYRO_DATA_Z1 0x15
-#define INV_ICM42607_REG_GYRO_DATA_Z0 0x16
+#define INV_ICM42607_REG_TEMP_DATA1 0x0009
+#define INV_ICM42607_REG_TEMP_DATA0 0x000A
+#define INV_ICM42607_REG_ACCEL_DATA_X1 0x000B
+#define INV_ICM42607_REG_ACCEL_DATA_X0 0x000C
+#define INV_ICM42607_REG_ACCEL_DATA_Y1 0x000D
+#define INV_ICM42607_REG_ACCEL_DATA_Y0 0x000E
+#define INV_ICM42607_REG_ACCEL_DATA_Z1 0x000F
+#define INV_ICM42607_REG_ACCEL_DATA_Z0 0x0010
+#define INV_ICM42607_REG_GYRO_DATA_X1 0x0011
+#define INV_ICM42607_REG_GYRO_DATA_X0 0x0012
+#define INV_ICM42607_REG_GYRO_DATA_Y1 0x0013
+#define INV_ICM42607_REG_GYRO_DATA_Y0 0x0014
+#define INV_ICM42607_REG_GYRO_DATA_Z1 0x0015
+#define INV_ICM42607_REG_GYRO_DATA_Z0 0x0016
#define INV_ICM42607_DATA_INVALID -32768
-#define INV_ICM42607_REG_TMST_FSYNCH 0x17
-#define INV_ICM42607_REG_TMST_FSYNCL 0x18
+#define INV_ICM42607_REG_TMST_FSYNCH 0x0017
+#define INV_ICM42607_REG_TMST_FSYNCL 0x0018
/* APEX Data Registers */
-#define INV_ICM42607_REG_APEX_DATA0 0x31
-#define INV_ICM42607_REG_APEX_DATA1 0x32
-#define INV_ICM42607_REG_APEX_DATA2 0x33
-#define INV_ICM42607_REG_APEX_DATA3 0x34
-#define INV_ICM42607_REG_APEX_DATA4 0x1D
-#define INV_ICM42607_REG_APEX_DATA5 0x1E
-
-#define INV_ICM42607_REG_PWR_MGMT0 0x1F
+#define INV_ICM42607_REG_APEX_DATA0 0x0031
+#define INV_ICM42607_REG_APEX_DATA1 0x0032
+#define INV_ICM42607_REG_APEX_DATA2 0x0033
+#define INV_ICM42607_REG_APEX_DATA3 0x0034
+#define INV_ICM42607_REG_APEX_DATA4 0x001D
+#define INV_ICM42607_REG_APEX_DATA5 0x001E
+
+#define INV_ICM42607_REG_PWR_MGMT0 0x001F
#define INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL BIT(7)
#define INV_ICM42607_PWR_MGMT0_IDLE BIT(4)
#define INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK GENMASK(3, 2)
#define INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK GENMASK(1, 0)
-#define INV_ICM42607_REG_GYRO_CONFIG0 0x20
+#define INV_ICM42607_REG_GYRO_CONFIG0 0x0020
#define INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK GENMASK(6, 5)
#define INV_ICM42607_GYRO_CONFIG0_ODR_MASK GENMASK(3, 0)
-#define INV_ICM42607_REG_ACCEL_CONFIG0 0x21
+#define INV_ICM42607_REG_ACCEL_CONFIG0 0x0021
#define INV_ICM42607_ACCEL_CONFIG0_FS_SEL_MASK GENMASK(6, 5)
#define INV_ICM42607_ACCEL_CONFIG0_ODR_MASK GENMASK(3, 0)
-#define INV_ICM42607_REG_TEMP_CONFIG0 0x22
+#define INV_ICM42607_REG_TEMP_CONFIG0 0x0022
#define INV_ICM42607_TEMP_CONFIG0_FILTER_MASK GENMASK(6, 4)
-#define INV_ICM42607_REG_GYRO_CONFIG1 0x23
+#define INV_ICM42607_REG_GYRO_CONFIG1 0x0023
#define INV_ICM42607_GYRO_CONFIG1_FILTER_MASK GENMASK(2, 0)
-#define INV_ICM42607_REG_ACCEL_CONFIG1 0x24
+#define INV_ICM42607_REG_ACCEL_CONFIG1 0x0024
#define INV_ICM42607_ACCEL_CONFIG1_AVG_MASK GENMASK(6, 4)
#define INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK GENMASK(2, 0)
-#define INV_ICM42607_REG_APEX_CONFIG0 0x25
+#define INV_ICM42607_REG_APEX_CONFIG0 0x0025
#define INV_ICM42607_APEX_CONFIG0_DMP_POWER_SAVE_EN BIT(3)
#define INV_ICM42607_APEX_CONFIG0_DMP_INIT_EN BIT(2)
#define INV_ICM42607_APEX_CONFIG0_DMP_MEM_RESET_EN BIT(0)
-#define INV_ICM42607_REG_APEX_CONFIG1 0x26
+#define INV_ICM42607_REG_APEX_CONFIG1 0x0026
#define INV_ICM42607_APEX_CONFIG1_SMD_ENABLE BIT(6)
#define INV_ICM42607_APEX_CONFIG1_FF_ENABLE BIT(5)
#define INV_ICM42607_APEX_CONFIG1_TILT_ENABLE BIT(4)
#define INV_ICM42607_APEX_CONFIG1_PED_ENABLE BIT(3)
#define INV_ICM42607_APEX_CONFIG1_DMP_ODR_MASK GENMASK(1, 0)
-#define INV_ICM42607_REG_WOM_CONFIG 0x27
+#define INV_ICM42607_REG_WOM_CONFIG 0x0027
#define INV_ICM42607_WOM_CONFIG_INT_DUR_MASK GENMASK(4, 3)
#define INV_ICM42607_WOM_CONFIG_INT_MODE BIT(2)
#define INV_ICM42607_WOM_CONFIG_MODE BIT(1)
#define INV_ICM42607_WOM_CONFIG_EN BIT(0)
-#define INV_ICM42607_REG_FIFO_CONFIG1 0x28
+#define INV_ICM42607_REG_FIFO_CONFIG1 0x0028
#define INV_ICM42607_FIFO_CONFIG1_MODE BIT(1)
#define INV_ICM42607_FIFO_CONFIG1_BYPASS BIT(0)
-#define INV_ICM42607_REG_FIFO_CONFIG2 0x29
-#define INV_ICM42607_REG_FIFO_CONFIG3 0x2A
+#define INV_ICM42607_REG_FIFO_CONFIG2 0x0029
+#define INV_ICM42607_REG_FIFO_CONFIG3 0x002A
#define INV_ICM42607_FIFO_WATERMARK_VAL(_wm) \
cpu_to_le16((_wm) & GENMASK(11, 0))
/* FIFO is 2048 bytes, let 12 samples for reading latency */
@@ -284,7 +312,7 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_FIFO_1SENSOR_PACKET_SIZE 8
#define INV_ICM42607_FIFO_2SENSORS_PACKET_SIZE 16
-#define INV_ICM42607_REG_INT_SOURCE0 0x2B
+#define INV_ICM42607_REG_INT_SOURCE0 0x002B
#define INV_ICM42607_INT_SOURCE0_ST_INT1_EN BIT(7)
#define INV_ICM42607_INT_SOURCE0_FSYNC_INT1_EN BIT(6)
#define INV_ICM42607_INT_SOURCE0_PLL_RDY_INT1_EN BIT(5)
@@ -294,12 +322,12 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INT_SOURCE0_FIFO_FULL_INT1_EN BIT(1)
#define INV_ICM42607_INT_SOURCE0_AGC_RDY_INT1_EN BIT(0)
-#define INV_ICM42607_REG_INT_SOURCE1 0x2C
+#define INV_ICM42607_REG_INT_SOURCE1 0x002C
#define INV_ICM42607_INT_SOURCE1_I3C_ERROR_INT1_EN BIT(6)
#define INV_ICM42607_INT_SOURCE1_SMD_INT1_EN BIT(3)
#define INV_ICM42607_INT_SOURCE1_WOM_INT1_EN GENMASK(2, 0)
-#define INV_ICM42607_REG_INT_SOURCE3 0x2D
+#define INV_ICM42607_REG_INT_SOURCE3 0x002D
#define INV_ICM42607_INT_SOURCE3_ST_INT2_EN BIT(7)
#define INV_ICM42607_INT_SOURCE3_FSYNC_INT2_EN BIT(6)
#define INV_ICM42607_INT_SOURCE3_PLL_RDY_INT2_EN BIT(5)
@@ -309,17 +337,17 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INT_SOURCE3_FIFO_FULL_INT2_EN BIT(1)
#define INV_ICM42607_INT_SOURCE3_AGC_RDY_INT2_EN BIT(0)
-#define INV_ICM42607_REG_INT_SOURCE4 0x2E
+#define INV_ICM42607_REG_INT_SOURCE4 0x002E
#define INV_ICM42607_INT_SOURCE4_I3C_ERROR_INT2_EN BIT(6)
#define INV_ICM42607_INT_SOURCE4_SMD_INT2_EN BIT(3)
#define INV_ICM42607_INT_SOURCE4_WOM_Z_INT2_EN BIT(2)
#define INV_ICM42607_INT_SOURCE4_WOM_Y_INT2_EN BIT(1)
#define INV_ICM42607_INT_SOURCE4_WOM_X_INT2_EN BIT(0)
-#define INV_ICM42607_REG_FIFO_LOST_PKT0 0x2F
-#define INV_ICM42607_REG_FIFO_LOST_PKT1 0x30
+#define INV_ICM42607_REG_FIFO_LOST_PKT0 0x002F
+#define INV_ICM42607_REG_FIFO_LOST_PKT1 0x0030
-#define INV_ICM42607_REG_INTF_CONFIG0 0x35
+#define INV_ICM42607_REG_INTF_CONFIG0 0x0035
#define INV_ICM42607_INTF_CONFIG0_FIFO_COUNT_FORMAT BIT(6)
#define INV_ICM42607_INTF_CONFIG0_FIFO_COUNT_ENDIAN BIT(5)
#define INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN BIT(4)
@@ -327,7 +355,7 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS 2
#define INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS 3
-#define INV_ICM42607_REG_INTF_CONFIG1 0x36
+#define INV_ICM42607_REG_INTF_CONFIG1 0x0036
#define INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN BIT(3)
#define INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN BIT(2)
#define INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK GENMASK(1, 0)
@@ -335,10 +363,10 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL 1
#define INV_ICM42607_INTF_CONFIG1_CLKSEL_OFF 2
-#define INV_ICM42607_REG_INT_STATUS_DRDY 0x39
+#define INV_ICM42607_REG_INT_STATUS_DRDY 0x0039
#define INV_ICM42607_INT_STATUS_DRDY_DATA_RDY BIT(0)
-#define INV_ICM42607_REG_INT_STATUS 0x3A
+#define INV_ICM42607_REG_INT_STATUS 0x003A
#define INV_ICM42607_INT_STATUS_ST BIT(7)
#define INV_ICM42607_INT_STATUS_FSYNC BIT(6)
#define INV_ICM42607_INT_STATUS_PLL_RDY BIT(5)
@@ -347,11 +375,11 @@ struct inv_icm42607_sensor_state {
#define INV_ICM42607_INT_STATUS_FIFO_FULL BIT(1)
#define INV_ICM42607_INT_STATUS_AGC_RDY BIT(0)
-#define INV_ICM42607_REG_INT_STATUS2 0x3B
+#define INV_ICM42607_REG_INT_STATUS2 0x003B
#define INV_ICM42607_INT_STATUS2_SMD BIT(3)
#define INV_ICM42607_INT_STATUS2_WOM_INT GENMASK(2, 0)
-#define INV_ICM42607_REG_INT_STATUS3 0x3C
+#define INV_ICM42607_REG_INT_STATUS3 0x003C
#define INV_ICM42607_INT_STATUS3_STEP_DET BIT(5)
#define INV_ICM42607_INT_STATUS3_STEP_CNT_OVF BIT(4)
#define INV_ICM42607_INT_STATUS3_TILT_DET BIT(3)
@@ -362,15 +390,35 @@ struct inv_icm42607_sensor_state {
* FIFO count is 16 bits (2 registers) big-endian
* FIFO data is a continuous read register to read FIFO content
*/
-#define INV_ICM42607_REG_FIFO_COUNTH 0x3D
-#define INV_ICM42607_REG_FIFO_COUNTL 0x3E
-#define INV_ICM42607_REG_FIFO_DATA 0x3F
+#define INV_ICM42607_REG_FIFO_COUNTH 0x003D
+#define INV_ICM42607_REG_FIFO_COUNTL 0x003E
+#define INV_ICM42607_REG_FIFO_DATA 0x003F
-#define INV_ICM42607_REG_WHOAMI 0x75
+#define INV_ICM42607_REG_WHOAMI 0x0075
#define INV_ICM42370P_WHOAMI 0x0D
#define INV_ICM42607P_WHOAMI 0x60
#define INV_ICM42607_WHOAMI 0x67
+#define INV_ICM42607_REG_BLK_SEL_W 0x0079
+#define INV_ICM42607_REG_MADDR_W 0x007A
+#define INV_ICM42607_REG_M_W 0x007B
+#define INV_ICM42607_REG_BLK_SEL_R 0x007C
+#define INV_ICM42607_REG_MADDR_R 0x007D
+#define INV_ICM42607_REG_M_R 0x007E
+
+/* User Bank MREG 1 registers */
+#define INV_ICM42607_REG_OFFSET_USER0 0x014E
+#define INV_ICM42607_REG_OFFSET_USER1 0x014F
+#define INV_ICM42607_REG_OFFSET_USER2 0x0150
+#define INV_ICM42607_REG_OFFSET_USER3 0x0151
+#define INV_ICM42607_REG_OFFSET_USER4 0x0152
+#define INV_ICM42607_REG_OFFSET_USER5 0x0153
+#define INV_ICM42607_REG_OFFSET_USER6 0x0154
+#define INV_ICM42607_REG_OFFSET_USER7 0x0155
+#define INV_ICM42607_REG_OFFSET_USER8 0x0156
+
+/* User Bank MREG 3 registers */
+#define INV_ICM42607_REG_ZG_ST_DATA 0x5005
/*
* Timings as listed in section 3 of datasheet, all values listed in datasheet
* in ms except temp startup time... setting all values in us and using
@@ -391,7 +439,6 @@ struct inv_icm42607_sensor_state {
typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
-extern const struct regmap_config inv_icm42607_regmap_config;
extern const struct inv_icm42607_hw inv_icm42607_hw_data;
extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
extern const struct inv_icm42607_hw inv_icm42370p_hw_data;
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 87b1499a3689f..ea0d50237817b 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -18,63 +18,262 @@
#include <linux/time.h>
#include <linux/timekeeping.h>
#include <linux/types.h>
+#include <linux/unaligned.h>
#include <asm/byteorder.h>
#include "inv_icm42607.h"
-static bool inv_icm42607_is_readable_reg(struct device *dev, unsigned int reg)
+static int inv_icm42607_mclk_get(struct regmap *map, bool *idle_set)
{
- switch (reg) {
- case INV_ICM42607_REG_MCLK_RDY ... INV_ICM42607_REG_INT_CONFIG:
- case INV_ICM42607_REG_TEMP_DATA1 ... INV_ICM42607_REG_TMST_FSYNCL:
- case INV_ICM42607_REG_APEX_DATA4 ... INV_ICM42607_REG_INTF_CONFIG1:
- case INV_ICM42607_REG_INT_STATUS_DRDY ... INV_ICM42607_REG_FIFO_DATA:
- case INV_ICM42607_REG_WHOAMI:
- return true;
+ unsigned int val;
+ int ret, ret2;
+
+ *idle_set = false;
+
+ ret = regmap_read(map, INV_ICM42607_REG_MCLK_RDY, &val);
+ if (ret)
+ return ret;
+
+ if (val & INV_ICM42607_MCLK_RDY_BIT)
+ return 0;
+
+ /*
+ * Clock isn't running: we're either in Sleep mode or in Accel LP mode
+ * running on WUOSC. Force the RC oscillator on via IDLE and wait for
+ * MCLK_RDY. Datasheet gives 10us (accel startup) to 200us (accel
+ * transition from OFF) for this to complete.
+ */
+ ret = regmap_set_bits(map, INV_ICM42607_REG_PWR_MGMT0,
+ INV_ICM42607_PWR_MGMT0_IDLE);
+ if (ret)
+ return ret;
+
+ *idle_set = true;
+
+ ret = regmap_read_poll_timeout(map, INV_ICM42607_REG_MCLK_RDY, val,
+ val & INV_ICM42607_MCLK_RDY_BIT, 10, 200);
+ if (ret) {
+ ret2 = regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
+ INV_ICM42607_PWR_MGMT0_IDLE);
+ if (ret2)
+ dev_err(regmap_get_device(map),
+ "failed to clear IDLE after MCLK timeout: %d\n", ret2);
+ else
+ *idle_set = false;
}
- return false;
+ return ret;
}
-static bool inv_icm42607_is_writeable_reg(struct device *dev, unsigned int reg)
+static void inv_icm42607_mclk_put(struct regmap *map, bool idle_set)
{
- switch (reg) {
- case INV_ICM42607_REG_DEVICE_CONFIG ... INV_ICM42607_REG_INT_CONFIG:
- case INV_ICM42607_REG_PWR_MGMT0 ... INV_ICM42607_REG_INT_SOURCE4:
- case INV_ICM42607_REG_INTF_CONFIG0 ... INV_ICM42607_REG_INTF_CONFIG1:
- return true;
+ if (idle_set)
+ regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
+ INV_ICM42607_PWR_MGMT0_IDLE);
+}
+
+static int inv_icm42607_blk_sel(unsigned int reg, u8 *blk_sel)
+{
+ u8 bank = FIELD_GET(INV_ICM42607_REG_BANK_MASK, reg);
+
+ switch (bank) {
+ case INV_ICM42607_MREG1:
+ *blk_sel = INV_ICM42607_MREG1_BLK_SEL;
+ return 0;
+ case INV_ICM42607_MREG2:
+ case INV_ICM42607_MREG3:
+ *blk_sel = bank;
+ return 0;
+ default:
+ return -EINVAL;
}
+}
- return false;
+static int inv_icm42607_mreg_read(struct regmap *map, unsigned int reg,
+ u8 *data, size_t count)
+{
+ unsigned int val;
+ bool idle_set;
+ u8 blk_sel;
+ int ret;
+
+ /* MREG access is one byte per transaction, no burst support. */
+ if (count != 1)
+ return -EINVAL;
+
+ ret = inv_icm42607_blk_sel(reg, &blk_sel);
+ if (ret)
+ return ret;
+
+ ret = inv_icm42607_mclk_get(map, &idle_set);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, blk_sel);
+ if (ret)
+ goto out;
+
+ ret = regmap_write(map, INV_ICM42607_REG_MADDR_R,
+ FIELD_GET(INV_ICM42607_REG_ADDR_MASK, reg));
+ if (ret)
+ goto out;
+
+ fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
+
+ ret = regmap_read(map, INV_ICM42607_REG_M_R, &val);
+ if (ret)
+ goto out;
+
+ fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
+
+ *data = val;
+out:
+ /* Restore direct access. */
+ ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, 0);
+ inv_icm42607_mclk_put(map, idle_set);
+
+ return ret;
}
+static int inv_icm42607_mreg_write(struct regmap *map, unsigned int reg,
+ const u8 *data, size_t count)
+{
+ bool idle_set;
+ u8 blk_sel;
+ int ret;
+
+ /* MREG access is one byte per transaction, no burst support. */
+ if (count != 1)
+ return -EINVAL;
+
+ ret = inv_icm42607_blk_sel(reg, &blk_sel);
+ if (ret)
+ return ret;
+
+ ret = inv_icm42607_mclk_get(map, &idle_set);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_W, blk_sel);
+ if (ret)
+ goto out;
+
+ ret = regmap_write(map, INV_ICM42607_REG_MADDR_W,
+ FIELD_GET(INV_ICM42607_REG_ADDR_MASK, reg));
+ if (ret)
+ goto out;
+
+ ret = regmap_write(map, INV_ICM42607_REG_M_W, *data);
+ if (ret)
+ goto out;
+
+ fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
+
+out:
+ /* Restore direct access. */
+ ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_W, 0);
+ inv_icm42607_mclk_put(map, idle_set);
+
+ return ret;
+}
+
+static int inv_icm42607_read(void *context, const void *reg_buf, size_t reg_size,
+ void *val_buf, size_t val_size)
+{
+ u16 reg = get_unaligned_be16(reg_buf);
+ struct regmap *map = context;
+
+ if (FIELD_GET(INV_ICM42607_REG_BANK_MASK, reg) != INV_ICM42607_BANK0)
+ return inv_icm42607_mreg_read(map, reg, val_buf, val_size);
+
+ return regmap_bulk_read(map, FIELD_GET(INV_ICM42607_REG_ADDR_MASK, reg),
+ val_buf, val_size);
+}
+
+static int inv_icm42607_write(void *context, const void *data, size_t count)
+{
+ unsigned int reg = get_unaligned_be16(data);
+ struct regmap *map = context;
+ const u8 *d = data;
+
+ if (FIELD_GET(INV_ICM42607_REG_BANK_MASK, reg) != INV_ICM42607_BANK0)
+ return inv_icm42607_mreg_write(map, reg, d + 2, count - 2);
+
+ return regmap_bulk_write(map, FIELD_GET(INV_ICM42607_REG_ADDR_MASK, reg),
+ d + 2, count - 2);
+}
+
+static const struct regmap_bus inv_icm42607_regmap_bus = {
+ .read = inv_icm42607_read,
+ .write = inv_icm42607_write,
+};
+
static bool inv_icm42607_is_volatile_reg(struct device *dev, unsigned int reg)
{
+ /*
+ * MREG banks are indirect and accessed one byte at a time. Keep them
+ * out of the cache so that regcache_sync() never coalesces them into
+ * an unsupported multi-byte write.
+ */
+ if (FIELD_GET(INV_ICM42607_REG_BANK_MASK, reg) != INV_ICM42607_BANK0)
+ return true;
+
switch (reg) {
case INV_ICM42607_REG_MCLK_RDY:
case INV_ICM42607_REG_SIGNAL_PATH_RESET:
case INV_ICM42607_REG_TEMP_DATA1 ... INV_ICM42607_REG_APEX_DATA5:
+ /* PWR_MGMT0 IDLE bit is toggled behind the cache during MREG access. */
+ case INV_ICM42607_REG_PWR_MGMT0:
case INV_ICM42607_REG_APEX_CONFIG0:
case INV_ICM42607_REG_FIFO_LOST_PKT0 ... INV_ICM42607_REG_APEX_DATA3:
case INV_ICM42607_REG_INT_STATUS_DRDY:
case INV_ICM42607_REG_INT_STATUS ... INV_ICM42607_REG_FIFO_DATA:
+ case INV_ICM42607_REG_BLK_SEL_W ... INV_ICM42607_REG_M_R:
return true;
}
return false;
}
-const struct regmap_config inv_icm42607_regmap_config = {
- .reg_bits = 8,
+static bool inv_icm42607_is_readable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case INV_ICM42607_REG_MCLK_RDY ... INV_ICM42607_REG_INT_CONFIG:
+ case INV_ICM42607_REG_TEMP_DATA1 ... INV_ICM42607_REG_TMST_FSYNCL:
+ case INV_ICM42607_REG_APEX_DATA4 ... INV_ICM42607_REG_INTF_CONFIG1:
+ case INV_ICM42607_REG_INT_STATUS_DRDY ... INV_ICM42607_REG_FIFO_DATA:
+ case INV_ICM42607_REG_WHOAMI:
+ case INV_ICM42607_REG_OFFSET_USER0 ... INV_ICM42607_REG_OFFSET_USER8:
+ return true;
+ }
+
+ return false;
+}
+
+static bool inv_icm42607_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case INV_ICM42607_REG_DEVICE_CONFIG ... INV_ICM42607_REG_INT_CONFIG:
+ case INV_ICM42607_REG_PWR_MGMT0 ... INV_ICM42607_REG_INT_SOURCE4:
+ case INV_ICM42607_REG_INTF_CONFIG0 ... INV_ICM42607_REG_INTF_CONFIG1:
+ case INV_ICM42607_REG_OFFSET_USER0 ... INV_ICM42607_REG_OFFSET_USER8:
+ return true;
+ }
+
+ return false;
+}
+
+static const struct regmap_config inv_icm42607_virt_regmap_config = {
+ .name = "banks",
+ .reg_bits = 16,
.val_bits = 8,
.writeable_reg = inv_icm42607_is_writeable_reg,
.readable_reg = inv_icm42607_is_readable_reg,
.volatile_reg = inv_icm42607_is_volatile_reg,
- .max_register = INV_ICM42607_REG_WHOAMI,
+ .max_register = INV_ICM42607_REG_ZG_ST_DATA,
.cache_type = REGCACHE_MAPLE,
};
-EXPORT_SYMBOL_NS_GPL(inv_icm42607_regmap_config, "IIO_ICM42607");
/* chip initial default configuration */
static const struct inv_icm42607_conf inv_icm42607_default_conf = {
@@ -596,8 +795,15 @@ int inv_icm42607_core_probe(struct regmap *regmap,
{
struct device *dev = regmap_get_device(regmap);
struct inv_icm42607_state *st;
+ struct regmap *regmap_custom;
int ret;
+ regmap_custom = devm_regmap_init(dev, &inv_icm42607_regmap_bus, regmap,
+ &inv_icm42607_virt_regmap_config);
+ if (IS_ERR(regmap_custom))
+ return dev_err_probe(dev, PTR_ERR(regmap_custom),
+ "Failed to register regmap\n");
+
st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
if (!st)
return -ENOMEM;
@@ -609,7 +815,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
return ret;
st->hw = hw;
- st->map = regmap;
+ st->map = regmap_custom;
ret = iio_read_mount_matrix(dev, &st->orientation);
if (ret)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
index cfafba2e3e68e..b1d60c777c21c 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
@@ -12,6 +12,11 @@
#include "inv_icm42607.h"
+static const struct regmap_config inv_icm42607_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
static int inv_icm42607_i2c_bus_setup(struct inv_icm42607_state *st)
{
unsigned int val;
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
index cd739375f6b2c..cb27ee470b06f 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
@@ -12,6 +12,11 @@
#include "inv_icm42607.h"
+static const struct regmap_config inv_icm42607_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
static int inv_icm42607_spi_bus_setup(struct inv_icm42607_state *st)
{
unsigned int val;
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access
2026-10-02 11:54 ` [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access Kanak Shilledar
@ 2026-10-02 13:12 ` Andy Shevchenko
2026-10-02 14:25 ` Kanak Shilledar
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-10-02 13:12 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
On Fri, Oct 02, 2026 at 01:54:29PM +0200, Kanak Shilledar wrote:
> The device supports indirect register access to different banks. A
> specific routine needs to be followed when accessing the registers in
> another bank as documented in the datasheet (section 13). This is
> required for accessing registers configured via the user and
> implementing buffer support. The implementation is inspired from the
> icm45600 driver. The banks are defined based on their initial bank
> access code which are written to the BLK_SEL_* regs. However, there is a
> variation for MREG1 as User Bank 0 and MREG1 both share the same 0x00,
> so User Bank 1 has 0x00 and MREG1 has 0x01. When MREG1 register is
> accessed, then BLK_SEL_* is written with 0x00 instead of 0x01. All the
> register access goes through the 16 bit virtual regmap layered on top of
> the 8bit bus regmap. Bank id in the upper byte and the register address
> in the lower byte. Bank 0 accesses are forwarded to the bus regmap with
> the bank field stripped, while MREG accesses are forwarded to bank
> switching sequence.
...
> + unsigned int val;
> + int ret, ret2;
> +
> + *idle_set = false;
> +
> + ret = regmap_read(map, INV_ICM42607_REG_MCLK_RDY, &val);
> + if (ret)
> + return ret;
> +
> + if (val & INV_ICM42607_MCLK_RDY_BIT)
> + return 0;
Why not regmap_test_bits()?
> + /*
> + * Clock isn't running: we're either in Sleep mode or in Accel LP mode
> + * running on WUOSC. Force the RC oscillator on via IDLE and wait for
> + * MCLK_RDY. Datasheet gives 10us (accel startup) to 200us (accel
> + * transition from OFF) for this to complete.
> + */
> + ret = regmap_set_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> + INV_ICM42607_PWR_MGMT0_IDLE);
> + if (ret)
> + return ret;
> +
> + *idle_set = true;
> +
> + ret = regmap_read_poll_timeout(map, INV_ICM42607_REG_MCLK_RDY, val,
> + val & INV_ICM42607_MCLK_RDY_BIT, 10, 200);
> + if (ret) {
> + ret2 = regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> + INV_ICM42607_PWR_MGMT0_IDLE);
> + if (ret2)
> + dev_err(regmap_get_device(map),
> + "failed to clear IDLE after MCLK timeout: %d\n", ret2);
Broken indentation.
Is it really important message?
> + else
> + *idle_set = false;
> }
...
> +static void inv_icm42607_mclk_put(struct regmap *map, bool idle_set)
> {
> + if (idle_set)
> + regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> + INV_ICM42607_PWR_MGMT0_IDLE);
> +}
What about
if (!idle_set)
return;
regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0, INV_ICM42607_PWR_MGMT0_IDLE);
?
...
> +static int inv_icm42607_mreg_read(struct regmap *map, unsigned int reg,
> + u8 *data, size_t count)
> +{
> + unsigned int val;
> + bool idle_set;
> + u8 blk_sel;
> + int ret;
> +
> + /* MREG access is one byte per transaction, no burst support. */
> + if (count != 1)
> + return -EINVAL;
> +
> + ret = inv_icm42607_blk_sel(reg, &blk_sel);
> + if (ret)
> + return ret;
> +
> + ret = inv_icm42607_mclk_get(map, &idle_set);
> + if (ret)
> + return ret;
> + ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, blk_sel);
> + if (ret)
> + goto out;
So, can we use regmap ranges instead?
> + ret = regmap_write(map, INV_ICM42607_REG_MADDR_R,
> + FIELD_GET(INV_ICM42607_REG_ADDR_MASK, reg));
> + if (ret)
> + goto out;
> +
> + fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> +
> + ret = regmap_read(map, INV_ICM42607_REG_M_R, &val);
> + if (ret)
> + goto out;
> +
> + fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> +
> + *data = val;
> +out:
> + /* Restore direct access. */
> + ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, 0);
> + inv_icm42607_mclk_put(map, idle_set);
> +
> + return ret;
> }
...
> +static const struct regmap_config inv_icm42607_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
No cache? Why?
> +};
...
> +static const struct regmap_config inv_icm42607_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
Ditto. And why they can't be deduplicated?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access
2026-10-02 13:12 ` Andy Shevchenko
@ 2026-10-02 14:25 ` Kanak Shilledar
0 siblings, 0 replies; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 14:25 UTC (permalink / raw)
To: andriy.shevchenko
Cc: andy, robh, Kernel, macromorgan, linux-kernel, conor+dt,
joshua.crofts1, devicetree, jean-baptiste.maneyrol, dlechner,
nuno.sa, krzk+dt, jic23, marcelo.schmitt1, Henrik Grimler,
linux-iio
[-- Attachment #1: Type: text/plain, Size: 5541 bytes --]
Hi Andy,
On Fri, 2026-10-02 at 16:12 +0300, Andy Shevchenko wrote:
> On Fri, Oct 02, 2026 at 01:54:29PM +0200, Kanak Shilledar wrote:
> > The device supports indirect register access to different banks. A
> > specific routine needs to be followed when accessing the registers
> > in
> > another bank as documented in the datasheet (section 13). This is
> > required for accessing registers configured via the user and
> > implementing buffer support. The implementation is inspired from
> > the
> > icm45600 driver. The banks are defined based on their initial bank
> > access code which are written to the BLK_SEL_* regs. However, there
> > is a
> > variation for MREG1 as User Bank 0 and MREG1 both share the same
> > 0x00,
> > so User Bank 1 has 0x00 and MREG1 has 0x01. When MREG1 register is
> > accessed, then BLK_SEL_* is written with 0x00 instead of 0x01. All
> > the
> > register access goes through the 16 bit virtual regmap layered on
> > top of
> > the 8bit bus regmap. Bank id in the upper byte and the register
> > address
> > in the lower byte. Bank 0 accesses are forwarded to the bus regmap
> > with
> > the bank field stripped, while MREG accesses are forwarded to bank
> > switching sequence.
>
> ...
>
> > + unsigned int val;
> > + int ret, ret2;
> > +
> > + *idle_set = false;
> > +
> > + ret = regmap_read(map, INV_ICM42607_REG_MCLK_RDY, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if (val & INV_ICM42607_MCLK_RDY_BIT)
> > + return 0;
>
> Why not regmap_test_bits()?
I will fix it.
> > + /*
> > + * Clock isn't running: we're either in Sleep mode or in
> > Accel LP mode
> > + * running on WUOSC. Force the RC oscillator on via IDLE
> > and wait for
> > + * MCLK_RDY. Datasheet gives 10us (accel startup) to 200us
> > (accel
> > + * transition from OFF) for this to complete.
> > + */
> > + ret = regmap_set_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> > + INV_ICM42607_PWR_MGMT0_IDLE);
> > + if (ret)
> > + return ret;
> > +
> > + *idle_set = true;
> > +
> > + ret = regmap_read_poll_timeout(map,
> > INV_ICM42607_REG_MCLK_RDY, val,
> > + val &
> > INV_ICM42607_MCLK_RDY_BIT, 10, 200);
> > + if (ret) {
> > + ret2 = regmap_clear_bits(map,
> > INV_ICM42607_REG_PWR_MGMT0,
> > +
> > INV_ICM42607_PWR_MGMT0_IDLE);
> > + if (ret2)
> > + dev_err(regmap_get_device(map),
> > + "failed to clear IDLE
> > after MCLK timeout: %d\n", ret2);
>
> Broken indentation.
> Is it really important message?
It is useful for indicating failure in MCLK, but I will lower the
priority to either _info or _debug and fix the indentation.
> > + else
> > + *idle_set = false;
> > }
>
> ...
>
> > +static void inv_icm42607_mclk_put(struct regmap *map, bool
> > idle_set)
> > {
> > + if (idle_set)
> > + regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> > + INV_ICM42607_PWR_MGMT0_IDLE);
> > +}
>
> What about
>
> if (!idle_set)
> return;
>
> regmap_clear_bits(map, INV_ICM42607_REG_PWR_MGMT0,
> INV_ICM42607_PWR_MGMT0_IDLE);
>
> ?
Will incorporate the suggestion.
> ...
>
> > +static int inv_icm42607_mreg_read(struct regmap *map, unsigned int
> > reg,
> > + u8 *data, size_t count)
> > +{
> > + unsigned int val;
> > + bool idle_set;
> > + u8 blk_sel;
> > + int ret;
> > +
> > + /* MREG access is one byte per transaction, no burst
> > support. */
> > + if (count != 1)
> > + return -EINVAL;
> > +
> > + ret = inv_icm42607_blk_sel(reg, &blk_sel);
> > + if (ret)
> > + return ret;
> > +
> > + ret = inv_icm42607_mclk_get(map, &idle_set);
> > + if (ret)
> > + return ret;
>
> > + ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R,
> > blk_sel);
> > + if (ret)
> > + goto out;
>
> So, can we use regmap ranges instead?
We can't use regmap ranges because the register accesses for different
banks guarded by a specific routine of writing the bank selector, the
address pointer and then finally accessing the value along with
checking for timings and clocks. There is also a limitation that,
accessing the registers in banks other than USER BANK 0 can only be
done serially. It doesn't support bulk reads. This is documented in
section 13 of the datasheet [1].
> > + ret = regmap_write(map, INV_ICM42607_REG_MADDR_R,
> > + FIELD_GET(INV_ICM42607_REG_ADDR_MASK,
> > reg));
> > + if (ret)
> > + goto out;
> > +
> > + fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> > +
> > + ret = regmap_read(map, INV_ICM42607_REG_M_R, &val);
> > + if (ret)
> > + goto out;
> > +
> > + fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> > +
> > + *data = val;
> > +out:
> > + /* Restore direct access. */
> > + ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, 0);
> > + inv_icm42607_mclk_put(map, idle_set);
> > +
> > + return ret;
> > }
>
> ...
>
> > +static const struct regmap_config inv_icm42607_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
>
> No cache? Why?
As the virtual regmap config has some caching for USER BANK 0 registers
only. The indirect banks doesn't support caching [1].
> > +};
>
> ...
>
> > +static const struct regmap_config inv_icm42607_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
>
> Ditto. And why they can't be deduplicated?
I will remove the duplication of these regmap_configs.
Thanks and Regards,
Kanak Shilledar
[1] Datasheet: https://www.lcsc.com/product-detail/C5129967.html
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support
2026-10-02 11:54 [PATCH v5 0/6] Add support for InvenSense ICM-42370-P accelerometer Kanak Shilledar
` (4 preceding siblings ...)
2026-10-02 11:54 ` [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access Kanak Shilledar
@ 2026-10-02 11:54 ` Kanak Shilledar
2026-10-02 13:18 ` Andy Shevchenko
5 siblings, 1 reply; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 11:54 UTC (permalink / raw)
To: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan
Cc: Kanak Shilledar, kernel, linux-iio, devicetree, linux-kernel
Expose IIO_CHAN_INFO_CALIBBIAS on the accelerometer channels. The
registers are stored in MREG1. The calibration bias is written to
OFFSET_USER4 to OFFSET_USER8 registers in MREG1. Reject the out of
limit calibbias values instead of clamping it.
Note: The accelerometer functionality is tested with Invensense,
ICM42370-P development board.
Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
Datasheet: https://www.lcsc.com/product-detail/C5129967.html
Signed-off-by: Kanak Shilledar <kanak.shilledar@axis.com>
---
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c | 194 +++++++++++++++++++++-
1 file changed, 192 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
index 9a3ace3e7fcf9..98cef0058b64c 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
@@ -13,6 +13,7 @@
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/types.h>
+#include <linux/units.h>
#include "inv_icm42607.h"
#include "inv_icm42607_temp.h"
@@ -22,9 +23,11 @@
.type = IIO_ACCEL, \
.modified = 1, \
.channel2 = _modifier, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
- .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS), \
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.scan_index = _index, \
@@ -56,6 +59,16 @@ static const struct iio_chan_spec inv_icm42607_accel_channels[] = {
INV_ICM42607_TEMP_CHAN(INV_ICM42607_ACCEL_SCAN_TEMP),
};
+/*
+ * Calibration bias values, IIO range format int + micro.
+ * Value is limited to +/-1g coded on 12 bits signed. Step is 0.5mg.
+ */
+static const int inv_icm42607_accel_calibbias[] = {
+ -10, 42010, /* Min : -2^11 * 0.0005 * 9.80665 = -10.042010 m/s² */
+ 0, 4903, /* Step: 0.0005 * 9.80665 = 0.004903 m/s² */
+ 10, 37106, /* Max : (2^11 - 1) * 0.0005 * 9.80665 = 10.037106 m/s² */
+};
+
static const int inv_icm42607_accel_scale_nano[][2] = {
[INV_ICM42607_ACCEL_FS_16G] = { 0, 4788403 },
[INV_ICM42607_ACCEL_FS_8G] = { 0, 2394202 },
@@ -176,6 +189,171 @@ static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev,
return inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
}
+static int inv_icm42607_accel_read_offset(struct inv_icm42607_state *st,
+ struct iio_chan_spec const *chan, int *val, int *val2)
+{
+ struct device *dev = regmap_get_device(st->map);
+ unsigned int lo_val, hi_val;
+ unsigned int reg;
+ s16 offset;
+ s64 val64;
+ s32 bias;
+ int ret;
+
+ if (chan->type != IIO_ACCEL)
+ return -EINVAL;
+
+ switch (chan->channel2) {
+ case IIO_MOD_X:
+ reg = INV_ICM42607_REG_OFFSET_USER4;
+ break;
+ case IIO_MOD_Y:
+ reg = INV_ICM42607_REG_OFFSET_USER6;
+ break;
+ case IIO_MOD_Z:
+ reg = INV_ICM42607_REG_OFFSET_USER7;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&st->lock);
+
+ ret = regmap_read(st->map, reg, &lo_val);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->map, reg + 1, &hi_val);
+ if (ret)
+ return ret;
+
+ /* 12 bits signed value */
+ switch (chan->channel2) {
+ case IIO_MOD_X:
+ case IIO_MOD_Z:
+ offset = sign_extend32(((lo_val & 0xF0) << 4) | hi_val, 11);
+ break;
+ case IIO_MOD_Y:
+ offset = sign_extend32(((hi_val & 0x0F) << 8) | lo_val, 11);
+ break;
+ default:
+ return -EINVAL;
+ }
+ /*
+ * Convert raw offset to g then to m/s²
+ * 12 bits signed raw step 0.5mg to g: 5 / 10000
+ * g to m/s²: 9.806650
+ * Result in micro (1000000)
+ * (offset * 5 * 9.806650 * 1000000) / 10000
+ */
+ val64 = (s64)offset * 5LL * 9806650LL;
+ /* For rounding, add + or - divisor (10000) divided by 2 */
+ if (val64 >= 0)
+ val64 += 10000LL / 2LL;
+ else
+ val64 -= 10000LL / 2LL;
+
+ bias = div_s64(val64, 10000L);
+ *val = bias / (long)MEGA;
+ *val2 = bias % (long)MEGA;
+
+ return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int inv_icm42607_accel_write_offset(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2)
+{
+ struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+ struct device *dev = regmap_get_device(st->map);
+ s32 min, max;
+ s16 offset;
+ s64 val64;
+ int ret;
+
+ if (chan->type != IIO_ACCEL)
+ return -EINVAL;
+
+ /* inv_icm42607_accel_calibbias: min - step - max in micro */
+ min = inv_icm42607_accel_calibbias[0] * (long)MEGA -
+ inv_icm42607_accel_calibbias[1];
+ max = inv_icm42607_accel_calibbias[4] * (long)MEGA +
+ inv_icm42607_accel_calibbias[5];
+
+ val64 = val * (s64)MEGA;
+ if (val >= 0)
+ val64 += val2;
+ else
+ val64 -= val2;
+
+ if (val64 < min || val64 > max)
+ return -EINVAL;
+
+ /*
+ * Convert m/s² to g then to raw value
+ * m/s² to g: 1 / 9.806650
+ * g to raw 12 bits signed, step 0.5mg: 10000 / 5
+ * val in micro (1000000)
+ * val * 10000 / (9.806650 * 1000000 * 5)
+ */
+ val64 *= 10000LL;
+
+ /* For rounding, add + or - divisor (9806650 * 5) divided by 2 */
+ if (val64 >= 0)
+ val64 += 9806650 * 5 / 2;
+ else
+ val64 -= 9806650 * 5 / 2;
+ offset = div_s64(val64, 9806650 * 5);
+
+ /* Value is limited to 12 bits signed, return -EINVAL if out of range */
+ if (offset < -2048 || offset > 2047)
+ return -EINVAL;
+
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&st->lock);
+
+ switch (chan->channel2) {
+ case IIO_MOD_X:
+ /* OFFSET_USER4 upper nibble is shared. */
+ ret = regmap_update_bits(st->map, INV_ICM42607_REG_OFFSET_USER4,
+ GENMASK(7, 4), (offset & 0xF00) >> 4);
+ if (ret)
+ return ret;
+
+ return regmap_write(st->map, INV_ICM42607_REG_OFFSET_USER5,
+ offset & 0xFF);
+ case IIO_MOD_Y:
+ /* OFFSET_USER7 lower nibble is shared. */
+ ret = regmap_update_bits(st->map, INV_ICM42607_REG_OFFSET_USER7,
+ GENMASK(3, 0), (offset & 0xF00) >> 8);
+ if (ret)
+ return ret;
+
+ return regmap_write(st->map, INV_ICM42607_REG_OFFSET_USER6,
+ offset & 0xFF);
+ case IIO_MOD_Z:
+ /* OFFSET_USER7 upper nibble is shared. */
+ ret = regmap_update_bits(st->map, INV_ICM42607_REG_OFFSET_USER7,
+ GENMASK(7, 4), (offset & 0xF00) >> 4);
+ if (ret)
+ return ret;
+
+ return regmap_write(st->map, INV_ICM42607_REG_OFFSET_USER8,
+ offset & 0xFF);
+ default:
+ return -EINVAL;
+ }
+}
+
static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
@@ -207,6 +385,8 @@ static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
return inv_icm42607_accel_read_scale(indio_dev, val, val2);
case IIO_CHAN_INFO_SAMP_FREQ:
return inv_icm42607_accel_read_odr(st, val, val2);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return inv_icm42607_accel_read_offset(st, chan, val, val2);
default:
return -EINVAL;
}
@@ -231,6 +411,12 @@ static int inv_icm42607_accel_read_avail(struct iio_dev *indio_dev,
*length = (ARRAY_SIZE(inv_icm42607_accel_odr) -
INV_ICM42607_ODR_1600HZ) * 2;
return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ if (chan->type != IIO_ACCEL)
+ return -EINVAL;
+ *vals = inv_icm42607_accel_calibbias;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ return IIO_AVAIL_RANGE;
default:
return -EINVAL;
}
@@ -250,6 +436,8 @@ static int inv_icm42607_accel_write_raw(struct iio_dev *indio_dev,
return ret;
case IIO_CHAN_INFO_SAMP_FREQ:
return inv_icm42607_accel_write_odr(indio_dev, val, val2);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return inv_icm42607_accel_write_offset(indio_dev, chan, val, val2);
default:
return -EINVAL;
}
@@ -266,6 +454,8 @@ static int inv_icm42607_accel_write_raw_get_fmt(struct iio_dev *indio_dev,
return IIO_VAL_INT_PLUS_NANO;
case IIO_CHAN_INFO_SAMP_FREQ:
return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support
2026-10-02 11:54 ` [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support Kanak Shilledar
@ 2026-10-02 13:18 ` Andy Shevchenko
2026-10-02 14:03 ` Kanak Shilledar
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-10-02 13:18 UTC (permalink / raw)
To: Kanak Shilledar
Cc: Henrik Grimler, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean-Baptiste Maneyrol, Joshua Crofts, Marcelo Schmitt,
Chris Morgan, kernel, linux-iio, devicetree, linux-kernel
On Fri, Oct 02, 2026 at 01:54:30PM +0200, Kanak Shilledar wrote:
> Expose IIO_CHAN_INFO_CALIBBIAS on the accelerometer channels. The
> registers are stored in MREG1. The calibration bias is written to
> OFFSET_USER4 to OFFSET_USER8 registers in MREG1. Reject the out of
> limit calibbias values instead of clamping it.
>
> Note: The accelerometer functionality is tested with Invensense,
> ICM42370-P development board.
...
> + case IIO_MOD_X:
> + case IIO_MOD_Z:
> + offset = sign_extend32(((lo_val & 0xF0) << 4) | hi_val, 11);
> + break;
> + case IIO_MOD_Y:
> + offset = sign_extend32(((hi_val & 0x0F) << 8) | lo_val, 11);
> + break;
Why do we have hi/lo and not proper __le16 or __be16 type for that to begin
with?
...
> + val64 = (s64)offset * 5LL * 9806650LL;
> + /* For rounding, add + or - divisor (10000) divided by 2 */
> + if (val64 >= 0)
> + val64 += 10000LL / 2LL;
> + else
> + val64 -= 10000LL / 2LL;
> +
> + bias = div_s64(val64, 10000L);
We have DIV_S64_ROUND_CLOSEST().
...
Overall, the feeling is that this is cumbersome change and may be split to
smaller and more isolated logical updates.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 6/6] iio: imu: inv_icm42607: Add accelerometer calibbias support
2026-10-02 13:18 ` Andy Shevchenko
@ 2026-10-02 14:03 ` Kanak Shilledar
0 siblings, 0 replies; 15+ messages in thread
From: Kanak Shilledar @ 2026-10-02 14:03 UTC (permalink / raw)
To: andriy.shevchenko
Cc: andy, robh, Kernel, macromorgan, linux-kernel, conor+dt,
joshua.crofts1, devicetree, jean-baptiste.maneyrol, dlechner,
nuno.sa, krzk+dt, jic23, marcelo.schmitt1, Henrik Grimler,
linux-iio
[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]
Hi Andy,
Thanks for going through the patches.
On Fri, 2026-10-02 at 16:18 +0300, Andy Shevchenko wrote:
> On Fri, Oct 02, 2026 at 01:54:30PM +0200, Kanak Shilledar wrote:
> > Expose IIO_CHAN_INFO_CALIBBIAS on the accelerometer channels. The
> > registers are stored in MREG1. The calibration bias is written to
> > OFFSET_USER4 to OFFSET_USER8 registers in MREG1. Reject the out of
> > limit calibbias values instead of clamping it.
> >
> > Note: The accelerometer functionality is tested with Invensense,
> > ICM42370-P development board.
>
> ...
>
> > + case IIO_MOD_X:
> > + case IIO_MOD_Z:
> > + offset = sign_extend32(((lo_val & 0xF0) << 4) |
> > hi_val, 11);
> > + break;
> > + case IIO_MOD_Y:
> > + offset = sign_extend32(((hi_val & 0x0F) << 8) |
> > lo_val, 11);
> > + break;
>
> Why do we have hi/lo and not proper __le16 or __be16 type for that to
> begin
> with?
The reason for having hi/lo is because the actual offset values are
split between two registers, as described in the section 16.33 to
section 16.37 of the datasheet [1].
MREG1 register Contents
-------------- --------------------------------
OFFSET_USER4 X[11:8] | other bits
OFFSET_USER5 X[7:0]
OFFSET_USER6 Y[7:0]
OFFSET_USER7 Z[11:8] | Y[11:8]
OFFSET_USER8 Z[7:0]
> ...
>
> > + val64 = (s64)offset * 5LL * 9806650LL;
> > + /* For rounding, add + or - divisor (10000) divided by 2
> > */
> > + if (val64 >= 0)
> > + val64 += 10000LL / 2LL;
> > + else
> > + val64 -= 10000LL / 2LL;
> > +
> > + bias = div_s64(val64, 10000L);
>
> We have DIV_S64_ROUND_CLOSEST().
I will replace it with the suggested one.
> ...
>
> Overall, the feeling is that this is cumbersome change and may be
> split to
> smaller and more isolated logical updates.
Do you have any advice on how to split this patch series?
Thanks and Regards,
Kanak Shilledar
[1] https://www.lcsc.com/product-detail/C5129967.html
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread