RE: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

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?

Liviu