From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: joshua.crofts1@gmail.com
Cc: "Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 8/8] iio: light: si1133: prevent race condition on timeout
Date: Wed, 29 Apr 2026 22:27:46 +0300 [thread overview]
Message-ID: <afJbshe8SURhqKqA@ashevche-desk.local> (raw)
In-Reply-To: <20260429-si1133-checkup-v3-8-469f21d960eb@gmail.com>
On Wed, Apr 29, 2026 at 05:04:56PM +0200, Joshua Crofts via B4 Relay wrote:
> 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.
> 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>
Reported-by?
Also with this seems the Fixes tag is appropriate (and in the other patch).
Also note, fixes should go first in the series.
...
> - if (!wait_for_completion_timeout(&data->completion, timeout))
> + if (!wait_for_completion_timeout(&data->completion, timeout)) {
> + /* Mask the IRQ to prevent delayed interrupt waking up
> + * any subsequent command.
> + */
> + regmap_write(data->regmap, SI1133_REG_IRQ_ENABLE, 0);
> return -ETIMEDOUT;
> + }
Seems like you dropped {} and reinstantiated them. It's not good style. Just
make sure that the first patch that does something leaves it in the form that
next patches won't have too many + or - lines that basically revert previous
subchanges. I call this style ping-pong and highly discourage from using it.
...
> /*
> - * reset counter on err to prevent sofware and hardware
> - * counters being out of sync
> + * Reset counter on err to prevent sofware and hardware
> + * counters being out of sync.
> */
Ha-ha, doesn't belong to this change (see also previous reply).
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2026-04-29 19:27 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-29 15:04 [PATCH v3 0/8] iio: light: si1133: driver cleanup Joshua Crofts via B4 Relay
2026-04-29 15:04 ` [PATCH v3 1/8] iio: light: si1133: remove unused macros Joshua Crofts via B4 Relay
2026-04-29 19:14 ` Andy Shevchenko
2026-04-30 7:50 ` Joshua Crofts
2026-04-29 15:04 ` [PATCH v3 2/8] iio: light: si1133: prefer complex macros enclosed in parenthesis Joshua Crofts via B4 Relay
2026-04-29 15:04 ` [PATCH v3 3/8] iio: light: si1133: add missing include headers Joshua Crofts via B4 Relay
2026-04-29 19:16 ` Andy Shevchenko
2026-04-30 7:52 ` Joshua Crofts
2026-04-29 15:04 ` [PATCH v3 4/8] iio: light: si1133: group generic <linux/*> headers Joshua Crofts via B4 Relay
2026-04-29 15:04 ` [PATCH v3 5/8] iio: light: si1133: use guard(mutex)() macro Joshua Crofts via B4 Relay
2026-04-29 19:19 ` Andy Shevchenko
2026-04-30 7:55 ` Joshua Crofts
2026-04-29 15:04 ` [PATCH v3 6/8] iio: light: si1133: add local variable for timeout Joshua Crofts via B4 Relay
2026-04-29 19:21 ` Andy Shevchenko
2026-04-29 15:04 ` [PATCH v3 7/8] iio: light: si1133: reset counter to prevent race condition Joshua Crofts via B4 Relay
2026-04-29 19:23 ` Andy Shevchenko
2026-04-30 7:49 ` Joshua Crofts
2026-04-29 15:04 ` [PATCH v3 8/8] iio: light: si1133: prevent race condition on timeout Joshua Crofts via B4 Relay
2026-04-29 19:27 ` Andy Shevchenko [this message]
2026-04-30 7:48 ` Joshua Crofts
2026-04-29 19:28 ` [PATCH v3 0/8] iio: light: si1133: driver cleanup Andy Shevchenko
2026-04-30 7:58 ` Joshua Crofts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afJbshe8SURhqKqA@ashevche-desk.local \
--to=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=joshua.crofts1@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox