Re: [PATCH v6 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced polling mechanism in `ads1100_poll_data_ready` uses an inverted wait condition that guarantees failure, silently swallows I2C errors, and incorrectly relies on a status bit that is always zero in continuous mode.
--

--- Patch [1]: [PATCH v6 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode ---
commit 9efe89d3c5acb6ee87bd34da568f0f0c50cd6e20
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>

iio: adc: 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 actual wait for new measurement, so to
be sure that read after change is correct functions that changes
datarate and gain will wait for new data.

> 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

[ ... ]

> +static bool ads1100_new_data_not_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 true;

[Severity: High]
If an I2C read error occurs, this returns true. Since the polling condition
below checks for non-zero, won't this cause the poll to exit immediately and
silently treat the hardware failure as a successful wait?

> +	}
> +
> +	return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> +}
> +
> +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);
> +	bool 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;
> +	}
> +
> +	return readx_poll_timeout(ads1100_new_data_not_ready, data,
> +				 data_ready, data_ready != 0,
> +				 wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);

[Severity: High]
Is this condition inverted? The readx_poll_timeout macro stops polling when
the condition evaluates to true. With data_ready != 0, it stops immediately
if the device is busy, and spins until timeout if the device is ready.

Additionally, because acquiring PM runtime resumes the device into continuous
mode, the ST/BSY bit is always 0. Does this mean every write to the scale or
data rate attributes will hang for 7.5 seconds and return -ETIMEDOUT?

> +}

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