[PATCH v3] iio: light: vcnl4000: use lock guard()

[PATCH v3] iio: light: vcnl4000: use lock guard()

[Raffael Raiel Trindade]

From: Raffael Raiel Trindade <raffaelraiel@usp.br>

Use guard() for handling mutex lock instead of manually locking and
unlocking. Remove gotos in error handling logic. This prevents forgotten
locks on early exits.

Signed-off-by: Raffael Raiel Trindade <raffaelraiel@usp.br>
Co-developed-by: Kim Carvalho <kim.ca@usp.br>
Signed-off-by: Kim Carvalho <kim.ca@usp.br>
---
v3:
 - fix indentation problems on line breaks

v2:
 - remove unnecessary ret attributions on non-error logic
 - remove breaks on switch-case after return

 drivers/iio/light/vcnl4000.c | 55 +++++++++++-------------------------
 1 file changed, 16 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index c08927e34b4e..023176ff14c7 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -18,6 +18,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -268,46 +269,36 @@ static ssize_t vcnl4000_write_als_enable(struct vcnl4000_data *data, bool en)
 {
 	int ret;
 
-	mutex_lock(&data->vcnl4000_lock);
+	guard(mutex)(&data->vcnl4000_lock);
 
 	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	if (en)
 		ret &= ~VCNL4040_ALS_CONF_ALS_SHUTDOWN;
 	else
 		ret |= VCNL4040_ALS_CONF_ALS_SHUTDOWN;
 
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, ret);
-
-out:
-	mutex_unlock(&data->vcnl4000_lock);
-
-	return ret;
+	return i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, ret);
 }
 
 static ssize_t vcnl4000_write_ps_enable(struct vcnl4000_data *data, bool en)
 {
 	int ret;
 
-	mutex_lock(&data->vcnl4000_lock);
+	guard(mutex)(&data->vcnl4000_lock);
 
 	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	if (en)
 		ret &= ~VCNL4040_PS_CONF1_PS_SHUTDOWN;
 	else
 		ret |= VCNL4040_PS_CONF1_PS_SHUTDOWN;
 
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret);
-
-out:
-	mutex_unlock(&data->vcnl4000_lock);
-
-	return ret;
+	return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret);
 }
 
 static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
@@ -660,20 +651,15 @@ static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val)
 
 	data->vcnl4200_ps.sampling_rate = ktime_set(0, val * 60 * NSEC_PER_USEC);
 
-	mutex_lock(&data->vcnl4000_lock);
+	guard(mutex)(&data->vcnl4000_lock);
 
 	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	regval = (ret & ~VCNL4040_PS_CONF2_PS_IT) |
 	    FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index);
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
-					regval);
-
-out:
-	mutex_unlock(&data->vcnl4000_lock);
-	return ret;
+	return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, regval);
 }
 
 static ssize_t vcnl4040_read_als_period(struct vcnl4000_data *data, int *val, int *val2)
@@ -1480,13 +1466,13 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
 	u16 val, mask;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
-	mutex_lock(&data->vcnl4000_lock);
+	guard(mutex)(&data->vcnl4000_lock);
 
 	switch (chan->type) {
 	case IIO_LIGHT:
 		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
 		if (ret < 0)
-			goto out;
+			return ret;
 
 		mask = VCNL4040_ALS_CONF_INT_EN;
 		if (state)
@@ -1495,13 +1481,11 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
 			val = (ret & ~mask);
 
 		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, val);
-		ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
-						val);
-		break;
+		return i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, val);
 	case IIO_PROXIMITY:
 		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
 		if (ret < 0)
-			goto out;
+			return ret;
 
 		if (dir == IIO_EV_DIR_RISING)
 			mask = VCNL4040_PS_IF_AWAY;
@@ -1511,17 +1495,10 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
 		val = state ? (ret | mask) : (ret & ~mask);
 
 		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
-		ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
-						val);
-		break;
+		return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val);
 	default:
-		break;
+		return ret;
 	}
-
-out:
-	mutex_unlock(&data->vcnl4000_lock);
-
-	return ret;
 }
 
 static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
-- 
2.51.0

Re: [PATCH v3] iio: light: vcnl4000: use lock guard()

[Andy Shevchenko]

On Mon, Apr 20, 2026 at 04:59:33PM -0300, Raffael Raiel Trindade wrote:

> Use guard() for handling mutex lock instead of manually locking and
> unlocking. Remove gotos in error handling logic. This prevents forgotten
> locks on early exits.

...

>  	regval = (ret & ~VCNL4040_PS_CONF2_PS_IT) |
>  	    FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index);

Side note: This can be simplified with FIELD_MODIFY().


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3] iio: light: vcnl4000: use lock guard()

[Andy Shevchenko]

On Mon, Apr 20, 2026 at 11:19:11PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 20, 2026 at 04:59:33PM -0300, Raffael Raiel Trindade wrote:
> 
> > Use guard() for handling mutex lock instead of manually locking and
> > unlocking. Remove gotos in error handling logic. This prevents forgotten
> > locks on early exits.

...

> >  	regval = (ret & ~VCNL4040_PS_CONF2_PS_IT) |
> >  	    FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index);
> 
> Side note: This can be simplified with FIELD_MODIFY().

To be clear, the patch LGTM, the above is a material for another if you
wish/have time to do so.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3] iio: light: vcnl4000: use lock guard()

[Raffael Raiel Trindade]

On Mon, Apr 20, 2026 at 5:26 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Apr 20, 2026 at 11:19:11PM +0300, Andy Shevchenko wrote:
> > On Mon, Apr 20, 2026 at 04:59:33PM -0300, Raffael Raiel Trindade wrote:
> >
> > > Use guard() for handling mutex lock instead of manually locking and
> > > unlocking. Remove gotos in error handling logic. This prevents forgotten
> > > locks on early exits.

...

> > Side note: This can be simplified with FIELD_MODIFY().
>
> To be clear, the patch LGTM, the above is a material for another if you
> wish/have time to do so.

Hello, Andy

Thank you for the clarification and the LGTM.
I will consider applying your suggestion with a new patch
in the near future.

--
Best Regards,
Raffael Raiel Trindade

Re: [PATCH v3] iio: light: vcnl4000: use lock guard()

[Jonathan Cameron]

On Mon, 20 Apr 2026 16:59:33 -0300
Raffael Raiel Trindade <raffaelraiel@usp.br> wrote:

> From: Raffael Raiel Trindade <raffaelraiel@usp.br>
> 
> Use guard() for handling mutex lock instead of manually locking and
> unlocking. Remove gotos in error handling logic. This prevents forgotten
> locks on early exits.
> 
> Signed-off-by: Raffael Raiel Trindade <raffaelraiel@usp.br>
> Co-developed-by: Kim Carvalho <kim.ca@usp.br>
> Signed-off-by: Kim Carvalho <kim.ca@usp.br>


There are some functions in here where guard() is useful but you haven't
updated. e.g. vcnl4000_measure(), vcnl4040_write_als_it()
vcnl4040_write_als_period() and maybe more.

I don't mind a mix of guard() and non guard() but only when that is
because one approach is better suited to a particular bit of code than the other.
Here it looks like at least some other functions would benefit similar to the
changes this patch makes.

So I'd like a more complete patch.

One other thing inline.

Thanks,

Jonathan




> @@ -1480,13 +1466,13 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
>  	u16 val, mask;
>  	struct vcnl4000_data *data = iio_priv(indio_dev);
>  
> -	mutex_lock(&data->vcnl4000_lock);
> +	guard(mutex)(&data->vcnl4000_lock);
>  
>  	switch (chan->type) {
>  	case IIO_LIGHT:
>  		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
>  		if (ret < 0)
> -			goto out;
> +			return ret;
>  
>  		mask = VCNL4040_ALS_CONF_INT_EN;
>  		if (state)
> @@ -1495,13 +1481,11 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
>  			val = (ret & ~mask);
>  
>  		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, val);
> -		ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
> -						val);
> -		break;
> +		return i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, val);
>  	case IIO_PROXIMITY:
>  		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
>  		if (ret < 0)
> -			goto out;
> +			return ret;
>  
>  		if (dir == IIO_EV_DIR_RISING)
>  			mask = VCNL4040_PS_IF_AWAY;
> @@ -1511,17 +1495,10 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
>  		val = state ? (ret | mask) : (ret & ~mask);
>  
>  		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
> -		ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
> -						val);
> -		break;
> +		return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val);
>  	default:
> -		break;
> +		return ret;

		return -EVINAL;

and drop the intialization of ret.

>  	}
> -
> -out:
> -	mutex_unlock(&data->vcnl4000_lock);
> -
> -	return ret;
>  }
>  
>  static irqreturn_t vcnl4040_irq_thread(int irq, void *p)