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

[David Lechner]

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?

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

> 
> Signed-off-by: Paul Geurts <paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Fixes: 93a39542d3c3 ("iio: adc: Add driver for ADS7128 / ADS7138")
> ---
> 
> V1 -> V2: Checked return values and prefixed iio: in commit msg
> 
> v1: https://lore.kernel.org/all/20260619075646.4100193-1-paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx/
> ---
>  drivers/iio/adc/ti-ads7138.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/iio/adc/ti-ads7138.c b/drivers/iio/adc/ti-ads7138.c
> index ee5c1b8e3a8e..81380fd2badc 100644
> --- a/drivers/iio/adc/ti-ads7138.c
> +++ b/drivers/iio/adc/ti-ads7138.c
> @@ -237,11 +237,21 @@ static int ads7138_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> +		/* Disable statistics update so the value is not updated mid read */
> +		ret = ads7138_i2c_clear_bit(data->client, ADS7138_REG_GENERAL_CFG,
> +					    ADS7138_GENERAL_CFG_STATS_EN);
> +		if (ret)
> +			return ret;
>  		ret = ads7138_i2c_read_block(data->client,
>  					     ADS7138_REG_RECENT_LSB_CH(chan->channel),
>  					     values, ARRAY_SIZE(values));
>  		if (ret)
>  			return ret;
> +		/* Enable statistics update after read */
> +		ret = ads7138_i2c_set_bit(data->client, ADS7138_REG_GENERAL_CFG,
> +					  ADS7138_GENERAL_CFG_STATS_EN);
> +		if (ret)
> +			return ret;
>  
>  		*val = get_unaligned_le16(values);
>  		return IIO_VAL_INT;