Re: [PATCH v3 2/3] iio: light: ltr501: use automatic cleanup of locks

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

On Wed, Apr 22, 2026 at 08:12:56PM -0300, rafasales@usp.br wrote:
> 
> Replace `mutex_lock()` and `mutex_unlock()` calls with guards
> to reduce boilerplate and allow for simpler code blocks.

...

> -	mutex_lock(&data->lock_als);
> -	ret = regmap_field_write(data->reg_als_rate, i);
> -	mutex_unlock(&data->lock_als);
> -
> -	return ret;
> +	guard(mutex)(&data->lock_als);

+ blank line.

We usually consider guard()() as not semantically linked to any code above
or below.

> +	return regmap_field_write(data->reg_als_rate, i);

Ditto for the same cases below.

...

> +		scoped_guard(mutex, &data->lock_als) {
> +			ret = regmap_field_write(data->reg_als_prst, new_val);
> +		}

Read Coding Style, please. The {} are not used for single statement bodies.

> +

This is an opposite to the above, the conditional is semantically coupled with
previous code, no blank line is needed.

>  		if (ret >= 0)
>  			data->als_period = period;

Ditto for the rest.

...

> +			scoped_guard(mutex, &data->lock_als)
> +				ret = ltr501_read_als(data, buf);

You see, even inside one patch there is an inconsistency. That's not good.
(Note, this piece is correct nevertheless by style).

>  			if (ret < 0)
>  				return ret;

...

>  		switch (dir) {
> +		case IIO_EV_DIR_RISING: {
> +			guard(mutex)(&data->lock_als);
> +			return regmap_bulk_write(data->regmap,
> +						 LTR501_ALS_THRESH_UP,
> +						 &val, 2);
> +			}
> +		case IIO_EV_DIR_FALLING: {
> +			guard(mutex)(&data->lock_als);
> +			return regmap_bulk_write(data->regmap,
> +						 LTR501_ALS_THRESH_LOW,
> +						 &val, 2);
> +			}
>  		default:
>  			return -EINVAL;
>  		}

Can you check if instead of doing like that, you can simply move guard()()
level up and make all this simpler? Same Q to the rest of similar code.

-- 
With Best Regards,
Andy Shevchenko