* [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking
@ 2026-05-05 22:07 Maxwell Doose
2026-05-06 9:06 ` Andy Shevchenko
2026-05-06 16:05 ` Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Maxwell Doose @ 2026-05-05 22:07 UTC (permalink / raw)
To: jic23
Cc: David Lechner, Nuno Sá, Andy Shevchenko,
open list:IIO SUBSYSTEM AND DRIVERS, open list
Include linux/cleanup.h to take advantage of new macros.
Replace manual mutex_lock() and mutex_unlock() calls across the file
with guard(mutex)() and scoped_guard() where appropriate. This will help
modernize the driver with up-to-date functions/macros.
Remove now redundant gotos and ret variables, as the new RAII macros
make them unneeded.
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
v2:
- Remove redundant blank line per Andy.
- Put kmx61_set_mode() function call in kmx61_runtime_suspend() on one
line per Andy.
v3:
- Add dedicated scope for guards.
v4:
- Fix certain scoping in switch-case blocks.
- Remove stray ret variable.
- Used scoped_guard() in kmx61_trigger_handler() instead of guard()().
drivers/iio/imu/kmx61.c | 98 ++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 60 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 3cd91d8a89ee..1f1a695ab161 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -7,6 +7,7 @@
* IIO driver for KMX61 (7-bit I2C slave address 0x0E or 0x0F).
*/
+#include <linux/cleanup.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/mod_devicetable.h>
@@ -783,7 +784,7 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
struct kmx61_data *data = kmx61_get_data(indio_dev);
switch (mask) {
- case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_RAW: {
switch (chan->type) {
case IIO_ACCEL:
base_reg = KMX61_ACC_XOUT_L;
@@ -794,28 +795,24 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
ret = kmx61_set_power_state(data, true, chan->address);
- if (ret) {
- mutex_unlock(&data->lock);
+ if (ret)
return ret;
- }
ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
if (ret < 0) {
kmx61_set_power_state(data, false, chan->address);
- mutex_unlock(&data->lock);
return ret;
}
*val = sign_extend32(ret >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
ret = kmx61_set_power_state(data, false, chan->address);
-
- mutex_unlock(&data->lock);
if (ret)
return ret;
return IIO_VAL_INT;
+ }
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_ACCEL:
@@ -830,17 +827,17 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
- case IIO_CHAN_INFO_SAMP_FREQ:
+ case IIO_CHAN_INFO_SAMP_FREQ: {
if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
return -EINVAL;
- mutex_lock(&data->lock);
- ret = kmx61_get_odr(data, val, val2, chan->address);
- mutex_unlock(&data->lock);
+ scoped_guard(mutex, &data->lock)
+ ret = kmx61_get_odr(data, val, val2, chan->address);
if (ret)
return -EINVAL;
return IIO_VAL_INT_PLUS_MICRO;
}
+ }
return -EINVAL;
}
@@ -848,27 +845,24 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val,
int val2, long mask)
{
- int ret;
struct kmx61_data *data = kmx61_get_data(indio_dev);
switch (mask) {
- case IIO_CHAN_INFO_SAMP_FREQ:
+ case IIO_CHAN_INFO_SAMP_FREQ: {
if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
return -EINVAL;
- mutex_lock(&data->lock);
- ret = kmx61_set_odr(data, val, val2, chan->address);
- mutex_unlock(&data->lock);
- return ret;
+ guard(mutex)(&data->lock);
+ return kmx61_set_odr(data, val, val2, chan->address);
+ }
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
- case IIO_ACCEL:
+ case IIO_ACCEL: {
if (val != 0)
return -EINVAL;
- mutex_lock(&data->lock);
- ret = kmx61_set_scale(data, val2);
- mutex_unlock(&data->lock);
- return ret;
+ guard(mutex)(&data->lock);
+ return kmx61_set_scale(data, val2);
+ }
default:
return -EINVAL;
}
@@ -945,28 +939,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
if (state && data->ev_enable_state)
return 0;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
if (!state && data->motion_trig_on) {
data->ev_enable_state = false;
- goto err_unlock;
+ return ret;
}
ret = kmx61_set_power_state(data, state, KMX61_ACC);
if (ret < 0)
- goto err_unlock;
+ return ret;
ret = kmx61_setup_any_motion_interrupt(data, state);
if (ret < 0) {
kmx61_set_power_state(data, false, KMX61_ACC);
- goto err_unlock;
+ return ret;
}
data->ev_enable_state = state;
-err_unlock:
- mutex_unlock(&data->lock);
-
return ret;
}
@@ -1020,11 +1011,11 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct kmx61_data *data = kmx61_get_data(indio_dev);
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
if (!state && data->ev_enable_state && data->motion_trig_on) {
data->motion_trig_on = false;
- goto err_unlock;
+ return ret;
}
if (data->acc_dready_trig == trig || data->motion_trig == trig)
@@ -1034,7 +1025,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
ret = kmx61_set_power_state(data, state, device);
if (ret < 0)
- goto err_unlock;
+ return ret;
if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
ret = kmx61_setup_new_data_interrupt(data, state, device);
@@ -1042,7 +1033,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
ret = kmx61_setup_any_motion_interrupt(data, state);
if (ret < 0) {
kmx61_set_power_state(data, false, device);
- goto err_unlock;
+ return ret;
}
if (data->acc_dready_trig == trig)
@@ -1051,8 +1042,6 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
data->mag_dready_trig_on = state;
else
data->motion_trig_on = state;
-err_unlock:
- mutex_unlock(&data->lock);
return ret;
}
@@ -1195,19 +1184,18 @@ static irqreturn_t kmx61_trigger_handler(int irq, void *p)
else
base = KMX61_MAG_XOUT_L;
- mutex_lock(&data->lock);
- iio_for_each_active_channel(indio_dev, bit) {
- ret = kmx61_read_measurement(data, base, bit);
- if (ret < 0) {
- mutex_unlock(&data->lock);
- goto err;
+ scoped_guard(mutex, &data->lock) {
+ iio_for_each_active_channel(indio_dev, bit) {
+ ret = kmx61_read_measurement(data, base, bit);
+ if (ret < 0) {
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+ }
+ buffer[i++] = ret;
}
- buffer[i++] = ret;
}
- mutex_unlock(&data->lock);
iio_push_to_buffers(indio_dev, buffer);
-err:
iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED;
@@ -1419,22 +1407,16 @@ static void kmx61_remove(struct i2c_client *client)
iio_trigger_unregister(data->motion_trig);
}
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
- mutex_unlock(&data->lock);
}
static int kmx61_suspend(struct device *dev)
{
- int ret;
struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
- mutex_lock(&data->lock);
- ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
- false);
- mutex_unlock(&data->lock);
-
- return ret;
+ guard(mutex)(&data->lock);
+ return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, false);
}
static int kmx61_resume(struct device *dev)
@@ -1453,13 +1435,9 @@ static int kmx61_resume(struct device *dev)
static int kmx61_runtime_suspend(struct device *dev)
{
struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
- int ret;
- mutex_lock(&data->lock);
- ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
- mutex_unlock(&data->lock);
-
- return ret;
+ guard(mutex)(&data->lock);
+ return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
}
static int kmx61_runtime_resume(struct device *dev)
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking
2026-05-05 22:07 [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking Maxwell Doose
@ 2026-05-06 9:06 ` Andy Shevchenko
2026-05-06 14:58 ` Maxwell Doose
2026-05-06 16:05 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-05-06 9:06 UTC (permalink / raw)
To: Maxwell Doose
Cc: jic23, David Lechner, Nuno Sá, Andy Shevchenko,
open list:IIO SUBSYSTEM AND DRIVERS, open list
On Tue, May 05, 2026 at 05:07:19PM -0500, Maxwell Doose wrote:
> Include linux/cleanup.h to take advantage of new macros.
>
> Replace manual mutex_lock() and mutex_unlock() calls across the file
> with guard(mutex)() and scoped_guard() where appropriate. This will help
> modernize the driver with up-to-date functions/macros.
>
> Remove now redundant gotos and ret variables, as the new RAII macros
> make them unneeded.
...
> - case IIO_CHAN_INFO_SAMP_FREQ:
> + case IIO_CHAN_INFO_SAMP_FREQ: {
> if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> return -EINVAL;
>
> - mutex_lock(&data->lock);
> - ret = kmx61_get_odr(data, val, val2, chan->address);
> - mutex_unlock(&data->lock);
> + scoped_guard(mutex, &data->lock)
> + ret = kmx61_get_odr(data, val, val2, chan->address);
> if (ret)
> return -EINVAL;
> return IIO_VAL_INT_PLUS_MICRO;
> }
> + }
> return -EINVAL;
For making the above less ambiguous, add default case here instead of the
standalone return.
}
default:
return -EINVAL;
}
> }
...
> static int kmx61_runtime_suspend(struct device *dev)
> {
> struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> - int ret;
>
> - mutex_lock(&data->lock);
> - ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> - mutex_unlock(&data->lock);
> -
> - return ret;
> + guard(mutex)(&data->lock);
Leave a blank line here. Same applies to all guard()() cases like this.
> + return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking
2026-05-06 9:06 ` Andy Shevchenko
@ 2026-05-06 14:58 ` Maxwell Doose
0 siblings, 0 replies; 5+ messages in thread
From: Maxwell Doose @ 2026-05-06 14:58 UTC (permalink / raw)
To: Andy Shevchenko
Cc: jic23, David Lechner, Nuno Sá, Andy Shevchenko,
open list:IIO SUBSYSTEM AND DRIVERS, open list
On Wed, May 6, 2026 at 4:06 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 05, 2026 at 05:07:19PM -0500, Maxwell Doose wrote:
[snip]
> > - case IIO_CHAN_INFO_SAMP_FREQ:
> > + case IIO_CHAN_INFO_SAMP_FREQ: {
> > if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> > return -EINVAL;
> >
> > - mutex_lock(&data->lock);
> > - ret = kmx61_get_odr(data, val, val2, chan->address);
> > - mutex_unlock(&data->lock);
> > + scoped_guard(mutex, &data->lock)
> > + ret = kmx61_get_odr(data, val, val2, chan->address);
> > if (ret)
> > return -EINVAL;
> > return IIO_VAL_INT_PLUS_MICRO;
> > }
> > + }
> > return -EINVAL;
>
> For making the above less ambiguous, add default case here instead of the
> standalone return.
>
Sounds good, I'll get that into the next version.
>
> }
> default:
> return -EINVAL;
> }
>
> > }
>
> ...
>
> > static int kmx61_runtime_suspend(struct device *dev)
> > {
> > struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> > - int ret;
> >
> > - mutex_lock(&data->lock);
> > - ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> > - mutex_unlock(&data->lock);
> > -
> > - return ret;
> > + guard(mutex)(&data->lock);
>
> Leave a blank line here. Same applies to all guard()() cases like this.
>
Ah, got it. I'm used to grouping related stuff in the code so to me
it's a bit strange but I'll also add that in the next version.
best regards,
max
>
> > + return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> > }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking
2026-05-05 22:07 [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking Maxwell Doose
2026-05-06 9:06 ` Andy Shevchenko
@ 2026-05-06 16:05 ` Jonathan Cameron
2026-05-06 16:56 ` Maxwell Doose
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2026-05-06 16:05 UTC (permalink / raw)
To: Maxwell Doose
Cc: David Lechner, Nuno Sá, Andy Shevchenko,
open list:IIO SUBSYSTEM AND DRIVERS, open list
On Tue, 5 May 2026 17:07:19 -0500
Maxwell Doose <m32285159@gmail.com> wrote:
> Include linux/cleanup.h to take advantage of new macros.
>
> Replace manual mutex_lock() and mutex_unlock() calls across the file
> with guard(mutex)() and scoped_guard() where appropriate. This will help
> modernize the driver with up-to-date functions/macros.
>
> Remove now redundant gotos and ret variables, as the new RAII macros
> make them unneeded.
>
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
Hi Maxwell
I'd slow down a bit to give more time for reviews.
I'd imagine some of the below was true in earlier versions.
Jonathan
> @@ -830,17 +827,17 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> - case IIO_CHAN_INFO_SAMP_FREQ:
> + case IIO_CHAN_INFO_SAMP_FREQ: {
Don't need {} in this case as no local variables added in this scope.
scoped_guard() is a trick with a for loop so has it's own scope definition
as part of the macro implementation.
> if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> return -EINVAL;
>
> - mutex_lock(&data->lock);
> - ret = kmx61_get_odr(data, val, val2, chan->address);
> - mutex_unlock(&data->lock);
> + scoped_guard(mutex, &data->lock)
> + ret = kmx61_get_odr(data, val, val2, chan->address);
> if (ret)
> return -EINVAL;
> return IIO_VAL_INT_PLUS_MICRO;
> }
> + }
> return -EINVAL;
> }
> @@ -945,28 +939,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> if (state && data->ev_enable_state)
> return 0;
>
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> if (!state && data->motion_trig_on) {
> data->ev_enable_state = false;
> - goto err_unlock;
> + return ret;
> }
>
> ret = kmx61_set_power_state(data, state, KMX61_ACC);
> if (ret < 0)
> - goto err_unlock;
> + return ret;
>
> ret = kmx61_setup_any_motion_interrupt(data, state);
> if (ret < 0) {
> kmx61_set_power_state(data, false, KMX61_ACC);
> - goto err_unlock;
> + return ret;
> }
>
> data->ev_enable_state = state;
>
> -err_unlock:
> - mutex_unlock(&data->lock);
> -
> return ret;
If we know this is 0 (probably is) then return 0.
> }
> @@ -1195,19 +1184,18 @@ static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> else
> base = KMX61_MAG_XOUT_L;
>
> - mutex_lock(&data->lock);
> - iio_for_each_active_channel(indio_dev, bit) {
> - ret = kmx61_read_measurement(data, base, bit);
> - if (ret < 0) {
> - mutex_unlock(&data->lock);
> - goto err;
> + scoped_guard(mutex, &data->lock) {
> + iio_for_each_active_channel(indio_dev, bit) {
> + ret = kmx61_read_measurement(data, base, bit);
> + if (ret < 0) {
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
The duplication is a bit ugly. I'd be tempted to use a helper to clean this up
Would be something like (not even compiled!)
static int kmx61_read_all(struct iio_dev *indio_dev, s16 buffer[at_least 8])
{
struct kmx61_data *data = kmx61_get_data(indio_dev);
u8 base;
int ret, i, bit;
if (indio_dev == data->acc_indio_dev)
base = KMX61_ACC_XOUT_L;
else
base = KMX61_MAG_XOUT_L;
guard(mutex)(&data->lock);
iio_for_each_active_channel(indio_dev, bit) {
ret = kmx61_read_measurement(data, base, bit);
if (ret < 0)
return ret;
buffer[i++] = ret;
}
return 0;
}
static irqreturn_t kmx61_trigger_handler(int irq, void *p)
{
struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
s16 buffer[8] = { };
u8 base;
if (kmx61_read_all(indio_dev, buffer))
goto err;
iio_push_to_buffers(indio_dev, buffer);
err:
iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED;
}
> + }
> + buffer[i++] = ret;
> }
> - buffer[i++] = ret;
> }
> - mutex_unlock(&data->lock);
>
> iio_push_to_buffers(indio_dev, buffer);
> -err:
> iio_trigger_notify_done(indio_dev->trig);
>
> return IRQ_HANDLED;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking
2026-05-06 16:05 ` Jonathan Cameron
@ 2026-05-06 16:56 ` Maxwell Doose
0 siblings, 0 replies; 5+ messages in thread
From: Maxwell Doose @ 2026-05-06 16:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá, Andy Shevchenko,
open list:IIO SUBSYSTEM AND DRIVERS, open list
Hi Jonathan,
On Wed, May 6, 2026 at 11:05 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 5 May 2026 17:07:19 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
>
> > Include linux/cleanup.h to take advantage of new macros.
> >
> > Replace manual mutex_lock() and mutex_unlock() calls across the file
> > with guard(mutex)() and scoped_guard() where appropriate. This will help
> > modernize the driver with up-to-date functions/macros.
> >
> > Remove now redundant gotos and ret variables, as the new RAII macros
> > make them unneeded.
> >
> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> Hi Maxwell
>
> I'd slow down a bit to give more time for reviews.
> I'd imagine some of the below was true in earlier versions.
>
Yeah...sorry about that. I tend to want to get my latest fixes out
fast so patches can be merged faster and we all waste less time. I can
wait until tomorrow evening to send this out if you'd like.
>
> Jonathan
>
> > @@ -830,17 +827,17 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> > default:
> > return -EINVAL;
> > }
> > - case IIO_CHAN_INFO_SAMP_FREQ:
> > + case IIO_CHAN_INFO_SAMP_FREQ: {
>
> Don't need {} in this case as no local variables added in this scope.
> scoped_guard() is a trick with a for loop so has it's own scope definition
> as part of the macro implementation.
>
Ah. See, I'm a bit skeptical of scoped_guard() nowadays because of the
hidden for. But I'll replace that {} with the scoped_guard().
>
> > if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> > return -EINVAL;
> >
> > - mutex_lock(&data->lock);
> > - ret = kmx61_get_odr(data, val, val2, chan->address);
> > - mutex_unlock(&data->lock);
> > + scoped_guard(mutex, &data->lock)
> > + ret = kmx61_get_odr(data, val, val2, chan->address);
> > if (ret)
> > return -EINVAL;
> > return IIO_VAL_INT_PLUS_MICRO;
> > }
> > + }
> > return -EINVAL;
> > }
>
> > @@ -945,28 +939,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> > if (state && data->ev_enable_state)
> > return 0;
> >
> > - mutex_lock(&data->lock);
> > + guard(mutex)(&data->lock);
> >
> > if (!state && data->motion_trig_on) {
> > data->ev_enable_state = false;
> > - goto err_unlock;
> > + return ret;
> > }
> >
> > ret = kmx61_set_power_state(data, state, KMX61_ACC);
> > if (ret < 0)
> > - goto err_unlock;
> > + return ret;
> >
> > ret = kmx61_setup_any_motion_interrupt(data, state);
> > if (ret < 0) {
> > kmx61_set_power_state(data, false, KMX61_ACC);
> > - goto err_unlock;
> > + return ret;
> > }
> >
> > data->ev_enable_state = state;
> >
> > -err_unlock:
> > - mutex_unlock(&data->lock);
> > -
> > return ret;
> If we know this is 0 (probably is) then return 0.
>
Will fix for v5.
>
> > }
>
> > @@ -1195,19 +1184,18 @@ static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> > else
> > base = KMX61_MAG_XOUT_L;
> >
> > - mutex_lock(&data->lock);
> > - iio_for_each_active_channel(indio_dev, bit) {
> > - ret = kmx61_read_measurement(data, base, bit);
> > - if (ret < 0) {
> > - mutex_unlock(&data->lock);
> > - goto err;
> > + scoped_guard(mutex, &data->lock) {
> > + iio_for_each_active_channel(indio_dev, bit) {
> > + ret = kmx61_read_measurement(data, base, bit);
> > + if (ret < 0) {
> > + iio_trigger_notify_done(indio_dev->trig);
> > + return IRQ_HANDLED;
>
> The duplication is a bit ugly. I'd be tempted to use a helper to clean this up
> Would be something like (not even compiled!)
>
Likely a good idea, I can do that, would probably be better to keep
the guard()() out of that scope.
>
> static int kmx61_read_all(struct iio_dev *indio_dev, s16 buffer[at_least 8])
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
> u8 base;
> int ret, i, bit;
>
> if (indio_dev == data->acc_indio_dev)
> base = KMX61_ACC_XOUT_L;
> else
> base = KMX61_MAG_XOUT_L;
>
> guard(mutex)(&data->lock);
>
> iio_for_each_active_channel(indio_dev, bit) {
> ret = kmx61_read_measurement(data, base, bit);
> if (ret < 0)
> return ret;
> buffer[i++] = ret;
> }
> return 0;
> }
>
> static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> {
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> s16 buffer[8] = { };
> u8 base;
>
> if (kmx61_read_all(indio_dev, buffer))
> goto err;
>
> iio_push_to_buffers(indio_dev, buffer);
> err:
> iio_trigger_notify_done(indio_dev->trig);
>
> return IRQ_HANDLED;
> }
>
I'll likely take the above and tune it up so that it will work as intended.
best regards,
max
> > + }
> > + buffer[i++] = ret;
> > }
> > - buffer[i++] = ret;
> > }
> > - mutex_unlock(&data->lock);
> >
> > iio_push_to_buffers(indio_dev, buffer);
> > -err:
> > iio_trigger_notify_done(indio_dev->trig);
> >
> > return IRQ_HANDLED;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-06 16:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 22:07 [PATCH v4] iio: imu: kmx61: Use guard(mutex)() over manual locking Maxwell Doose
2026-05-06 9:06 ` Andy Shevchenko
2026-05-06 14:58 ` Maxwell Doose
2026-05-06 16:05 ` Jonathan Cameron
2026-05-06 16:56 ` Maxwell Doose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox