* [PATCH v4 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup
2025-09-09 7:11 [PATCH v4 0/5] iio: imu: inv_icm42600: pm_runtime fixes + various changes Sean Nyekjaer
@ 2025-09-09 7:11 ` Sean Nyekjaer
2025-09-09 7:11 ` [PATCH v4 2/5] iio: imu: inv_icm42600: Drop redundant pm_runtime reinitialization in resume Sean Nyekjaer
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Sean Nyekjaer @ 2025-09-09 7:11 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol, rafael, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-iio, linux-kernel, linux-pm, Jean-Baptiste Maneyrol,
Jonathan Cameron, Sean Nyekjaer
Rework the power management in inv_icm42600_core_probe() to use
devm_pm_runtime_set_active_enabled(), which simplifies the runtime PM
setup by handling activation and enabling in one step.
Remove the separate inv_icm42600_disable_pm callback, as it's no longer
needed with the devm-managed approach.
Using devm_pm_runtime_enable() also fixes the missing disable of
autosuspend.
Update inv_icm42600_disable_vddio_reg() to only disable the regulator if
the device is not suspended i.e. powered-down, preventing unbalanced
disables.
Also remove redundant error msg on regulator_disable(), the regulator
framework already emits an error message when regulator_disable() fails.
This simplifies the PM setup and avoids manipulating the usage counter
unnecessarily.
Fixes: 31c24c1e93c3 ("iio: imu: inv_icm42600: add core of new inv_icm42600 driver")
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index a4d42e7e21807f7954def431e9cf03dffaa5bd5e..76d8e4f14d87a552a35d7a9e0cb3305781d85ca9 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -711,20 +711,12 @@ static void inv_icm42600_disable_vdd_reg(void *_data)
static void inv_icm42600_disable_vddio_reg(void *_data)
{
struct inv_icm42600_state *st = _data;
- const struct device *dev = regmap_get_device(st->map);
- int ret;
-
- ret = regulator_disable(st->vddio_supply);
- if (ret)
- dev_err(dev, "failed to disable vddio error %d\n", ret);
-}
+ struct device *dev = regmap_get_device(st->map);
-static void inv_icm42600_disable_pm(void *_data)
-{
- struct device *dev = _data;
+ if (pm_runtime_status_suspended(dev))
+ return;
- pm_runtime_put_sync(dev);
- pm_runtime_disable(dev);
+ regulator_disable(st->vddio_supply);
}
int inv_icm42600_core_probe(struct regmap *regmap, int chip,
@@ -824,16 +816,14 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip,
return ret;
/* setup runtime power management */
- ret = pm_runtime_set_active(dev);
+ ret = devm_pm_runtime_set_active_enabled(dev);
if (ret)
return ret;
- pm_runtime_get_noresume(dev);
- pm_runtime_enable(dev);
+
pm_runtime_set_autosuspend_delay(dev, INV_ICM42600_SUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(dev);
- pm_runtime_put(dev);
- return devm_add_action_or_reset(dev, inv_icm42600_disable_pm, dev);
+ return ret;
}
EXPORT_SYMBOL_NS_GPL(inv_icm42600_core_probe, "IIO_ICM42600");
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 4/5] iio: imu: inv_icm42600: Use devm_regulator_get_enable() for vdd regulator
2025-09-09 7:11 [PATCH v4 0/5] iio: imu: inv_icm42600: pm_runtime fixes + various changes Sean Nyekjaer
` (2 preceding siblings ...)
2025-09-09 7:11 ` [PATCH v4 3/5] iio: imu: inv_icm42600: Avoid configuring if already pm_runtime suspended Sean Nyekjaer
@ 2025-09-09 7:11 ` Sean Nyekjaer
2025-09-09 7:11 ` [PATCH v4 5/5] iio: imu: inv_icm42600: use guard() to release mutexes Sean Nyekjaer
4 siblings, 0 replies; 11+ messages in thread
From: Sean Nyekjaer @ 2025-09-09 7:11 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol, rafael, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-iio, linux-kernel, linux-pm, Jean-Baptiste Maneyrol,
Jonathan Cameron, Sean Nyekjaer
The vdd regulator is not used for runtime power management, so it does
not need explicit enable/disable handling.
Use devm_regulator_get_enable() to let the regulator be managed
automatically by devm.
This simplifies the code by removing the manual enable and cleanup
logic.
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
drivers/iio/imu/inv_icm42600/inv_icm42600.h | 1 -
drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 29 +++++-------------------
2 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index 1430ab4f1dea5d5ba6277d74275fc44a6cd30eb8..c8b48a5c5ed0677bb35201d32934936faaf7a1a6 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -167,7 +167,6 @@ struct inv_icm42600_state {
enum inv_icm42600_chip chip;
const char *name;
struct regmap *map;
- struct regulator *vdd_supply;
struct regulator *vddio_supply;
int irq;
struct iio_mount_matrix orientation;
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index ee780f530dc8612cd25dc2216b153ef4e8c32b7b..4bf436c46f1cfd7e7e1bb911d94a0a566d63e791 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -697,17 +697,6 @@ static int inv_icm42600_enable_regulator_vddio(struct inv_icm42600_state *st)
return 0;
}
-static void inv_icm42600_disable_vdd_reg(void *_data)
-{
- struct inv_icm42600_state *st = _data;
- const struct device *dev = regmap_get_device(st->map);
- int ret;
-
- ret = regulator_disable(st->vdd_supply);
- if (ret)
- dev_err(dev, "failed to disable vdd error %d\n", ret);
-}
-
static void inv_icm42600_disable_vddio_reg(void *_data)
{
struct inv_icm42600_state *st = _data;
@@ -765,23 +754,17 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip,
return ret;
}
- st->vdd_supply = devm_regulator_get(dev, "vdd");
- if (IS_ERR(st->vdd_supply))
- return PTR_ERR(st->vdd_supply);
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to get vdd regulator\n");
+
+ msleep(INV_ICM42600_POWER_UP_TIME_MS);
st->vddio_supply = devm_regulator_get(dev, "vddio");
if (IS_ERR(st->vddio_supply))
return PTR_ERR(st->vddio_supply);
- ret = regulator_enable(st->vdd_supply);
- if (ret)
- return ret;
- msleep(INV_ICM42600_POWER_UP_TIME_MS);
-
- ret = devm_add_action_or_reset(dev, inv_icm42600_disable_vdd_reg, st);
- if (ret)
- return ret;
-
ret = inv_icm42600_enable_regulator_vddio(st);
if (ret)
return ret;
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 5/5] iio: imu: inv_icm42600: use guard() to release mutexes
2025-09-09 7:11 [PATCH v4 0/5] iio: imu: inv_icm42600: pm_runtime fixes + various changes Sean Nyekjaer
` (3 preceding siblings ...)
2025-09-09 7:11 ` [PATCH v4 4/5] iio: imu: inv_icm42600: Use devm_regulator_get_enable() for vdd regulator Sean Nyekjaer
@ 2025-09-09 7:11 ` Sean Nyekjaer
2025-09-09 11:39 ` Andy Shevchenko
2025-09-09 14:03 ` David Lechner
4 siblings, 2 replies; 11+ messages in thread
From: Sean Nyekjaer @ 2025-09-09 7:11 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol, rafael, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-iio, linux-kernel, linux-pm, Jean-Baptiste Maneyrol,
Jonathan Cameron, Sean Nyekjaer
Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
for cleaner and safer mutex handling.
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 24 +++------
drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 41 +++++++--------
drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 59 ++++++++--------------
drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 24 +++------
4 files changed, 56 insertions(+), 92 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index 48014b61ced335eb2c8549cfc2e79ccde1934308..54760d8f92a279334338fd09e3ab74b2d939a46d 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -561,11 +561,10 @@ static int inv_icm42600_accel_write_scale(struct iio_dev *indio_dev,
conf.fs = idx / 2;
pm_runtime_get_sync(dev);
- mutex_lock(&st->lock);
- ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
+ scoped_guard(mutex, &st->lock)
+ ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
- mutex_unlock(&st->lock);
pm_runtime_put_autosuspend(dev);
return ret;
@@ -986,16 +985,11 @@ static int inv_icm42600_accel_hwfifo_set_watermark(struct iio_dev *indio_dev,
unsigned int val)
{
struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
- int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
st->fifo.watermark.accel = val;
- ret = inv_icm42600_buffer_update_watermark(st);
-
- mutex_unlock(&st->lock);
-
- return ret;
+ return inv_icm42600_buffer_update_watermark(st);
}
static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev,
@@ -1007,15 +1001,13 @@ static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev,
if (count == 0)
return 0;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = inv_icm42600_buffer_hwfifo_flush(st, count);
- if (!ret)
- ret = st->fifo.nb.accel;
-
- mutex_unlock(&st->lock);
+ if (ret)
+ return ret;
- return ret;
+ return st->fifo.nb.accel;
}
static int inv_icm42600_accel_read_event_config(struct iio_dev *indio_dev,
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
index 36d69a0face655bf2dc9229c52d07448e9b2ca02..15391961bf969237acf67e328e92c2911c77924f 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
@@ -283,9 +283,8 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev)
pm_runtime_get_sync(dev);
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
inv_sensors_timestamp_reset(ts);
- mutex_unlock(&st->lock);
return 0;
}
@@ -299,43 +298,41 @@ static int inv_icm42600_buffer_postenable(struct iio_dev *indio_dev)
struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
/* exit if FIFO is already on */
if (st->fifo.on) {
- ret = 0;
- goto out_on;
+ st->fifo.on++;
+ return 0;
}
/* set FIFO threshold interrupt */
ret = regmap_set_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
if (ret)
- goto out_unlock;
+ return ret;
/* flush FIFO data */
ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
if (ret)
- goto out_unlock;
+ return ret;
/* set FIFO in streaming mode */
ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
INV_ICM42600_FIFO_CONFIG_STREAM);
if (ret)
- goto out_unlock;
+ return ret;
/* workaround: first read of FIFO count after reset is always 0 */
ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT, st->buffer, 2);
if (ret)
- goto out_unlock;
+ return ret;
-out_on:
/* increase FIFO on counter */
st->fifo.on++;
-out_unlock:
- mutex_unlock(&st->lock);
- return ret;
+
+ return 0;
}
static int inv_icm42600_buffer_predisable(struct iio_dev *indio_dev)
@@ -343,38 +340,36 @@ static int inv_icm42600_buffer_predisable(struct iio_dev *indio_dev)
struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
/* exit if there are several sensors using the FIFO */
if (st->fifo.on > 1) {
- ret = 0;
- goto out_off;
+ st->fifo.on--;
+ return 0;
}
/* set FIFO in bypass mode */
ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
INV_ICM42600_FIFO_CONFIG_BYPASS);
if (ret)
- goto out_unlock;
+ return ret;
/* flush FIFO data */
ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
if (ret)
- goto out_unlock;
+ return ret;
/* disable FIFO threshold interrupt */
ret = regmap_clear_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
if (ret)
- goto out_unlock;
+ return ret;
-out_off:
/* decrease FIFO on counter */
st->fifo.on--;
-out_unlock:
- mutex_unlock(&st->lock);
- return ret;
+
+ return 0;
}
static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 4bf436c46f1cfd7e7e1bb911d94a0a566d63e791..76eb22488e5f31623a0f93d856fb4d10182f1ac2 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -439,18 +439,13 @@ int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
unsigned int writeval, unsigned int *readval)
{
struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
- int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
if (readval)
- ret = regmap_read(st->map, reg, readval);
- else
- ret = regmap_write(st->map, reg, writeval);
-
- mutex_unlock(&st->lock);
+ return regmap_read(st->map, reg, readval);
- return ret;
+ return regmap_write(st->map, reg, writeval);
}
static int inv_icm42600_set_conf(struct inv_icm42600_state *st,
@@ -820,22 +815,22 @@ static int inv_icm42600_suspend(struct device *dev)
struct device *accel_dev;
bool wakeup;
int accel_conf;
- int ret = 0;
+ int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
st->suspended.gyro = st->conf.gyro.mode;
st->suspended.accel = st->conf.accel.mode;
st->suspended.temp = st->conf.temp_en;
if (pm_runtime_suspended(dev))
- goto out_unlock;
+ return 0;
/* disable FIFO data streaming */
if (st->fifo.on) {
ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
INV_ICM42600_FIFO_CONFIG_BYPASS);
if (ret)
- goto out_unlock;
+ return ret;
}
/* keep chip on and wake-up capable if APEX and wakeup on */
@@ -851,7 +846,7 @@ static int inv_icm42600_suspend(struct device *dev)
if (st->apex.wom.enable) {
ret = inv_icm42600_disable_wom(st);
if (ret)
- goto out_unlock;
+ return ret;
}
accel_conf = INV_ICM42600_SENSOR_MODE_OFF;
}
@@ -859,15 +854,13 @@ static int inv_icm42600_suspend(struct device *dev)
ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
accel_conf, false, NULL);
if (ret)
- goto out_unlock;
+ return ret;
/* disable vddio regulator if chip is sleeping */
if (!wakeup)
regulator_disable(st->vddio_supply);
-out_unlock:
- mutex_unlock(&st->lock);
- return ret;
+ return 0;
}
/*
@@ -881,12 +874,12 @@ static int inv_icm42600_resume(struct device *dev)
struct inv_icm42600_sensor_state *accel_st = iio_priv(st->indio_accel);
struct device *accel_dev;
bool wakeup;
- int ret = 0;
+ int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
if (pm_runtime_suspended(dev))
- goto out_unlock;
+ return 0;
/* check wakeup capability */
accel_dev = &st->indio_accel->dev;
@@ -898,7 +891,7 @@ static int inv_icm42600_resume(struct device *dev)
} else {
ret = inv_icm42600_enable_regulator_vddio(st);
if (ret)
- goto out_unlock;
+ return ret;
}
/* restore sensors state */
@@ -906,13 +899,13 @@ static int inv_icm42600_resume(struct device *dev)
st->suspended.accel,
st->suspended.temp, NULL);
if (ret)
- goto out_unlock;
+ return ret;
/* restore APEX features if disabled */
if (!wakeup && st->apex.wom.enable) {
ret = inv_icm42600_enable_wom(st);
if (ret)
- goto out_unlock;
+ return ret;
}
/* restore FIFO data streaming */
@@ -923,9 +916,7 @@ static int inv_icm42600_resume(struct device *dev)
INV_ICM42600_FIFO_CONFIG_STREAM);
}
-out_unlock:
- mutex_unlock(&st->lock);
- return ret;
+ return 0;
}
/* Runtime suspend will turn off sensors that are enabled by iio devices. */
@@ -934,34 +925,28 @@ static int inv_icm42600_runtime_suspend(struct device *dev)
struct inv_icm42600_state *st = dev_get_drvdata(dev);
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
/* disable all sensors */
ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
INV_ICM42600_SENSOR_MODE_OFF, false,
NULL);
if (ret)
- goto error_unlock;
+ return ret;
regulator_disable(st->vddio_supply);
-error_unlock:
- mutex_unlock(&st->lock);
- return ret;
+ return 0;
}
/* Sensors are enabled by iio devices, no need to turn them back on here. */
static int inv_icm42600_runtime_resume(struct device *dev)
{
struct inv_icm42600_state *st = dev_get_drvdata(dev);
- int ret;
- mutex_lock(&st->lock);
-
- ret = inv_icm42600_enable_regulator_vddio(st);
+ guard(mutex)(&st->lock);
- mutex_unlock(&st->lock);
- return ret;
+ return inv_icm42600_enable_regulator_vddio(st);
}
EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = {
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
index 8a7cc91276319f0b1714ad11d46e409688b258c4..7ef0a25ec74f6b005ca6e86058d67d0be67327df 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
@@ -277,11 +277,10 @@ static int inv_icm42600_gyro_write_scale(struct iio_dev *indio_dev,
conf.fs = idx / 2;
pm_runtime_get_sync(dev);
- mutex_lock(&st->lock);
- ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
+ scoped_guard(mutex, &st->lock)
+ ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
- mutex_unlock(&st->lock);
pm_runtime_put_autosuspend(dev);
return ret;
@@ -688,16 +687,11 @@ static int inv_icm42600_gyro_hwfifo_set_watermark(struct iio_dev *indio_dev,
unsigned int val)
{
struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
- int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
st->fifo.watermark.gyro = val;
- ret = inv_icm42600_buffer_update_watermark(st);
-
- mutex_unlock(&st->lock);
-
- return ret;
+ return inv_icm42600_buffer_update_watermark(st);
}
static int inv_icm42600_gyro_hwfifo_flush(struct iio_dev *indio_dev,
@@ -709,15 +703,13 @@ static int inv_icm42600_gyro_hwfifo_flush(struct iio_dev *indio_dev,
if (count == 0)
return 0;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = inv_icm42600_buffer_hwfifo_flush(st, count);
- if (!ret)
- ret = st->fifo.nb.gyro;
-
- mutex_unlock(&st->lock);
+ if (ret)
+ return ret;
- return ret;
+ return st->fifo.nb.gyro;
}
static const struct iio_info inv_icm42600_gyro_info = {
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread