* [PATCH v4] iio: light: vcnl4000: use lock guard()
@ 2026-05-06 21:05 Raffael Raiel Trindade
2026-05-07 16:36 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Raffael Raiel Trindade @ 2026-05-06 21:05 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy
Cc: Raffael Raiel Trindade, Kim Carvalho, linux-iio
From: Raffael Raiel Trindade <raffaelraiel@usp.br>
Use guard() and scoped_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>
---
v4:
- use guard() and scoped_guard() instead of
lock() and unlock() in all functions
- drop ret = -EINVAL initialization
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 | 148 +++++++++++------------------------
1 file changed, 45 insertions(+), 103 deletions(-)
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index c08927e34b4e..d87a95a05329 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)
@@ -442,18 +433,18 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
int tries = 20;
int ret;
- mutex_lock(&data->vcnl4000_lock);
+ guard(mutex)(&data->vcnl4000_lock);
ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
req_mask);
if (ret < 0)
- goto fail;
+ return ret;
/* wait for data to become ready */
while (tries--) {
ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
if (ret < 0)
- goto fail;
+ return ret;
if (ret & rdy_mask)
break;
msleep(20); /* measurement takes up to 100 ms */
@@ -462,20 +453,13 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
if (tries < 0) {
dev_err(&data->client->dev,
"vcnl4000_measure() failed, data not ready\n");
- ret = -EIO;
- goto fail;
+ return -EIO;
}
ret = vcnl4000_read_data(data, data_reg, val);
if (ret < 0)
- goto fail;
-
- mutex_unlock(&data->vcnl4000_lock);
-
- return 0;
+ return ret;
-fail:
- mutex_unlock(&data->vcnl4000_lock);
return ret;
}
@@ -486,16 +470,14 @@ static int vcnl4200_measure(struct vcnl4000_data *data,
s64 delta;
ktime_t next_measurement;
- mutex_lock(&chan->lock);
-
- next_measurement = ktime_add(chan->last_measurement,
- chan->sampling_rate);
- delta = ktime_us_delta(next_measurement, ktime_get());
- if (delta > 0)
- usleep_range(delta, delta + 500);
- chan->last_measurement = ktime_get();
-
- mutex_unlock(&chan->lock);
+ scoped_guard(mutex, &chan->lock) {
+ next_measurement = ktime_add(chan->last_measurement,
+ chan->sampling_rate);
+ delta = ktime_us_delta(next_measurement, ktime_get());
+ if (delta > 0)
+ usleep_range(delta, delta + 500);
+ chan->last_measurement = ktime_get();
+ }
ret = i2c_smbus_read_word_data(data->client, chan->reg);
if (ret < 0)
@@ -606,21 +588,15 @@ static ssize_t vcnl4040_write_als_it(struct vcnl4000_data *data, int val)
(*data->chip_spec->als_it_times)[0][1]),
val);
- 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_unlock;
+ return ret;
regval = FIELD_PREP(VCNL4040_ALS_CONF_IT, i);
regval |= (ret & ~VCNL4040_ALS_CONF_IT);
- ret = i2c_smbus_write_word_data(data->client,
- VCNL4200_AL_CONF,
- regval);
-
-out_unlock:
- mutex_unlock(&data->vcnl4000_lock);
- return ret;
+ return i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, regval);
}
static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2)
@@ -660,20 +636,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)
@@ -721,20 +692,15 @@ static ssize_t vcnl4040_write_als_period(struct vcnl4000_data *data, int val, in
break;
}
- 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_unlock;
+ return ret;
regval = FIELD_PREP(VCNL4040_ALS_CONF_PERS, i);
regval |= (ret & ~VCNL4040_ALS_CONF_PERS);
- ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
- regval);
-
-out_unlock:
- mutex_unlock(&data->vcnl4000_lock);
- return ret;
+ return i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, regval);
}
static ssize_t vcnl4040_read_ps_period(struct vcnl4000_data *data, int *val, int *val2)
@@ -783,20 +749,15 @@ static ssize_t vcnl4040_write_ps_period(struct vcnl4000_data *data, int val, int
}
}
- 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_unlock;
+ return ret;
regval = FIELD_PREP(VCNL4040_CONF1_PS_PERS, i);
regval |= (ret & ~VCNL4040_CONF1_PS_PERS);
- ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
- regval);
-
-out_unlock:
- mutex_unlock(&data->vcnl4000_lock);
- return ret;
+ return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, regval);
}
static ssize_t vcnl4040_read_ps_oversampling_ratio(struct vcnl4000_data *data, int *val)
@@ -830,20 +791,15 @@ static ssize_t vcnl4040_write_ps_oversampling_ratio(struct vcnl4000_data *data,
if (i >= ARRAY_SIZE(vcnl4040_ps_oversampling_ratio))
return -EINVAL;
- mutex_lock(&data->vcnl4000_lock);
+ guard(mutex)(&data->vcnl4000_lock);
ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
if (ret < 0)
- goto out_unlock;
+ return ret;
regval = FIELD_PREP(VCNL4040_PS_CONF3_MPS, i);
regval |= (ret & ~VCNL4040_PS_CONF3_MPS);
- ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
- regval);
-
-out_unlock:
- mutex_unlock(&data->vcnl4000_lock);
- return ret;
+ return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3, regval);
}
static ssize_t vcnl4040_read_ps_calibbias(struct vcnl4000_data *data, int *val, int *val2)
@@ -878,20 +834,15 @@ static ssize_t vcnl4040_write_ps_calibbias(struct vcnl4000_data *data, int val)
if (i >= ARRAY_SIZE(vcnl4040_ps_calibbias_ua))
return -EINVAL;
- mutex_lock(&data->vcnl4000_lock);
+ guard(mutex)(&data->vcnl4000_lock);
ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
if (ret < 0)
- goto out_unlock;
+ return ret;
regval = (ret & ~VCNL4040_PS_MS_LED_I);
regval |= FIELD_PREP(VCNL4040_PS_MS_LED_I, i);
- ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
- regval);
-
-out_unlock:
- mutex_unlock(&data->vcnl4000_lock);
- return ret;
+ return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3, regval);
}
static int vcnl4000_read_raw(struct iio_dev *indio_dev,
@@ -1476,17 +1427,17 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
enum iio_event_direction dir,
bool state)
{
- int ret = -EINVAL;
+ int ret;
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 +1446,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 +1460,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 -EINVAL;
}
-
-out:
- mutex_unlock(&data->vcnl4000_lock);
-
- return ret;
}
static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v4] iio: light: vcnl4000: use lock guard()
2026-05-06 21:05 [PATCH v4] iio: light: vcnl4000: use lock guard() Raffael Raiel Trindade
@ 2026-05-07 16:36 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2026-05-07 16:36 UTC (permalink / raw)
To: Raffael Raiel Trindade; +Cc: dlechner, nuno.sa, andy, Kim Carvalho, linux-iio
On Wed, 6 May 2026 18:05:33 -0300
Raffael Raiel Trindade <raffaelraiel@usp.br> wrote:
> From: Raffael Raiel Trindade <raffaelraiel@usp.br>
>
> Use guard() and scoped_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>
Hi All,
One remaining minor point and a suggestion for where there may be
some more useful cleanup to be done on this driver
Thanks,
Jonathan
> ---
> v4:
> - use guard() and scoped_guard() instead of
> lock() and unlock() in all functions
> - drop ret = -EINVAL initialization
>
> 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 | 148 +++++++++++------------------------
> 1 file changed, 45 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index c08927e34b4e..d87a95a05329 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>
>
> static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
> @@ -442,18 +433,18 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
> int tries = 20;
> int ret;
>
> - mutex_lock(&data->vcnl4000_lock);
> + guard(mutex)(&data->vcnl4000_lock);
>
> ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
> req_mask);
> if (ret < 0)
> - goto fail;
> + return ret;
>
> /* wait for data to become ready */
> while (tries--) {
> ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
> if (ret < 0)
> - goto fail;
> + return ret;
> if (ret & rdy_mask)
> break;
> msleep(20); /* measurement takes up to 100 ms */
> @@ -462,20 +453,13 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
> if (tries < 0) {
> dev_err(&data->client->dev,
> "vcnl4000_measure() failed, data not ready\n");
> - ret = -EIO;
> - goto fail;
> + return -EIO;
> }
>
> ret = vcnl4000_read_data(data, data_reg, val);
> if (ret < 0)
> - goto fail;
> -
> - mutex_unlock(&data->vcnl4000_lock);
> -
> - return 0;
> + return ret;
>
> -fail:
> - mutex_unlock(&data->vcnl4000_lock);
> return ret;
return 0;
Check for any other cases that have been introduced where the final return
is always success (e.g. 0)
> }
> static ssize_t vcnl4040_read_ps_calibbias(struct vcnl4000_data *data, int *val, int *val2)
> @@ -878,20 +834,15 @@ static ssize_t vcnl4040_write_ps_calibbias(struct vcnl4000_data *data, int val)
> if (i >= ARRAY_SIZE(vcnl4040_ps_calibbias_ua))
> return -EINVAL;
>
> - mutex_lock(&data->vcnl4000_lock);
> + guard(mutex)(&data->vcnl4000_lock);
>
> ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
> if (ret < 0)
> - goto out_unlock;
> + return ret;
>
> regval = (ret & ~VCNL4040_PS_MS_LED_I);
> regval |= FIELD_PREP(VCNL4040_PS_MS_LED_I, i);
If you want more stuff to do on this driver, look at FIELD_MODIFY() and where
it can be usefully applied.
> - ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
> - regval);
> -
> -out_unlock:
> - mutex_unlock(&data->vcnl4000_lock);
> - return ret;
> + return i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3, regval);
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-07 16:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 21:05 [PATCH v4] iio: light: vcnl4000: use lock guard() Raffael Raiel Trindade
2026-05-07 16:36 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox