[PATCH V7 05/11] iio: imu: inv_icm42607: Add PM support for icm42607
Jonathan Cameron
jic23 at kernel.org
Fri May 15 11:59:54 PDT 2026
On Fri, 15 May 2026 08:00:10 -0500
Chris Morgan <macroalpha82 at gmail.com> wrote:
> From: Chris Morgan <macromorgan at hotmail.com>
>
> Add power management support for the ICM42607 device driver.
>
> Signed-off-by: Chris Morgan <macromorgan at hotmail.com>
Sashiko had a comment but I'd ignore it as it's about the rather
complex magic that lets us do various forms of PM configs without
lots of __maybe_unused etc and it works so don't touch it :)
I'm too lazy to check what the dependency ordering is other than
pm_ptr() is true in cases where pm_sleep_ptr() whereas the
opposite is not true and that means the actual dead code removal
occurs at a different point in the code those macros are producing.
Anyhow a few really minor things inline. Otherwise LGTM
> ---
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 12 ++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 143 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 +
> .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 +
> 4 files changed, 157 insertions(+)
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 2c20e95b237a..5f37999e39a5 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 1088c5c7076f..e27ad0319a12 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> static int inv_icm42607_set_conf(struct inv_icm42607_state *st,
> const struct inv_icm42607_conf *conf)
> {
> @@ -198,11 +255,15 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> {
> int ret;
>
> + if (st->vddio_en)
> + return 0;
> +
> ret = regulator_enable(st->vddio_supply);
> if (ret)
> return ret;
>
> fsleep(INV_ICM42607_POWER_UP_TIME_US);
> + st->vddio_en = 1;
It's a bool (which is good) so = true
>
> return 0;
> }
> @@ -211,7 +272,10 @@ static void inv_icm42607_disable_vddio_reg(void *_data)
> {
> struct inv_icm42607_state *st = _data;
>
> + if (!st->vddio_en)
> + return;
> regulator_disable(st->vddio_supply);
> + st->vddio_en = 0;
= false
> }
>
> +static int inv_icm42607_suspend(struct device *dev)
> +{
> + struct inv_icm42607_state *st = dev_get_drvdata(dev);
> + int ret;
> +
> + guard(mutex)(&st->lock);
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + ret = inv_icm42607_set_pwr_mgmt0(st, INV_ICM42607_SENSOR_MODE_OFF,
> + INV_ICM42607_SENSOR_MODE_OFF,
> + false, NULL);
> + if (ret)
> + return ret;
Trivial but blank line here
> + inv_icm42607_disable_vddio_reg(st);
> +
> + return 0;
> +}
> +
> +static int inv_icm42607_runtime_suspend(struct device *dev)
> +{
> + struct inv_icm42607_state *st = dev_get_drvdata(dev);
> + int ret = 0;
Assigned in all paths I think? If that stops being true I'd probably still
introduce the assignment when it becomes the case even though it's a little
more code churn.
> +
> + guard(mutex)(&st->lock);
> +
> + ret = inv_icm42607_set_pwr_mgmt0(st, INV_ICM42607_SENSOR_MODE_OFF,
> + INV_ICM42607_SENSOR_MODE_OFF, false,
> + NULL);
> + if (ret)
> + return ret;
> +
> + inv_icm42607_disable_vddio_reg(st);
> +
> + return 0;
> +}
More information about the Linux-rockchip
mailing list