[PATCH 0/5] iio: imu: New driver for InvenSense ICM-20948 9-Axis sensor

[PATCH 0/5] iio: imu: New driver for InvenSense ICM-20948 9-Axis sensor

[Bharadwaj Raju]

This series adds a driver for the InvenSense ICM-20948 9-axis IMU.

The IMU includes a gyrometer, accelerometer, magnetometer, and a
temperature sensor.

For now, this series only adds support for the gyrometer and temperature
sensor, and doesn't yet support the FIFO features.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
Bharadwaj Raju (5):
      dt-bindings: iio: imu: Add ICM-20948
      iio: imu: add inv_icm20948
      iio: imu: icm20948: add support for gyroscope
      MAINTAINERS: add entry for inv_icm20948 driver
      iio: imu: icm20948: add runtime power management support

 .../bindings/iio/imu/invensense,icm20948.yaml      |  36 +++
 MAINTAINERS                                        |   7 +
 drivers/iio/imu/Kconfig                            |   1 +
 drivers/iio/imu/Makefile                           |   1 +
 drivers/iio/imu/inv_icm20948/Kconfig               |  17 +
 drivers/iio/imu/inv_icm20948/Makefile              |  10 +
 drivers/iio/imu/inv_icm20948/inv_icm20948.h        |  92 ++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_core.c   | 128 ++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c   | 355 +++++++++++++++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c    |  48 +++
 drivers/iio/imu/inv_icm20948/inv_icm20948_power.c  |  73 +++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c   | 115 +++++++
 12 files changed, 883 insertions(+)
---
base-commit: 8742b2d8935f476449ef37e263bc4da3295c7b58
change-id: 20250825-icm20948-07fcc432a458

Best regards,
-- 
Bharadwaj Raju <bharadwaj.raju777@gmail.com>

[PATCH 1/5] dt-bindings: iio: imu: Add ICM-20948

[Bharadwaj Raju]

Document device-tree bindings for the InvenSense ICM-20948 device.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
 .../bindings/iio/imu/invensense,icm20948.yaml      | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm20948.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm20948.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..00c8210a0285972f8084137c69a4618a1f674485
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm20948.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/invensense,icm20948.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: InvenSense ICM-20948 Inertial Measurement Unit
+
+maintainers:
+  - Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+
+description: |
+  9-axis motion-tracking device that combines a 3-axis gyroscope, 3-axis
+  accelerometer, and a 3-axis magnetometer.
+
+  https://invensense.tdk.com/wp-content/uploads/2024/03/DS-000189-ICM-20948-v1.6.pdf
+
+properties:
+  compatible:
+    enum:
+      - invensense,icm20948
+
+  reg:
+    maxItems: 1
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      icm20948 {
+        compatible = "invensense,icm20948";
+        reg = <0x69>;
+      }
+    }

-- 
2.51.0

Re: [PATCH 1/5] dt-bindings: iio: imu: Add ICM-20948

[Krzysztof Kozlowski]

On 30/08/2025 20:42, Bharadwaj Raju wrote:
> +description: |
> +  9-axis motion-tracking device that combines a 3-axis gyroscope, 3-axis
> +  accelerometer, and a 3-axis magnetometer.
> +
> +  https://invensense.tdk.com/wp-content/uploads/2024/03/DS-000189-ICM-20948-v1.6.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - invensense,icm20948
> +

That's pretty incomplete. See existing invense bindings. Explain in
commit msg how this is different than existing devices.

> +  reg:
> +    maxItems: 1
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      icm20948 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).


> +        compatible = "invensense,icm20948";
> +        reg = <0x69>;
> +      }
> +    }
> 


Best regards,
Krzysztof

Re: [PATCH 1/5] dt-bindings: iio: imu: Add ICM-20948

[Krzysztof Kozlowski]

On Sun, Aug 31, 2025 at 12:12:45AM +0530, Bharadwaj Raju wrote:
> +properties:
> +  compatible:
> +    enum:
> +      - invensense,icm20948
> +
> +  reg:
> +    maxItems: 1
> +

... and this was never tested. Missing additional props.

> +examples:

Best regards,
Krzysztof

[PATCH 2/5] iio: imu: add inv_icm20948

[Bharadwaj Raju]

Core parts of the new ICM20948 driver.

Add register definitions, probing, setup, and an IIO device for
reading the onboard temperature sensor.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
 drivers/iio/imu/Kconfig                          |   1 +
 drivers/iio/imu/Makefile                         |   1 +
 drivers/iio/imu/inv_icm20948/Kconfig             |  17 ++++
 drivers/iio/imu/inv_icm20948/Makefile            |   8 ++
 drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  47 +++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_core.c | 122 +++++++++++++++++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c  |  48 +++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c | 108 ++++++++++++++++++++
 8 files changed, 352 insertions(+)

diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 15612f0f189b5114deb414ef840339678abdc562..d59e5b0087398cfbd2719ca914fd147ab067155f 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -109,6 +109,7 @@ config KMX61
 	  be called kmx61.
 
 source "drivers/iio/imu/inv_icm42600/Kconfig"
+source "drivers/iio/imu/inv_icm20948/Kconfig"
 source "drivers/iio/imu/inv_mpu6050/Kconfig"
 
 config SMI240
diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
index e901aea498d37e5897e8b71268356a19eac2cb59..79e49bae59038c1ca1d54a64cf49b6ca5f57cb0b 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_FXOS8700_I2C) += fxos8700_i2c.o
 obj-$(CONFIG_FXOS8700_SPI) += fxos8700_spi.o
 
 obj-y += inv_icm42600/
+obj-y += inv_icm20948/
 obj-y += inv_mpu6050/
 
 obj-$(CONFIG_KMX61) += kmx61.o
diff --git a/drivers/iio/imu/inv_icm20948/Kconfig b/drivers/iio/imu/inv_icm20948/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..79ffbe273e71f27ec33fffa0286eafd7dd11aa29
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+config INV_ICM20948
+	tristate
+
+config INV_ICM20948_I2C
+	tristate "InvenSense ICM-20948 I2C driver"
+	depends on I2C
+	select INV_ICM20948
+	select REGMAP_I2C
+	help
+	  This driver supports the InvenSense ICM-20948 motion tracking
+	  device over I2C.
+
+	  This driver can be built as a module. The module will be called
+	  inv-icm20948-i2c.
+
diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..c508c2dc3eee2c32be20067e3e0868a203d8aa1a
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
+inv-icm20948-y += inv_icm20948_core.o
+inv-icm20948-y += inv_icm20948_temp.o
+
+obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
+inv-icm20948-i2c-y += inv_icm20948_i2c.o
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
new file mode 100644
index 0000000000000000000000000000000000000000..f9830645fbe96fd02eef7c54d1e5908647d5a0fe
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#ifndef INV_ICM20948_H_
+#define INV_ICM20948_H_
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/err.h>
+
+/* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
+#define INV_ICM20948_SLEEP_WAKEUP_MS 35
+
+#define INV_ICM20948_REG_BANK_SEL 0x7F
+#define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
+
+#define INV_ICM20948_REG_WHOAMI 0x0000
+#define INV_ICM20948_WHOAMI 0xEA
+
+#define INV_ICM20948_REG_FIFO_RW 0x0072
+
+#define INV_ICM20948_REG_PWR_MGMT_1 0x0006
+#define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
+#define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
+
+#define INV_ICM20948_REG_TEMP_DATA 0x0039
+
+extern const struct regmap_config inv_icm20948_regmap_config;
+
+struct inv_icm20948_state {
+	struct device *dev;
+	struct regmap *regmap;
+	struct iio_dev *temp_dev;
+	struct mutex lock;
+};
+
+extern int inv_icm20948_core_probe(struct regmap *regmap);
+
+struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
+
+#endif
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
new file mode 100644
index 0000000000000000000000000000000000000000..ee9e4159cffa261f0326b146a4b3df2cbfbd7697
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#include "inv_icm20948.h"
+
+static const struct regmap_range_cfg inv_icm20948_regmap_ranges[] = {
+	{
+		.name = "user banks",
+		.range_min = 0x0000,
+		.range_max = 0x3FFF,
+		.selector_reg = INV_ICM20948_REG_BANK_SEL,
+		.selector_mask = INV_ICM20948_BANK_SEL_MASK,
+		.window_start = 0,
+		.window_len = 0x1000,
+	},
+};
+
+static const struct regmap_range inv_icm20948_regmap_volatile_yes_ranges[] = {
+	/* WHOAMI */
+	regmap_reg_range(0x0000, 0x0000),
+	/* PWR_MGMT_1 */
+	regmap_reg_range(0x0006, 0x0006),
+	/* I2C and INT status */
+	regmap_reg_range(0x0017, 0x001C),
+	/* Sensor readouts */
+	regmap_reg_range(0x0028, 0x0052),
+	/* FIFO count and data */
+	regmap_reg_range(0x0070, 0x0072),
+	/* Data ready status */
+	regmap_reg_range(0x0074, 0x0074),
+	/* GYRO_CONFIG_1 */
+	regmap_reg_range(0x2001, 0x2001),
+	/* I2C SLV4 data in */
+	regmap_reg_range(0x307F, 0x307F),
+};
+
+static const struct regmap_access_table inv_icm20948_regmap_volatile_accesses = {
+	.yes_ranges = inv_icm20948_regmap_volatile_yes_ranges,
+	.n_yes_ranges = ARRAY_SIZE(inv_icm20948_regmap_volatile_yes_ranges),
+};
+
+static const struct regmap_range inv_icm20948_rd_noinc_no_ranges[] = {
+	regmap_reg_range(0x0000, INV_ICM20948_REG_FIFO_RW - 1),
+	regmap_reg_range(INV_ICM20948_REG_FIFO_RW + 1, 0x3FFF),
+};
+
+static const struct regmap_access_table inv_icm20948_regmap_rd_noinc_table = {
+	.no_ranges = inv_icm20948_rd_noinc_no_ranges,
+	.n_no_ranges = ARRAY_SIZE(inv_icm20948_rd_noinc_no_ranges),
+};
+
+const struct regmap_config inv_icm20948_regmap_config = {
+	.name = "inv_icm20948",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x3FFF,
+	.ranges = inv_icm20948_regmap_ranges,
+	.num_ranges = ARRAY_SIZE(inv_icm20948_regmap_ranges),
+	.volatile_table = &inv_icm20948_regmap_volatile_accesses,
+	.rd_noinc_table = &inv_icm20948_regmap_rd_noinc_table,
+	.cache_type = REGCACHE_MAPLE,
+};
+EXPORT_SYMBOL_NS_GPL(inv_icm20948_regmap_config, "IIO_ICM20948");
+
+static int inv_icm20948_setup(struct inv_icm20948_state *state)
+{
+	guard(mutex)(&state->lock);
+
+	int reported_whoami;
+	int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
+			      &reported_whoami);
+	if (ret)
+		return ret;
+	if (reported_whoami != INV_ICM20948_WHOAMI) {
+		dev_err(state->dev, "invalid whoami %d, expected %d\n",
+			reported_whoami, INV_ICM20948_WHOAMI);
+		return -ENODEV;
+	}
+
+	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+				INV_ICM20948_PWR_MGMT_1_DEV_RESET,
+				INV_ICM20948_PWR_MGMT_1_DEV_RESET);
+	if (ret)
+		return ret;
+	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
+
+	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+				INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
+	if (ret)
+		return ret;
+	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
+
+	state->temp_dev = inv_icm20948_temp_init(state);
+	if (IS_ERR(state->temp_dev))
+		return PTR_ERR(state->temp_dev);
+
+	return 0;
+}
+
+int inv_icm20948_core_probe(struct regmap *regmap)
+{
+	struct device *dev = regmap_get_device(regmap);
+
+	struct inv_icm20948_state *state;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->regmap = regmap;
+	state->dev = dev;
+
+	mutex_init(&state->lock);
+
+	return inv_icm20948_setup(state);
+}
+
+MODULE_AUTHOR("Bharadwaj Raju <bharadwaj.raju777@gmail.com>");
+MODULE_DESCRIPTION("InvenSense ICM-20948 device driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c
new file mode 100644
index 0000000000000000000000000000000000000000..cf04d82e014a2497592c9a15bbde6e36f431dd56
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/property.h>
+
+#include "inv_icm20948.h"
+
+static int inv_icm20948_probe(struct i2c_client *client)
+{
+	struct regmap *regmap =
+		devm_regmap_init_i2c(client, &inv_icm20948_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return inv_icm20948_core_probe(regmap);
+}
+
+static const struct i2c_device_id inv_icm20948_id[] = { { "icm20948" }, {} };
+MODULE_DEVICE_TABLE(i2c, inv_icm20948_id);
+
+static const struct of_device_id inv_icm20948_of_matches[] = {
+	{ .compatible = "invensense,icm20948" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, inv_icm20948_of_matches);
+
+static struct i2c_driver inv_icm20948_driver = {
+	.driver = {
+		.name = "icm20948",
+		.of_match_table = inv_icm20948_of_matches,
+	},
+	.probe = inv_icm20948_probe,
+	.id_table = inv_icm20948_id,
+};
+module_i2c_driver(inv_icm20948_driver);
+
+MODULE_AUTHOR("Bharadwaj Raju <bharadwaj.raju777@gmail.com>");
+MODULE_DESCRIPTION("InvenSense ICM-20948 device driver (I2C)");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_ICM20948");
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
new file mode 100644
index 0000000000000000000000000000000000000000..916053740cc5acda0316c76504d4086eff5ec7f0
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#include <linux/bits.h>
+
+#include <linux/iio/iio.h>
+
+#include "inv_icm20948.h"
+
+static const struct iio_chan_spec
+	inv_icm20948_temp_chan = { .type = IIO_TEMP,
+				   .info_mask_separate =
+					   BIT(IIO_CHAN_INFO_RAW) |
+					   BIT(IIO_CHAN_INFO_OFFSET) |
+					   BIT(IIO_CHAN_INFO_SCALE),
+				   .scan_index = 0,
+				   .scan_type = {
+					   .sign = 's',
+					   .realbits = 16,
+				   } };
+
+static int inv_icm20948_temp_read_sensor(struct inv_icm20948_state *state,
+					 s16 *temp)
+{
+	guard(mutex)(&state->lock);
+
+	__be16 raw;
+	int ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
+				   &raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	*temp = __be16_to_cpu(raw);
+
+	return 0;
+}
+
+static int inv_icm20948_temp_read_raw(struct iio_dev *temp_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val, int *val2, long mask)
+{
+	struct inv_icm20948_state *state = iio_device_get_drvdata(temp_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(temp_dev))
+			return -EBUSY;
+		s16 temp;
+		int ret = inv_icm20948_temp_read_sensor(state, &temp);
+
+		if (ret)
+			return ret;
+		iio_device_release_direct(temp_dev);
+		*val = temp;
+		return IIO_VAL_INT;
+	/*
+	 * Sensitivity = 333.87
+	 * RoomTempOff = 21
+	 * T_degC = ((T_raw - RoomTempOff) / Sensitivity) + RoomTempOff
+	 * T_degC = ((T_raw - 21) / 333.87) + 21
+	 * T_milliDegC = 1000 * (((T_raw - 21) / 333.87) + 21)
+	 * T_milliDegC = (1000 / 333.87) * (T_raw - 21 + (21 * 333.87))
+	 * T_milliDegC = (T_raw + 6990.27) * 2.995177
+
+	 * scale = 2.995177
+	 * offset = 6990.27
+	 */
+	case IIO_CHAN_INFO_SCALE:
+		*val = 2;
+		*val2 = 995177;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 6990;
+		*val2 = 270000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info inv_icm20948_temp_info = {
+	.read_raw = inv_icm20948_temp_read_raw,
+};
+
+struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state)
+{
+	struct iio_dev *temp_dev = devm_iio_device_alloc(state->dev, 0);
+
+	if (!temp_dev)
+		return ERR_PTR(-ENOMEM);
+
+	iio_device_set_drvdata(temp_dev, state);
+
+	temp_dev->name = "icm20948-temp";
+	temp_dev->info = &inv_icm20948_temp_info;
+	temp_dev->modes = INDIO_DIRECT_MODE;
+	temp_dev->channels = &inv_icm20948_temp_chan;
+	temp_dev->num_channels = 1;
+
+	int ret = devm_iio_device_register(state->dev, temp_dev);
+
+	if (ret)
+		return ERR_PTR(ret);
+
+	return temp_dev;
+}

-- 
2.51.0

Re: [PATCH 2/5] iio: imu: add inv_icm20948

[Andy Shevchenko]

On Sat, Aug 30, 2025 at 9:43 PM Bharadwaj Raju
<bharadwaj.raju777@gmail.com> wrote:
>
> Core parts of the new ICM20948 driver.
>
> Add register definitions, probing, setup, and an IIO device for
> reading the onboard temperature sensor.

...

>  drivers/iio/imu/Kconfig                          |   1 +
>  drivers/iio/imu/Makefile                         |   1 +
>  drivers/iio/imu/inv_icm20948/Kconfig             |  17 ++++
>  drivers/iio/imu/inv_icm20948/Makefile            |   8 ++
>  drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  47 +++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_core.c | 122 +++++++++++++++++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c  |  48 +++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c | 108 ++++++++++++++++++++
>  8 files changed, 352 insertions(+)

This looks strange. The few files are created here, and there is no
comment like "new file ..."
That said, the MAINTAINER is not updated here and hence the file will
be orphaned till the MAINTAINER changes. So, update incrementally
MAINTAINERS starting from the first patch.

...

> +#ifndef INV_ICM20948_H_
> +#define INV_ICM20948_H_
> +
> +#include <linux/bits.h>

> +#include <linux/bitfield.h>

Unused.

> +#include <linux/mutex.h>

> +#include <linux/regmap.h>

Can be replaced with proper forward declarations.

> +#include <linux/i2c.h>

Unused

> +#include <linux/iio/iio.h>

Forward declaration.

> +#include <linux/err.h>

Unused.

Keep it ordered and follow the IWYU principle (include what you use).
Currently this is a semi-random list of the inclusions. And missed
forward declarations.

...

> +#define INV_ICM20948_REG_BANK_SEL 0x7F

Make all register offsets to be fixed-width, like 0x007F

> +#define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
> +
> +#define INV_ICM20948_REG_WHOAMI 0x0000
> +#define INV_ICM20948_WHOAMI 0xEA
> +
> +#define INV_ICM20948_REG_FIFO_RW 0x0072
> +
> +#define INV_ICM20948_REG_PWR_MGMT_1 0x0006
> +#define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
> +#define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
> +
> +#define INV_ICM20948_REG_TEMP_DATA 0x0039
> +
> +extern const struct regmap_config inv_icm20948_regmap_config;
> +
> +struct inv_icm20948_state {
> +       struct device *dev;
> +       struct regmap *regmap;
> +       struct iio_dev *temp_dev;
> +       struct mutex lock;
> +};
> +
> +extern int inv_icm20948_core_probe(struct regmap *regmap);
> +
> +struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
> +
> +#endif

...

> +#include "inv_icm20948.h"

This file uses much more than the private header. Please, fix this properly.

...

> +static const struct regmap_range_cfg inv_icm20948_regmap_ranges[] = {
> +       {
> +               .name = "user banks",
> +               .range_min = 0x0000,
> +               .range_max = 0x3FFF,

Are you sure about range_min? This will cause circular caching and
unpleasant side-effects.

> +               .selector_reg = INV_ICM20948_REG_BANK_SEL,
> +               .selector_mask = INV_ICM20948_BANK_SEL_MASK,
> +               .window_start = 0,
> +               .window_len = 0x1000,
> +       },
> +};
> +
> +static const struct regmap_range inv_icm20948_regmap_volatile_yes_ranges[] = {
> +       /* WHOAMI */
> +       regmap_reg_range(0x0000, 0x0000),
> +       /* PWR_MGMT_1 */
> +       regmap_reg_range(0x0006, 0x0006),
> +       /* I2C and INT status */
> +       regmap_reg_range(0x0017, 0x001C),
> +       /* Sensor readouts */
> +       regmap_reg_range(0x0028, 0x0052),
> +       /* FIFO count and data */
> +       regmap_reg_range(0x0070, 0x0072),
> +       /* Data ready status */
> +       regmap_reg_range(0x0074, 0x0074),

So, the above are real, the below are virtual? How does it work?

> +       /* GYRO_CONFIG_1 */
> +       regmap_reg_range(0x2001, 0x2001),
> +       /* I2C SLV4 data in */
> +       regmap_reg_range(0x307F, 0x307F),
> +};

...

> +static const struct regmap_range inv_icm20948_rd_noinc_no_ranges[] = {
> +       regmap_reg_range(0x0000, INV_ICM20948_REG_FIFO_RW - 1),
> +       regmap_reg_range(INV_ICM20948_REG_FIFO_RW + 1, 0x3FFF),

Make 0x3fff a constant and use it everywhere.

> +};

...

> +const struct regmap_config inv_icm20948_regmap_config = {
> +       .name = "inv_icm20948",
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +       .max_register = 0x3FFF,

See above.

> +       .ranges = inv_icm20948_regmap_ranges,
> +       .num_ranges = ARRAY_SIZE(inv_icm20948_regmap_ranges),
> +       .volatile_table = &inv_icm20948_regmap_volatile_accesses,
> +       .rd_noinc_table = &inv_icm20948_regmap_rd_noinc_table,
> +       .cache_type = REGCACHE_MAPLE,
> +};

...

> +static int inv_icm20948_setup(struct inv_icm20948_state *state)
> +{
> +       guard(mutex)(&state->lock);
> +
> +       int reported_whoami;
> +       int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
> +                             &reported_whoami);

Just no. We almost do not allow a mixture of the definitions and code.

> +       if (ret)
> +               return ret;
> +       if (reported_whoami != INV_ICM20948_WHOAMI) {
> +               dev_err(state->dev, "invalid whoami %d, expected %d\n",
> +                       reported_whoami, INV_ICM20948_WHOAMI);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +                               INV_ICM20948_PWR_MGMT_1_DEV_RESET,
> +                               INV_ICM20948_PWR_MGMT_1_DEV_RESET);
> +       if (ret)
> +               return ret;

Any long sleeps (and even 1ms is already long on modern fast CPUs)
must be explained.

> +       msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> +
> +       ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +                               INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
> +       if (ret)
> +               return ret;

> +       msleep(INV_ICM20948_SLEEP_WAKEUP_MS);

Ditto.

> +       state->temp_dev = inv_icm20948_temp_init(state);

> +       if (IS_ERR(state->temp_dev))
> +               return PTR_ERR(state->temp_dev);
> +
> +       return 0;


return PTR_ERR_OR_ZERO(...);

> +}
> +
> +int inv_icm20948_core_probe(struct regmap *regmap)
> +{
> +       struct device *dev = regmap_get_device(regmap);

> +

Have you ever run checkpatch.pl? We do not allow blank lines in the
definition blocks.

> +       struct inv_icm20948_state *state;
> +
> +       state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> +       if (!state)
> +               return -ENOMEM;
> +
> +       state->regmap = regmap;
> +       state->dev = dev;

> +       mutex_init(&state->lock);

devm_mutex_init()

> +       return inv_icm20948_setup(state);
> +}

...

> +#include <linux/kernel.h>

This header must not be used in the regular drivers. Follow the IWYU principle.

> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>

Keep them ordered.

...

> +static int inv_icm20948_probe(struct i2c_client *client)
> +{
> +       struct regmap *regmap =
> +               devm_regmap_init_i2c(client, &inv_icm20948_regmap_config);
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);

This is wrong stylistically.
First of all, there must be a blank line between definition block and
the code, BUT in this case the code will be harder to maintain and it
will be prone to subtle errors, that's why it's recommended to split
definition and the assignment and move assignment closest to the
check.

> +       return inv_icm20948_core_probe(regmap);
> +}
> +
> +static const struct i2c_device_id inv_icm20948_id[] = { { "icm20948" }, {} };

No, please make it readable.

> +MODULE_DEVICE_TABLE(i2c, inv_icm20948_id);
> +
> +static const struct of_device_id inv_icm20948_of_matches[] = {
> +       { .compatible = "invensense,icm20948" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm20948_of_matches);
> +
> +static struct i2c_driver inv_icm20948_driver = {
> +       .driver = {
> +               .name = "icm20948",
> +               .of_match_table = inv_icm20948_of_matches,
> +       },
> +       .probe = inv_icm20948_probe,
> +       .id_table = inv_icm20948_id,
> +};
> +module_i2c_driver(inv_icm20948_driver);
> +
> +MODULE_AUTHOR("Bharadwaj Raju <bharadwaj.raju777@gmail.com>");
> +MODULE_DESCRIPTION("InvenSense ICM-20948 device driver (I2C)");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_ICM20948");

I stop here as I believe the whole series needs much more work to
follow IWYU, the style, coding standards, etc...
When you are done with that, come up with a new version.

Thanks!

--
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] iio: imu: add inv_icm20948

[Jonathan Cameron]

On Sun, 31 Aug 2025 00:12:46 +0530
Bharadwaj Raju <bharadwaj.raju777@gmail.com> wrote:

> Core parts of the new ICM20948 driver.
> 
> Add register definitions, probing, setup, and an IIO device for
> reading the onboard temperature sensor.
> 
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
Hi Bharadwaj,

I see you've already gotten some reviews on this so I'll take only
a fairly quick look.  I may also overlap somewhat with the other reviewers.

Biggest question is I'm not seeing why you need an IIO device just
for the on die (and that is important vs on board) temperature sensor.

Jonathan

> ---
>  drivers/iio/imu/Kconfig                          |   1 +
>  drivers/iio/imu/Makefile                         |   1 +
>  drivers/iio/imu/inv_icm20948/Kconfig             |  17 ++++
>  drivers/iio/imu/inv_icm20948/Makefile            |   8 ++
>  drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  47 +++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_core.c | 122 +++++++++++++++++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c  |  48 +++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c | 108 ++++++++++++++++++++
>  8 files changed, 352 insertions(+)
> 
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 15612f0f189b5114deb414ef840339678abdc562..d59e5b0087398cfbd2719ca914fd147ab067155f 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -109,6 +109,7 @@ config KMX61
>  	  be called kmx61.
>  
>  source "drivers/iio/imu/inv_icm42600/Kconfig"
> +source "drivers/iio/imu/inv_icm20948/Kconfig"

Alphabetical/numeric order.

>  source "drivers/iio/imu/inv_mpu6050/Kconfig"
>  
>  config SMI240
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index e901aea498d37e5897e8b71268356a19eac2cb59..79e49bae59038c1ca1d54a64cf49b6ca5f57cb0b 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_FXOS8700_I2C) += fxos8700_i2c.o
>  obj-$(CONFIG_FXOS8700_SPI) += fxos8700_spi.o
>  
>  obj-y += inv_icm42600/
> +obj-y += inv_icm20948/

here as well.

>  obj-y += inv_mpu6050/
>  
>  obj-$(CONFIG_KMX61) += kmx61.o

> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..f9830645fbe96fd02eef7c54d1e5908647d5a0fe
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> + */
> +
> +#ifndef INV_ICM20948_H_
> +#define INV_ICM20948_H_
> +
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/err.h>
> +
> +/* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
> +#define INV_ICM20948_SLEEP_WAKEUP_MS 35
> +
> +#define INV_ICM20948_REG_BANK_SEL 0x7F
> +#define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
> +
> +#define INV_ICM20948_REG_WHOAMI 0x0000
> +#define INV_ICM20948_WHOAMI 0xEA
> +
> +#define INV_ICM20948_REG_FIFO_RW 0x0072
> +
> +#define INV_ICM20948_REG_PWR_MGMT_1 0x0006
> +#define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
> +#define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
> +
> +#define INV_ICM20948_REG_TEMP_DATA 0x0039
> +
> +extern const struct regmap_config inv_icm20948_regmap_config;
> +
> +struct inv_icm20948_state {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct iio_dev *temp_dev;
> +	struct mutex lock;

Lock scope needs to always have a comment. What 'data' is this protecting
from concurrent accesses?

> +};
> +
> +extern int inv_icm20948_core_probe(struct regmap *regmap);
> +
> +struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
> +
> +#endif
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ee9e4159cffa261f0326b146a4b3df2cbfbd7697
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c

> +
> +static int inv_icm20948_setup(struct inv_icm20948_state *state)
> +{
> +	guard(mutex)(&state->lock);
> +
As below.
> +	int reported_whoami;
> +	int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
> +			      &reported_whoami);
> +	if (ret)
> +		return ret;
> +	if (reported_whoami != INV_ICM20948_WHOAMI) {
> +		dev_err(state->dev, "invalid whoami %d, expected %d\n",
> +			reported_whoami, INV_ICM20948_WHOAMI);

This breaks fallback compatibles used in device tree to support
newer parts that the driver has not yet been updated for, as long
as they are backwards compatible with older ones.
We still have some old drivers with hard checks like this, but current
practice is to at most print a message and then carry on anyway.

> +		return -ENODEV;
> +	}
> +
> +	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +				INV_ICM20948_PWR_MGMT_1_DEV_RESET,
> +				INV_ICM20948_PWR_MGMT_1_DEV_RESET);
> +	if (ret)
> +		return ret;
> +	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> +
> +	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +				INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
> +	if (ret)
> +		return ret;
> +	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> +
> +	state->temp_dev = inv_icm20948_temp_init(state);
> +	if (IS_ERR(state->temp_dev))
> +		return PTR_ERR(state->temp_dev);
> +
> +	return 0;
> +}
> +
> +int inv_icm20948_core_probe(struct regmap *regmap)
> +{
> +	struct device *dev = regmap_get_device(regmap);
> +
> +	struct inv_icm20948_state *state;
> +
> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);

Given question on why you need separate struct iio_dev instances below
I suspect this will change a lot and we won't end up with another state
structure here.

> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->regmap = regmap;
> +	state->dev = dev;
> +
> +	mutex_init(&state->lock);
	ret = devm_mutex_init()
	if (ret)
		return ret;

The advantages of this are small, but it costs us little so
lets enable the lock debugging stuff that the destroy_mutex() this
will call enables.

> +
> +	return inv_icm20948_setup(state);
> +}

> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..cf04d82e014a2497592c9a15bbde6e36f431dd56
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_i2c.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#include "inv_icm20948.h"
> +
> +static int inv_icm20948_probe(struct i2c_client *client)
> +{
> +	struct regmap *regmap =
> +		devm_regmap_init_i2c(client, &inv_icm20948_regmap_config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return inv_icm20948_core_probe(regmap);
> +}
> +
> +static const struct i2c_device_id inv_icm20948_id[] = { { "icm20948" }, {} };

static const struct i2c_device_id inv_icm20948_id[] = {
	{ "icm20948" },
	{ }
};

> +MODULE_DEVICE_TABLE(i2c, inv_icm20948_id);
> +
> +static const struct of_device_id inv_icm20948_of_matches[] = {
> +	{ .compatible = "invensense,icm20948" },
> +	{}
Trivial style preference that I'm trying to generalize
across IIO for
	{ }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm20948_of_matches);

> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..916053740cc5acda0316c76504d4086eff5ec7f0
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> + */
> +
> +#include <linux/bits.h>
> +
> +#include <linux/iio/iio.h>

Andy covered the need to follow include what you use principles for includes.

> +
> +#include "inv_icm20948.h"
> +
> +static const struct iio_chan_spec
> +	inv_icm20948_temp_chan = { .type = IIO_TEMP,
> +				   .info_mask_separate =
> +					   BIT(IIO_CHAN_INFO_RAW) |
> +					   BIT(IIO_CHAN_INFO_OFFSET) |
> +					   BIT(IIO_CHAN_INFO_SCALE),
> +				   .scan_index = 0,
> +				   .scan_type = {
> +					   .sign = 's',
> +					   .realbits = 16,
> +				   } };
> +
Preferred format is something like this.
static const struct iio_chan_spec inv_icm20948_temp_chan = {
	.type = IIO_TEMP,
	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
			      BIT(IIO_CHAN_INFO_OFFSET) |
			      BIT(IIO_CHAN_INFO_SCALE),
	.scan_index = 0,
	.scan_type = {
		.sign = 's',
		.realbits = 16,
	},
};
Note the trailing comma on scan_type that will make this easier to extent
in future.

> +
> +static int inv_icm20948_temp_read_raw(struct iio_dev *temp_dev,
> +				      struct iio_chan_spec const *chan,
> +				      int *val, int *val2, long mask)
> +{
> +	struct inv_icm20948_state *state = iio_device_get_drvdata(temp_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(temp_dev))
> +			return -EBUSY;
> +		s16 temp;
> +		int ret = inv_icm20948_temp_read_sensor(state, &temp);

As already pointed out we normally don't mix declarations and code.
There is an exception for cleanup.h cases but that doesn't apply here.

> +
> +		if (ret)
> +			return ret;
> +		iio_device_release_direct(temp_dev);
> +		*val = temp;
> +		return IIO_VAL_INT;

> +}

> +struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state)
> +{
> +	struct iio_dev *temp_dev = devm_iio_device_alloc(state->dev, 0);
> +
> +	if (!temp_dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	iio_device_set_drvdata(temp_dev, state);
> +
> +	temp_dev->name = "icm20948-temp";
> +	temp_dev->info = &inv_icm20948_temp_info;
> +	temp_dev->modes = INDIO_DIRECT_MODE;
> +	temp_dev->channels = &inv_icm20948_temp_chan;
> +	temp_dev->num_channels = 1;
> +
> +	int ret = devm_iio_device_register(state->dev, temp_dev);

It's unusual for it to make sense to register a separate iio device instance for the
temperature sensor in an IMU.  They tend to be there only to allow
temperature compensation based on measures of die temperature.
Why does it make sense for this device?  There are a few reasons
I could think of that might justify it but I took at quick read
of the datasheet and I can't see a reason to split it up at all. Looks
like one iio_dev will do for the whole thing.

Jonathan



> +
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return temp_dev;
> +}
> 

[PATCH 3/5] iio: imu: icm20948: add support for gyroscope

[Bharadwaj Raju]

Add support for reading the gyroscope, which is exposed as another IIO
device under the icm20948 driver.

For now, the only configuration supported is changing the full-scale
range.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
 drivers/iio/imu/inv_icm20948/Makefile            |   1 +
 drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  78 ++++--
 drivers/iio/imu/inv_icm20948/inv_icm20948_core.c |  55 ++--
 drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c | 343 +++++++++++++++++++++++
 4 files changed, 432 insertions(+), 45 deletions(-)

diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
index c508c2dc3eee2c32be20067e3e0868a203d8aa1a..88a37be159e1d6f575da1c070c84ac94cd963020 100644
--- a/drivers/iio/imu/inv_icm20948/Makefile
+++ b/drivers/iio/imu/inv_icm20948/Makefile
@@ -3,6 +3,7 @@
 obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
 inv-icm20948-y += inv_icm20948_core.o
 inv-icm20948-y += inv_icm20948_temp.o
+inv-icm20948-y += inv_icm20948_gyro.o
 
 obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
 inv-icm20948-i2c-y += inv_icm20948_i2c.o
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
index f9830645fbe96fd02eef7c54d1e5908647d5a0fe..ca2513114378cdcba5bc315fc94cd61f930b4dfa 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948.h
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
@@ -3,45 +3,83 @@
  * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
  */
 
-#ifndef INV_ICM20948_H_
-#define INV_ICM20948_H_
+ #ifndef INV_ICM20948_H_
+ #define INV_ICM20948_H_
 
-#include <linux/bits.h>
-#include <linux/bitfield.h>
-#include <linux/mutex.h>
-#include <linux/regmap.h>
-#include <linux/i2c.h>
-#include <linux/iio/iio.h>
-#include <linux/err.h>
+ #include <linux/bits.h>
+ #include <linux/bitfield.h>
+ #include <linux/mutex.h>
+ #include <linux/regmap.h>
+ #include <linux/i2c.h>
+ #include <linux/iio/iio.h>
+ #include <linux/err.h>
 
 /* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
-#define INV_ICM20948_SLEEP_WAKEUP_MS 35
+ #define INV_ICM20948_SLEEP_WAKEUP_MS 35
 
-#define INV_ICM20948_REG_BANK_SEL 0x7F
-#define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
+ #define INV_ICM20948_REG_BANK_SEL 0x7F
+ #define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
 
-#define INV_ICM20948_REG_WHOAMI 0x0000
-#define INV_ICM20948_WHOAMI 0xEA
+ #define INV_ICM20948_REG_WHOAMI 0x0000
+ #define INV_ICM20948_WHOAMI 0xEA
 
-#define INV_ICM20948_REG_FIFO_RW 0x0072
+ #define INV_ICM20948_REG_FIFO_RW 0x0072
 
-#define INV_ICM20948_REG_PWR_MGMT_1 0x0006
-#define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
-#define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
+ #define INV_ICM20948_REG_PWR_MGMT_1 0x0006
+ #define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
+ #define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
 
-#define INV_ICM20948_REG_TEMP_DATA 0x0039
+ #define INV_ICM20948_REG_TEMP_DATA 0x0039
+
+ #define INV_ICM20948_REG_GYRO_DATA_X 0x0033
+ #define INV_ICM20948_REG_GYRO_DATA_Y 0x0035
+ #define INV_ICM20948_REG_GYRO_DATA_Z 0x0037
+
+ #define INV_ICM20948_REG_GYRO_CONFIG_1 0x2001
+ #define INV_ICM20948_GYRO_CONFIG_ENABLE_DLPF BIT(0)
+ #define INV_ICM20948_GYRO_CONFIG_FULLSCALE GENMASK(2, 1)
+ #define INV_ICM20948_GYRO_CONFIG_DLP_CONFIG GENMASK(5, 3)
+
+ #define INV_ICM20948_REG_GYRO_USER_OFFSET_X 0x2003
+ #define INV_ICM20948_REG_GYRO_USER_OFFSET_Y 0x2005
+ #define INV_ICM20948_REG_GYRO_USER_OFFSET_Z 0x2007
 
 extern const struct regmap_config inv_icm20948_regmap_config;
 
+enum inv_icm20948_gyro_fs {
+	INV_ICM20948_GYRO_FS_250 = 0,
+	INV_ICM20948_GYRO_FS_500 = 1,
+	INV_ICM20948_GYRO_FS_1000 = 2,
+	INV_ICM20948_GYRO_FS_2000 = 3,
+};
+
+enum inv_icm20948_gyro_avg {
+	INV_ICM20948_GYRO_AVG_1X = 0,
+	INV_ICM20948_GYRO_AVG_2X = 1,
+	INV_ICM20948_GYRO_AVG_4X = 2,
+	INV_ICM20948_GYRO_AVG_8X = 3,
+	INV_ICM20948_GYRO_AVG_16X = 4,
+	INV_ICM20948_GYRO_AVG_32X = 5,
+	INV_ICM20948_GYRO_AVG_64X = 6,
+	INV_ICM20948_GYRO_AVG_128X = 7,
+};
+
+struct inv_icm20948_gyro_config {
+	int fsr;
+};
+
 struct inv_icm20948_state {
 	struct device *dev;
 	struct regmap *regmap;
 	struct iio_dev *temp_dev;
+	struct iio_dev *gyro_dev;
+	struct inv_icm20948_gyro_config *gyro_conf;
 	struct mutex lock;
 };
 
 extern int inv_icm20948_core_probe(struct regmap *regmap);
 
 struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
+struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state);
 
-#endif
+ #endif
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
index ee9e4159cffa261f0326b146a4b3df2cbfbd7697..eb4f940de7013bf4ddeb69b6380a60fbde49964a 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
@@ -3,7 +3,7 @@
  * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
  */
 
-#include "inv_icm20948.h"
+ #include "inv_icm20948.h"
 
 static const struct regmap_range_cfg inv_icm20948_regmap_ranges[] = {
 	{
@@ -66,36 +66,41 @@ EXPORT_SYMBOL_NS_GPL(inv_icm20948_regmap_config, "IIO_ICM20948");
 
 static int inv_icm20948_setup(struct inv_icm20948_state *state)
 {
-	guard(mutex)(&state->lock);
-
-	int reported_whoami;
-	int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
-			      &reported_whoami);
-	if (ret)
-		return ret;
-	if (reported_whoami != INV_ICM20948_WHOAMI) {
-		dev_err(state->dev, "invalid whoami %d, expected %d\n",
-			reported_whoami, INV_ICM20948_WHOAMI);
-		return -ENODEV;
+	scoped_guard(mutex, &state->lock) {
+		int reported_whoami;
+		int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
+				      &reported_whoami);
+		if (ret)
+			return ret;
+		if (reported_whoami != INV_ICM20948_WHOAMI) {
+			dev_err(state->dev, "invalid whoami %d, expected %d\n",
+				reported_whoami, INV_ICM20948_WHOAMI);
+			return -ENODEV;
+		}
+
+		ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+					INV_ICM20948_PWR_MGMT_1_DEV_RESET,
+					INV_ICM20948_PWR_MGMT_1_DEV_RESET);
+		if (ret)
+			return ret;
+		msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
+
+		ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+					INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
+		if (ret)
+			return ret;
+
+		msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
 	}
 
-	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
-				INV_ICM20948_PWR_MGMT_1_DEV_RESET,
-				INV_ICM20948_PWR_MGMT_1_DEV_RESET);
-	if (ret)
-		return ret;
-	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
-
-	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
-				INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
-	if (ret)
-		return ret;
-	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
-
 	state->temp_dev = inv_icm20948_temp_init(state);
 	if (IS_ERR(state->temp_dev))
 		return PTR_ERR(state->temp_dev);
 
+	state->gyro_dev = inv_icm20948_gyro_init(state);
+	if (IS_ERR(state->gyro_dev))
+		return PTR_ERR(state->gyro_dev);
+
 	return 0;
 }
 
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
new file mode 100644
index 0000000000000000000000000000000000000000..2d4d598eb21c8ce98d4ee3c72504554ab49ea596
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#include <linux/bits.h>
+
+#include <linux/iio/iio.h>
+
+#include "inv_icm20948.h"
+
+/* IIO int + nano format */
+static const int inv_icm20948_gyro_scale[] = {
+	/* 250 dps == 0.000133158 rad/s per LSB */
+	[2 * INV_ICM20948_GYRO_FS_250] = 0,
+	[2 * INV_ICM20948_GYRO_FS_250 + 1] = 133158,
+	/* 500 dps == 0.000266316 rad/s per LSB */
+	[2 * INV_ICM20948_GYRO_FS_500] = 0,
+	[2 * INV_ICM20948_GYRO_FS_500 + 1] = 266316,
+	/* 1000 dps == 0.000532632 rad/s per LSB */
+	[2 * INV_ICM20948_GYRO_FS_1000] = 0,
+	[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 532632,
+	/* 2000 dps == 0.001065264 rad/s per LSB */
+	[2 * INV_ICM20948_GYRO_FS_1000] = 0,
+	[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 1065264,
+};
+
+/* Calibration bias, IIO range format int + nano */
+/* raw value -2**15 to +2**15, 0.0305 dps per LSB step */
+static const int inv_icm20948_gyro_calibbias_range[] = {
+	-17, 443239423, /* min */
+	0,   532325, /* step */
+	+17, 443239423, /* max */
+};
+
+#define INV_ICM20948_GYRO_CHAN(_dir) \
+	{		\
+		.type = IIO_ANGL_VEL,		\
+		.modified = 1,		\
+		.channel2 = IIO_MOD_##_dir,		\
+		.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) |		\
+		  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 = INV_ICM20948_GYRO_SCAN_##_dir,		\
+		.scan_type = {		\
+			.sign = 's',		\
+			.realbits = 16,		\
+			.endianness = IIO_BE,		\
+		},		\
+	}
+
+enum inv_icm20948_gyro_scan {
+	INV_ICM20948_GYRO_SCAN_X,
+	INV_ICM20948_GYRO_SCAN_Y,
+	INV_ICM20948_GYRO_SCAN_Z,
+};
+
+static const struct iio_chan_spec inv_icm20948_gyro_channels[] = {
+	INV_ICM20948_GYRO_CHAN(X),
+	INV_ICM20948_GYRO_CHAN(Y),
+	INV_ICM20948_GYRO_CHAN(Z),
+};
+
+static int inv_icm20948_gyro_apply_config(struct inv_icm20948_state *state)
+{
+	guard(mutex)(&state->lock);
+
+	return regmap_write_bits(state->regmap, INV_ICM20948_REG_GYRO_CONFIG_1,
+				 INV_ICM20948_GYRO_CONFIG_FULLSCALE,
+				 state->gyro_conf->fsr << 1);
+}
+
+static int inv_icm20948_gyro_read_sensor(struct inv_icm20948_state *state,
+					 struct iio_chan_spec const *chan,
+					 s16 *val)
+{
+	unsigned int reg;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = INV_ICM20948_REG_GYRO_DATA_X;
+		break;
+	case IIO_MOD_Y:
+		reg = INV_ICM20948_REG_GYRO_DATA_Y;
+		break;
+	case IIO_MOD_Z:
+		reg = INV_ICM20948_REG_GYRO_DATA_Z;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	__be16 raw;
+	int ret = regmap_bulk_read(state->regmap, reg, &raw, sizeof(raw));
+
+	if (ret)
+		return ret;
+
+	*val = (s16)be16_to_cpu(raw);
+
+	return 0;
+}
+
+static int inv_icm20948_gyro_read_calibbias(struct inv_icm20948_state *state,
+					    struct iio_chan_spec const *chan,
+					    int *val, int *val2)
+{
+	guard(mutex)(&state->lock);
+
+	unsigned int reg;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_X;
+		break;
+	case IIO_MOD_Y:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Y;
+		break;
+	case IIO_MOD_Z:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Z;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	__be16 offset_raw;
+	int ret = regmap_bulk_read(state->regmap, reg, &offset_raw,
+				   sizeof(offset_raw));
+
+	if (ret)
+		return ret;
+	int offset = be16_to_cpu(offset_raw);
+
+	/* step size = 0.0305 dps/LSB => +/- 999.24 dps over 16-bit range */
+	/* offset * 0.0305 * Pi * 10**9 (for nano) / 180 */
+	/* offset * 95818575.93448868 / 180 */
+	/* offset * 95818576 / 180 */
+	s64 val64 = (s64)offset * 95818576;
+	/* for rounding, add or subtract divisor/2 */
+	if (val64 >= 0)
+		val64 += 180 / 2;
+	else
+		val64 -= 180 / 2;
+
+	s64 bias = div_s64(val64, 180);
+
+	*val = bias / 1000000000L;
+	*val2 = bias % 1000000000L;
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int inv_icm20948_gyro_read_raw(struct iio_dev *gyro_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val, int *val2, long mask)
+{
+	struct inv_icm20948_state *state = iio_device_get_drvdata(gyro_dev);
+
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(gyro_dev))
+			return -EBUSY;
+		s16 raw;
+		int ret = inv_icm20948_gyro_read_sensor(state, chan, &raw);
+
+		iio_device_release_direct(gyro_dev);
+		if (ret)
+			return ret;
+		*val = raw;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = inv_icm20948_gyro_scale[2 * state->gyro_conf->fsr + 1];
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return inv_icm20948_gyro_read_calibbias(state, chan, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm20948_gyro_write_scale(struct inv_icm20948_state *state,
+					 int val, int val2)
+{
+	/* all supported scales are < 1.0 and > 0.0 */
+	if (val != 0)
+		return -EINVAL;
+
+	int idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(inv_icm20948_gyro_scale); idx += 2) {
+		if (val2 == inv_icm20948_gyro_scale[idx + 1])
+			break;
+	}
+
+	if (idx >= ARRAY_SIZE(inv_icm20948_gyro_scale))
+		return -EINVAL;
+
+	state->gyro_conf->fsr = idx / 2;
+	return inv_icm20948_gyro_apply_config(state);
+}
+
+static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
+					struct iio_chan_spec const *chan,
+					int val, int val2)
+{
+	guard(mutex)(&state->lock);
+
+	unsigned int reg;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_X;
+		break;
+	case IIO_MOD_Y:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Y;
+		break;
+	case IIO_MOD_Z:
+		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Z;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	s64 bias = (s64)val * 100000000L + val2;
+	s64 val64 = bias * 180;
+
+	if (val64 >= 0)
+		val64 -= 180 / 2;
+	else
+		val64 += 180 / 2;
+
+	s64 offset64 = div_s64(val64, 95818576L);
+	s16 offset = clamp(offset64, (s64)S16_MIN, (s64)S16_MAX);
+	__be16 offset_write = cpu_to_be16(offset);
+
+	return regmap_bulk_write(state->regmap, reg, &offset_write,
+				 sizeof(offset_write));
+}
+
+static int inv_icm20948_gyro_write_raw(struct iio_dev *gyro_dev,
+				       struct iio_chan_spec const *chan,
+				       int val, int val2, long mask)
+{
+	struct inv_icm20948_state *state = iio_device_get_drvdata(gyro_dev);
+
+	int ret;
+
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		if (!iio_device_claim_direct(gyro_dev))
+			return -EBUSY;
+		ret = inv_icm20948_gyro_write_scale(state, val, val2);
+		iio_device_release_direct(gyro_dev);
+		return ret;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (!iio_device_claim_direct(gyro_dev))
+			return -EBUSY;
+		ret = inv_icm20948_write_calibbias(state, chan, val, val2);
+		iio_device_release_direct(gyro_dev);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm20948_gyro_read_avail(struct iio_dev *gyro_dev,
+					struct iio_chan_spec const *chan,
+					const int **vals, int *type,
+					int *length, long mask)
+{
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = inv_icm20948_gyro_scale;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(inv_icm20948_gyro_scale);
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*vals = inv_icm20948_gyro_calibbias_range;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(inv_icm20948_gyro_calibbias_range);
+		return IIO_AVAIL_RANGE;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info inv_icm20948_gyro_info = {
+	.read_raw = inv_icm20948_gyro_read_raw,
+	.write_raw = inv_icm20948_gyro_write_raw,
+	.read_avail = inv_icm20948_gyro_read_avail,
+};
+
+struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state)
+{
+	struct iio_dev *gyro_dev = devm_iio_device_alloc(state->dev, 0);
+
+	if (!gyro_dev)
+		return ERR_PTR(-ENOMEM);
+
+	iio_device_set_drvdata(gyro_dev, state);
+
+	gyro_dev->name = "icm20948-gyro";
+	gyro_dev->info = &inv_icm20948_gyro_info;
+	gyro_dev->modes = INDIO_DIRECT_MODE;
+	gyro_dev->channels = inv_icm20948_gyro_channels;
+	gyro_dev->num_channels = ARRAY_SIZE(inv_icm20948_gyro_channels);
+
+	int ret = devm_iio_device_register(state->dev, gyro_dev);
+
+	if (ret)
+		return ERR_PTR(ret);
+
+	state->gyro_conf =
+		devm_kzalloc(state->dev, sizeof(*state->gyro_conf), GFP_KERNEL);
+	if (!state->gyro_conf)
+		return ERR_PTR(-ENOMEM);
+
+	state->gyro_conf->fsr = INV_ICM20948_GYRO_FS_250;
+	ret = inv_icm20948_gyro_apply_config(state);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return gyro_dev;
+}

-- 
2.51.0

Re: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope

[Krzysztof Kozlowski]

On 30/08/2025 20:42, Bharadwaj Raju wrote:
> Add support for reading the gyroscope, which is exposed as another IIO
> device under the icm20948 driver.
> 
> For now, the only configuration supported is changing the full-scale
> range.
> 
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> ---
>  drivers/iio/imu/inv_icm20948/Makefile            |   1 +
>  drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  78 ++++--
>  drivers/iio/imu/inv_icm20948/inv_icm20948_core.c |  55 ++--
>  drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c | 343 +++++++++++++++++++++++
>  4 files changed, 432 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
> index c508c2dc3eee2c32be20067e3e0868a203d8aa1a..88a37be159e1d6f575da1c070c84ac94cd963020 100644
> --- a/drivers/iio/imu/inv_icm20948/Makefile
> +++ b/drivers/iio/imu/inv_icm20948/Makefile
> @@ -3,6 +3,7 @@
>  obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
>  inv-icm20948-y += inv_icm20948_core.o
>  inv-icm20948-y += inv_icm20948_temp.o
> +inv-icm20948-y += inv_icm20948_gyro.o
>  
>  obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
>  inv-icm20948-i2c-y += inv_icm20948_i2c.o
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> index f9830645fbe96fd02eef7c54d1e5908647d5a0fe..ca2513114378cdcba5bc315fc94cd61f930b4dfa 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> @@ -3,45 +3,83 @@
>   * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
>   */
>  
> -#ifndef INV_ICM20948_H_
> -#define INV_ICM20948_H_
> + #ifndef INV_ICM20948_H_
> + #define INV_ICM20948_H_
>  
> -#include <linux/bits.h>
> -#include <linux/bitfield.h>
> -#include <linux/mutex.h>
> -#include <linux/regmap.h>
> -#include <linux/i2c.h>
> -#include <linux/iio/iio.h>
> -#include <linux/err.h>
> + #include <linux/bits.h>
> + #include <linux/bitfield.h>
> + #include <linux/mutex.h>
> + #include <linux/regmap.h>
> + #include <linux/i2c.h>
> + #include <linux/iio/iio.h>
> + #include <linux/err.h>
>  
>  /* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
> -#define INV_ICM20948_SLEEP_WAKEUP_MS 35
> + #define INV_ICM20948_SLEEP_WAKEUP_MS 35
>  
> -#define INV_ICM20948_REG_BANK_SEL 0x7F
> -#define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
> + #define INV_ICM20948_REG_BANK_SEL 0x7F
> + #define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
>  
> -#define INV_ICM20948_REG_WHOAMI 0x0000
> -#define INV_ICM20948_WHOAMI 0xEA
> + #define INV_ICM20948_REG_WHOAMI 0x0000
> + #define INV_ICM20948_WHOAMI 0xEA
>  
> -#define INV_ICM20948_REG_FIFO_RW 0x0072
> + #define INV_ICM20948_REG_FIFO_RW 0x0072
>  
> -#define INV_ICM20948_REG_PWR_MGMT_1 0x0006
> -#define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)
> -#define INV_ICM20948_PWR_MGMT_1_SLEEP BIT(6)
> + #define INV_ICM20948_REG_PWR_MGMT_1 0x0006
> + #define INV_ICM20948_PWR_MGMT_1_DEV_RESET BIT(7)


This makes no sense.

Best regards,
Krzysztof

Re: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope

[kernel test robot]

Hi Bharadwaj,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 8742b2d8935f476449ef37e263bc4da3295c7b58]

url:    https://github.com/intel-lab-lkp/linux/commits/Bharadwaj-Raju/dt-bindings-iio-imu-Add-ICM-20948/20250831-024726
base:   8742b2d8935f476449ef37e263bc4da3295c7b58
patch link:    https://lore.kernel.org/r/20250831-icm20948-v1-3-1fe560a38de4%40gmail.com
patch subject: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope
config: arm-randconfig-r123-20250901 (https://download.01.org/0day-ci/archive/20250901/202509010654.5oiN6YTZ-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project ac23f7465eedd0dd565ffb201f573e7a69695fa3)
reproduce: (https://download.01.org/0day-ci/archive/20250901/202509010654.5oiN6YTZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509010654.5oiN6YTZ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c:21:12: sparse: sparse: Initializer entry defined twice
   drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c:24:12: sparse:   also defined here

vim +21 drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c

    11	
    12	/* IIO int + nano format */
    13	static const int inv_icm20948_gyro_scale[] = {
    14		/* 250 dps == 0.000133158 rad/s per LSB */
    15		[2 * INV_ICM20948_GYRO_FS_250] = 0,
    16		[2 * INV_ICM20948_GYRO_FS_250 + 1] = 133158,
    17		/* 500 dps == 0.000266316 rad/s per LSB */
    18		[2 * INV_ICM20948_GYRO_FS_500] = 0,
    19		[2 * INV_ICM20948_GYRO_FS_500 + 1] = 266316,
    20		/* 1000 dps == 0.000532632 rad/s per LSB */
  > 21		[2 * INV_ICM20948_GYRO_FS_1000] = 0,
    22		[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 532632,
    23		/* 2000 dps == 0.001065264 rad/s per LSB */
    24		[2 * INV_ICM20948_GYRO_FS_1000] = 0,
    25		[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 1065264,
    26	};
    27	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope

[kernel test robot]

Hi Bharadwaj,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8742b2d8935f476449ef37e263bc4da3295c7b58]

url:    https://github.com/intel-lab-lkp/linux/commits/Bharadwaj-Raju/dt-bindings-iio-imu-Add-ICM-20948/20250831-024726
base:   8742b2d8935f476449ef37e263bc4da3295c7b58
patch link:    https://lore.kernel.org/r/20250831-icm20948-v1-3-1fe560a38de4%40gmail.com
patch subject: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope
config: parisc-randconfig-r132-20250901 (https://download.01.org/0day-ci/archive/20250901/202509010950.z3ZOTLrS-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 10.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250901/202509010950.z3ZOTLrS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509010950.z3ZOTLrS-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "__divdi3" [drivers/iio/imu/inv_icm20948/inv-icm20948.ko] undefined!
>> ERROR: modpost: "__moddi3" [drivers/iio/imu/inv_icm20948/inv-icm20948.ko] undefined!
ERROR: modpost: "inv_icm20948_core_probe" [drivers/iio/imu/inv_icm20948/inv-icm20948-i2c.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/5] iio: imu: icm20948: add support for gyroscope

[Jonathan Cameron]

On Sun, 31 Aug 2025 00:12:47 +0530
Bharadwaj Raju <bharadwaj.raju777@gmail.com> wrote:

> Add support for reading the gyroscope, which is exposed as another IIO
> device under the icm20948 driver.
> 
> For now, the only configuration supported is changing the full-scale
> range.
> 
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>

Some questions on earlier patch follow through to here.

Also check your patches for the sort of reformatting you have here.
It's both wrong and if it were correct it should have been in the
earlier patch.

Jonathan

> ---
>  drivers/iio/imu/inv_icm20948/Makefile            |   1 +
>  drivers/iio/imu/inv_icm20948/inv_icm20948.h      |  78 ++++--
>  drivers/iio/imu/inv_icm20948/inv_icm20948_core.c |  55 ++--
>  drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c | 343 +++++++++++++++++++++++
>  4 files changed, 432 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
> index c508c2dc3eee2c32be20067e3e0868a203d8aa1a..88a37be159e1d6f575da1c070c84ac94cd963020 100644
> --- a/drivers/iio/imu/inv_icm20948/Makefile
> +++ b/drivers/iio/imu/inv_icm20948/Makefile
> @@ -3,6 +3,7 @@
>  obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
>  inv-icm20948-y += inv_icm20948_core.o
>  inv-icm20948-y += inv_icm20948_temp.o
> +inv-icm20948-y += inv_icm20948_gyro.o
>  
>  obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
>  inv-icm20948-i2c-y += inv_icm20948_i2c.o
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> index f9830645fbe96fd02eef7c54d1e5908647d5a0fe..ca2513114378cdcba5bc315fc94cd61f930b4dfa 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> @@ -3,45 +3,83 @@
>   * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
>   */
>  

>  
>  extern int inv_icm20948_core_probe(struct regmap *regmap);
>  
>  struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
> +struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state);
>  
> -#endif
> + #endif
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> index ee9e4159cffa261f0326b146a4b3df2cbfbd7697..eb4f940de7013bf4ddeb69b6380a60fbde49964a 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> @@ -3,7 +3,7 @@
>   * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
>   */
>  
> -#include "inv_icm20948.h"
> + #include "inv_icm20948.h"

Definitely not.
Please run checkpatch.pl over each patch in turn.

>  
>  static const struct regmap_range_cfg inv_icm20948_regmap_ranges[] = {
>  	{
> @@ -66,36 +66,41 @@ EXPORT_SYMBOL_NS_GPL(inv_icm20948_regmap_config, "IIO_ICM20948");
>  
>  static int inv_icm20948_setup(struct inv_icm20948_state *state)
>  {
> -	guard(mutex)(&state->lock);
> -
> -	int reported_whoami;
> -	int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
> -			      &reported_whoami);
> -	if (ret)
> -		return ret;
> -	if (reported_whoami != INV_ICM20948_WHOAMI) {
> -		dev_err(state->dev, "invalid whoami %d, expected %d\n",
> -			reported_whoami, INV_ICM20948_WHOAMI);
> -		return -ENODEV;
> +	scoped_guard(mutex, &state->lock) {
> +		int reported_whoami;
> +		int ret = regmap_read(state->regmap, INV_ICM20948_REG_WHOAMI,
> +				      &reported_whoami);
> +		if (ret)
> +			return ret;
> +		if (reported_whoami != INV_ICM20948_WHOAMI) {
> +			dev_err(state->dev, "invalid whoami %d, expected %d\n",
> +				reported_whoami, INV_ICM20948_WHOAMI);
> +			return -ENODEV;
> +		}
> +
> +		ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +					INV_ICM20948_PWR_MGMT_1_DEV_RESET,
> +					INV_ICM20948_PWR_MGMT_1_DEV_RESET);
> +		if (ret)
> +			return ret;
> +		msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> +
> +		ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +					INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
> +		if (ret)
> +			return ret;
> +
> +		msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
>  	}
>  
> -	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> -				INV_ICM20948_PWR_MGMT_1_DEV_RESET,
> -				INV_ICM20948_PWR_MGMT_1_DEV_RESET);
> -	if (ret)
> -		return ret;
> -	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> -
> -	ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> -				INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
> -	if (ret)
> -		return ret;
> -	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> -

If this change makes sense it should be in the earlier patch.
Don't add code that you then move around later.

> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d4d598eb21c8ce98d4ee3c72504554ab49ea596
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> + */
> +
> +#include <linux/bits.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#include "inv_icm20948.h"
> +
> +/* IIO int + nano format */
> +static const int inv_icm20948_gyro_scale[] = {
> +	/* 250 dps == 0.000133158 rad/s per LSB */
> +	[2 * INV_ICM20948_GYRO_FS_250] = 0,
> +	[2 * INV_ICM20948_GYRO_FS_250 + 1] = 133158,
> +	/* 500 dps == 0.000266316 rad/s per LSB */
> +	[2 * INV_ICM20948_GYRO_FS_500] = 0,
> +	[2 * INV_ICM20948_GYRO_FS_500 + 1] = 266316,
> +	/* 1000 dps == 0.000532632 rad/s per LSB */
> +	[2 * INV_ICM20948_GYRO_FS_1000] = 0,
> +	[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 532632,
> +	/* 2000 dps == 0.001065264 rad/s per LSB */
> +	[2 * INV_ICM20948_GYRO_FS_1000] = 0,
> +	[2 * INV_ICM20948_GYRO_FS_1000 + 1] = 1065264,
> +};
> +
> +/* Calibration bias, IIO range format int + nano */
> +/* raw value -2**15 to +2**15, 0.0305 dps per LSB step */

Use multiline comment format.
/*
 * Calibration bias...
 * raw value...
 */

> +static const int inv_icm20948_gyro_calibbias_range[] = {
> +	-17, 443239423, /* min */
> +	0,   532325, /* step */
> +	+17, 443239423, /* max */
> +};
> +
> +#define INV_ICM20948_GYRO_CHAN(_dir) \
> +	{		\
> +		.type = IIO_ANGL_VEL,		\
> +		.modified = 1,		\
> +		.channel2 = IIO_MOD_##_dir,		\
> +		.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) |		\
> +		  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 = INV_ICM20948_GYRO_SCAN_##_dir,		\
> +		.scan_type = {		\
> +			.sign = 's',		\
> +			.realbits = 16,		\
> +			.endianness = IIO_BE,		\
> +		},		\
> +	}

See example in previous patch (or pretty much any drivers in tree)
for how to format these.


> +
> +static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
> +					struct iio_chan_spec const *chan,
> +					int val, int val2)
> +{
> +	guard(mutex)(&state->lock);
> +
> +	unsigned int reg;
> +
> +	switch (chan->channel2) {
> +	case IIO_MOD_X:
> +		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_X;
> +		break;
> +	case IIO_MOD_Y:
> +		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Y;
> +		break;
> +	case IIO_MOD_Z:
> +		reg = INV_ICM20948_REG_GYRO_USER_OFFSET_Z;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	s64 bias = (s64)val * 100000000L + val2;
> +	s64 val64 = bias * 180;
> +
> +	if (val64 >= 0)
> +		val64 -= 180 / 2;
> +	else
> +		val64 += 180 / 2;
> +
> +	s64 offset64 = div_s64(val64, 95818576L);
> +	s16 offset = clamp(offset64, (s64)S16_MIN, (s64)S16_MAX);
> +	__be16 offset_write = cpu_to_be16(offset);

Declarations at top of scope.

> +
> +	return regmap_bulk_write(state->regmap, reg, &offset_write,
> +				 sizeof(offset_write));

I think you only support i2c so far. When adding SPI we should be using
DMA safe buffers for bulk accesses.  Look at how we do __aligned(IIO_DMA_MINALIGN)
in many IIO drivers.  Separate heap allocations will give you what is needed
as an alternative but stack variables are never suitable.

> +}
> +
> +static int inv_icm20948_gyro_write_raw(struct iio_dev *gyro_dev,
> +				       struct iio_chan_spec const *chan,
> +				       int val, int val2, long mask)
> +{
> +	struct inv_icm20948_state *state = iio_device_get_drvdata(gyro_dev);
> +

no blank line here.

> +	int ret;
> +
> +	if (chan->type != IIO_ANGL_VEL)
> +		return -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (!iio_device_claim_direct(gyro_dev))
> +			return -EBUSY;
> +		ret = inv_icm20948_gyro_write_scale(state, val, val2);
> +		iio_device_release_direct(gyro_dev);
> +		return ret;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (!iio_device_claim_direct(gyro_dev))
> +			return -EBUSY;
> +		ret = inv_icm20948_write_calibbias(state, chan, val, val2);
> +		iio_device_release_direct(gyro_dev);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state)
> +{
> +	struct iio_dev *gyro_dev = devm_iio_device_alloc(state->dev, 0);

Look at what that second parameter is for and how we use iio_priv()
extensively in IIo drivers.

> +
> +	if (!gyro_dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	iio_device_set_drvdata(gyro_dev, state);
The above IIO priv comment will probably mean this can go away.

> +
> +	gyro_dev->name = "icm20948-gyro";
> +	gyro_dev->info = &inv_icm20948_gyro_info;
> +	gyro_dev->modes = INDIO_DIRECT_MODE;
> +	gyro_dev->channels = inv_icm20948_gyro_channels;
> +	gyro_dev->num_channels = ARRAY_SIZE(inv_icm20948_gyro_channels);
> +
> +	int ret = devm_iio_device_register(state->dev, gyro_dev);

As with the temperature sensor I'm not yet setting any justification for
there being more than one iio device for the whole sensor.  There are reasons
to do that, but why does that apply for this device?  It is the last
option when we can't do anything else, not where we start from when
consider the design.

> +
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	state->gyro_conf =
> +		devm_kzalloc(state->dev, sizeof(*state->gyro_conf), GFP_KERNEL);

This should be using iio_priv and if you do have a pointer in the parent state
it should be to the struct iio_dev *gyro_dev, not this.  Currently you have
one of those as well which is unnecessary.

> +	if (!state->gyro_conf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	state->gyro_conf->fsr = INV_ICM20948_GYRO_FS_250;
> +	ret = inv_icm20948_gyro_apply_config(state);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return gyro_dev;
> +}
> 

[PATCH 4/5] MAINTAINERS: add entry for inv_icm20948 driver

[Bharadwaj Raju]

Add MAINTAINERS entry for the InvenSense ICM-20948 driver in IIO.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa45799dfe07de2f54de6d6a1ce0615..930c80c1cc40a4cfb4bf270e1ab7680b88d97ff3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12783,6 +12783,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/media/i2c/isil,isl79987.yaml
 F:	drivers/media/i2c/isl7998x.c
 
+INVENSENSE ICM-20948 IMU DRIVER
+M:	Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/imu/invensense,icm20948.yaml
+F:	drivers/iio/imu/inv_icm20948/
+
 INVENSENSE ICM-426xx IMU DRIVER
 M:	Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
 L:	linux-iio@vger.kernel.org

-- 
2.51.0

Re: [PATCH 4/5] MAINTAINERS: add entry for inv_icm20948 driver

[Jonathan Cameron]

On Sun, 31 Aug 2025 00:12:48 +0530
Bharadwaj Raju <bharadwaj.raju777@gmail.com> wrote:

> Add MAINTAINERS entry for the InvenSense ICM-20948 driver in IIO.
> 
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
Preferred to build this up as the code is added.
So initial entry in the DT binding patch, then add the directory
when you bring that in.

Jonathan

> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe168477caa45799dfe07de2f54de6d6a1ce0615..930c80c1cc40a4cfb4bf270e1ab7680b88d97ff3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12783,6 +12783,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/media/i2c/isil,isl79987.yaml
>  F:	drivers/media/i2c/isl7998x.c
>  
> +INVENSENSE ICM-20948 IMU DRIVER
> +M:	Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/imu/invensense,icm20948.yaml
> +F:	drivers/iio/imu/inv_icm20948/
> +
>  INVENSENSE ICM-426xx IMU DRIVER
>  M:	Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
>  L:	linux-iio@vger.kernel.org
> 

[PATCH 5/5] iio: imu: icm20948: add runtime power management support

[Bharadwaj Raju]

Implement runtime power management support for the ICM20948
sensor. The device autosuspends after 2 seconds of idle time.

Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
---
 drivers/iio/imu/inv_icm20948/Makefile             |  1 +
 drivers/iio/imu/inv_icm20948/inv_icm20948.h       |  7 +++
 drivers/iio/imu/inv_icm20948/inv_icm20948_core.c  |  3 +-
 drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c  | 28 ++++++---
 drivers/iio/imu/inv_icm20948/inv_icm20948_power.c | 73 +++++++++++++++++++++++
 drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c  | 15 +++--
 6 files changed, 114 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
index 88a37be159e1d6f575da1c070c84ac94cd963020..0a17ad1c003e6a93f3431f7a998e56cdf975d245 100644
--- a/drivers/iio/imu/inv_icm20948/Makefile
+++ b/drivers/iio/imu/inv_icm20948/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
 inv-icm20948-y += inv_icm20948_core.o
 inv-icm20948-y += inv_icm20948_temp.o
 inv-icm20948-y += inv_icm20948_gyro.o
+inv-icm20948-y += inv_icm20948_power.o
 
 obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
 inv-icm20948-i2c-y += inv_icm20948_i2c.o
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
index ca2513114378cdcba5bc315fc94cd61f930b4dfa..194dcccabc2162334779b285320187c7ff1f5236 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948.h
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
@@ -13,10 +13,13 @@
  #include <linux/i2c.h>
  #include <linux/iio/iio.h>
  #include <linux/err.h>
+ #include <linux/pm_runtime.h>
 
 /* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
  #define INV_ICM20948_SLEEP_WAKEUP_MS 35
 
+ #define INV_ICM20948_SUSPEND_DELAY_MS 2000
+
  #define INV_ICM20948_REG_BANK_SEL 0x7F
  #define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
 
@@ -46,6 +49,8 @@
 
 extern const struct regmap_config inv_icm20948_regmap_config;
 
+extern const struct dev_pm_ops inv_icm20948_pm_ops;
+
 enum inv_icm20948_gyro_fs {
 	INV_ICM20948_GYRO_FS_250 = 0,
 	INV_ICM20948_GYRO_FS_500 = 1,
@@ -82,4 +87,6 @@ extern int inv_icm20948_core_probe(struct regmap *regmap);
 struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
 struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state);
 
+int inv_icm20948_pm_setup(struct inv_icm20948_state *state);
+
  #endif
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
index eb4f940de7013bf4ddeb69b6380a60fbde49964a..e6e670d96e40c3663e55d1545b52f609603a02ed 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
@@ -101,7 +101,7 @@ static int inv_icm20948_setup(struct inv_icm20948_state *state)
 	if (IS_ERR(state->gyro_dev))
 		return PTR_ERR(state->gyro_dev);
 
-	return 0;
+	return inv_icm20948_pm_setup(state);
 }
 
 int inv_icm20948_core_probe(struct regmap *regmap)
@@ -113,6 +113,7 @@ int inv_icm20948_core_probe(struct regmap *regmap)
 	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
 		return -ENOMEM;
+	dev_set_drvdata(dev, state);
 
 	state->regmap = regmap;
 	state->dev = dev;
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
index 2d4d598eb21c8ce98d4ee3c72504554ab49ea596..9cefb47a46b1a323202aa84f0de647d7b7b89728 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
@@ -73,10 +73,14 @@ static const struct iio_chan_spec inv_icm20948_gyro_channels[] = {
 static int inv_icm20948_gyro_apply_config(struct inv_icm20948_state *state)
 {
 	guard(mutex)(&state->lock);
+	pm_runtime_get_sync(state->dev);
 
-	return regmap_write_bits(state->regmap, INV_ICM20948_REG_GYRO_CONFIG_1,
+	int ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_GYRO_CONFIG_1,
 				 INV_ICM20948_GYRO_CONFIG_FULLSCALE,
 				 state->gyro_conf->fsr << 1);
+
+	pm_runtime_put_autosuspend(state->dev);
+	return ret;
 }
 
 static int inv_icm20948_gyro_read_sensor(struct inv_icm20948_state *state,
@@ -99,23 +103,25 @@ static int inv_icm20948_gyro_read_sensor(struct inv_icm20948_state *state,
 		return -EINVAL;
 	}
 
+	pm_runtime_get_sync(state->dev);
+
 	__be16 raw;
 	int ret = regmap_bulk_read(state->regmap, reg, &raw, sizeof(raw));
 
 	if (ret)
-		return ret;
+		goto out;
 
 	*val = (s16)be16_to_cpu(raw);
 
-	return 0;
+out:
+	pm_runtime_put_autosuspend(state->dev);
+	return ret;
 }
 
 static int inv_icm20948_gyro_read_calibbias(struct inv_icm20948_state *state,
 					    struct iio_chan_spec const *chan,
 					    int *val, int *val2)
 {
-	guard(mutex)(&state->lock);
-
 	unsigned int reg;
 
 	switch (chan->channel2) {
@@ -133,8 +139,11 @@ static int inv_icm20948_gyro_read_calibbias(struct inv_icm20948_state *state,
 	}
 
 	__be16 offset_raw;
+
+	pm_runtime_get_sync(state->dev);
 	int ret = regmap_bulk_read(state->regmap, reg, &offset_raw,
 				   sizeof(offset_raw));
+	pm_runtime_put_autosuspend(state->dev);
 
 	if (ret)
 		return ret;
@@ -216,8 +225,6 @@ static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
 					struct iio_chan_spec const *chan,
 					int val, int val2)
 {
-	guard(mutex)(&state->lock);
-
 	unsigned int reg;
 
 	switch (chan->channel2) {
@@ -246,8 +253,13 @@ static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
 	s16 offset = clamp(offset64, (s64)S16_MIN, (s64)S16_MAX);
 	__be16 offset_write = cpu_to_be16(offset);
 
-	return regmap_bulk_write(state->regmap, reg, &offset_write,
+	pm_runtime_get_sync(state->dev);
+	mutex_lock(&state->lock);
+	int ret = regmap_bulk_write(state->regmap, reg, &offset_write,
 				 sizeof(offset_write));
+	mutex_unlock(&state->lock);
+	pm_runtime_put_autosuspend(state->dev);
+	return ret;
 }
 
 static int inv_icm20948_gyro_write_raw(struct iio_dev *gyro_dev,
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c
new file mode 100644
index 0000000000000000000000000000000000000000..1281a5e5acb539cd3f91ca8ed8d52371f330b60a
--- /dev/null
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
+ */
+
+#include "inv_icm20948.h"
+
+static int inv_icm20948_suspend(struct device *dev)
+{
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	struct inv_icm20948_state *state = dev_get_drvdata(dev);
+
+	guard(mutex)(&state->lock);
+
+	return regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+				 INV_ICM20948_PWR_MGMT_1_SLEEP,
+				 INV_ICM20948_PWR_MGMT_1_SLEEP);
+}
+
+static int inv_icm20948_resume(struct device *dev)
+{
+	struct inv_icm20948_state *state = dev_get_drvdata(dev);
+
+	guard(mutex)(&state->lock);
+
+	pm_runtime_disable(state->dev);
+	pm_runtime_set_active(state->dev);
+	pm_runtime_enable(state->dev);
+
+	int ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
+				    INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
+	if (ret)
+		return ret;
+
+	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
+
+	return 0;
+}
+
+static void inv_icm20948_pm_disable(void *data)
+{
+	struct device *dev = data;
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+}
+
+int inv_icm20948_pm_setup(struct inv_icm20948_state *state)
+{
+	struct device *dev = state->dev;
+
+	guard(mutex)(&state->lock);
+
+	int ret;
+
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		return ret;
+	pm_runtime_get_noresume(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, INV_ICM20948_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_put(dev);
+
+	return devm_add_action_or_reset(dev, inv_icm20948_pm_disable, dev);
+}
+
+EXPORT_NS_GPL_DEV_PM_OPS(inv_icm20948_pm_ops, IIO_ICM20948) = {
+	SYSTEM_SLEEP_PM_OPS(inv_icm20948_suspend, inv_icm20948_resume)
+	RUNTIME_PM_OPS(inv_icm20948_suspend, inv_icm20948_resume, NULL)
+};
diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
index 916053740cc5acda0316c76504d4086eff5ec7f0..6e17b3719301d6d7f005d545587f558fcadd2f40 100644
--- a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
+++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
@@ -24,17 +24,24 @@ static const struct iio_chan_spec
 static int inv_icm20948_temp_read_sensor(struct inv_icm20948_state *state,
 					 s16 *temp)
 {
-	guard(mutex)(&state->lock);
+	int ret;
+
+	pm_runtime_get_sync(state->dev);
+	mutex_lock(&state->lock);
 
 	__be16 raw;
-	int ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
+	ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
 				   &raw, sizeof(raw));
 	if (ret)
-		return ret;
+		goto out;
 
 	*temp = __be16_to_cpu(raw);
+	ret = 0;
 
-	return 0;
+out:
+	mutex_unlock(&state->lock);
+	pm_runtime_put_autosuspend(state->dev);
+	return ret;
 }
 
 static int inv_icm20948_temp_read_raw(struct iio_dev *temp_dev,

-- 
2.51.0

Re: [PATCH 5/5] iio: imu: icm20948: add runtime power management support

[Markus Elfring]

…
> @@ -24,17 +24,24 @@ static const struct iio_chan_spec
>  static int inv_icm20948_temp_read_sensor(struct inv_icm20948_state *state,
>  					 s16 *temp)
>  {
> -	guard(mutex)(&state->lock);
> +	int ret;
> +
> +	pm_runtime_get_sync(state->dev);
> +	mutex_lock(&state->lock);
>  
>  	__be16 raw;
> -	int ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
> +	ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
>  				   &raw, sizeof(raw));
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	*temp = __be16_to_cpu(raw);
> +	ret = 0;
>  
> -	return 0;
> +out:
> +	mutex_unlock(&state->lock);
> +	pm_runtime_put_autosuspend(state->dev);
> +	return ret;
>  }
…

Does anything hinder you with the continued application of scope-based resource management
for such a function implementation?

Regards,
Markus

Re: [PATCH 5/5] iio: imu: icm20948: add runtime power management support

[Greg KH]

On Sun, Aug 31, 2025 at 09:23:08PM +0200, Markus Elfring wrote:
> …
> > @@ -24,17 +24,24 @@ static const struct iio_chan_spec
> >  static int inv_icm20948_temp_read_sensor(struct inv_icm20948_state *state,
> >  					 s16 *temp)
> >  {
> > -	guard(mutex)(&state->lock);
> > +	int ret;
> > +
> > +	pm_runtime_get_sync(state->dev);
> > +	mutex_lock(&state->lock);
> >  
> >  	__be16 raw;
> > -	int ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
> > +	ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
> >  				   &raw, sizeof(raw));
> >  	if (ret)
> > -		return ret;
> > +		goto out;
> >  
> >  	*temp = __be16_to_cpu(raw);
> > +	ret = 0;
> >  
> > -	return 0;
> > +out:
> > +	mutex_unlock(&state->lock);
> > +	pm_runtime_put_autosuspend(state->dev);
> > +	return ret;
> >  }
> …
> 
> Does anything hinder you with the continued application of scope-based resource management
> for such a function implementation?
> 
> Regards,
> Markus
> 



Hi,

This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
You have sent him a patch that has triggered this response.

Right now, the development tree you have sent a patch for is "closed"
due to the timing of the merge window.  Don't worry, the patch(es) you
have sent are not lost, and will be looked at after the merge window is
over (after the -rc1 kernel is released by Linus).

So thank you for your patience and your patches will be reviewed at this
later time, you do not have to do anything further, this is just a short
note to let you know the patch status and so you don't worry they didn't
make it through.

thanks,

greg k-h's patch email bot

Re: [PATCH 5/5] iio: imu: icm20948: add runtime power management support

[Jonathan Cameron]

On Sun, 31 Aug 2025 00:12:49 +0530
Bharadwaj Raju <bharadwaj.raju777@gmail.com> wrote:

> Implement runtime power management support for the ICM20948
> sensor. The device autosuspends after 2 seconds of idle time.

This is an unusual feature to bring in at this point in developing
a driver, but fair enough if you want to it doesn't hurt!

Anyhow, various comments inline and requests for more information.

Jonathan


> 
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> ---
>  drivers/iio/imu/inv_icm20948/Makefile             |  1 +
>  drivers/iio/imu/inv_icm20948/inv_icm20948.h       |  7 +++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_core.c  |  3 +-
>  drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c  | 28 ++++++---
>  drivers/iio/imu/inv_icm20948/inv_icm20948_power.c | 73 +++++++++++++++++++++++
>  drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c  | 15 +++--
>  6 files changed, 114 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm20948/Makefile b/drivers/iio/imu/inv_icm20948/Makefile
> index 88a37be159e1d6f575da1c070c84ac94cd963020..0a17ad1c003e6a93f3431f7a998e56cdf975d245 100644
> --- a/drivers/iio/imu/inv_icm20948/Makefile
> +++ b/drivers/iio/imu/inv_icm20948/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_INV_ICM20948) += inv-icm20948.o
>  inv-icm20948-y += inv_icm20948_core.o
>  inv-icm20948-y += inv_icm20948_temp.o
>  inv-icm20948-y += inv_icm20948_gyro.o
> +inv-icm20948-y += inv_icm20948_power.o
>  
>  obj-$(CONFIG_INV_ICM20948_I2C) += inv-icm20948-i2c.o
>  inv-icm20948-i2c-y += inv_icm20948_i2c.o
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948.h b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> index ca2513114378cdcba5bc315fc94cd61f930b4dfa..194dcccabc2162334779b285320187c7ff1f5236 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948.h
> @@ -13,10 +13,13 @@
>   #include <linux/i2c.h>
>   #include <linux/iio/iio.h>
>   #include <linux/err.h>
> + #include <linux/pm_runtime.h>
>  
>  /* accel takes 20ms, gyro takes 35ms to wake from full-chip sleep */
>   #define INV_ICM20948_SLEEP_WAKEUP_MS 35
>  
> + #define INV_ICM20948_SUSPEND_DELAY_MS 2000
I'd just use the value inline.  It should only be in one place
and the meaning of the value there is well understood by reviewers.

> +
>   #define INV_ICM20948_REG_BANK_SEL 0x7F
>   #define INV_ICM20948_BANK_SEL_MASK GENMASK(5, 4)
>  
> @@ -46,6 +49,8 @@
>  
>  extern const struct regmap_config inv_icm20948_regmap_config;
>  
> +extern const struct dev_pm_ops inv_icm20948_pm_ops;
> +
>  enum inv_icm20948_gyro_fs {
>  	INV_ICM20948_GYRO_FS_250 = 0,
>  	INV_ICM20948_GYRO_FS_500 = 1,
> @@ -82,4 +87,6 @@ extern int inv_icm20948_core_probe(struct regmap *regmap);
>  struct iio_dev *inv_icm20948_temp_init(struct inv_icm20948_state *state);
>  struct iio_dev *inv_icm20948_gyro_init(struct inv_icm20948_state *state);
>  
> +int inv_icm20948_pm_setup(struct inv_icm20948_state *state);
> +
>   #endif
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> index eb4f940de7013bf4ddeb69b6380a60fbde49964a..e6e670d96e40c3663e55d1545b52f609603a02ed 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_core.c
> @@ -101,7 +101,7 @@ static int inv_icm20948_setup(struct inv_icm20948_state *state)
>  	if (IS_ERR(state->gyro_dev))
>  		return PTR_ERR(state->gyro_dev);
>  
> -	return 0;
> +	return inv_icm20948_pm_setup(state);
>  }
>  
>  int inv_icm20948_core_probe(struct regmap *regmap)
> @@ -113,6 +113,7 @@ int inv_icm20948_core_probe(struct regmap *regmap)
>  	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
>  	if (!state)
>  		return -ENOMEM;
> +	dev_set_drvdata(dev, state);
>  
>  	state->regmap = regmap;
>  	state->dev = dev;
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
> index 2d4d598eb21c8ce98d4ee3c72504554ab49ea596..9cefb47a46b1a323202aa84f0de647d7b7b89728 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_gyro.c

>  
>  static int inv_icm20948_gyro_read_sensor(struct inv_icm20948_state *state,
> @@ -99,23 +103,25 @@ static int inv_icm20948_gyro_read_sensor(struct inv_icm20948_state *state,
>  		return -EINVAL;
>  	}
>  
> +	pm_runtime_get_sync(state->dev);
> +
>  	__be16 raw;
>  	int ret = regmap_bulk_read(state->regmap, reg, &raw, sizeof(raw));
>  
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	*val = (s16)be16_to_cpu(raw);
>  
> -	return 0;
> +out:
> +	pm_runtime_put_autosuspend(state->dev);

A common thing to do when runtime pm is involved is to have a wrapper function
around the main code.  That wrapper then deals with runtime pm, but lets you
use direct returns in the inner function, which tends to improve readability.

> +	return ret;
>  }
>  
>  static int inv_icm20948_gyro_read_calibbias(struct inv_icm20948_state *state,
>  					    struct iio_chan_spec const *chan,
>  					    int *val, int *val2)
>  {
> -	guard(mutex)(&state->lock);
> -
>  	unsigned int reg;
>  
>  	switch (chan->channel2) {
> @@ -133,8 +139,11 @@ static int inv_icm20948_gyro_read_calibbias(struct inv_icm20948_state *state,
>  	}
>  
>  	__be16 offset_raw;
> +
> +	pm_runtime_get_sync(state->dev);
>  	int ret = regmap_bulk_read(state->regmap, reg, &offset_raw,
>  				   sizeof(offset_raw));
> +	pm_runtime_put_autosuspend(state->dev);
>  
>  	if (ret)
>  		return ret;
> @@ -216,8 +225,6 @@ static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
>  					struct iio_chan_spec const *chan,
>  					int val, int val2)
>  {
> -	guard(mutex)(&state->lock);
> -
>  	unsigned int reg;
>  
>  	switch (chan->channel2) {
> @@ -246,8 +253,13 @@ static int inv_icm20948_write_calibbias(struct inv_icm20948_state *state,
>  	s16 offset = clamp(offset64, (s64)S16_MIN, (s64)S16_MAX);
>  	__be16 offset_write = cpu_to_be16(offset);
>  
> -	return regmap_bulk_write(state->regmap, reg, &offset_write,
> +	pm_runtime_get_sync(state->dev);
> +	mutex_lock(&state->lock);
> +	int ret = regmap_bulk_write(state->regmap, reg, &offset_write,
>  				 sizeof(offset_write));
> +	mutex_unlock(&state->lock);
> +	pm_runtime_put_autosuspend(state->dev);
> +	return ret;
>  }
>  
>  static int inv_icm20948_gyro_write_raw(struct iio_dev *gyro_dev,
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1281a5e5acb539cd3f91ca8ed8d52371f330b60a
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_power.c

Don't have a separate file for this. It is not that much code so much more
obvious to just have it in the core file.

> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 Bharadwaj Raju <bharadwaj.raju777@gmail.com>
> + */
> +
> +#include "inv_icm20948.h"
> +
> +static int inv_icm20948_suspend(struct device *dev)
> +{
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	struct inv_icm20948_state *state = dev_get_drvdata(dev);
> +
> +	guard(mutex)(&state->lock);

What data is this mutex protecting here?  Regmap has it's own locks
internally and I'm not immediately sure what else needs to be protected
against races.

> +
> +	return regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +				 INV_ICM20948_PWR_MGMT_1_SLEEP,
> +				 INV_ICM20948_PWR_MGMT_1_SLEEP);
> +}
> +
> +static int inv_icm20948_resume(struct device *dev)
> +{
> +	struct inv_icm20948_state *state = dev_get_drvdata(dev);
> +
> +	guard(mutex)(&state->lock);
> +
> +	pm_runtime_disable(state->dev);
> +	pm_runtime_set_active(state->dev);
> +	pm_runtime_enable(state->dev);

Which device is this on?  I'd not expect to typically see runtime pm state
manipulated in runtime pm ops for another device. The parent /child relationships
etc (more complex options exist) should deal with that.
> +
> +	int ret = regmap_write_bits(state->regmap, INV_ICM20948_REG_PWR_MGMT_1,
> +				    INV_ICM20948_PWR_MGMT_1_SLEEP, 0);
> +	if (ret)
> +		return ret;
> +
> +	msleep(INV_ICM20948_SLEEP_WAKEUP_MS);
> +
> +	return 0;
> +}
> +
> +static void inv_icm20948_pm_disable(void *data)
> +{
> +	struct device *dev = data;
> +
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +}
> +
> +int inv_icm20948_pm_setup(struct inv_icm20948_state *state)
> +{
> +	struct device *dev = state->dev;
> +
> +	guard(mutex)(&state->lock);
> +
> +	int ret;
> +
> +	ret = pm_runtime_set_active(dev);
> +	if (ret)
> +		return ret;
> +	pm_runtime_get_noresume(dev);
> +	pm_runtime_enable(dev);
> +	pm_runtime_set_autosuspend_delay(dev, INV_ICM20948_SUSPEND_DELAY_MS);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_put(dev);
> +
> +	return devm_add_action_or_reset(dev, inv_icm20948_pm_disable, dev);
> +}
> +
> +EXPORT_NS_GPL_DEV_PM_OPS(inv_icm20948_pm_ops, IIO_ICM20948) = {
> +	SYSTEM_SLEEP_PM_OPS(inv_icm20948_suspend, inv_icm20948_resume)
> +	RUNTIME_PM_OPS(inv_icm20948_suspend, inv_icm20948_resume, NULL)
If you want to use runtime pm ops for both this is not how it is done.
See DEFINE_RUNTIME_DEV_PM_OPS()

> +};
> diff --git a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
> index 916053740cc5acda0316c76504d4086eff5ec7f0..6e17b3719301d6d7f005d545587f558fcadd2f40 100644
> --- a/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
> +++ b/drivers/iio/imu/inv_icm20948/inv_icm20948_temp.c
> @@ -24,17 +24,24 @@ static const struct iio_chan_spec
>  static int inv_icm20948_temp_read_sensor(struct inv_icm20948_state *state,
>  					 s16 *temp)
>  {
> -	guard(mutex)(&state->lock);
> +	int ret;
> +
> +	pm_runtime_get_sync(state->dev);
> +	mutex_lock(&state->lock);
>  
>  	__be16 raw;
> -	int ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
> +	ret = regmap_bulk_read(state->regmap, INV_ICM20948_REG_TEMP_DATA,
>  				   &raw, sizeof(raw));
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	*temp = __be16_to_cpu(raw);
> +	ret = 0;
>  
> -	return 0;
> +out:
> +	mutex_unlock(&state->lock);
> +	pm_runtime_put_autosuspend(state->dev);
> +	return ret;
>  }
>  
>  static int inv_icm20948_temp_read_raw(struct iio_dev *temp_dev,
>