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

[Andy Shevchenko]

On Sun, Feb 15, 2026 at 06:53:16PM -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.

...

>  	thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
>  	thresh_val_id = GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l);

>  

Also drop this blank line, it will be aligned with the similar style in the
next patch.

> -	if (thresh_val_id > GP2AP020A00F_THRESH_PH) {
> -		err = -EINVAL;
> -		goto error_unlock;
> -	}
> +	if (thresh_val_id > GP2AP020A00F_THRESH_PH)
> +		return -EINVAL;

...

>  	thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);

>  

Ditto.

> -	if (thresh_reg_l > GP2AP020A00F_PH_L_REG) {
> -		err = -EINVAL;
> -		goto error_unlock;
> -	}
> +	if (thresh_reg_l > GP2AP020A00F_PH_L_REG)
> +		return -EINVAL;

...

>  	data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
>  	if (!data->buffer)

>  		err = -ENOMEM;

Now simply

		return -ENOMEM;

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

And with the above I believe this becomes

	return 0;

>  }

...

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

Would you need the err assignment now?

> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  
>  	iio_for_each_active_channel(indio_dev, i) {
>  		switch (i) {

...

>  	if (err == 0)
>  		kfree(data->buffer);

With the guard()() in place this can be inverted to have something
(I haven't seen the full context, though) like

	if (err)
		return err;

	kfree(...);

> -	mutex_unlock(&data->lock);
> -
>  	return err;

Ideally this should become

	return 0;

-- 
With Best Regards,
Andy Shevchenko