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

[Jonathan Cameron]

On Wed, 2 Sep 2026 09:44:11 +0300
Andrei Stancovici <andrei.stancovici@xxxxxxxxxx> wrote:

> The LTC2499 has an internal PTAT (proportional to absolute temperature)
> sensor that is activated by a second I2C configuration byte (EN2 | IM).
> Expose it as an IIO_TEMP channel providing raw, scale and offset so the
> standard IIO formula
> 
> 	T[m°C] = (raw + offset) * scale
> 
> reconstructs the temperature.

Andy raised the point about too much description on the next patch and
it definitely applies here as well. Think about what matters to those
reading the patch. E.g. something like.

"The LTC2499 includes a temperatures sensor. Expose the raw reading and
appropriate scale and offset."


> 
> The PTAT sensor yields the absolute temperature as
> 
> 	T(K) = DATAOUT24 * Vref / 1570		(Vref in volts)
> 
> The raw value exported here is sign-extended and normalised to
> 2^(resolution + 1) == 2^25, i.e. raw = 2 * DATAOUT24, so on the IIO
> milli-degree-Celsius convention
> 
> 	scale[m°C/LSB] = Vref_uV / 3140000
> 	offset         = -273150 * 3140000 / Vref_uV

If this stuff belongs anywhere it is as comments where the maths is
in the code. You have it it there, no point in repeating here.

> 
> The scale and offset are derived from the reference voltage returned by
> regulator_get_voltage(); its error is propagated as before, so a board
> that fails to describe vref-supply gets a clear read error instead of a
> silently wrong temperature. No board-specific reference value is assumed
> in the driver.
This bit is useful but focus on what matters a little more. Something like
"As scale and offset depend on the reference voltage, if no regulator
 is specified by firmware, reading them will return an error."
I'd talk about why as well. e.g. "As this is a new feature simply failing
probe is not a option."
> 
> The single temperature channel is appended as the last entry of the
> shared channel array and excluded via num_channels for parts without an
> internal sensor, so the existing LTC2497 channel layout and device name
> are unchanged.

Implementation detail this description shouldn't mention.

> 
> The LTC2499 latches its converter configuration from the second command
> byte and only re-evaluates it when that byte has EN2 set. EN2 | IM
> selects the internal temperature sensor. Because a single-byte command,
> or a second byte with EN2 = 0, means "keep previous", a one-byte channel
> select cannot pull the device back out of temperature mode: after a
> temperature read every subsequent voltage read would keep returning the
> PTAT result instead of the selected input. Temperature support is
> therefore only correct if the voltage path also emits a second command
> byte that re-selects an external input.

What does this mean to the user?  

> 
> Send two-byte commands for all conversions on parts that have the sensor
> (has_temp):
> 
> 	temperature:	EN2 | IM
> 	voltage:	EN2			(IM = 0 -> external input)
> 
> The LTC2497 and LTC2496, which lack the second-byte mechanism, keep using
> the original single-byte channel select and are unchanged.

I'd not bother talking about what happens for those that are unaffected
(given it is the obvious answer of nothing!)

> 
> Signed-off-by: Andrei Stancovici <andrei.stancovici@xxxxxxxxxx>
> ---
>  drivers/iio/adc/ltc2497-core.c | 58 +++++++++++++++++++++++++++++++---
>  drivers/iio/adc/ltc2497.c      | 33 +++++++++++++++++++
>  drivers/iio/adc/ltc2497.h      | 12 +++++++
>  3 files changed, 99 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> index 2dc5c7044269..6df9c72bd8cf 100644
> --- a/drivers/iio/adc/ltc2497-core.c
> +++ b/drivers/iio/adc/ltc2497-core.c
> @@ -9,9 +9,11 @@

...

> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index c1668b5a351e..79c87bac05e1 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -84,6 +84,38 @@ static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
>  			return 0;
>  	}
>  
> +	/*
> +	 * Parts with the internal PTAT sensor (LTC2499) latch their converter
> +	 * configuration via a second command byte and only re-evaluate it when
> +	 * that byte has EN2 set; a single byte, or a second byte with EN2 = 0,
> +	 * means "keep previous".  A one-byte channel select therefore cannot pull
> +	 * the device back out of temperature mode, so a voltage read after a
> +	 * temperature read would keep returning the PTAT result.  Always drive the
> +	 * second byte with EN2 set on these parts: IM = 1 for a temperature read,
> +	 * EN2 alone (IM = 0) to (re)select an external input.  FA = FB = 0 keeps
> +	 * the power-on simultaneous 50/60Hz rejection, whose worst-case
> +	 * conversion time the driver's wait already covers.

Say why it is 'always' done.  Seems you could do it only when the configuration
needs to change.  Thus if someone keeps reading a particular channel don't send
the second byte?  Fine to just say it is for simplicity.

> +	 */
> +	if (ddata->chip_info->has_temp) {
> +		u8 cmd[2];
> +
> +		if (address == LTC2497_TEMP_ADDR) {
> +			cmd[0] = LTC2497_ENABLE | LTC2497_CONFIG_DEFAULT;
> +			cmd[1] = LTC2499_EN2 | LTC2499_IM;
> +		} else {
> +			cmd[0] = LTC2497_ENABLE | address;
> +			cmd[1] = LTC2499_EN2;
> +		}
> +
> +		ret = i2c_master_send(st->client, cmd, sizeof(cmd));
> +		if (ret < 0) {
> +			dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
> +				ERR_PTR(ret));
> +			return ret;
> +		}
> +		return 0;
> +	}
> +