Re: [PATCH v3 8/8] iio: light: si1133: prevent race condition on timeout

Joshua Crofts
2026-04-30T07:48:28Z

On Wed, 29 Apr 2026 at 21:27, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> 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.

Yeah, makes sense.

> ...
>
> > - 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.

Agreed, I seem to have missed this.

>
> > /*
>
> > - * 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).

I noticed this immediately after sending the series :((( I must've edited
it while rebasing...

--
Kind regards

CJD