Re: [PATCH v8 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Fri, 17 Jul 2026 20:50:33 +0200
Jakub Szczudlo <jakubszczudlo40@gmail.com> wrote:

> Add ADS1110 support that have faster datarate than ADS1100, it also uses
> internal voltage reference of 2.048V for measurement.
> 
> Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Hi Jakub,

Seems some stuff in here should be in patch 1.

Thanks

Jonathan


> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 363f790ca5d5..19533667e6cd 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c

...


> @@ -139,14 +177,16 @@ static int ads1100_new_data_is_ready(struct ads1100_data *data)
>  		return ret;
>  	}
>  
> -	return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]) ? 0 : 1;
> +	return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);

This change and the use below seem odd to be in this patch. I guess
they should be back in patch 1.

>  }
>  
>  static int ads1100_poll_data_ready(struct ads1100_data *data)
>  {
> -	int data_rate_Hz = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> +	int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
> +	int data_rate_Hz = data->chip_info->available_data_rate_hz[data_rate_index];
>  	/* To be sure we wait 5 times more than data rate */
> -	unsigned long wait_time_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, 2 * data_rate_Hz);
> +	unsigned long period_us = DIV_ROUND_CLOSEST(USEC_PER_SEC, data_rate_Hz);
> +	unsigned long wait_time_us = 5UL * period_us;
>  	int data_ready;
>  	u8 buffer[3];
>  	int ret;
> @@ -159,7 +199,7 @@ 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,
> +				 data_ready, data_ready == 0,
see above. Seems to be in wrong patch.
>  				 wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);

Sashiko points out that the polling period is very long.  Was this meant to be period_us?

>  	if (data_ready < 0)
>  		return data_ready;