Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604
From: Jonathan Cameron
Date: Fri May 22 2026 - 13:42:14 EST
On Fri, 22 May 2026 14:24:27 +0000
"Stan, Liviu" <Liviu.Stan@xxxxxxxxxx> wrote:
> On Fri, 22 May 2026 14:34:44 +0100 Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> > On Thu, 21 May 2026 19:43:01 +0300
> > Liviu Stan <liviu.stan@xxxxxxxxxx> wrote:
> >
> ...
> >
> > The sashiko comment about the completion race looks real to me.
> > Can you take a look at that. Might be some odd reason why we can't
> > reinit_completion() before triggering the capture but that would
> > be very unusual.
> >
>
> I don't see any reason why we couldn't put it before the regmap_write,
> it has also already been done in ltc2983_eeprom_cmd(). So, I will change
> it in v4.
>
> > Otherwise just two really small things I missed in earlier reviews.
> > If everything else was ready to go I'd have tweaked these whilst
> > applying but seems you'll be doing a v4 so over to you!
>
> Understood, will change them in v4!:) Thanks!
>
> On Fri, 22 May 2026 09:09:33 -0500 David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> > On 5/22/26 8:23 AM, Liviu Stan wrote:
> > > On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@xxxxxxxxxx wrote:
> > >
> > >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> ...
> > >> [Severity: High]
> > >> This is a pre-existing issue, but can this function experience spurious
> > >> read timeouts due to a race condition with the completion?
> > >>
> > >> Looking at the rest of this function:
> > >>
> > >> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion);
> > >> if (ret)
> > >> return ret;
> > >>
> > >> reinit_completion(&st->completion);
> > >>
> > >> If the hardware conversion finishes very quickly after regmap_write(),
> > >> could the interrupt fire and call complete() before the CPU executes
> > >> reinit_completion()? This would overwrite the completion event, causing the
> > >> subsequent wait_for_completion_timeout() to spuriously time out.
> > >>
> > >> Would it be safer to call reinit_completion() before initiating the hardware
> > >> action?
> > >
> > > This is theoretically valid but in practice, the conversion requires at minimum
> > > two 82ms cycles (167ms), so there is no realistic window for the interrupt to
> > > race with reinit_completion(). What do you guys think?
> >
> > I would still move it before the write so that it looks correct
> > and others can copy/paste the logic on other devices.
>
> That makes sense. Thanks!
>
> Would this be considered a fix?
Yes, though not an urgent one given no particular reports of it being hit.
Just put it at the start of the series and I'll apply it with the rest.
Can get backported after next merge window.
Thanks,
Jonathan
>
> Liviu