[PATCH v5 0/8] iio: light: si1133: driver cleanup

[PATCH v5 0/8] 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
 - race condition fixes reported by Sashiko

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v5:
- PATCH 5: fix commit message
- Pick up Andy's Reviewed-by tag
- Link to v4: https://lore.kernel.org/r/20260430-si1133-checkup-v4-0-fb3e9dce41bf@gmail.com

Changes in v4:
- Changed logical ordering of patchset
- PATCH 1: fix typo in comment, edit commit message
- PATCH 2: edit commit message
- PATCH 3: edit commit message
- PATCH 5: remove irqreturn.h and add mod_devicetable.h
- Link to v3: https://lore.kernel.org/r/20260429-si1133-checkup-v3-0-469f21d960eb@gmail.com

Changes in v3:
- PATCH 6: add local variable for timeout
- PATCH 7 & 8: fix potential race conditions as pointed out by Sashiko
- Link to v2: https://lore.kernel.org/r/20260428-si1133-checkup-v2-0-70ad14bfefe2@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 (8):
      iio: light: si1133: reset counter to prevent race condition
      iio: light: si1133: prevent race condition on timeout
      iio: light: si1133: remove unused macros
      iio: light: si1133: prefer complex macros enclosed in parenthesis
      iio: light: si1133: add missing include headers
      iio: light: si1133: group generic <linux/*> headers
      iio: light: si1133: add local variable for timeout
      iio: light: si1133: use guard(mutex)() macro

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

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

[PATCH v5 1/8] iio: light: si1133: reset counter to prevent race condition

[Joshua Crofts via B4 Relay]

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

Sashiko reported a potential race condition happening when the driver
returns an errno after a timeout in the si1133_command() function. The
premature exit causes the hardware and software counters to become out
of sync by not updating data->rsp_seq, therefore the internal hardware
counter keeps incrementing.

Fix this by adding a call to si1133_cmd_reset_counter() before returning
from timeout.

Fixes: e01e7eaf37d8 ("iio: light: introduce si1133")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260428-si1133-checkup-v2-5-70ad14bfefe2%40gmail.com
Assisted-by: gemini:gemini-3.1-pro-preview
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index 44fa152dbd24c26c97cc778cbe641d92ecd56afa..c88c79202be2e21abb72067426e31b6bf97c61cf 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -427,6 +427,11 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 			dev_warn(dev,
 				 "Failed to read command 0x%02x, ret=%d\n",
 				 cmd, err);
+			/*
+			 * Reset counter on err to prevent software and hardware
+			 * counters being out of sync.
+			 */
+			si1133_cmd_reset_counter(data);
 			goto out;
 		}
 	}

-- 
2.47.3

[PATCH v5 2/8] iio: light: si1133: prevent race condition on timeout

[Joshua Crofts via B4 Relay]

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

Sashiko reported a bug where the si1133_command exits on timeout
without halting the sensor or masking the interrupt. If the sensor
completes the command later, any subsequent command to the sensor
will cause the IRQ handler to complete immediately, returning stale
data to the driver all while the command hasn't finished yet, shifting
all potential reads in the future.

Fix this by masking the IRQ if wait_for_completion_timeout() fails.
When initiating a new command, do a dummy read of the IRQ_STATUS
register and turn the IRQ back on.

Fixes: e01e7eaf37d8 ("iio: light: introduce si1133")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260428-si1133-checkup-v2-5-70ad14bfefe2%40gmail.com
Assisted-by: gemini:gemini-3.1-pro-preview
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index c88c79202be2e21abb72067426e31b6bf97c61cf..bf7bf0f1631d49a1d04b89f172646e4afaf46438 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -395,8 +395,14 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 
 	expected_seq = (data->rsp_seq + 1) & SI1133_MAX_CMD_CTR;
 
-	if (cmd == SI1133_CMD_FORCE)
+	if (cmd == SI1133_CMD_FORCE) {
+		/* Flush pending IRQs from a previous timeout. */
+		regmap_read(data->regmap, SI1133_REG_IRQ_STATUS, &resp);
+		regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE,
+			     SI1133_IRQ_CHANNEL_ENABLE);
+
 		reinit_completion(&data->completion);
+	}
 
 	err = regmap_write(data->regmap, SI1133_REG_COMMAND, cmd);
 	if (err) {
@@ -409,6 +415,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 		/* wait for irq */
 		if (!wait_for_completion_timeout(&data->completion,
 			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
+			regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
 			err = -ETIMEDOUT;
 			goto out;
 		}

-- 
2.47.3

[PATCH v5 3/8] iio: light: si1133: remove unused macros

[Joshua Crofts via B4 Relay]

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

Remove unused macros unrelated to hardware definition.

No functional change.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index bf7bf0f1631d49a1d04b89f172646e4afaf46438..ed9b3e0a12d66d749f09f570de1ccdda2174b6ba 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -86,13 +86,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

[PATCH v5 4/8] 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.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index ed9b3e0a12d66d749f09f570de1ccdda2174b6ba..9abc0a7e2ecf0d2fb1bd3af88eb675e9f1be4665 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,11 +87,11 @@
 #define SI1133_CMD_MINSLEEP_US_HIGH	7500
 #define SI1133_CMD_TIMEOUT_MS		25
 
-#define SI1133_REG_HOSTOUT(x)		(x) + 0x13
+#define SI1133_REG_HOSTOUT(x)		((x) + 0x13)
 
 #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 v5 5/8] 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,
jiffies.h, math.h, mod_devicetable.h, mutex.h, types.h).

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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 9abc0a7e2ecf0d2fb1bd3af88eb675e9f1be4665..ab66a5a9ffb4a89bbba7474b6e984ddb61cfeebe 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/jiffies.h>
+#include <linux/math.h>
+#include <linux/mod_devicetable.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 v5 6/8] 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.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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 ab66a5a9ffb4a89bbba7474b6e984ddb61cfeebe..fdffdee16e493277d95c369ec2f6c0678cf9f61c 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

[PATCH v5 7/8] iio: light: si1133: add local variable for timeout

[Joshua Crofts via B4 Relay]

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

Add local variable for timeout to improve readability.

No functional change.

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

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index fdffdee16e493277d95c369ec2f6c0678cf9f61c..a3e4ab25acdc105d3800994f4391a7e4291e1c74 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -390,6 +390,7 @@ static int si1133_cmd_reset_counter(struct si1133_data *data)
 
 static int si1133_command(struct si1133_data *data, u8 cmd)
 {
+	unsigned long timeout = msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS);
 	struct device *dev = &data->client->dev;
 	u32 resp;
 	int err;
@@ -417,8 +418,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 
 	if (cmd == SI1133_CMD_FORCE) {
 		/* wait for irq */
-		if (!wait_for_completion_timeout(&data->completion,
-			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
+		if (!wait_for_completion_timeout(&data->completion, timeout)) {
 			regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
 			err = -ETIMEDOUT;
 			goto out;

-- 
2.47.3

Re: [PATCH v5 7/8] iio: light: si1133: add local variable for timeout

[Jonathan Cameron]

On Tue, 05 May 2026 09:31:32 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Add local variable for timeout to improve readability.
> 
> No functional change.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Tweaked as I mentioned in my late reply to v4 thread.

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index a3e4ab25acdc..ef5c38e303a6 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -390,7 +390,7 @@ static int si1133_cmd_reset_counter(struct si1133_data *data)
 
 static int si1133_command(struct si1133_data *data, u8 cmd)
 {
-       unsigned long timeout = msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS);
+       unsigned long timeout;
        struct device *dev = &data->client->dev;
        u32 resp;
        int err;
@@ -418,6 +418,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 
        if (cmd == SI1133_CMD_FORCE) {
                /* wait for irq */
+               timeout = msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS);
                if (!wait_for_completion_timeout(&data->completion, timeout)) {
                        regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
                        err = -ETIMEDOUT;


> ---
>  drivers/iio/light/si1133.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index fdffdee16e493277d95c369ec2f6c0678cf9f61c..a3e4ab25acdc105d3800994f4391a7e4291e1c74 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -390,6 +390,7 @@ static int si1133_cmd_reset_counter(struct si1133_data *data)
>  
>  static int si1133_command(struct si1133_data *data, u8 cmd)
>  {
> +	unsigned long timeout = msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS);
>  	struct device *dev = &data->client->dev;
>  	u32 resp;
>  	int err;
> @@ -417,8 +418,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
>  
>  	if (cmd == SI1133_CMD_FORCE) {
>  		/* wait for irq */
> -		if (!wait_for_completion_timeout(&data->completion,
> -			msecs_to_jiffies(SI1133_COMPLETION_TIMEOUT_MS))) {
> +		if (!wait_for_completion_timeout(&data->completion, timeout)) {
>  			regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
>  			err = -ETIMEDOUT;
>  			goto out;
> 

[PATCH v5 8/8] 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.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/light/si1133.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index a3e4ab25acdc105d3800994f4391a7e4291e1c74..55660dccfc7956e77b1d8105b7261221176c1736 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>
@@ -396,7 +397,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;
 
@@ -413,19 +414,18 @@ 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, timeout)) {
 			regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
-			err = -ETIMEDOUT;
-			goto out;
+			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,
@@ -443,7 +443,7 @@ static int si1133_command(struct si1133_data *data, u8 cmd)
 			 * counters being out of sync.
 			 */
 			si1133_cmd_reset_counter(data);
-			goto out;
+			return err;
 		}
 	}
 
@@ -454,9 +454,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 v5 0/8] iio: light: si1133: driver cleanup

[Jonathan Cameron]

On Tue, 05 May 2026 09:31:25 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> 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
>  - race condition fixes reported by Sashiko
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Applied.

Sashiko is far from happy but I think what it is picking up on
are either wrong (like the i2c stuff) or about other potential issues
in the driver.

Correctly recovering from every error case is fine so I tend
to think of that as best effort only rather than worrying too much
about the more complex parts.

So applied with that one tweak to where timeout is assigned in patch 7.

Thanks,

Jonathan

> ---
> Changes in v5:
> - PATCH 5: fix commit message
> - Pick up Andy's Reviewed-by tag
> - Link to v4: https://lore.kernel.org/r/20260430-si1133-checkup-v4-0-fb3e9dce41bf@gmail.com
> 
> Changes in v4:
> - Changed logical ordering of patchset
> - PATCH 1: fix typo in comment, edit commit message
> - PATCH 2: edit commit message
> - PATCH 3: edit commit message
> - PATCH 5: remove irqreturn.h and add mod_devicetable.h
> - Link to v3: https://lore.kernel.org/r/20260429-si1133-checkup-v3-0-469f21d960eb@gmail.com
> 
> Changes in v3:
> - PATCH 6: add local variable for timeout
> - PATCH 7 & 8: fix potential race conditions as pointed out by Sashiko
> - Link to v2: https://lore.kernel.org/r/20260428-si1133-checkup-v2-0-70ad14bfefe2@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 (8):
>       iio: light: si1133: reset counter to prevent race condition
>       iio: light: si1133: prevent race condition on timeout
>       iio: light: si1133: remove unused macros
>       iio: light: si1133: prefer complex macros enclosed in parenthesis
>       iio: light: si1133: add missing include headers
>       iio: light: si1133: group generic <linux/*> headers
>       iio: light: si1133: add local variable for timeout
>       iio: light: si1133: use guard(mutex)() macro
> 
>  drivers/iio/light/si1133.c | 69 +++++++++++++++++++++++++++-------------------
>  1 file changed, 41 insertions(+), 28 deletions(-)
> ---
> base-commit: d86db1905add39f905cf9f04252804b359914ed6
> change-id: 20260427-si1133-checkup-abcc5280adf3
> 
> Best regards,

Re: [PATCH v5 0/8] iio: light: si1133: driver cleanup

[Joshua Crofts]

On Tue, 5 May 2026 at 14:34, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 05 May 2026 09:31:25 +0200
> Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:
>
> > 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
> >  - race condition fixes reported by Sashiko
> >
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Applied.
>
> Sashiko is far from happy but I think what it is picking up on
> are either wrong (like the i2c stuff) or about other potential issues
> in the driver.
>
> Correctly recovering from every error case is fine so I tend
> to think of that as best effort only rather than worrying too much
> about the more complex parts.

It seems that Sashiko ends up finding more regressions after fixing
one it reported. It's a bit misleading when it reports multiple regressions
and once you open the report it's all "this wasn't introduced by the patch,
but - ".

> So applied with that one tweak to where timeout is assigned in patch 7.

Thanks, it does look better this way.

-- 
Kind regards

CJD