Re: [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sun, Feb 15, 2026 at 08:54:52PM -0500, Neel Bullywon wrote:
> Use guard() and scoped_guard() to replace manual mutex lock/unlock
> calls. This simplifies error handling and ensures RAII-style cleanup.
> 
> guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
> suspend, and resume. Case blocks using guard() in read_raw and
> write_raw are wrapped in braces at the case label level to ensure
> clear scope for the cleanup guards.
> 
> scoped_guard() is used in remove and runtime_suspend where a short
> mutex-protected scope is needed for a single function call.
> 
> The trigger_handler function is left unchanged as mixing guard() with
> goto error paths can be fragile.

...

> -	mutex_lock(&data->mutex);
> -	bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);
> -	mutex_unlock(&data->mutex);
> +	scoped_guard(mutex, &data->mutex)
> +		bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);

Wonder if we can move this to a helper:

static int bmc150_magn_set_power_mode_locked(data, mode) // locked or unlocked, dunno with proper naming
{
	guard(mutex)(&data->mutex)
	return bmc150_magn_set_power_mode(data, mode, true);
}

...

> -	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> -					 true);
> -	mutex_unlock(&data->mutex);
> +	scoped_guard(mutex, &data->mutex)
> +		ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> +						 true);
>  	if (ret < 0) {
>  		dev_err(dev, "powering off device failed\n");
>  		return ret;
>  	}

Ditto.


...

>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct bmc150_magn_data *data = iio_priv(indio_dev);
> -	int ret;
> -
> -	mutex_lock(&data->mutex);
> -	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> -					 true);
> -	mutex_unlock(&data->mutex);
>  
> -	return ret;
> +	guard(mutex)(&data->mutex);
> +	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> +					  true);

Ditto.

>  }
>  
>  static int bmc150_magn_resume(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct bmc150_magn_data *data = iio_priv(indio_dev);
> -	int ret;
> -
> -	mutex_lock(&data->mutex);
> -	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
> -					 true);
> -	mutex_unlock(&data->mutex);
>  
> -	return ret;
> +	guard(mutex)(&data->mutex);
> +	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
> +					  true);

Ditto.

>  }

-- 
With Best Regards,
Andy Shevchenko