Re: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The polling logic in `ads1100_poll_data_ready` is fundamentally broken due to an inverted loop condition and impossible hardware polling state in continuous mode.

Pre-existing issues:
- [High] The driver consistently returns stale or invalid data on the first read after waking up from runtime suspend because it does not wait for a new conversion to complete.
- [Medium] Missing return value check for `ads1100_set_config_bits()` in `ads1100_set_scale()` suppresses I2C write errors.
--

--- Patch [1]: [PATCH v7 1/3] iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode ---
commit 110a1aa46c3c440dfe6b3dac8a6c6e80760ff636
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>

iio: adc: ti-ads1100: Fix incorrect reading when datarate changed in single mode

When device is suspended and it is in single mode then changing
datarate doesn't make it actually wait for a new measurement, so to
be sure that read after change is correct functions that changes
datarate and gain will wait for new data.

Fixes: 541880542f2b ("iio: adc: Add TI ADS1100 and ADS1000")

> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 9fe8d54cce83c..f0a30ae139afa 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -123,10 +128,50 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does ads1100_get_adc_result() return stale
or invalid data on the first read after waking up from runtime suspend?

The code executes the I2C read immediately after the runtime PM resume
returns, bypassing the necessary hardware conversion delay:

ads1100_get_adc_result()
    ret = pm_runtime_resume_and_get(&data->client->dev);
    if (ret < 0)
        return ret;

    ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));

If the read occurs immediately before the conversion finishes (for example,
125ms delay is needed at 8 SPS), will this return the power-on reset value
of 0 or garbage data instead of a valid measurement?

> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> +	int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> +	/* To be sure we wait 5 times more than data rate */
> +	unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);
> +	int data_ready;
> +	u8 buffer[3];
> +	int ret;
> +
> +	/* To be sure that polled value will have value after config change */
> +	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
> +				 data_ready, data_ready != 0,
> +				 wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);

[Severity: High]
Does this readx_poll_timeout() call have an inverted loop condition for
data_ready?

Because the condition is data_ready != 0, it appears the driver will break
early when the device is busy, and spin up to 7.5 seconds when the device
is actually ready.

Since this polling state might be impossible in continuous mode, could this
cause the driver to consistently hit the 7.5-second timeout, completely
breaking data rate and scale configuration when userspace writes to the
in_voltage_scale or in_voltage_sampling_frequency sysfs attributes?

[ ... ]

> @@ -149,22 +199,35 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
>  
>  	ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
>  

[Severity: Medium]
This is a pre-existing issue, but does ads1100_set_scale() suppress I2C write
errors by missing a return value check for ads1100_set_config_bits()?

If I2C bus errors occur during scale configuration (for example, if the bus
is unstable or the device is disconnected), the driver will continue as if
the configuration succeeded.

Could this lead to incorrect data conversion scales being reported to
userspace?

> -	return 0;
> +	return ads1100_poll_data_ready(data);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714194717.596193-1-jakubszczudlo40@gmail.com?part=1