Thread (9 messages) flat view 9 messages, 4 authors, 11d ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-09 08:39:50

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 [off-list ref]

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
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help