Re: [PATCH] iio: imu: kmx61: Use guard(mutex)() family over manual locking

Andy Shevchenko
2026-05-04T08:36:16Z

On Fri, May 01, 2026 at 10:24:54PM -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.

...

> *val = sign_extend32(ret >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> ret = kmx61_set_power_state(data, false, chan->address);

>

Now this blank line becomes a bit confusing as the following conditional is
tightly coupled with the above code, remove it.

> - mutex_unlock(&data->lock);
> if (ret)
> return ret;
> return IIO_VAL_INT;

...

> - mutex_lock(&data->lock);
> + guard(mutex)(&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;
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;

Hmm... Is the HANDLED a right choice?

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

If the answer is yes, I'm wondering if we may deduplicate that...

...

> 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,
> + guard(mutex)(&data->lock);

> + return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
> false);

One line is okay in this case.

> - mutex_unlock(&data->lock);
> -
> - return ret;
> }

--
With Best Regards,
Andy Shevchenko