Re: [PATCH v4 1/9] iio: light: gp2ap020a00f: simplify locking with guard()

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

On Tue, Feb 17, 2026 at 10:37:20PM -0600, Ethan Tidmore wrote:
> Use the guard() cleanup handler to manage the device lock.
> This simplifies the code by removing the need for manual unlocking
> and goto error handling paths.

> static int gp2ap020a00f_write_event_config(struct iio_dev *indio_dev,

>  	enum gp2ap020a00f_cmd cmd;
>  	int err;
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  
>  	switch (chan->type) {
>  	case IIO_PROXIMITY:

>  		err = -EINVAL;
>  	}
>  
> -	mutex_unlock(&data->lock);
> -
>  	return err;
>  }

What I meant in the my cover letter is that you need to take some pieces from
my patches and incorporate them here, so this becomes

	return 0;


...

> static int gp2ap020a00f_read_event_config(struct iio_dev *indio_dev,

>  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
>  	int event_en = 0;
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  
>  	switch (chan->type) {
>  	case IIO_PROXIMITY:
> @@ -1223,8 +1204,6 @@ static int gp2ap020a00f_read_event_config(struct iio_dev *indio_dev,
>  		break;
>  	}
>  
> -	mutex_unlock(&data->lock);
> -
>  	return event_en;

Same here, now event_en is redundant.

>  }

...

> static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)

>  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
>  	int i, err = 0;

(Do you need this '= 0'? Check it).

>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);


>  	}
>  
>  	if (err < 0)
> -		goto error_unlock;
> +		return err;
>  
>  	data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
>  	if (!data->buffer)

>  		err = -ENOMEM;
>  
> -error_unlock:
> -	mutex_unlock(&data->lock);
> -
>  	return err;

And here, this should become

		return -ENOMEM;

	return 0;


>  }

...

> static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)

>  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
>  	int i, err = 0;
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  
>  	iio_for_each_active_channel(indio_dev, i) {
>  		switch (i) {
> @@ -1452,8 +1428,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
>  	if (err == 0)
>  		kfree(data->buffer);
>  
> -	mutex_unlock(&data->lock);
> -
>  	return err;
>  }

Same here.

-- 
With Best Regards,
Andy Shevchenko