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 2 potential issue(s) to consider:

New issues:
- [High] Polling logic is broken due to an inverted termination condition, leading to immediate exits when the device is busy.

Pre-existing issues:
- [High] The device driver reads stale or default power-on data immediately after runtime PM resumes it.
--

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

This commit introduces a polling loop to wait for new data to be ready
when the datarate or gain is changed while the device is in single mode.
It adds ads1100_poll_data_ready() and calls it after configuration changes.

> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> --- 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)
[ ... ]
>  	ret = pm_runtime_resume_and_get(&data->client->dev);
>  	if (ret < 0)
>  		return ret;
>  
>  	ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));

[Severity: High]
This is a pre-existing issue, but does this immediately read a stale or
power-on value (0) instead of a valid measurement after autosuspend?

When pm_runtime_resume_and_get() wakes the device, the ADC is powered on
and placed in continuous mode. However, we immediately execute an I2C
read without waiting for the ADC's first conversion to complete (which can
take up to 125ms at 8 SPS).

>  
>  	pm_runtime_put_autosuspend(&data->client->dev);
>  
>  	if (ret < 0) {
[ ... ]
> +static int ads1100_new_data_is_ready(struct ads1100_data *data)
> +{
> +	u8 buffer[3];
> +	int ret;
> +
> +	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;
> +	}
> +
> +	return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> +}
> +
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
[ ... ]
> +	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]
Is the termination condition for the polling loop inverted?

ads1100_new_data_is_ready() returns the ST/BSY bit, which is 1 when the ADC
is busy and 0 when it is ready.

Since readx_poll_timeout() exits when the condition evaluates to true,
data_ready != 0 will cause the loop to exit immediately while the device is
still busy, bypassing the wait entirely.

> +	if (ret)
> +		return ret;
> +
> +	return data_ready < 0 ? data_ready : 0;
> +}

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