[PATCH v2 0/5] iio: imu: inv_icm42600: pm_runtime fixes + various changes

[PATCH v2 0/5] iio: imu: inv_icm42600: pm_runtime fixes + various changes

[Sean Nyekjaer]

This series was triggered by "Runtime PM usage count underflow!" when
unloading the module(s).
By testing the driver in various use cases and reading code it was
obvious that it could need some tiding up.

I'm still not 100% satisfied with suspend/resume is calling directly to
vddio_enable/disable. In my mind it should be managed by pm_runtime.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes in v2:
- Removed patch iio: imu: inv_icm42600: Use inv_icm42600_disable_vddio_reg()
- Moved changes from patch iio: imu: inv_icm42600: Remove redundant
  error msg on regulator_disable() into iio: imu: inv_icm42600: Simplify
  pm_runtime setup.
- Move associated sleep close to enabling of vdd
- Pass regulator as the parameter to inv_icm42600_disable_vddio_reg()
- Use devm_pm_runtime_set_active_enabled() to simplify even more
- Added a new commit that uses guard() to release mutexes
- Link to v1: https://lore.kernel.org/r/20250709-icm42pmreg-v1-0-3d0e793c99b2@geanix.com

---
Sean Nyekjaer (5):
      iio: imu: inv_icm42600: Simplify pm_runtime setup
      iio: imu: inv_icm42600: Drop redundant pm_runtime reinitialization in resume
      iio: imu: inv_icm42600: Avoid configuring if already pm_runtime suspended
      iio: imu: inv_icm42600: Use devm_regulator_get_enable() for vdd regulator
      iio: imu: inv_icm42600: use guard() to release mutexes

 drivers/iio/imu/inv_icm42600/inv_icm42600.h        |   1 -
 drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c  |  11 +--
 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c |  27 +++---
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c   | 105 +++++++--------------
 drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c   |  11 +--
 5 files changed, 51 insertions(+), 104 deletions(-)
---
base-commit: a5651af7f531ca57b9fc3114de77d9050012eb8d
change-id: 20250708-icm42pmreg-24d824d978c4

Best regards,
-- 
Sean Nyekjaer <sean@geanix.com>

[PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[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, 6 insertions(+), 18 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..c19615750c717101312f358a9160dd2c455cfb14 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -711,20 +711,10 @@ 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);
-}
-
-static void inv_icm42600_disable_pm(void *_data)
-{
-	struct device *dev = _data;
+	struct device *dev = regmap_get_device(st->map);
 
-	pm_runtime_put_sync(dev);
-	pm_runtime_disable(dev);
+	if (!pm_runtime_status_suspended(dev))
+		regulator_disable(st->vddio_supply);
 }
 
 int inv_icm42600_core_probe(struct regmap *regmap, int chip,
@@ -824,16 +814,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

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Andy Shevchenko]

On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
>
> 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.

...

> +       struct device *dev = regmap_get_device(st->map);
>
> +       if (!pm_runtime_status_suspended(dev))
> +               regulator_disable(st->vddio_supply);

I would rather use positive conditional as it seems to me more scalable

>  }


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Jonathan Cameron]

On Fri, 8 Aug 2025 23:37:51 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
> >
> > 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.  
> 
> ...
> 
> > +       struct device *dev = regmap_get_device(st->map);
> >
> > +       if (!pm_runtime_status_suspended(dev))
> > +               regulator_disable(st->vddio_supply);  
> 
> I would rather use positive conditional as it seems to me more scalable
Hi Andy,
> 

To potentially save time when Sean looks at this.  I don't follow. Do you mean
something like
	if (pm_runtime_status_suspended(dev))
		return;

	regulator_disable(st->vddio_supply);

?

If so I'm not seeing why we'd want this to scale as it's a single use
devm_set_action_or_reset() callback doing just one thing.

Jonathan

> >  }  
> 
> 

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Andy Shevchenko]

On Sat, Aug 9, 2025 at 8:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 8 Aug 2025 23:37:51 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:

...

> > > +       struct device *dev = regmap_get_device(st->map);
> > >
> > > +       if (!pm_runtime_status_suspended(dev))
> > > +               regulator_disable(st->vddio_supply);
> >
> > I would rather use positive conditional as it seems to me more scalable
>
> To potentially save time when Sean looks at this.  I don't follow. Do you mean
> something like
>         if (pm_runtime_status_suspended(dev))
>                 return;
>
>         regulator_disable(st->vddio_supply);
>
> ?

Yes.

> If so I'm not seeing why we'd want this to scale as it's a single use
> devm_set_action_or_reset() callback doing just one thing.

While I agree in _this_ case, in general the check and return
immediately is more scalable for reading purposes, e.g., indentation
will be one level less. Also it won't require additional churn in
adding {, i.e. changing conditional line just for that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Sean Nyekjaer]

On Sat, Aug 09, 2025 at 10:27:52PM +0100, Andy Shevchenko wrote:
> On Sat, Aug 9, 2025 at 8:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Fri, 8 Aug 2025 23:37:51 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
> 
> ...
> 
> > > > +       struct device *dev = regmap_get_device(st->map);
> > > >
> > > > +       if (!pm_runtime_status_suspended(dev))
> > > > +               regulator_disable(st->vddio_supply);
> > >
> > > I would rather use positive conditional as it seems to me more scalable
> >
> > To potentially save time when Sean looks at this.  I don't follow. Do you mean
> > something like
> >         if (pm_runtime_status_suspended(dev))
> >                 return;
> >
> >         regulator_disable(st->vddio_supply);
> >
> > ?
> 
> Yes.
> 
> > If so I'm not seeing why we'd want this to scale as it's a single use
> > devm_set_action_or_reset() callback doing just one thing.
> 
> While I agree in _this_ case, in general the check and return
> immediately is more scalable for reading purposes, e.g., indentation
> will be one level less. Also it won't require additional churn in
> adding {, i.e. changing conditional line just for that.
> 

I like the return early if pm_runtime_status_suspended() is true.

Andy, when doing reviews please keep the function name, as it's much
easier to add the changes.

Jonathan, do we think checking pm_runtime_status_suspended() is a viable
option? Should we ask on the linux-pm list?

If it's ok; I will patch:
drivers/iio/adc/ti-ads1100.c
drivers/iio/pressure/bmp280-core.c
drivers/iio/pressure/icp10100.c

/Sean

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Andy Shevchenko]

On Mon, Aug 11, 2025 at 4:21 PM Sean Nyekjaer <sean@geanix.com> wrote:
> On Sat, Aug 09, 2025 at 10:27:52PM +0100, Andy Shevchenko wrote:
> > On Sat, Aug 9, 2025 at 8:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Fri, 8 Aug 2025 23:37:51 +0200
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:

...

> > > > > +       struct device *dev = regmap_get_device(st->map);
> > > > >
> > > > > +       if (!pm_runtime_status_suspended(dev))
> > > > > +               regulator_disable(st->vddio_supply);


> Andy, when doing reviews please keep the function name, as it's much
> easier to add the changes.

Sure, sorry I missed it, I thought it was obvious, but you are right,
it's better to keep more context.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Sean Nyekjaer]

On Mon, Aug 11, 2025 at 02:21:26PM +0100, Sean Nyekjaer wrote:
> On Sat, Aug 09, 2025 at 10:27:52PM +0100, Andy Shevchenko wrote:
> > On Sat, Aug 9, 2025 at 8:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Fri, 8 Aug 2025 23:37:51 +0200
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
> > 
> > ...
> > 
> > > > > +       struct device *dev = regmap_get_device(st->map);
> > > > >
> > > > > +       if (!pm_runtime_status_suspended(dev))
> > > > > +               regulator_disable(st->vddio_supply);
> > > >
> > > > I would rather use positive conditional as it seems to me more scalable
> > >
> > > To potentially save time when Sean looks at this.  I don't follow. Do you mean
> > > something like
> > >         if (pm_runtime_status_suspended(dev))
> > >                 return;
> > >
> > >         regulator_disable(st->vddio_supply);
> > >
> > > ?
> > 
> > Yes.
> > 
> > > If so I'm not seeing why we'd want this to scale as it's a single use
> > > devm_set_action_or_reset() callback doing just one thing.
> > 
> > While I agree in _this_ case, in general the check and return
> > immediately is more scalable for reading purposes, e.g., indentation
> > will be one level less. Also it won't require additional churn in
> > adding {, i.e. changing conditional line just for that.
> > 
> 
> I like the return early if pm_runtime_status_suspended() is true.
> 
> Andy, when doing reviews please keep the function name, as it's much
> easier to add the changes.
> 
> Jonathan, do we think checking pm_runtime_status_suspended() is a viable
> option? Should we ask on the linux-pm list?
> 
> If it's ok; I will patch:
> drivers/iio/adc/ti-ads1100.c
> drivers/iio/pressure/bmp280-core.c
> drivers/iio/pressure/icp10100.c

Hi Jonathan,

Did you see my question here?
Just ignore this if you are vacationing...

/Sean

Re: [PATCH v2 1/5] iio: imu: inv_icm42600: Simplify pm_runtime setup

[Jonathan Cameron]

On Mon, 25 Aug 2025 15:14:15 +0000
Sean Nyekjaer <sean@geanix.com> wrote:

> On Mon, Aug 11, 2025 at 02:21:26PM +0100, Sean Nyekjaer wrote:
> > On Sat, Aug 09, 2025 at 10:27:52PM +0100, Andy Shevchenko wrote:  
> > > On Sat, Aug 9, 2025 at 8:06 PM Jonathan Cameron <jic23@kernel.org> wrote:  
> > > > On Fri, 8 Aug 2025 23:37:51 +0200
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > > > On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:  
> > > 
> > > ...
> > >   
> > > > > > +       struct device *dev = regmap_get_device(st->map);
> > > > > >
> > > > > > +       if (!pm_runtime_status_suspended(dev))
> > > > > > +               regulator_disable(st->vddio_supply);  
> > > > >
> > > > > I would rather use positive conditional as it seems to me more scalable  
> > > >
> > > > To potentially save time when Sean looks at this.  I don't follow. Do you mean
> > > > something like
> > > >         if (pm_runtime_status_suspended(dev))
> > > >                 return;
> > > >
> > > >         regulator_disable(st->vddio_supply);
> > > >
> > > > ?  
> > > 
> > > Yes.
> > >   
> > > > If so I'm not seeing why we'd want this to scale as it's a single use
> > > > devm_set_action_or_reset() callback doing just one thing.  
> > > 
> > > While I agree in _this_ case, in general the check and return
> > > immediately is more scalable for reading purposes, e.g., indentation
> > > will be one level less. Also it won't require additional churn in
> > > adding {, i.e. changing conditional line just for that.
> > >   
> > 
> > I like the return early if pm_runtime_status_suspended() is true.
> > 
> > Andy, when doing reviews please keep the function name, as it's much
> > easier to add the changes.
> > 
> > Jonathan, do we think checking pm_runtime_status_suspended() is a viable
> > option? Should we ask on the linux-pm list?

Perhaps +CC linux-pm and Rafael on one of the patches and see if any replies.
> > 
> > If it's ok; I will patch:
> > drivers/iio/adc/ti-ads1100.c
> > drivers/iio/pressure/bmp280-core.c
> > drivers/iio/pressure/icp10100.c  
> 
> Hi Jonathan,
> 
> Did you see my question here?

Indeed not. Wise to poke!

> Just ignore this if you are vacationing...
> 
> /Sean
> 
> 

[PATCH v2 2/5] iio: imu: inv_icm42600: Drop redundant pm_runtime reinitialization in resume

[Sean Nyekjaer]

Remove unnecessary calls to pm_runtime_disable(), pm_runtime_set_active(),
and pm_runtime_enable() from the resume path. These operations are not
required here and can interfere with proper pm_runtime state handling,
especially when resuming from a pm_runtime suspended state.

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 | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index c19615750c717101312f358a9160dd2c455cfb14..bb0dbbb7ad03bb57ffd3180785a686f2cdb5c845 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -915,10 +915,6 @@ static int inv_icm42600_resume(struct device *dev)
 			goto out_unlock;
 	}
 
-	pm_runtime_disable(dev);
-	pm_runtime_set_active(dev);
-	pm_runtime_enable(dev);
-
 	/* restore sensors state */
 	ret = inv_icm42600_set_pwr_mgmt0(st, st->suspended.gyro,
 					 st->suspended.accel,

-- 
2.50.1

[PATCH v2 3/5] iio: imu: inv_icm42600: Avoid configuring if already pm_runtime suspended

[Sean Nyekjaer]

Do as in suspend, skip resume configuration steps if the device is already
pm_runtime suspended. This avoids reconfiguring a device that is already
in the correct low-power state and ensures that pm_runtime handles the
power state transitions properly.

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 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index bb0dbbb7ad03bb57ffd3180785a686f2cdb5c845..39ba94e2e473abe7af64c655f0399bd756ddfe11 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -835,17 +835,15 @@ static int inv_icm42600_suspend(struct device *dev)
 	struct device *accel_dev;
 	bool wakeup;
 	int accel_conf;
-	int ret;
+	int ret = 0;
 
 	mutex_lock(&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)) {
-		ret = 0;
+	if (pm_runtime_suspended(dev))
 		goto out_unlock;
-	}
 
 	/* disable FIFO data streaming */
 	if (st->fifo.on) {
@@ -898,10 +896,13 @@ 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;
+	int ret = 0;
 
 	mutex_lock(&st->lock);
 
+	if (pm_runtime_suspended(dev))
+		goto out_unlock;
+
 	/* check wakeup capability */
 	accel_dev = &st->indio_accel->dev;
 	wakeup = st->apex.on && device_may_wakeup(accel_dev);

-- 
2.50.1

[PATCH v2 4/5] iio: imu: inv_icm42600: Use devm_regulator_get_enable() for vdd regulator

[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.

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 39ba94e2e473abe7af64c655f0399bd756ddfe11..52f61e90fec3e08effc1d398293bece5413406d1 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;
@@ -763,23 +752,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

[PATCH v2 5/5] iio: imu: inv_icm42600: use guard() to release mutexes

[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  | 11 ++----
 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 27 ++++++--------
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c   | 43 +++++++++-------------
 drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c   | 11 ++----
 4 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index 7a28051330b79098bfa94b8c8c78c2bce20b7230..1e58253b3ddc0028d0899ec04cabbc65c6bf2b72 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -562,11 +562,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);
+	guard(mutex)(&st->lock);
 
 	ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
 
-	mutex_unlock(&st->lock);
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
@@ -993,13 +992,11 @@ static int inv_icm42600_accel_hwfifo_set_watermark(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);
 
 	st->fifo.watermark.accel = val;
 	ret = inv_icm42600_buffer_update_watermark(st);
 
-	mutex_unlock(&st->lock);
-
 	return ret;
 }
 
@@ -1012,14 +1009,12 @@ 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);
-
 	return ret;
 }
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
index 7c4ed981db043b4e8f3967b0802655d34f863954..d052b7069dc5c99dfea634415c107e188994c995 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
@@ -292,9 +292,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;
 }
@@ -308,7 +307,7 @@ 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) {
@@ -320,30 +319,29 @@ static int inv_icm42600_buffer_postenable(struct iio_dev *indio_dev)
 	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;
 }
 
@@ -352,7 +350,7 @@ 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) {
@@ -364,25 +362,24 @@ static int inv_icm42600_buffer_predisable(struct iio_dev *indio_dev)
 	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;
 }
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 52f61e90fec3e08effc1d398293bece5413406d1..82e6c8b3fba5b63d4eebb0a68bd88b950dbe881d 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -441,15 +441,13 @@ int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
 	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 ret;
 }
 
@@ -820,20 +818,21 @@ static int inv_icm42600_suspend(struct device *dev)
 	int accel_conf;
 	int ret = 0;
 
-	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;
+	ret = pm_runtime_suspended(dev);
+	if (ret)
+		return ret;
 
 	/* 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 */
@@ -849,7 +848,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;
 	}
@@ -857,14 +856,12 @@ 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;
 }
 
@@ -881,10 +878,11 @@ static int inv_icm42600_resume(struct device *dev)
 	bool wakeup;
 	int ret = 0;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
-	if (pm_runtime_suspended(dev))
-		goto out_unlock;
+	ret = pm_runtime_suspended(dev);
+	if (ret)
+		return ret;
 
 	/* check wakeup capability */
 	accel_dev = &st->indio_accel->dev;
@@ -896,7 +894,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 */
@@ -904,13 +902,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 */
@@ -921,8 +919,6 @@ static int inv_icm42600_resume(struct device *dev)
 				   INV_ICM42600_FIFO_CONFIG_STREAM);
 	}
 
-out_unlock:
-	mutex_unlock(&st->lock);
 	return ret;
 }
 
@@ -932,19 +928,17 @@ 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;
 }
 
@@ -954,11 +948,10 @@ static int inv_icm42600_runtime_resume(struct device *dev)
 	struct inv_icm42600_state *st = dev_get_drvdata(dev);
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = inv_icm42600_enable_regulator_vddio(st);
 
-	mutex_unlock(&st->lock);
 	return ret;
 }
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
index 9ba6f13628e6af5e19d047476ae93754f07aa95f..875e48748d70678bcd3963187be6de8c5a2b2fcc 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
@@ -278,11 +278,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);
+	guard(mutex)(&st->lock);
 
 	ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
 
-	mutex_unlock(&st->lock);
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
@@ -695,13 +694,11 @@ static int inv_icm42600_gyro_hwfifo_set_watermark(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);
 
 	st->fifo.watermark.gyro = val;
 	ret = inv_icm42600_buffer_update_watermark(st);
 
-	mutex_unlock(&st->lock);
-
 	return ret;
 }
 
@@ -714,14 +711,12 @@ 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);
-
 	return ret;
 }
 

-- 
2.50.1

Re: [PATCH v2 5/5] iio: imu: inv_icm42600: use guard() to release mutexes

[Andy Shevchenko]

On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
>
> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
> for cleaner and safer mutex handling.

...

>         pm_runtime_get_sync(dev);
> -       mutex_lock(&st->lock);
> +       guard(mutex)(&st->lock);
>
>         ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
>
> -       mutex_unlock(&st->lock);
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_put_autosuspend(dev);

This makes PM calls under the mutex. In some cases it may lead to deadlocks.
I think you wanted to use scoped_guard() here and in similar cases.

...

>         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;

Now remove ret and use return directly.

>  }

> -       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);
> -
>         return ret;

In the similar way as above.

ret = _flush();
if (ret)
  return ret;

return ...nb.accel;

...

>         struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);

>         int ret;

Now unneeded, just return directly.

> -       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 ret;

...

>         int ret = 0;

Now unneeded assignment.

> -       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;
> +       ret = pm_runtime_suspended(dev);
> +       if (ret)
> +               return ret;

...

>         /* disable vddio regulator if chip is sleeping */
>         if (!wakeup)
>                 regulator_disable(st->vddio_supply);
>
> -out_unlock:
> -       mutex_unlock(&st->lock);
>         return ret;

Now return 0 to make it clear that this is a success.

...

> @@ -881,10 +878,11 @@ static int inv_icm42600_resume(struct device *dev)
>         bool wakeup;
>         int ret = 0;

Assignment is useless now.

> -       mutex_lock(&st->lock);
> +       guard(mutex)(&st->lock);
>
> -       if (pm_runtime_suspended(dev))
> -               goto out_unlock;
> +       ret = pm_runtime_suspended(dev);
> +       if (ret)
> +               return ret;

...

> -out_unlock:
> -       mutex_unlock(&st->lock);
>         return ret;

  return 0;

?

...

>         regulator_disable(st->vddio_supply);
>
> -error_unlock:
> -       mutex_unlock(&st->lock);
>         return ret;

Ditto.

>  }

...

>         int ret;

Now useless variable.

>
> -       mutex_lock(&st->lock);
> +       guard(mutex)(&st->lock);
>
>         ret = inv_icm42600_enable_regulator_vddio(st);
>
> -       mutex_unlock(&st->lock);
>         return ret;
>  }

...

>         int ret;

Ditto.

> -       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;
>  }

...

> -       mutex_lock(&st->lock);
> +       guard(mutex)(&st->lock);
>
>         ret = inv_icm42600_buffer_hwfifo_flush(st, count);
>         if (!ret)
>                 ret = st->fifo.nb.gyro;

Invert conditional and return ret directly.

> -       mutex_unlock(&st->lock);
> -
>         return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] iio: imu: inv_icm42600: use guard() to release mutexes

[Sean Nyekjaer]

On Fri, Aug 08, 2025 at 11:52:35PM +0100, Andy Shevchenko wrote:
> On Fri, Aug 8, 2025 at 5:58 PM Sean Nyekjaer <sean@geanix.com> wrote:
> >
> > Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
> > for cleaner and safer mutex handling.
> 
> ...
> 
> >         pm_runtime_get_sync(dev);
> > -       mutex_lock(&st->lock);
> > +       guard(mutex)(&st->lock);
> >
> >         ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
> >
> > -       mutex_unlock(&st->lock);
> >         pm_runtime_mark_last_busy(dev);
> >         pm_runtime_put_autosuspend(dev);
> 
> This makes PM calls under the mutex. In some cases it may lead to deadlocks.
> I think you wanted to use scoped_guard() here and in similar cases.

Oh, good catch :)

> 
> ...
> 
> >         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;
> 
> Now remove ret and use return directly.
> 
> >  }
> 
> > -       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);
> > -
> >         return ret;
> 
> In the similar way as above.
> 
> ret = _flush();
> if (ret)
>   return ret;
> 
> return ...nb.accel;
> 
> ...
> 
> >         struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> 
> >         int ret;
> 
> Now unneeded, just return directly.
> 
> > -       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 ret;
> 
> ...
> 
> >         int ret = 0;
> 
> Now unneeded assignment.
> 
> > -       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;
> > +       ret = pm_runtime_suspended(dev);
> > +       if (ret)
> > +               return ret;
> 
> ...
> 
> >         /* disable vddio regulator if chip is sleeping */
> >         if (!wakeup)
> >                 regulator_disable(st->vddio_supply);
> >
> > -out_unlock:
> > -       mutex_unlock(&st->lock);
> >         return ret;
> 
> Now return 0 to make it clear that this is a success.
> 
> ...
> 
> > @@ -881,10 +878,11 @@ static int inv_icm42600_resume(struct device *dev)
> >         bool wakeup;
> >         int ret = 0;
> 
> Assignment is useless now.
> 
> > -       mutex_lock(&st->lock);
> > +       guard(mutex)(&st->lock);
> >
> > -       if (pm_runtime_suspended(dev))
> > -               goto out_unlock;
> > +       ret = pm_runtime_suspended(dev);
> > +       if (ret)
> > +               return ret;
> 
> ...
> 
> > -out_unlock:
> > -       mutex_unlock(&st->lock);
> >         return ret;
> 
>   return 0;
> 
> ?
> 
> ...
> 
> >         regulator_disable(st->vddio_supply);
> >
> > -error_unlock:
> > -       mutex_unlock(&st->lock);
> >         return ret;
> 
> Ditto.
> 
> >  }
> 
> ...
> 
> >         int ret;
> 
> Now useless variable.
> 
> >
> > -       mutex_lock(&st->lock);
> > +       guard(mutex)(&st->lock);
> >
> >         ret = inv_icm42600_enable_regulator_vddio(st);
> >
> > -       mutex_unlock(&st->lock);
> >         return ret;
> >  }
> 
> ...
> 
> >         int ret;
> 
> Ditto.
> 
> > -       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;
> >  }
> 
> ...
> 
> > -       mutex_lock(&st->lock);
> > +       guard(mutex)(&st->lock);
> >
> >         ret = inv_icm42600_buffer_hwfifo_flush(st, count);
> >         if (!ret)
> >                 ret = st->fifo.nb.gyro;
> 
> Invert conditional and return ret directly.
> 
> > -       mutex_unlock(&st->lock);
> > -
> >         return ret;
> 
> --
> With Best Regards,
> Andy Shevchenko

Thanks for the review.

/Sean