[PATCH v3 0/2] iio: light: iqs621-als: use lock guards and prefer early error handling

[PATCH v3 0/2] iio: light: iqs621-als: use lock guards and prefer early error handling

[Pedro Barletta Gennari]

Use guard(mutex)() for handling mutex lock instead of manually
locking and unlocking the mutex and flip 'if (!ret)' to 'if (ret)'
to handle errors early in drivers/iio/light/iqs621-als.c

Patch 2 depends on patch 1

Signed-off-by: Pedro Barletta Gennari <pedro.pbg@usp.br>
---
v3:
  - Remove unnecessary { }
  - Remove unnecessary ret initialization
  - Flip 'if (!ret)' to 'if (ret)'
v2:
  - Keep include list ordered
  - Remove redundant 'else'
  - Remove unnecessary variable 'ret'
---

Pedro Barletta Gennari (2):
  iio: light: iqs621-als: use lock guards
  iio: light: iqs621-als: prefer early error handling over if (!ret)

 drivers/iio/light/iqs621-als.c | 111 +++++++++++++--------------------
 1 file changed, 43 insertions(+), 68 deletions(-)

-- 
2.51.0

[PATCH v3 1/2] iio: light: iqs621-als: use lock guards

[Pedro Barletta Gennari]

Use guard(mutex)() for handling mutex lock instead of
manually locking and unlocking the mutex. This prevents forgotten
locks due to early exits and removes the need of gotos.

Signed-off-by: Pedro Barletta Gennari <pedro.pbg@usp.br>
---
 drivers/iio/light/iqs621-als.c | 93 ++++++++++++----------------------
 1 file changed, 31 insertions(+), 62 deletions(-)

diff --git a/drivers/iio/light/iqs621-als.c b/drivers/iio/light/iqs621-als.c
index b9f230210f07..25a655b328fc 100644
--- a/drivers/iio/light/iqs621-als.c
+++ b/drivers/iio/light/iqs621-als.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2019 Jeff LaBundy <jeff@labundy.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
@@ -107,26 +108,20 @@ static int iqs621_als_notifier(struct notifier_block *notifier,
 	indio_dev = iqs621_als->indio_dev;
 	timestamp = iio_get_time_ns(indio_dev);
 
-	mutex_lock(&iqs621_als->lock);
+	guard(mutex)(&iqs621_als->lock);
 
 	if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) {
 		ret = iqs621_als_init(iqs621_als);
 		if (ret) {
 			dev_err(indio_dev->dev.parent,
 				"Failed to re-initialize device: %d\n", ret);
-			ret = NOTIFY_BAD;
-		} else {
-			ret = NOTIFY_OK;
+			return NOTIFY_BAD;
 		}
-
-		goto err_mutex;
+		return NOTIFY_OK;
 	}
 
-	if (!iqs621_als->light_en && !iqs621_als->range_en &&
-	    !iqs621_als->prox_en) {
-		ret = NOTIFY_DONE;
-		goto err_mutex;
-	}
+	if (!iqs621_als->light_en && !iqs621_als->range_en && !iqs621_als->prox_en)
+		return NOTIFY_DONE;
 
 	/* IQS621 only */
 	light_new = event_data->als_flags & IQS621_ALS_FLAGS_LIGHT;
@@ -181,12 +176,7 @@ static int iqs621_als_notifier(struct notifier_block *notifier,
 
 	iqs621_als->als_flags = event_data->als_flags;
 	iqs621_als->ir_flags = event_data->ir_flags;
-	ret = NOTIFY_OK;
-
-err_mutex:
-	mutex_unlock(&iqs621_als->lock);
-
-	return ret;
+	return NOTIFY_OK;
 }
 
 static void iqs621_als_notifier_unregister(void *context)
@@ -241,30 +231,22 @@ static int iqs621_als_read_event_config(struct iio_dev *indio_dev,
 					enum iio_event_direction dir)
 {
 	struct iqs621_als_private *iqs621_als = iio_priv(indio_dev);
-	int ret;
 
-	mutex_lock(&iqs621_als->lock);
+	guard(mutex)(&iqs621_als->lock);
 
 	switch (chan->type) {
 	case IIO_LIGHT:
-		ret = iqs621_als->light_en;
-		break;
+		return iqs621_als->light_en;
 
 	case IIO_INTENSITY:
-		ret = iqs621_als->range_en;
-		break;
+		return iqs621_als->range_en;
 
 	case IIO_PROXIMITY:
-		ret = iqs621_als->prox_en;
-		break;
+		return iqs621_als->prox_en;
 
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	mutex_unlock(&iqs621_als->lock);
-
-	return ret;
 }
 
 static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
@@ -278,11 +260,11 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 	unsigned int val;
 	int ret;
 
-	mutex_lock(&iqs621_als->lock);
+	guard(mutex)(&iqs621_als->lock);
 
 	ret = regmap_read(iqs62x->regmap, iqs62x->dev_desc->als_flags, &val);
 	if (ret)
-		goto err_mutex;
+		return ret;
 	iqs621_als->als_flags = val;
 
 	switch (chan->type) {
@@ -293,7 +275,7 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 									 0xFF);
 		if (!ret)
 			iqs621_als->light_en = state;
-		break;
+		return ret;
 
 	case IIO_INTENSITY:
 		ret = regmap_update_bits(iqs62x->regmap, IQS620_GLBL_EVENT_MASK,
@@ -302,12 +284,12 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 									 0xFF);
 		if (!ret)
 			iqs621_als->range_en = state;
-		break;
+		return ret;
 
 	case IIO_PROXIMITY:
 		ret = regmap_read(iqs62x->regmap, IQS622_IR_FLAGS, &val);
 		if (ret)
-			goto err_mutex;
+			return ret;
 		iqs621_als->ir_flags = val;
 
 		ret = regmap_update_bits(iqs62x->regmap, IQS620_GLBL_EVENT_MASK,
@@ -315,16 +297,11 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 					 state ? 0 : 0xFF);
 		if (!ret)
 			iqs621_als->prox_en = state;
-		break;
+		return ret;
 
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-err_mutex:
-	mutex_unlock(&iqs621_als->lock);
-
-	return ret;
 }
 
 static int iqs621_als_read_event_value(struct iio_dev *indio_dev,
@@ -335,33 +312,28 @@ static int iqs621_als_read_event_value(struct iio_dev *indio_dev,
 				       int *val, int *val2)
 {
 	struct iqs621_als_private *iqs621_als = iio_priv(indio_dev);
-	int ret = IIO_VAL_INT;
 
-	mutex_lock(&iqs621_als->lock);
+	guard(mutex)(&iqs621_als->lock);
 
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
 		*val = iqs621_als->thresh_light * 16;
-		break;
+		return IIO_VAL_INT;
 
 	case IIO_EV_DIR_FALLING:
 		*val = iqs621_als->thresh_dark * 4;
-		break;
+		return IIO_VAL_INT;
 
 	case IIO_EV_DIR_EITHER:
 		if (iqs621_als->ir_flags_mask == IQS622_IR_FLAGS_TOUCH)
 			*val = iqs621_als->thresh_prox * 4;
 		else
 			*val = iqs621_als->thresh_prox;
-		break;
+		return IIO_VAL_INT;
 
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	mutex_unlock(&iqs621_als->lock);
-
-	return ret;
 }
 
 static int iqs621_als_write_event_value(struct iio_dev *indio_dev,
@@ -375,9 +347,9 @@ static int iqs621_als_write_event_value(struct iio_dev *indio_dev,
 	struct iqs62x_core *iqs62x = iqs621_als->iqs62x;
 	unsigned int thresh_reg, thresh_val;
 	u8 ir_flags_mask, *thresh_cache;
-	int ret = -EINVAL;
+	int ret;
 
-	mutex_lock(&iqs621_als->lock);
+	guard(mutex)(&iqs621_als->lock);
 
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
@@ -426,30 +398,27 @@ static int iqs621_als_write_event_value(struct iio_dev *indio_dev,
 			break;
 
 		default:
-			goto err_mutex;
+			return -EINVAL;
 		}
 
 		thresh_cache = &iqs621_als->thresh_prox;
 		break;
 
 	default:
-		goto err_mutex;
+		return -EINVAL;
 	}
 
 	if (thresh_val > 0xFF)
-		goto err_mutex;
+		return -EINVAL;
 
 	ret = regmap_write(iqs62x->regmap, thresh_reg, thresh_val);
 	if (ret)
-		goto err_mutex;
+		return ret;
 
 	*thresh_cache = thresh_val;
 	iqs621_als->ir_flags_mask = ir_flags_mask;
 
-err_mutex:
-	mutex_unlock(&iqs621_als->lock);
-
-	return ret;
+	return 0;
 }
 
 static const struct iio_info iqs621_als_info = {
-- 
2.51.0

[PATCH v3 2/2] iio: light: iqs621-als: prefer early error handling over if (!ret)

[Pedro Barletta Gennari]

Handle errors as early as possible by replacing 'if (!ret)' with the
more common form 'if (ret)'. This makes the code easier to read.

Signed-off-by: Pedro Barletta Gennari <pedro.pbg@usp.br>
---
 drivers/iio/light/iqs621-als.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/iqs621-als.c b/drivers/iio/light/iqs621-als.c
index 25a655b328fc..cd5843e3e2c3 100644
--- a/drivers/iio/light/iqs621-als.c
+++ b/drivers/iio/light/iqs621-als.c
@@ -273,18 +273,22 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 					 iqs62x->dev_desc->als_mask,
 					 iqs621_als->range_en || state ? 0 :
 									 0xFF);
-		if (!ret)
-			iqs621_als->light_en = state;
-		return ret;
+		if (ret)
+			return ret;
+		iqs621_als->light_en = state;
+
+		return 0;
 
 	case IIO_INTENSITY:
 		ret = regmap_update_bits(iqs62x->regmap, IQS620_GLBL_EVENT_MASK,
 					 iqs62x->dev_desc->als_mask,
 					 iqs621_als->light_en || state ? 0 :
 									 0xFF);
-		if (!ret)
-			iqs621_als->range_en = state;
-		return ret;
+		if (ret)
+			return ret;
+		iqs621_als->range_en = state;
+
+		return 0;
 
 	case IIO_PROXIMITY:
 		ret = regmap_read(iqs62x->regmap, IQS622_IR_FLAGS, &val);
@@ -295,9 +299,11 @@ static int iqs621_als_write_event_config(struct iio_dev *indio_dev,
 		ret = regmap_update_bits(iqs62x->regmap, IQS620_GLBL_EVENT_MASK,
 					 iqs62x->dev_desc->ir_mask,
 					 state ? 0 : 0xFF);
-		if (!ret)
-			iqs621_als->prox_en = state;
-		return ret;
+		if (ret)
+			return ret;
+		iqs621_als->prox_en = state;
+
+		return 0;
 
 	default:
 		return -EINVAL;
-- 
2.51.0

Re: [PATCH v3 0/2] iio: light: iqs621-als: use lock guards and prefer early error handling

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 10:29:53PM -0300, Pedro Barletta Gennari wrote:
> Use guard(mutex)() for handling mutex lock instead of manually
> locking and unlocking the mutex and flip 'if (!ret)' to 'if (ret)'
> to handle errors early in drivers/iio/light/iqs621-als.c
> 
> Patch 2 depends on patch 1

Both LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/2] iio: light: iqs621-als: use lock guards and prefer early error handling

[Jonathan Cameron]

On Wed, 29 Apr 2026 23:01:34 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Apr 28, 2026 at 10:29:53PM -0300, Pedro Barletta Gennari wrote:
> > Use guard(mutex)() for handling mutex lock instead of manually
> > locking and unlocking the mutex and flip 'if (!ret)' to 'if (ret)'
> > to handle errors early in drivers/iio/light/iqs621-als.c
> > 
> > Patch 2 depends on patch 1  
> 
> Both LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
Applied to the testing branch of iio.git.

I'll push that out as togreg once I've caught up with the
backlog and the bots have finished poking at it.

Thanks,

Jonathan