[PATCH v2 0/5] iio: light: si1133: driver cleanup

[PATCH v2 0/5] iio: light: si1133: driver cleanup

[Joshua Crofts via B4 Relay]

This series deals with the cleanup and modernization of the Silicon
Labs SI1133 UV/Ambient light sensor.

What seemed like a simple checkpatch cleanup turned out to be more
complex, therefore I've ended up doing a patch series instead.

Changes include:
 - adding missing headers
 - removing unused macros
 - adding guard(mutex)() support
 - code style fixes

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v2:
- Change v2 to patch series
- PATCH 2: remove unused macros
- PATCH 3: add missing include headers
- PATCH 4: group generic <linux/*> headers
- PATCH 5: add guard(mutex)() support
- Link to v1: https://lore.kernel.org/r/20260427-si1133-checkup-v1-1-20f5f11eba6c@gmail.com

---
Joshua Crofts (5):
      iio: light: si1133: prefer complex macros enclosed in parenthesis
      iio: light: si1133: remove unused macros
      iio: light: si1133: add missing include headers
      iio: light: si1133: group generic <linux/*> headers
      iio: light: si1133: use guard(mutex)() macro

 drivers/iio/light/si1133.c | 57 ++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)
---
base-commit: d86db1905add39f905cf9f04252804b359914ed6
change-id: 20260427-si1133-checkup-abcc5280adf3

Best regards,
-- 
Joshua Crofts <joshua.crofts1@gmail.com>

[PATCH v2 1/5] iio: light: si1133: prefer complex macros enclosed in parenthesis

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Enclose complex macros in parenthesis per checkpatch.pl error to
improve code style.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index 44fa152dbd24c26c97cc778cbe641d92ecd56afa..52e4269dc35014b87b7c46a120e64723be716768 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -50,23 +50,23 @@
 #define SI1133_MAX_CMD_CTR		0xF
 
 #define SI1133_PARAM_REG_CHAN_LIST	0x01
-#define SI1133_PARAM_REG_ADCCONFIG(x)	((x) * 4) + 2
-#define SI1133_PARAM_REG_ADCSENS(x)	((x) * 4) + 3
-#define SI1133_PARAM_REG_ADCPOST(x)	((x) * 4) + 4
+#define SI1133_PARAM_REG_ADCCONFIG(x)	(((x) * 4) + 2)
+#define SI1133_PARAM_REG_ADCSENS(x)	(((x) * 4) + 3)
+#define SI1133_PARAM_REG_ADCPOST(x)	(((x) * 4) + 4)
 
 #define SI1133_ADCMUX_MASK 0x1F
 
-#define SI1133_ADCCONFIG_DECIM_RATE(x)	(x) << 5
+#define SI1133_ADCCONFIG_DECIM_RATE(x)	((x) << 5)
 
 #define SI1133_ADCSENS_SCALE_MASK 0x70
 #define SI1133_ADCSENS_SCALE_SHIFT 4
 #define SI1133_ADCSENS_HSIG_MASK BIT(7)
 #define SI1133_ADCSENS_HSIG_SHIFT 7
 #define SI1133_ADCSENS_HW_GAIN_MASK 0xF
-#define SI1133_ADCSENS_NB_MEAS(x)	fls(x) << SI1133_ADCSENS_SCALE_SHIFT
+#define SI1133_ADCSENS_NB_MEAS(x)	(fls(x) << SI1133_ADCSENS_SCALE_SHIFT)
 
 #define SI1133_ADCPOST_24BIT_EN BIT(6)
-#define SI1133_ADCPOST_POSTSHIFT_BITQTY(x) (x & GENMASK(2, 0)) << 3
+#define SI1133_ADCPOST_POSTSHIFT_BITQTY(x) (((x) & GENMASK(2, 0)) << 3)
 
 #define SI1133_PARAM_ADCMUX_SMALL_IR	0x0
 #define SI1133_PARAM_ADCMUX_MED_IR	0x1
@@ -87,15 +87,15 @@
 #define SI1133_CMD_MINSLEEP_US_HIGH	7500
 #define SI1133_CMD_TIMEOUT_MS		25
 #define SI1133_CMD_LUX_TIMEOUT_MS	5000
-#define SI1133_CMD_TIMEOUT_US		SI1133_CMD_TIMEOUT_MS * 1000
+#define SI1133_CMD_TIMEOUT_US		(SI1133_CMD_TIMEOUT_MS * 1000)
 
-#define SI1133_REG_HOSTOUT(x)		(x) + 0x13
+#define SI1133_REG_HOSTOUT(x)		((x) + 0x13)
 
 #define SI1133_MEASUREMENT_FREQUENCY 1250
 
 #define SI1133_X_ORDER_MASK            0x0070
 #define SI1133_Y_ORDER_MASK            0x0007
-#define si1133_get_x_order(m)          ((m) & SI1133_X_ORDER_MASK) >> 4
+#define si1133_get_x_order(m)          (((m) & SI1133_X_ORDER_MASK) >> 4)
 #define si1133_get_y_order(m)          ((m) & SI1133_Y_ORDER_MASK)
 
 #define SI1133_LUX_ADC_MASK		0xE

-- 
2.47.3

[PATCH v2 2/5] iio: light: si1133: remove unused macros

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Remove unused macros to improve readability.

No functional change.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index 52e4269dc35014b87b7c46a120e64723be716768..ada8da122b76c9a0a55751baa6522a145f0fc7e8 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -22,8 +22,6 @@
 #define SI1133_REG_PART_ID		0x00
 #define SI1133_REG_REV_ID		0x01
 #define SI1133_REG_MFR_ID		0x02
-#define SI1133_REG_INFO0		0x03
-#define SI1133_REG_INFO1		0x04
 
 #define SI1133_PART_ID			0x33
 
@@ -40,7 +38,6 @@
 #define SI1133_CMD_RESET_CTR		0x00
 #define SI1133_CMD_RESET_SW		0x01
 #define SI1133_CMD_FORCE		0x11
-#define SI1133_CMD_START_AUTONOMOUS	0x13
 #define SI1133_CMD_PARAM_SET		0x80
 #define SI1133_CMD_PARAM_QUERY		0x40
 #define SI1133_CMD_PARAM_MASK		0x3F
@@ -86,13 +83,9 @@
 #define SI1133_CMD_MINSLEEP_US_LOW	5000
 #define SI1133_CMD_MINSLEEP_US_HIGH	7500
 #define SI1133_CMD_TIMEOUT_MS		25
-#define SI1133_CMD_LUX_TIMEOUT_MS	5000
-#define SI1133_CMD_TIMEOUT_US		(SI1133_CMD_TIMEOUT_MS * 1000)
 
 #define SI1133_REG_HOSTOUT(x)		((x) + 0x13)
 
-#define SI1133_MEASUREMENT_FREQUENCY 1250
-
 #define SI1133_X_ORDER_MASK            0x0070
 #define SI1133_Y_ORDER_MASK            0x0007
 #define si1133_get_x_order(m)          (((m) & SI1133_X_ORDER_MASK) >> 4)

-- 
2.47.3

Re: [PATCH v2 2/5] iio: light: si1133: remove unused macros

[Jonathan Cameron]

On Tue, 28 Apr 2026 15:18:34 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Remove unused macros to improve readability.
Hi Joshua,

Don't update macros then remove them - that's just increasing
noise in the update patch. Reorder this patch to be before patch 1.

Otherwise, I think a few of these can stay. See below

Thanks,

Jonathan

> 
> No functional change.
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/light/si1133.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index 52e4269dc35014b87b7c46a120e64723be716768..ada8da122b76c9a0a55751baa6522a145f0fc7e8 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -22,8 +22,6 @@
>  #define SI1133_REG_PART_ID		0x00
>  #define SI1133_REG_REV_ID		0x01
>  #define SI1133_REG_MFR_ID		0x02
> -#define SI1133_REG_INFO0		0x03
> -#define SI1133_REG_INFO1		0x04

This one isn't in the category of things Andy was referring to.
It's a hardware register address definition.  We don't have hard rules on whether
a complete set of these should be defined, or just the ones used but it is definitely
acceptable to just define them all up front as then easy to check them against
a datasheet.

>  
>  #define SI1133_PART_ID			0x33
>  
> @@ -40,7 +38,6 @@
>  #define SI1133_CMD_RESET_CTR		0x00
>  #define SI1133_CMD_RESET_SW		0x01
>  #define SI1133_CMD_FORCE		0x11
> -#define SI1133_CMD_START_AUTONOMOUS	0x13
Similar for this.

>  #define SI1133_CMD_PARAM_SET		0x80
>  #define SI1133_CMD_PARAM_QUERY		0x40
>  #define SI1133_CMD_PARAM_MASK		0x3F
> @@ -86,13 +83,9 @@
>  #define SI1133_CMD_MINSLEEP_US_LOW	5000
>  #define SI1133_CMD_MINSLEEP_US_HIGH	7500
>  #define SI1133_CMD_TIMEOUT_MS		25
> -#define SI1133_CMD_LUX_TIMEOUT_MS	5000
> -#define SI1133_CMD_TIMEOUT_US		(SI1133_CMD_TIMEOUT_MS * 1000)

Dropping these and the one below is good as those aren't register or command
defines. This last one is entirely pointless as better to have the rescale
inline and only one definition up here.

>  
>  #define SI1133_REG_HOSTOUT(x)		((x) + 0x13)
>  
> -#define SI1133_MEASUREMENT_FREQUENCY 1250
> -
>  #define SI1133_X_ORDER_MASK            0x0070
>  #define SI1133_Y_ORDER_MASK            0x0007
>  #define si1133_get_x_order(m)          (((m) & SI1133_X_ORDER_MASK) >> 4)
> 

Re: [PATCH v2 2/5] iio: light: si1133: remove unused macros

[Joshua Crofts]

On Tue, 28 Apr 2026 at 17:12, Jonathan Cameron <jic23@kernel.org> wrote:
> Don't update macros then remove them - that's just increasing
> noise in the update patch. Reorder this patch to be before patch 1.

Yeah, my fault, I forgot to do a rebase.

> >  #define SI1133_REG_PART_ID           0x00
> >  #define SI1133_REG_REV_ID            0x01
> >  #define SI1133_REG_MFR_ID            0x02
> > -#define SI1133_REG_INFO0             0x03
> > -#define SI1133_REG_INFO1             0x04
>
> This one isn't in the category of things Andy was referring to.
> It's a hardware register address definition.  We don't have hard rules on whether
> a complete set of these should be defined, or just the ones used but it is definitely
> acceptable to just define them all up front as then easy to check them against
> a datasheet.
>
> >
> >  #define SI1133_PART_ID                       0x33
> >
> > @@ -40,7 +38,6 @@
> >  #define SI1133_CMD_RESET_CTR         0x00
> >  #define SI1133_CMD_RESET_SW          0x01
> >  #define SI1133_CMD_FORCE             0x11
> > -#define SI1133_CMD_START_AUTONOMOUS  0x13
> Similar for this.
>
> >  #define SI1133_CMD_PARAM_SET         0x80
> >  #define SI1133_CMD_PARAM_QUERY               0x40
> >  #define SI1133_CMD_PARAM_MASK                0x3F
> > @@ -86,13 +83,9 @@
> >  #define SI1133_CMD_MINSLEEP_US_LOW   5000
> >  #define SI1133_CMD_MINSLEEP_US_HIGH  7500
> >  #define SI1133_CMD_TIMEOUT_MS                25
> > -#define SI1133_CMD_LUX_TIMEOUT_MS    5000
> > -#define SI1133_CMD_TIMEOUT_US                (SI1133_CMD_TIMEOUT_MS * 1000)
>
> Dropping these and the one below is good as those aren't register or command
> defines. This last one is entirely pointless as better to have the rescale
> inline and only one definition up here.

Sure, I'll send a v2 tomorrow.

-- 
Kind regards

CJD

[PATCH v2 3/5] iio: light: si1133: add missing include headers

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Add missing include headers to prevent compilation relying on transient
dependencies (array_size.h, bitops.h, completion.h, dev_printk.h, err.h,
irqreturn.h, jiffies.h, math.h, mutex.h, types.h).

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index ada8da122b76c9a0a55751baa6522a145f0fc7e8..5c5c5ae0535a9592522c0b21496f47068044c6ff 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -6,11 +6,21 @@
  * Copyright 2018 Maxime Roussin-Belanger <maxime.roussinbelanger@gmail.com>
  */
 
+#include <linux/array_size.h>
+#include <linux/bitops.h>
+#include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/jiffies.h>
+#include <linux/math.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regmap.h>
+#include <linux/types.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>

-- 
2.47.3

[PATCH v2 4/5] iio: light: si1133: group generic <linux/*> headers

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Group generic <linux/*> include headers to improve code style.

No functional change.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index 5c5c5ae0535a9592522c0b21496f47068044c6ff..f2a79675c026c86bb56d85a65cd035d7540d0ef8 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -21,14 +21,12 @@
 #include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/util_macros.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include <linux/util_macros.h>
-
-#include <linux/unaligned.h>
-
 #define SI1133_REG_PART_ID		0x00
 #define SI1133_REG_REV_ID		0x01
 #define SI1133_REG_MFR_ID		0x02

-- 
2.47.3

Re: [PATCH v2 4/5] iio: light: si1133: group generic <linux/*> headers

[Jonathan Cameron]

On Tue, 28 Apr 2026 15:18:36 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Group generic <linux/*> include headers to improve code style.
Not particularly important but you could mention why these two are
where they are.  For the unaligned one that was a mass replacement
of asm/unaligned with linux/unaligned that didn't reflect local
ordering.  No idea for util_macros.h!

> 
> No functional change.
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/light/si1133.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index 5c5c5ae0535a9592522c0b21496f47068044c6ff..f2a79675c026c86bb56d85a65cd035d7540d0ef8 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -21,14 +21,12 @@
>  #include <linux/mutex.h>
>  #include <linux/regmap.h>
>  #include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/util_macros.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#include <linux/util_macros.h>
> -
> -#include <linux/unaligned.h>
> -
>  #define SI1133_REG_PART_ID		0x00
>  #define SI1133_REG_REV_ID		0x01
>  #define SI1133_REG_MFR_ID		0x02
> 

[PATCH v2 5/5] iio: light: si1133: use guard(mutex)() macro

[Joshua Crofts via B4 Relay]

From: Joshua Crofts <joshua.crofts1@gmail.com>

Remove mutex_lock()/mutex_unlock() and goto instances and add
guard(mutex)() macro to modernize driver and improve mutex handling.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index f2a79675c026c86bb56d85a65cd035d7540d0ef8..3b40defda383bcbe731dc0b413903e0267f03b01 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -8,6 +8,7 @@
 
 #include <linux/array_size.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
@@ -392,7 +393,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 	int err;
 	int expected_seq;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	expected_seq = (data->rsp_seq + 1) & SI1133_MAX_CMD_CTR;
 
@@ -403,19 +404,17 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 	if (err) {
 		dev_warn(dev, "Failed to write command 0x%02x, ret=%d\n", cmd,
 			 err);
-		goto out;
+		return err;
 	}
 
 	if (cmd == SI1133_CMD_FORCE) {
 		/* wait for irq */
 		if (!wait_for_completion_timeout(&data->completion,
-			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
-			err = -ETIMEDOUT;
-			goto out;
-		}
+			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS)))
+			return -ETIMEDOUT;
 		err = regmap_read(data->regmap, SI1133_REG_RESPONSE0, &resp);
 		if (err)
-			goto out;
+			return err;
 	} else {
 		err = regmap_read_poll_timeout(data->regmap,
 					       SI1133_REG_RESPONSE0, resp,
@@ -428,7 +427,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 			dev_warn(dev,
 				 "Failed to read command 0x%02x, ret=%d\n",
 				 cmd, err);
-			goto out;
+			return err;
 		}
 	}
 
@@ -439,9 +438,6 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 		data->rsp_seq = expected_seq;
 	}
 
-out:
-	mutex_unlock(&data->mutex);
-
 	return err;
 }
 

-- 
2.47.3

Re: [PATCH v2 5/5] iio: light: si1133: use guard(mutex)() macro

[Jonathan Cameron]

On Tue, 28 Apr 2026 15:18:37 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Remove mutex_lock()/mutex_unlock() and goto instances and add
> guard(mutex)() macro to modernize driver and improve mutex handling.
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
One comment that is more about the existing code than the changes here, but
as you are touching it nice to clean that bit up!

Nice series,

Jonathan

> ---
>  drivers/iio/light/si1133.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index f2a79675c026c86bb56d85a65cd035d7540d0ef8..3b40defda383bcbe731dc0b413903e0267f03b01 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -8,6 +8,7 @@
>  
>  #include <linux/array_size.h>
>  #include <linux/bitops.h>
> +#include <linux/cleanup.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/dev_printk.h>
> @@ -392,7 +393,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
>  	int err;
>  	int expected_seq;
>  
> -	mutex_lock(&data->mutex);
> +	guard(mutex)(&data->mutex);
>  
>  	expected_seq = (data->rsp_seq + 1) & SI1133_MAX_CMD_CTR;
>  
> @@ -403,19 +404,17 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
>  	if (err) {
>  		dev_warn(dev, "Failed to write command 0x%02x, ret=%d\n", cmd,
>  			 err);
> -		goto out;
> +		return err;
>  	}
>  
>  	if (cmd == SI1133_CMD_FORCE) {
>  		/* wait for irq */
>  		if (!wait_for_completion_timeout(&data->completion,
> -			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
> -			err = -ETIMEDOUT;
> -			goto out;
> -		}
> +			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS)))
> +			return -ETIMEDOUT;
This is some unfortunate indentation from readability point of view.
I'd reindent and not worry about the long line - it's very long but still under 100 so
tools won't complain and I think it is worth the pain here!

>  		if (!wait_for_completion_timeout(&data->completion,
						 msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS)))
			return -ETIMEDOUT;

or perhaps better yet, use a local variable for that timeout.


>  		err = regmap_read(data->regmap, SI1133_REG_RESPONSE0, &resp);
>  		if (err)
> -			goto out;
> +			return err;
>  	} else {
>  		err = regmap_read_poll_timeout(data->regmap,
>  					       SI1133_REG_RESPONSE0, resp,
> @@ -428,7 +427,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
>  			dev_warn(dev,
>  				 "Failed to read command 0x%02x, ret=%d\n",
>  				 cmd, err);
> -			goto out;
> +			return err;
>  		}
>  	}
>  
> @@ -439,9 +438,6 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
>  		data->rsp_seq = expected_seq;
>  	}
>  
> -out:
> -	mutex_unlock(&data->mutex);
> -
>  	return err;
>  }
>  
> 

Re: [PATCH v2 5/5] iio: light: si1133: use guard(mutex)() macro

[Joshua Crofts]

On Tue, 28 Apr 2026 at 17:20, Jonathan Cameron <jic23@kernel.org> wrote:
> >       if (cmd == SI1133_CMD_FORCE) {
> >               /* wait for irq */
> >               if (!wait_for_completion_timeout(&data->completion,
> > -                     msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
> > -                     err = -ETIMEDOUT;
> > -                     goto out;
> > -             }
> > +                     msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS)))
> > +                     return -ETIMEDOUT;
> This is some unfortunate indentation from readability point of view.
> I'd reindent and not worry about the long line - it's very long but still under 100 so
> tools won't complain and I think it is worth the pain here!
>
> >               if (!wait_for_completion_timeout(&data->completion,
>                                                  msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS)))
>                         return -ETIMEDOUT;
>
> or perhaps better yet, use a local variable for that timeout.

Okay, I'll probably do a separate patch for that though.

-- 
Kind regards

CJD