Add Invensense ICM42607

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
 .../bindings/iio/imu/invensense,icm42600.yaml  | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +17 -1

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
       - invensense,icm42600
       - invensense,icm42602
       - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
       - invensense,icm42622
       - invensense,icm42631
       - invensense,icm42686

@@ -67,10 +69,24 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts
 
 unevaluatedProperties: false
 

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

unevaluatedProperties: false

Signature · 1 line

2.43.0

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

Thanks,
JB

Quoted 55 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

unevaluatedProperties: false

Signature · 1 line

2.43.  

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

Thanks,
JB

Quoted 55 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

unevaluatedProperties: false

Signature · 1 line

2.43.0

From: Chris Morgan <[email protected]>
Sent: Tuesday, June 23, 2026 02:06
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron; Chris Morgan; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Krzysztof Kozlowski
Subject: Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607

On Mon, Jun 22, 2026 at 09: 23: 28AM +0000, Jean-Baptiste Maneyrol wrote: > Hello Chris and Jonathan, > > concerning dt bindings, my initial understanding was that we had a file per > driver. But here, Chris is doing a new driver for ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.

ZjQcmQRYFpfptBannerEnd

Quoted 6 lines · Jean-Baptiste Maneyrol

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Quoted 8 lines

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

For what it's worth I'm not using any of those registers in the driver currently; from what I see in the datasheets I was able to find on the web the 42607p doesn't do the indirect register access (again unless I'm misreading). To be fair I don't have any other icm42607 chips to test against. The 42607c does appear to do such register access.

Thank you,
Chris

Quoted 56 lines

Thanks,
JB

Quoted 51 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

Quoted 4 lines

Quoted 4 lines

unevaluatedProperties: false

--
2.43.0

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

Thanks,
JB

Quoted 51 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

Quoted 4 lines

unevaluatedProperties: false

--
2.43.0

Quoted 31 lines

From: Chris Morgan <[email protected]>
Sent: Tuesday, June 23, 2026 02:06
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron; Chris Morgan; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Krzysztof Kozlowski
Subject: Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607

On Mon, Jun 22, 2026 at 09: 23: 28AM +0000, Jean-Baptiste Maneyrol wrote: > Hello Chris and Jonathan, > > concerning dt bindings, my initial understanding was that we had a file per > driver. But here, Chris is doing a new driver for ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.

ZjQcmQRYFpfptBannerEnd

Quoted 6 lines · Jean-Baptiste Maneyrol

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Quoted 8 lines

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

Quoted 8 lines

For what it's worth I'm not using any of those registers in the driver currently; from what I see in the datasheets I was able to find on the web the 42607p doesn't do the indirect register access (again unless I'm misreading). To be fair I don't have any other icm42607 chips to test against. The 42607c does appear to do such register access.

Thank you,
Chris

Hello Chris,

here is a link to download ICM-42670-P datasheet, this chip is completely similar to ICM-42607-P:
https://www.invensense.tdk.com/en-us/download-resource/ds-000451-icm-42670-p-datasheet

Indirect register access is required when you want to use the FIFO for configuring which data is stored inside or when you want to update gyro/accel hardware offsets (calibbias iio attribute usually). Also required for a lot of more complex internal chip configuration.

I didn't had a chance to look at your driver currently. I hope to be able to have a look soon.

I can you give the figures for the required maximum sleep time for accel and gyro startups and stops. Usually, they are not provided in datasheet (only mean values).

Thanks for your work,
JB

Quoted 57 lines

Quoted 57 lines

Thanks,
JB

Quoted 51 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

Quoted 6 lines

Quoted 4 lines

Quoted 4 lines

unevaluatedProperties: false

--
2.43.0

________________________________________
From: Chris Morgan <[email protected]>
Sent: Tuesday, June 23, 2026 02:06
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron; Chris Morgan; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Krzysztof Kozlowski
Subject: Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607

On Mon, Jun 22, 2026 at 09: 23: 28AM +0000, Jean-Baptiste Maneyrol wrote: > Hello Chris and Jonathan, > > concerning dt bindings, my initial understanding was that we had a file per > driver. But here, Chris is doing a new driver for ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.

ZjQcmQRYFpfptBannerEnd

Quoted 6 lines · Jean-Baptiste Maneyrol

Hello Chris and Jonathan,

concerning dt bindings, my initial understanding was that we had a file per driver. But here, Chris is doing a new driver for icm42607 while adding new bindings here.

Quoted 8 lines

Does it means we don't have 1 binding file per driver, and there is no need to create a new binding file for inv_icm42607 driver?

Despite the naming, icm42607 chips are a complete new design very different than all other icm42600 chips. It using similar IPs for things like the FIFO, but all other parts are different. Especially, it doesn't use banks for registers access but indirect access delegated to the chip internals for accessing certain registers.

For what it's worth I'm not using any of those registers in the driver currently; from what I see in the datasheets I was able to find on the web the 42607p doesn't do the indirect register access (again unless I'm misreading). To be fair I don't have any other icm42607 chips to test against. The 42607c does appear to do such register access.

Thank you,
Chris

Quoted 59 lines

Thanks,
JB

Quoted 55 lines

From: Chris Morgan <[email protected]>

Add the ICM42607 and ICM42607P inertial measurement unit.

This device is functionally very similar to the icm42600 series with a very different register layout. The driver does not require an interrupt for these specific chip revisions.

Signed-off-by Chris Morgan <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
.../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +2 -0

index 9b2af104f186..81b6e85decd5 100644
@@ -30,6 +30,8 @@ properties:
      - invensense,icm42600
      - invensense,icm42602
      - invensense,icm42605
+      - invensense,icm42607
+      - invensense,icm42607p
      - invensense,icm42622
      - invensense,icm42631
      - invensense,icm42686

@@ -67,10 +69,24 @@ properties:

required:
- compatible
- reg
- - interrupts

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#

Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +15 -0

+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - invensense,icm42600
+              - invensense,icm42602
+              - invensense,icm42605
+              - invensense,icm42622
+              - invensense,icm42631
+              - invensense,icm42686
+              - invensense,icm42688
+    then:
+      required:
+        - interrupts

unevaluatedProperties: false

--
2.43.0

Add the core component of a new inv_icm42607 driver. This includes a few setup functions and the full register definition in the header file, as well as the bits necessary to compile and probe the device when used on an i2c bus.

+#include "inv_icm42607.h"

From: Chris Morgan <[email protected]>

Add power management support for the ICM42607 device driver.

Signed-off-by Chris Morgan <[email protected]>

---
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 139 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
 .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
 4 files changed, 159 insertions(+)

drivers/iio/imu/inv_icm42607/inv_icm42607.h +0 -0

index a6a58571935f..4f4f541027dc 100644

drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0

+
 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 dev_pm_ops inv_icm42607_pm_ops;
 
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +11 -0

index 4b8e19091786..64f5d263de4f 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device/devres.h>

@@ -11,6 +12,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/time.h>

@@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
+				      enum inv_icm42607_sensor_mode gyro,
+				      enum inv_icm42607_sensor_mode accel,
+				      bool temp, unsigned int *sleep_ms)
+{
+	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
+	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
+	bool oldtemp = st->conf.temp_en;
+	unsigned int sleepval_ms;

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +39 -0

+	 */
+	if (!gyro && oldgyro)
+		fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
+
+	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
+	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
+	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	st->conf.gyro.mode = gyro;
+	st->conf.accel.mode = accel;
+	st->conf.temp_en = temp;
+
+	/*
+	 * If a state change occurs from off to on, sleep for the startup
+	 * time of the sensor, unless a sleep_ms is specified. Since more
+	 * than one sensor can be transitioned from off to on, select the
+	 * maximum time from each of the sensors changing from off to on.
+	 */
+	sleepval_ms = 0;
+	if (temp && !oldtemp)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
+
+	if (accel && !oldaccel)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
+
+	if (gyro && !oldgyro)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
+
+	if (sleep_ms)
+		*sleep_ms = sleepval_ms;
+	else if (sleepval_ms)
+		fsleep(sleepval_ms * USEC_PER_MSEC);
+
+	return 0;
+}
 
 
 int inv_icm42607_core_probe(struct regmap *regmap,

@@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (!st)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, st);
+
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1 -0

+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:48 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add power management support for the ICM42607 device driver.

Signed-off-by Chris Morgan <[email protected]>

A few things from taking a look at the sashiko report:
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com

Quoted 13 lines

---
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 139 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
 .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
 4 files changed, 159 insertions(+)

drivers/iio/imu/inv_icm42607/inv_icm42607.h +0 -0

index a6a58571935f..4f4f541027dc 100644

drivers/iio/imu/inv_icm42607/inv_icm42607.h struct inv_icm42607_state { · line 352

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +5 -0

@@ -334,11 +345,18 @@ struct inv_icm42607_state {
 #define INV_ICM42607_GYRO_STOP_TIME_MS			45
 #define INV_ICM42607_TEMP_STARTUP_TIME_MS		77
 
+/*
+ * Suspend delay assumed from other icm42600 series device, not
+ * documented in datasheet.
+ */
+#define INV_ICM42607_SUSPEND_DELAY_MS			(2 * USEC_PER_MSEC)

Sashiko had a valid comment on this. MSEC_PER_SEC seems more appropriate given this is 2 seconds in milli seconds.

Quoted 49 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0

+
 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 dev_pm_ops inv_icm42607_pm_ops;
 
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 4b8e19091786..64f5d263de4f 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device/devres.h>

@@ -11,6 +12,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/time.h>

@@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
+				      enum inv_icm42607_sensor_mode gyro,
+				      enum inv_icm42607_sensor_mode accel,
+				      bool temp, unsigned int *sleep_ms)
+{
+	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
+	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
+	bool oldtemp = st->conf.temp_en;
+	unsigned int sleepval_ms;
+	unsigned int val;
+	int ret;
+
+	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
+		return 0;
+

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c const struct inv_icm42607_hw inv_icm42607p_hw_data = { · line 126

Quoted 4 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +4 -0

+	/*
+	 * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
+	 * for a minimum of 45ms. So if we transition from an on state to an
+	 * off state wait 45ms to ensure a sufficient pause before power off.

Sashiko commented on this.. I think what we could do with adding to the comment is what the path is that didn't pass through this function which would ensure we have been on for 30 of this msecs already.

Quoted 54 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +39 -0

+	 */
+	if (!gyro && oldgyro)
+		fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
+
+	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
+	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
+	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	st->conf.gyro.mode = gyro;
+	st->conf.accel.mode = accel;
+	st->conf.temp_en = temp;
+
+	/*
+	 * If a state change occurs from off to on, sleep for the startup
+	 * time of the sensor, unless a sleep_ms is specified. Since more
+	 * than one sensor can be transitioned from off to on, select the
+	 * maximum time from each of the sensors changing from off to on.
+	 */
+	sleepval_ms = 0;
+	if (temp && !oldtemp)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
+
+	if (accel && !oldaccel)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
+
+	if (gyro && !oldgyro)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
+
+	if (sleep_ms)
+		*sleep_ms = sleepval_ms;
+	else if (sleepval_ms)
+		fsleep(sleepval_ms * USEC_PER_MSEC);
+
+	return 0;
+}
 
 
 int inv_icm42607_core_probe(struct regmap *regmap,

@@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (!st)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, st);
+
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;

@@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (ret)
 		return ret;
 

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 350

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);

Sashiko does put out some stuff here. Please take a look and work out or test if it is right (I think not but haven't checked that carefully!) From a quick look I think that the auto disabling of autosuspend does a rpm_idle() that should result in it suspending...

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 354

Quoted 5 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1 -0

+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");

Quoted 85 lines · Jonathan Cameron

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:48 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add power management support for the ICM42607 device driver.

Signed-off-by Chris Morgan <[email protected]>

A few things from taking a look at the sashiko report:
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com

Quoted 13 lines

---
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 139 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
 .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
 4 files changed, 159 insertions(+)

drivers/iio/imu/inv_icm42607/inv_icm42607.h +0 -0

index a6a58571935f..4f4f541027dc 100644

drivers/iio/imu/inv_icm42607/inv_icm42607.h struct inv_icm42607_state { · line 352

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +5 -0

@@ -334,11 +345,18 @@ struct inv_icm42607_state {
 #define INV_ICM42607_GYRO_STOP_TIME_MS			45
 #define INV_ICM42607_TEMP_STARTUP_TIME_MS		77
 
+/*
+ * Suspend delay assumed from other icm42600 series device, not
+ * documented in datasheet.
+ */
+#define INV_ICM42607_SUSPEND_DELAY_MS			(2 * USEC_PER_MSEC)

Sashiko had a valid comment on this. MSEC_PER_SEC seems more appropriate given this is 2 seconds in milli seconds.

Quoted 49 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0

+
 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 dev_pm_ops inv_icm42607_pm_ops;
 
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 4b8e19091786..64f5d263de4f 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device/devres.h>

@@ -11,6 +12,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/time.h>

@@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
+				      enum inv_icm42607_sensor_mode gyro,
+				      enum inv_icm42607_sensor_mode accel,
+				      bool temp, unsigned int *sleep_ms)
+{
+	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
+	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
+	bool oldtemp = st->conf.temp_en;
+	unsigned int sleepval_ms;
+	unsigned int val;
+	int ret;
+
+	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
+		return 0;
+

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c const struct inv_icm42607_hw inv_icm42607p_hw_data = { · line 126

Quoted 4 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +4 -0

+	/*
+	 * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
+	 * for a minimum of 45ms. So if we transition from an on state to an
+	 * off state wait 45ms to ensure a sufficient pause before power off.

Sashiko commented on this.. I think what we could do with adding to the comment is what the path is that didn't pass through this function which would ensure we have been on for 30 of this msecs already.

I'm going to track whatever time the gyro started, and then if less than 45ms has elapsed just pause the remaining amount of time.

Quoted 54 lines

Quoted 54 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +39 -0

+	 */
+	if (!gyro && oldgyro)
+		fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
+
+	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
+	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
+	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	st->conf.gyro.mode = gyro;
+	st->conf.accel.mode = accel;
+	st->conf.temp_en = temp;
+
+	/*
+	 * If a state change occurs from off to on, sleep for the startup
+	 * time of the sensor, unless a sleep_ms is specified. Since more
+	 * than one sensor can be transitioned from off to on, select the
+	 * maximum time from each of the sensors changing from off to on.
+	 */
+	sleepval_ms = 0;
+	if (temp && !oldtemp)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
+
+	if (accel && !oldaccel)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
+
+	if (gyro && !oldgyro)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
+
+	if (sleep_ms)
+		*sleep_ms = sleepval_ms;
+	else if (sleepval_ms)
+		fsleep(sleepval_ms * USEC_PER_MSEC);
+
+	return 0;
+}
 
 
 int inv_icm42607_core_probe(struct regmap *regmap,

@@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (!st)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, st);
+
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;

@@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (ret)
 		return ret;
 

Quoted 11 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 350

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);

Sashiko does put out some stuff here. Please take a look and work out or test if it is right (I think not but haven't checked that carefully!) From a quick look I think that the auto disabling of autosuspend does a rpm_idle() that should result in it suspending...

I see a few other drivers adding one more call to devm_pm_runtime_enable() so I'm going to see how that works out.

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 354

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1 -0

+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");

Thank you,
Chris

Quoted 164 lines · Chris Morgan

Quoted 85 lines · Jonathan Cameron

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:48 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add power management support for the ICM42607 device driver.

Signed-off-by Chris Morgan <[email protected]>

A few things from taking a look at the sashiko report:
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com

Quoted 13 lines

---
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 139 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
 .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
 4 files changed, 159 insertions(+)

drivers/iio/imu/inv_icm42607/inv_icm42607.h +0 -0

index a6a58571935f..4f4f541027dc 100644

drivers/iio/imu/inv_icm42607/inv_icm42607.h struct inv_icm42607_state { · line 352

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +5 -0

@@ -334,11 +345,18 @@ struct inv_icm42607_state {
 #define INV_ICM42607_GYRO_STOP_TIME_MS			45
 #define INV_ICM42607_TEMP_STARTUP_TIME_MS		77
 
+/*
+ * Suspend delay assumed from other icm42600 series device, not
+ * documented in datasheet.
+ */
+#define INV_ICM42607_SUSPEND_DELAY_MS			(2 * USEC_PER_MSEC)  

Sashiko had a valid comment on this. MSEC_PER_SEC seems more appropriate given this is 2 seconds in milli seconds.

Quoted 49 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0

+
 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 dev_pm_ops inv_icm42607_pm_ops;
 
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 4b8e19091786..64f5d263de4f 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device/devres.h>

@@ -11,6 +12,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/time.h>

@@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
+				      enum inv_icm42607_sensor_mode gyro,
+				      enum inv_icm42607_sensor_mode accel,
+				      bool temp, unsigned int *sleep_ms)
+{
+	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
+	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
+	bool oldtemp = st->conf.temp_en;
+	unsigned int sleepval_ms;
+	unsigned int val;
+	int ret;
+
+	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
+		return 0;
+

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c const struct inv_icm42607_hw inv_icm42607p_hw_data = { · line 126

Quoted 4 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +4 -0

+	/*
+	 * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
+	 * for a minimum of 45ms. So if we transition from an on state to an
+	 * off state wait 45ms to ensure a sufficient pause before power off.  

Sashiko commented on this.. I think what we could do with adding to the comment is what the path is that didn't pass through this function which would ensure we have been on for 30 of this msecs already.

I'm going to track whatever time the gyro started, and then if less than 45ms has elapsed just pause the remaining amount of time.

Quoted 54 lines

Quoted 54 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +39 -0

+	 */
+	if (!gyro && oldgyro)
+		fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
+
+	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
+	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
+	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	st->conf.gyro.mode = gyro;
+	st->conf.accel.mode = accel;
+	st->conf.temp_en = temp;
+
+	/*
+	 * If a state change occurs from off to on, sleep for the startup
+	 * time of the sensor, unless a sleep_ms is specified. Since more
+	 * than one sensor can be transitioned from off to on, select the
+	 * maximum time from each of the sensors changing from off to on.
+	 */
+	sleepval_ms = 0;
+	if (temp && !oldtemp)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
+
+	if (accel && !oldaccel)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
+
+	if (gyro && !oldgyro)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
+
+	if (sleep_ms)
+		*sleep_ms = sleepval_ms;
+	else if (sleepval_ms)
+		fsleep(sleepval_ms * USEC_PER_MSEC);
+
+	return 0;
+}  
 
 
 int inv_icm42607_core_probe(struct regmap *regmap,

@@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (!st)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, st);
+
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;

@@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (ret)
 		return ret;
 

Quoted 11 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 350

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);  

Sashiko does put out some stuff here. Please take a look and work out or test if it is right (I think not but haven't checked that carefully!) From a quick look I think that the auto disabling of autosuspend does a rpm_idle() that should result in it suspending...

Quoted 2 lines

I see a few other drivers adding one more call to devm_pm_runtime_enable() so I'm going to see how that works out.

Obviously that didn't work, but digging in much deeper into the PM code I see that devm_pm_runtime_set_active_enabled() returns devm_pm_runtime_enable(), which sets devm_add_action_or_reset() on pm_runtime_disable_action(), which calls pm_runtime_dont_use_autosuspend() and pm_runtime_disable(). Shouldn't this work, or am I missing something? Basically when the driver detaches shouldn't this chain end up calling pm_runtime_dont_use_autosuspend()?

Thank you,
Chris

Quoted 9 lines

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 354

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1 -0

+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");  

Thank you,
Chris

Quoted 168 lines · On Tue, 23 Jun 2026 13:08:02 -0500 Chris Morgan <[email protected]> wrote:

Quoted 164 lines · Chris Morgan

Quoted 85 lines · Jonathan Cameron

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:48 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add power management support for the ICM42607 device driver.

Signed-off-by Chris Morgan <[email protected]>

A few things from taking a look at the sashiko report:
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com

Quoted 13 lines

---
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  18 +++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 139 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |   1 +
 .../iio/imu/inv_icm42607/inv_icm42607_spi.c   |   1 +
 4 files changed, 159 insertions(+)

drivers/iio/imu/inv_icm42607/inv_icm42607.h +0 -0

index a6a58571935f..4f4f541027dc 100644

drivers/iio/imu/inv_icm42607/inv_icm42607.h struct inv_icm42607_state { · line 352

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +5 -0

@@ -334,11 +345,18 @@ struct inv_icm42607_state {
 #define INV_ICM42607_GYRO_STOP_TIME_MS			45
 #define INV_ICM42607_TEMP_STARTUP_TIME_MS		77
 
+/*
+ * Suspend delay assumed from other icm42600 series device, not
+ * documented in datasheet.
+ */
+#define INV_ICM42607_SUSPEND_DELAY_MS			(2 * USEC_PER_MSEC)  

Sashiko had a valid comment on this. MSEC_PER_SEC seems more appropriate given this is 2 seconds in milli seconds.

Quoted 49 lines

drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0

+
 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 dev_pm_ops inv_icm42607_pm_ops;
 
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 4b8e19091786..64f5d263de4f 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device/devres.h>

@@ -11,6 +12,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/time.h>

@@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
+				      enum inv_icm42607_sensor_mode gyro,
+				      enum inv_icm42607_sensor_mode accel,
+				      bool temp, unsigned int *sleep_ms)
+{
+	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
+	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
+	bool oldtemp = st->conf.temp_en;
+	unsigned int sleepval_ms;
+	unsigned int val;
+	int ret;
+
+	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
+		return 0;
+

Quoted 8 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c const struct inv_icm42607_hw inv_icm42607p_hw_data = { · line 126

Quoted 4 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +4 -0

+	/*
+	 * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
+	 * for a minimum of 45ms. So if we transition from an on state to an
+	 * off state wait 45ms to ensure a sufficient pause before power off.  

Sashiko commented on this.. I think what we could do with adding to the comment is what the path is that didn't pass through this function which would ensure we have been on for 30 of this msecs already.

I'm going to track whatever time the gyro started, and then if less than 45ms has elapsed just pause the remaining amount of time.

Quoted 54 lines

Quoted 54 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +39 -0

+	 */
+	if (!gyro && oldgyro)
+		fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
+
+	val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
+	val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
+	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	st->conf.gyro.mode = gyro;
+	st->conf.accel.mode = accel;
+	st->conf.temp_en = temp;
+
+	/*
+	 * If a state change occurs from off to on, sleep for the startup
+	 * time of the sensor, unless a sleep_ms is specified. Since more
+	 * than one sensor can be transitioned from off to on, select the
+	 * maximum time from each of the sensors changing from off to on.
+	 */
+	sleepval_ms = 0;
+	if (temp && !oldtemp)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
+
+	if (accel && !oldaccel)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
+
+	if (gyro && !oldgyro)
+		sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
+
+	if (sleep_ms)
+		*sleep_ms = sleepval_ms;
+	else if (sleepval_ms)
+		fsleep(sleepval_ms * USEC_PER_MSEC);
+
+	return 0;
+}  
 
 
 int inv_icm42607_core_probe(struct regmap *regmap,

@@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (!st)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, st);
+
 	ret = devm_mutex_init(dev, &st->lock);
 	if (ret)
 		return ret;

@@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (ret)
 		return ret;
 

Quoted 11 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 350

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);  

Sashiko does put out some stuff here. Please take a look and work out or test if it is right (I think not but haven't checked that carefully!) From a quick look I think that the auto disabling of autosuspend does a rpm_idle() that should result in it suspending...

Quoted 2 lines

I see a few other drivers adding one more call to devm_pm_runtime_enable() so I'm going to see how that works out.

Quoted 8 lines

Obviously that didn't work, but digging in much deeper into the PM code I see that devm_pm_runtime_set_active_enabled() returns devm_pm_runtime_enable(), which sets devm_add_action_or_reset() on pm_runtime_disable_action(), which calls pm_runtime_dont_use_autosuspend() and pm_runtime_disable(). Shouldn't this work, or am I missing something? Basically when the driver detaches shouldn't this chain end up calling pm_runtime_dont_use_autosuspend()?

Agreed. That was how I read things. The only thing I didn't entirely confirm (states involved were fiddly enough I'd want to check it with the actual driver!) was that pm_runtime_dont_use_autosuspend() ends up calling rpm_idle() in this case.

Jonathan

Quoted 14 lines

Thank you,
Chris

Quoted 9 lines

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, · line 354

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1 -0

+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");  

Thank you,
Chris

From: Chris Morgan <[email protected]>

Add functions for reading temperature sensor data.

Signed-off-by Chris Morgan <[email protected]>

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 64f5d263de4f..644cd7f821b9 100644
@@ -162,6 +162,24 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
 	return 0;
 }
 
+int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
+			       unsigned int *sleep_ms)
+{
+	unsigned int val;
+	int ret;
+
+	val = FIELD_PREP(INV_ICM42607_TEMP_CONFIG0_FILTER_MASK,
+			 INV_ICM42607_FILTER_BW_34HZ);
+	ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
+				 INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
+	if (ret)
+		return ret;
+
+	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
+					  st->conf.accel.mode, enable,
+					  sleep_ms);
+}
 

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +4 -0

new file mode 100644
index 000000000000..9a60e1a478b0
 
+static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
+{
+	struct device *dev = regmap_get_device(st->map);
+	u8 raw[2];

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +16 -0

+	ret = inv_icm42607_set_temp_conf(st, st->conf.temp_en, NULL);
+	st->conf.temp_en = false;
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1,
+			       raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	*temp = get_unaligned_be16(raw);
+	if (*temp == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:49 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add functions for reading temperature sensor data.

Signed-off-by Chris Morgan <[email protected]>

Another sashiko reported thing. I'd definitely have missed this one and I think it is correct.

When I get caught up I'll post a thread to see if people feel we should generally just ask for Sashiko to reply on list.

Jonathan

Quoted 43 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0

index 64f5d263de4f..644cd7f821b9 100644
@@ -162,6 +162,24 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
 	return 0;
 }
 
+int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
+			       unsigned int *sleep_ms)
+{
+	unsigned int val;
+	int ret;
+
+	val = FIELD_PREP(INV_ICM42607_TEMP_CONFIG0_FILTER_MASK,
+			 INV_ICM42607_FILTER_BW_34HZ);
+	ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
+				 INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
+	if (ret)
+		return ret;
+
+	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
+					  st->conf.accel.mode, enable,
+					  sleep_ms);
+}
 

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +11 -0

new file mode 100644
index 000000000000..9a60e1a478b0
 
+static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
+{
+	struct device *dev = regmap_get_device(st->map);
+	u8 raw[2];
+	int ret;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c line 32

Quoted 3 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +3 -0

+	guard(mutex)(&st->lock);
+
+	st->conf.temp_en = true;

Sashiko points out (seems right) that this sets the internal state before the power management routine expects it to be set. So why is this here as opposed to just passing true into the function that follows?

Quoted 7 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +6 -0

+	ret = inv_icm42607_set_temp_conf(st, st->conf.temp_en, NULL);
+	st->conf.temp_en = false;
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1,

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c line 48

Quoted 10 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +10 -0

+			       raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	*temp = get_unaligned_be16(raw);
+	if (*temp == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}

Add icm42607 accelerometer sensor for icm42607.

drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +11 -0

+		if (i == odr)
+			break;
+	}
+	if (i >= ARRAY_SIZE(inv_icm42607_accel_odr))
+		return -EINVAL;
+
+	*val = inv_icm42607_accel_odr[i][0];
+	*val2 = inv_icm42607_accel_odr[i][1];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}

drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +18 -0

+		if (val == inv_icm42607_accel_odr[idx][0] &&
+		    val2 == inv_icm42607_accel_odr[idx][1])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42607_accel_odr))
+		return -EINVAL;
+
+	conf.odr = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_accel_conf(st, &conf, NULL);
+}

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +19 -0

+	if (conf->filter != oldconf->filter) {
+		if (conf->mode == INV_ICM42607_SENSOR_MODE_LOW_POWER) {
+			val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG1_AVG_MASK, conf->filter);
+			ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
+						 INV_ICM42607_ACCEL_CONFIG1_AVG_MASK, val);
+		} else {
+			val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK,
+					 conf->filter);
+			ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
+						 INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK, val);
+		}
+		if (ret)
+			return ret;
+		oldconf->filter = conf->filter;
+	}
+
+	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
+					  st->conf.temp_en, sleep_ms);
+}

From: Chris Morgan <[email protected]>

Add icm42607 accelerometer sensor for icm42607.

Signed-off-by Chris Morgan <[email protected]>

Add gyroscope functions to the icm42607 driver.

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
+			       struct inv_icm42607_sensor_conf *conf,
+			       unsigned int *sleep_ms)
+{
+	struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
+	unsigned int val;

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +25 -0

+	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
+				 conf->fs);
+		val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
+				  conf->odr);
+		ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+		if (ret)
+			return ret;
+		oldconf->fs = conf->fs;
+		oldconf->odr = conf->odr;
+	}
+
+	if (conf->filter != oldconf->filter) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
+				 conf->filter);
+		ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+					 INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+		if (ret)
+			return ret;
+		oldconf->filter = conf->filter;
+	}
+
+	return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
+					  st->conf.temp_en, sleep_ms);
+}

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +12 -0

+		if (i == odr)
+			break;
+	}
+	if (i >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	*val = inv_icm42607_gyro_odr[i][0];
+	*val2 = inv_icm42607_gyro_odr[i][1];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +8 -0

+		if (val == inv_icm42607_gyro_odr[idx][0] &&
+		    val2 == inv_icm42607_gyro_odr[idx][1])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	conf.odr = idx;

Quoted 1 line · Chris Morgan

Add gyroscope functions to the icm42607 driver.

...

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6 -0

+int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
+			       struct inv_icm42607_sensor_conf *conf,
+			       unsigned int *sleep_ms)
+{
+	struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
+	unsigned int val;

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st, · line 236

Quoted 10 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +10 -0

+	int ret;
+
+	if (conf->mode < 0)
+		conf->mode = oldconf->mode;
+	if (conf->fs < 0)
+		conf->fs = oldconf->fs;
+	if (conf->odr < 0)
+		conf->odr = oldconf->odr;
+	if (conf->filter < 0)
+		conf->filter = oldconf->filter;

Same comment as per previous patch. But looking at this, can you rather have a helper that answers the below two questions? Something like

void _assign_conf(..., bool *write_odr, bool *write_filter)

{
	...
}

EXPORT_...

in the core driver? But wight both approaches and choose either existing one (as in this patch series) or what I suggested.

Quoted 26 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +25 -0

+	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
+				 conf->fs);
+		val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
+				  conf->odr);
+		ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+		if (ret)
+			return ret;
+		oldconf->fs = conf->fs;
+		oldconf->odr = conf->odr;
+	}
+
+	if (conf->filter != oldconf->filter) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
+				 conf->filter);
+		ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+					 INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+		if (ret)
+			return ret;
+		oldconf->filter = conf->filter;
+	}
+
+	return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
+					  st->conf.temp_en, sleep_ms);
+}

...

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c line 191

Quoted 9 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +9 -0

+{
+	unsigned int odr;
+	unsigned int i;
+
+	guard(mutex)(&st->lock);
+
+	odr = st->conf.gyro.odr;
+
+	for (i = 5; i < ARRAY_SIZE(inv_icm42607_gyro_odr); ++i) {

Same comment, why pre-increment?

Quoted 13 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +12 -0

+		if (i == odr)
+			break;
+	}
+	if (i >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	*val = inv_icm42607_gyro_odr[i][0];
+	*val2 = inv_icm42607_gyro_odr[i][1];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c line 213

Quoted 10 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +10 -0

+static int inv_icm42607_gyro_write_odr(struct iio_dev *indio_dev,
+				       int val, int val2)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int idx;
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	int ret;
+
+	for (idx = 5; idx < ARRAY_SIZE(inv_icm42607_gyro_odr); ++idx) {

Ditto.

Quoted 12 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +12 -0

+		if (val == inv_icm42607_gyro_odr[idx][0] &&
+		    val2 == inv_icm42607_gyro_odr[idx][1])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	conf.odr = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c line 168

Quoted 6 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +6 -0

+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_gyro_conf(st, &conf, NULL);
+}

Can be some of the code deduplicated between gyro and accel?

Signature · 3 lines

-- 
With Best Regards,
Andy Shevchenko

Quoted 1 line · Andy Shevchenko

On Mon, Jun 15, 2026 at 12:25:51PM -0500, Chris Morgan wrote:

Quoted 1 line

Can be some of the code deduplicated between gyro and accel?

Probably a fair amount, but the deduplication will likely need to be undone somewhat if we get buffer, WoM or apex support added back (I don't have any devices with such functionality, so if anyone will do it then it won't be me). I can refactor more if you want, or we can keep it split like this to make it easy if someone else wants to tackle the buffers/IRQs stuff later? Your call.

Quoted 2 lines · Chris Morgan

Quoted 1 line · Andy Shevchenko

On Mon, Jun 15, 2026 at 12:25:51PM -0500, Chris Morgan wrote:

...

Quoted 8 lines

Quoted 1 line

Can be some of the code deduplicated between gyro and accel?

Probably a fair amount, but the deduplication will likely need to be undone somewhat if we get buffer, WoM or apex support added back (I don't have any devices with such functionality, so if anyone will do it then it won't be me). I can refactor more if you want, or we can keep it split like this to make it easy if someone else wants to tackle the buffers/IRQs stuff later? Your call.

Just asking. Jonathan, David, Nuno, what's your opinion on this? Personal opinion is to avoid solving the issues that do not exist. If you are not committing into those features, let's not prepare driver for them right now.

Quoted 15 lines · On Tue, 23 Jun 2026 09:52:36 +0300 Andy Shevchenko…

Quoted 2 lines · Chris Morgan

Quoted 1 line · Andy Shevchenko

On Mon, Jun 15, 2026 at 12:25:51PM -0500, Chris Morgan wrote:

...

Quoted 8 lines

Quoted 1 line

Can be some of the code deduplicated between gyro and accel?

Probably a fair amount, but the deduplication will likely need to be undone somewhat if we get buffer, WoM or apex support added back (I don't have any devices with such functionality, so if anyone will do it then it won't be me). I can refactor more if you want, or we can keep it split like this to make it easy if someone else wants to tackle the buffers/IRQs stuff later? Your call.

Quoted 5 lines

Just asking. Jonathan, David, Nuno, what's your opinion on this? Personal opinion is to avoid solving the issues that do not exist. If you are not committing into those features, let's not prepare driver for them right now.

Agreed. My only exception to this is normally when someone tells me they have the code and plan to post a follow up set very shortly. Then I get a bit more relaxed as they chances of 'preparing' for the future being relevant become much higher!

Jonathan

From: Chris Morgan <[email protected]>

Add gyroscope functions to the icm42607 driver.

Signed-off-by Chris Morgan <[email protected]>

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +42 -0

index eb239987a1ce..23ca7529825c 100644
@@ -218,6 +218,49 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,
 					  st->conf.temp_en, sleep_ms);
 }
 
+int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
+			       struct inv_icm42607_sensor_conf *conf,
+			       unsigned int *sleep_ms)
+{
+	struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
+	unsigned int val;
+	int ret;
+
+	if (conf->mode < 0)
+		conf->mode = oldconf->mode;
+	if (conf->fs < 0)
+		conf->fs = oldconf->fs;
+	if (conf->odr < 0)
+		conf->odr = oldconf->odr;
+	if (conf->filter < 0)
+		conf->filter = oldconf->filter;
+
+	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
+				 conf->fs);
+		val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
+				  conf->odr);
+		ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+		if (ret)
+			return ret;
+		oldconf->fs = conf->fs;
+		oldconf->odr = conf->odr;
+	}
+
+	if (conf->filter != oldconf->filter) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
+				 conf->filter);
+		ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+					 INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+		if (ret)
+			return ret;
+		oldconf->filter = conf->filter;
+	}
+
+	return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
+					  st->conf.temp_en, sleep_ms);
+}
 

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +30 -0

new file mode 100644
index 000000000000..ef73560b39d7
 
+static int inv_icm42607_gyro_read_sensor(struct iio_dev *indio_dev,
+					 struct iio_chan_spec const *chan,
+					 s16 *val)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int reg;
+	u8 data[2];
+	int ret;
+
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = INV_ICM42607_REG_GYRO_DATA_X1;
+		break;
+	case IIO_MOD_Y:
+		reg = INV_ICM42607_REG_GYRO_DATA_Y1;
+		break;
+	case IIO_MOD_Z:
+		reg = INV_ICM42607_REG_GYRO_DATA_Z1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +14 -0

+	if (ret)
+		return ret;
+
+	/* read gyro register data */
+	ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
+	if (ret)
+		return ret;
+
+	*val = get_unaligned_be16(data);
+	if (*val == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}

Quoted 5 lines · On Mon, 15 Jun 2026 12:25:51 -0500 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

Add gyroscope functions to the icm42607 driver.

Signed-off-by Chris Morgan <[email protected]>

There is another bit of sashiko stuff in here. Please take a look
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com

I think it is correct about there being a path in which the gyro ends up always enabled along side anything else after the first read.

In general there seems to be a bit of mix on whether the caller or the power management function should be responsible for the caching of state.

I know you look at Sashiko so I could just have waited a bit, but I was reviewing anyway so took a look and having done that might as well highlight some stuff!

Jonathan

Quoted 87 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +42 -0

index eb239987a1ce..23ca7529825c 100644
@@ -218,6 +218,49 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,
 					  st->conf.temp_en, sleep_ms);
 }
 
+int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
+			       struct inv_icm42607_sensor_conf *conf,
+			       unsigned int *sleep_ms)
+{
+	struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
+	unsigned int val;
+	int ret;
+
+	if (conf->mode < 0)
+		conf->mode = oldconf->mode;
+	if (conf->fs < 0)
+		conf->fs = oldconf->fs;
+	if (conf->odr < 0)
+		conf->odr = oldconf->odr;
+	if (conf->filter < 0)
+		conf->filter = oldconf->filter;
+
+	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
+				 conf->fs);
+		val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
+				  conf->odr);
+		ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+		if (ret)
+			return ret;
+		oldconf->fs = conf->fs;
+		oldconf->odr = conf->odr;
+	}
+
+	if (conf->filter != oldconf->filter) {
+		val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
+				 conf->filter);
+		ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+					 INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+		if (ret)
+			return ret;
+		oldconf->filter = conf->filter;
+	}
+
+	return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
+					  st->conf.temp_en, sleep_ms);
+}
 

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +30 -0

new file mode 100644
index 000000000000..ef73560b39d7
 
+static int inv_icm42607_gyro_read_sensor(struct iio_dev *indio_dev,
+					 struct iio_chan_spec const *chan,
+					 s16 *val)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int reg;
+	u8 data[2];
+	int ret;
+
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = INV_ICM42607_REG_GYRO_DATA_X1;
+		break;
+	case IIO_MOD_Y:
+		reg = INV_ICM42607_REG_GYRO_DATA_Y1;
+		break;
+	case IIO_MOD_Z:
+		reg = INV_ICM42607_REG_GYRO_DATA_Z1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c line 102

Quoted 10 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +10 -0

+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	/* enable gyro sensor */
+	conf.mode = gyro_st->power_mode;
+	conf.filter = gyro_st->filter;
+	ret = inv_icm42607_set_gyro_conf(st, &conf, NULL);

The sashiko report is basically:

This turns it on, and runtime pm will turn it off but the state cached ends up such that it is turned on again for any runtime pm resume.

Quoted 5 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +4 -0

+	if (ret)
+		return ret;
+
+	/* read gyro register data */

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c line 116

Quoted 10 lines

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +10 -0

+	ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
+	if (ret)
+		return ret;
+
+	*val = get_unaligned_be16(data);
+	if (*val == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}