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

From: Andy Shevchenko

Date: Wed Feb 18 2026 - 02:08:24 EST


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