[v5] iio: light: si1133: driver cleanup

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

Posted by Joshua Crofts via B4 Relay 1 month, 1 week ago

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>

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

Posted by Jonathan Cameron 1 month, 1 week ago

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

Posted by Joshua Crofts 1 month, 1 week ago

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