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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] I2C read failures during the data-ready polling loop are mishandled, causing a 7.5-second blocking spin-loop, driver stall, and excessive log spam.
- [Medium] The polling wait interval was incorrectly increased to 5 full conversion periods, injecting severe latency on every configuration change.
--

--- Patch [3]: [PATCH v8 3/3] iio: adc: ti-ads1100: Add ti-ads1110 support to  ti-ads1100 driver ---
commit 5c24a653a5a6dc7cd0b451448a77f86bfdf06445
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>

iio: adc: ti-ads1100: Add ti-ads1110 support to ti-ads1100 driver

Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.

> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 363f790ca5d5e..19533667e6cd8 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]);
>  }
>  
>  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;

[Severity: Medium]
Does this introduce a severe delay during configuration changes?

The previous code waited for half of a conversion period. For example, at 8Hz,
that would be ~62.5ms.

By changing this to 5 full conversion periods, the initial sleep inside
readx_poll_timeout() will block for over 600ms every time the data rate or
scale is updated.

>  	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,

[Severity: High]
Will this loop forever on I2C errors?

If i2c_master_recv() fails inside ads1100_new_data_is_ready(), it returns a
negative error code. Since data_ready == 0 evaluates to false for negative
values, the polling loop won't exit on failure.

This could cause the driver to hang for the entire 7.5-second timeout,
repeatedly reading and spamming the log with error messages.

Should the condition be data_ready <= 0 instead?

>  				 wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);
>  	if (data_ready < 0)
>  		return data_ready;

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