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

[Andy Shevchenko]

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