Re: [PATCH v2] iio: ti-ads7138: Disable STATS_EN bit while reading conversion results

[Paul Geurts]

> On 6/23/26 7:52 AM, Paul Geurts wrote:
>>> On Fri, Jun 19, 2026 at 09:42:08AM -0500, David Lechner wrote:
>>>> On 6/19/26 4:00 AM, Paul Geurts wrote:
>>>>> The device might update channel data while it's read by the host,
>>>>> providing a data race. Disable the update of the channel stats before
>>>>> reading the values.
>>>>
>>>> This description seems a bit short on details. It looks like this
>>>> driver doesn't support buffered reads. So if we disable statistics
>>>> during a direct read, why would we want to enable statistics in
>>>> the first place?
>>>
>>> I believe this patch is initiated by
>>> "Until a new conversion result is available, previous values can be read from
>>> the statistics registers. Before reading the statistics registers, set STATS_EN
>>> to 0 to prevent any updates to this register block."
>>> from the datasheet.
>>>
>>> I would rather like to know if this is IRL problem, or just a datasheet reading
>>> based code.
>> 
>> This is indeed the part of the datahseet I got the solution from. Our real life
>> scenario was that the register read from the RECENT register was constantly
>> flipping between values 0x24F0 and 0x2500. Due to the STATS_EN not being
>> cleared, we occasionally read 0x2400 and 0x25F0. So this is based on a real
>> life problem.
>
> Thanks, this is the kind of description we want in the commit message.

Ok I'll update that.

>
>> 
>>>
>>>> (This driver suffers from the comments say "what" rather than "why"
>>>> /* Enable statistics and digital window comparator */ so it is hard
>>>> to say what the original intention was.)
>>>>
>>>> Can you explain more what this race condition is about and what
>>>> happens before the fix vs. after the fix? People generally do this
>>>> with two columns showing concurrent function calls.
>>>>
>>>> It seems to me like disabling statistics would break IIO_CHAN_INFO_PEAK.
>> 
>> I think it indeed could break the PEAK value. But I think the data race is a
>> bigger issue. I'm not sure using the statistics at all is a good idea for
>> this device, but I'm also not sure there is another option. TBH, I don't
>> really have the time now to properly fix all of this. I just saw and
>> quickly fixed this, and thought it would be good to let you know this is
>> an issue.
>
> Now that it has been explained in more detail, I can see that your fix
> is correct. And we should also do the same when reading IIO_CHAN_INFO_PEAK
> and IIO_CHAN_INFO_TROUGH.
>
> This would make it so that we could never get the peak and trough from
> the same time period though since each read would reset all 3 values. Not
> sure if that is important or not.

Yes I can add that. It would probably not be ideal, but at least the data
being read is correct.

>
>> 
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>> 
>> br,
>> Paul

Thanks!
Paul