Re: [PATCH v4 2/3] iio: adc: ltc2497: add LTC2499 internal temperature channel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The IIO_TEMP channel scale and offset calculations omit the ADC resolution divisor, causing temperature readings to be incorrect by a factor of over 16 million.
--

commit 675eebaaf0b1a3975b7d05ae2a5dc9c63db82edc
Author: Andrei Stancovici <andrei.stancovici@analog.com>

iio: adc: ltc2497: add LTC2499 internal temperature channel
    
The LTC2499 includes an internal temperature sensor.  Expose it as an
IIO_TEMP channel with raw, scale and offset.

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2499fe.pdf

> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> index 2dc5c70442694..6df9c72bd8cf4 100644
> --- a/drivers/iio/adc/ltc2497-core.c
> +++ b/drivers/iio/adc/ltc2497-core.c
[ ... ]
> @@ -95,10 +97,45 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> -		*val = ret / 1000;
> -		*val2 = ddata->chip_info->resolution + 1;
> -
> -		return IIO_VAL_FRACTIONAL_LOG2;
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			/*
> +			 * raw is normalised to 2^(resolution + 1), i.e.
> +			 * raw = 2 * DATAOUT24, so the PTAT scale (datasheet
> +			 * Vref / 1570 per Kelvin) doubles its denominator and,
> +			 * in m°C, becomes Vref_uV / 3140000.
> +			 */
> +			*val = ret;
> +			*val2 = 3140000;
> +			return IIO_VAL_FRACTIONAL;

[Severity: High]
Does this scale calculation in ltc2497core_read_raw() omit the necessary
division by the ADC resolution (2^25)?

Without the resolution divisor, the temperature readings could be incorrect by
a factor of over 16 million when a userspace application reads the sysfs
attributes and applies the standard IIO formula (raw + offset) * scale.

> +		case IIO_VOLTAGE:
> +			*val = ret / (MICRO / MILLI);
> +			*val2 = ddata->chip_info->resolution + 1;
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			ret = regulator_get_voltage(ddata->ref);
> +			if (ret < 0)
> +				return ret;
> +			if (ret == 0)
> +				return -EINVAL;
> +			/*
> +			 * 0 °C == 273.15 K must map to raw + offset such that
> +			 * (raw + offset) * scale == 0 m°C, i.e.
> +			 *   offset = -273150 / scale
> +			 *          = -273150 * 3140000 / Vref_uV
> +			 * Computed in 64-bit to avoid overflow.
> +			 */
> +			*val = div_s64(ABSOLUTE_ZERO_MILLICELSIUS * 3140000LL, ret);

[Severity: High]
Since this offset in ltc2497core_read_raw() is calculated based on the
potentially broken scale from above, will this result in an offset that cannot
correct for the missing resolution divisor?

> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
>  
>  	default:
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909082755.366269-1-andrei.stancovici@analog.com?part=2