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

[Andy Shevchenko]

On Sun, Jun 28, 2026 at 09:43:39PM +0200, Jakub Szczudlo wrote:
> 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.

...

> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> +	int data_rate_hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];

I would use proper SI units here, id est _Hz.

> +	/* 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);

Not sure if it's less readable than in case of split assignments. Perhaps I
would take that.

> +	bool data_ready;
> +	u8 buffer[3];
> +	int ret;

	unsigned long wait_time_us;
	int data_rate_Hz;
	bool data_ready;
	u8 buffer[3];
	int ret;

	data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];

	/* To be sure we wait 5 times more than data rate */
	wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 5 * data_rate_Hz);


> +	/* To be sure that polled value will have value after config change */
> +	ret = i2c_master_recv(data->client, 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);

Split logically

				 data_ready, data_ready != 0,
				 wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);

> +}

...

>  static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
>  {
>  	unsigned int i;
>  	unsigned int size;
> +	int ret;
>  
>  	size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;

>  	for (i = 0; i < size; i++) {

Hmm... While at it, perhaps

	for (unsigned int i = 0; i < size; i++) {

but it's fine to leave as is.

> -		if (ads1100_data_rate[i] == rate)
> -			return ads1100_set_config_bits(data, ADS1100_DR_MASK,
> -						       FIELD_PREP(ADS1100_DR_MASK, i));
> +		if (i == size)
> +			return -EINVAL;
> +
> +		PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(&data->client->dev, pm);
> +		ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +		if (ret)
> +			return ret;
> +
> +		ret = ads1100_set_config_bits(data, ADS1100_DR_MASK,
> +					      FIELD_PREP(ADS1100_DR_MASK, i));
> +		if (ret)
> +			return ret;
> +
> +		return ads1100_poll_data_ready(data);
>  	}
>  
>  	return -EINVAL;

-- 
With Best Regards,
Andy Shevchenko