[PATCH v2 2/5] iio: light: tcs3472: convert remaining locking to guard(mutex)
From: Aldo Conte
Date: Tue May 12 2026 - 18:40:29 EST
Convert the locking in tcs3472_read_event(), tcs3472_write_event(),
tcs3472_read_event_config(), tcs3472_write_event_config(),
tcs3472_powerdown() and tcs3472_resume() to use guard(mutex)
instead of explicit mutex_lock()/mutex_unlock() pairs.
This avoids manual unlock calls on each return path, drops the goto
in tcs3472_write_event(), and removes 'ret' variables only needed to
return after the unlock.
No functional change.
Suggested-by: Andy Shevchenko <andy@xxxxxxxxxx>
Signed-off-by: Aldo Conte <aldocontelk@xxxxxxxxx>
---
v2:
(Suggested by Andy)
- Moved earlier in the series to avoid introducing manual locking in the devm
patch that would then be converted here.
- Dropped "found by code inspection" (this is a cleanup, not a bug)
- Test details moved to cover letter
drivers/iio/light/tcs3472.c | 59 +++++++++++++------------------------
1 file changed, 21 insertions(+), 38 deletions(-)
diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
index 9b0f64cd9c50..4fd4fd74d0d6 100644
--- a/drivers/iio/light/tcs3472.c
+++ b/drivers/iio/light/tcs3472.c
@@ -13,6 +13,7 @@
* TODO: wait time
*/
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/module.h>
@@ -220,32 +221,24 @@ static int tcs3472_read_event(struct iio_dev *indio_dev,
int *val2)
{
struct tcs3472_data *data = iio_priv(indio_dev);
- int ret;
unsigned int period;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
switch (info) {
case IIO_EV_INFO_VALUE:
*val = (dir == IIO_EV_DIR_RISING) ?
data->high_thresh : data->low_thresh;
- ret = IIO_VAL_INT;
- break;
+ return IIO_VAL_INT;
case IIO_EV_INFO_PERIOD:
period = (256 - data->atime) * 2400 *
tcs3472_intr_pers[data->apers];
*val = period / USEC_PER_SEC;
*val2 = period % USEC_PER_SEC;
- ret = IIO_VAL_INT_PLUS_MICRO;
- break;
+ return IIO_VAL_INT_PLUS_MICRO;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
-
- mutex_unlock(&data->lock);
-
- return ret;
}
static int tcs3472_write_event(struct iio_dev *indio_dev,
@@ -259,7 +252,8 @@ static int tcs3472_write_event(struct iio_dev *indio_dev,
int period;
int i;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
+
switch (info) {
case IIO_EV_INFO_VALUE:
switch (dir) {
@@ -270,18 +264,18 @@ static int tcs3472_write_event(struct iio_dev *indio_dev,
command = TCS3472_AILT;
break;
default:
- ret = -EINVAL;
- goto error;
+ return -EINVAL;
}
ret = i2c_smbus_write_word_data(data->client, command, val);
if (ret)
- goto error;
+ return ret;
if (dir == IIO_EV_DIR_RISING)
data->high_thresh = val;
else
data->low_thresh = val;
- break;
+
+ return 0;
case IIO_EV_INFO_PERIOD:
period = val * USEC_PER_SEC + val2;
for (i = 1; i < ARRAY_SIZE(tcs3472_intr_pers) - 1; i++) {
@@ -291,18 +285,14 @@ static int tcs3472_write_event(struct iio_dev *indio_dev,
}
ret = i2c_smbus_write_byte_data(data->client, TCS3472_PERS, i);
if (ret)
- goto error;
+ return ret;
data->apers = i;
- break;
+
+ return 0;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
-error:
- mutex_unlock(&data->lock);
-
- return ret;
}
static int tcs3472_read_event_config(struct iio_dev *indio_dev,
@@ -310,13 +300,10 @@ static int tcs3472_read_event_config(struct iio_dev *indio_dev,
enum iio_event_direction dir)
{
struct tcs3472_data *data = iio_priv(indio_dev);
- int ret;
- mutex_lock(&data->lock);
- ret = !!(data->enable & TCS3472_ENABLE_AIEN);
- mutex_unlock(&data->lock);
+ guard(mutex)(&data->lock);
- return ret;
+ return !!(data->enable & TCS3472_ENABLE_AIEN);
}
static int tcs3472_write_event_config(struct iio_dev *indio_dev,
@@ -327,7 +314,7 @@ static int tcs3472_write_event_config(struct iio_dev *indio_dev,
int ret = 0;
u8 enable_old;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
enable_old = data->enable;
@@ -342,7 +329,6 @@ static int tcs3472_write_event_config(struct iio_dev *indio_dev,
if (ret)
data->enable = enable_old;
}
- mutex_unlock(&data->lock);
return ret;
}
@@ -379,6 +365,7 @@ static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
} scan = { };
int ret = tcs3472_req_data(data);
+
if (ret < 0)
goto done;
@@ -544,15 +531,13 @@ static int tcs3472_powerdown(struct tcs3472_data *data)
int ret;
u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
data->enable & ~enable_mask);
if (!ret)
data->enable &= ~enable_mask;
- mutex_unlock(&data->lock);
-
return ret;
}
@@ -581,15 +566,13 @@ static int tcs3472_resume(struct device *dev)
int ret;
u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
data->enable | enable_mask);
if (!ret)
data->enable |= enable_mask;
- mutex_unlock(&data->lock);
-
return ret;
}
--
2.54.0