[PATCH v3 3/8] iio: tcs3472: convert remaining locking to guard(mutex)

From: Aldo Conte

Date: Fri May 22 2026 - 09:06:38 EST


Convert several functions to use guard(mutex)()

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.

While the conversion is in progress, take the opportunity to make a
few small cleanups that guard() enables:

- In tcs3472_read_event_config(), replace '!!(...)' with
'(...) ? 1 : 0' for readability.

- In tcs3472_write_event_config(), tcs3472_powerdown() and
tcs3472_resume() use an early return on the I2C
write failure path.

No functional change.

Signed-off-by: Aldo Conte <aldocontelk@xxxxxxxxx>
---
v3:
- Drop Suggested-by: Andy (inappropriate tag).
- In read_event_config(): !! -> ? 1 : 0.
- In write_event_config(): early return on I2C error.
- In powerdown() and resume(): early return on I2C error.
- Remove stray blank line in trigger_handler().

drivers/iio/light/tcs3472.c | 76 ++++++++++++++++---------------------
1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
index 1803bb882804..b31714040a02 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) ? 1 : 0;
}

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;

@@ -339,12 +326,13 @@ static int tcs3472_write_event_config(struct iio_dev *indio_dev,
if (enable_old != data->enable) {
ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
data->enable);
- if (ret)
+ if (ret) {
data->enable = enable_old;
+ return ret;
+ }
}
- mutex_unlock(&data->lock);

- return ret;
+ return 0;
}

static irqreturn_t tcs3472_event_handler(int irq, void *priv)
@@ -445,16 +433,16 @@ 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;
+ if (ret)
+ return ret;

- mutex_unlock(&data->lock);
+ data->enable &= ~enable_mask;

- return ret;
+ return 0;
}

static int tcs3472_probe(struct i2c_client *client)
@@ -583,16 +571,16 @@ 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;
+ if (ret)
+ return ret;

- mutex_unlock(&data->lock);
+ data->enable |= enable_mask;

- return ret;
+ return 0;
}

static DEFINE_SIMPLE_DEV_PM_OPS(tcs3472_pm_ops, tcs3472_suspend,
--
2.54.0