Re: [PATCH v2 08/10] iio: light: opt3001: move driver to guard(mutex)() use

[Jonathan Cameron]

On Tue, 12 May 2026 12:57:28 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@xxxxxxxxxx> wrote:

> From: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
> 
> Move driver to use guard(mutex)() macro, to facilitate automatic
> locking/unlocking of resources. This modernizes the driver and
> improves code style.
> 
> While at it, remove unnecessary gotos and return variables.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Signed-off-by: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
A couple of very trivial things where I'd have done it differently.
I don't mind much however if you want to keep it as is.

> @@ -615,18 +601,12 @@ static int opt3001_write_event_value(struct iio_dev *iio,
>  		opt->low_thresh_exp = exponent;
>  		break;
>  	default:
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	ret = i2c_smbus_write_word_swapped(client, reg, value);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dev, "failed to write register %02x\n", reg);
> -		goto err;
Similar to below.  Though very much a matter of personal taste - I'd
definitely not have asked for a respin for this alone!
> -	}
> -
> -err:
> -	mutex_unlock(&opt->lock);
>  
>  	return ret;
>  }
> @@ -660,7 +640,7 @@ static int opt3001_write_event_config(struct iio_dev *iio,
>  	if (!state && opt->mode == OPT3001_CONFIGURATION_M_SHUTDOWN)
>  		return 0;
>  
> -	mutex_lock(&opt->lock);
> +	guard(mutex)(&opt->lock);
>  
>  	mode = state ? OPT3001_CONFIGURATION_M_CONTINUOUS
>  		: OPT3001_CONFIGURATION_M_SHUTDOWN;
> @@ -669,21 +649,16 @@ static int opt3001_write_event_config(struct iio_dev *iio,
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read register %02x\n",
>  			OPT3001_CONFIGURATION);
> -		goto err;
> +		return ret;
>  	}
>  
>  	reg = ret;
>  	opt3001_set_mode(opt, &reg, mode);
>  
>  	ret = i2c_smbus_write_word_swapped(client, OPT3001_CONFIGURATION, reg);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dev, "failed to write register %02x\n",
>  			OPT3001_CONFIGURATION);
> -		goto err;

I'd return ret here

> -	}
> -

Then return 0; here

Makes it a little easier to spot the good path.


> -err:
> -	mutex_unlock(&opt->lock);
>  
>  	return ret;
>  }
>