Re: [PATCH v7 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] Inverted polling condition in ads1100_poll_data_ready causes 7.5s hangs and missed data.
- [Medium] Unhandled negative error code from ads1100_get_vref_millivolts() leaks to userspace.
--

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

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

This commit adds support for the ADS1110 ADC. It provides faster data rates
than the ADS1100 and uses an internal voltage reference of 2.048V.

> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index f0a30ae139afa..5d798bfcc74e1 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -144,7 +182,8 @@ static int ads1100_new_data_is_ready(struct ads1100_data *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)];
> +	int data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
> +	int data_rate_Hz = data->ads_config->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, 5 * data_rate_Hz);
>  	int data_ready;

[Severity: High]
Does the inverted break condition in ads1100_poll_data_ready() cause the
polling loop to hang when data is ready?

Further down in this function, the readx_poll_timeout() call evaluates
the data_ready variable:

    ret = readx_poll_timeout(ads1100_new_data_is_ready, data,
                             data_ready, data_ready != 0,
                             wait_time_us, ADS1100_MAX_DRDY_TIMEOUT_US);

It looks like ads1100_new_data_is_ready() returns 0 when ready, 1 when busy,
and a negative code on I2C error.

Since readx_poll_timeout() stops looping when its condition evaluates to true,
data_ready != 0 would cause it to silently exit on busy or error states, but
hang and loop until the 7.5s timeout when data is actually ready.

[ ... ]
> @@ -230,14 +269,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
>  	return ads1100_poll_data_ready(data);
>  }
>  
> -static int ads1100_get_vdd_millivolts(struct ads1100_data *data)
> -{
> -	return regulator_get_voltage(data->reg_vdd) / (MICRO / MILLI);
> -}
> -
>  static void ads1100_calc_scale_avail(struct ads1100_data *data)
>  {
> -	int millivolts = ads1100_get_vdd_millivolts(data);
> +	int millivolts = ads1100_get_vref_millivolts(data);

[Severity: Medium]
If regulator_get_voltage() fails inside ads1100_get_vref_millivolts(),
will this permanently cache a bogus negative error code (like -EINVAL) in
the scale_avail array?

>  	unsigned int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
[ ... ]
> @@ -296,12 +331,12 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		/* full-scale is the supply voltage in millivolts */
> -		*val = ads1100_get_vdd_millivolts(data);
> +		*val = ads1100_get_vref_millivolts(data);

[Severity: Medium]
Can an unhandled negative error code from ads1100_get_vref_millivolts() leak
to userspace here?

If the VDD regulator driver fails to return its voltage, *val will contain
the negative error code. Because the function returns IIO_VAL_FRACTIONAL_LOG2
instead of the error, this masks the failure from the IIO core.

>  		*val2 = 15 + FIELD_GET(ADS1100_PGA_MASK, data->config);
>  		return IIO_VAL_FRACTIONAL_LOG2;

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