| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <31dea455-f5c4-47a9-9ba8-6a284b12ccfe@baylibre.com>
Date: Thu, 14 Aug 2025 14:19:12 -0500
From: David Lechner <dlechner@...libre.com>
To: Yusuf Alper Bilgin <y.alperbilgin@...il.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Nuno Sá
<nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Liam Beguin <liambeguin@...il.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] iio: adc: ltc2497: add temperature sensor support
On 8/14/25 6:00 AM, Yusuf Alper Bilgin wrote:
> Support for reading the internal temperature sensor on LTC2495 and
> LTC2499 via a standard IIO temperature channel.
>
> Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@...il.com>
> ---
> drivers/iio/adc/ltc2497-core.c | 141 ++++++++++++++++++++++++++++++-----------
> drivers/iio/adc/ltc2497.c | 28 +++++++-
> drivers/iio/adc/ltc2497.h | 15 +++++
> 3 files changed, 145 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> index 2dc5c704426949a4ec62c42591d6c2c40ffb79cc..bbb7948f81933ee35103e37cb2ba94354f61b32e 100644
> --- a/drivers/iio/adc/ltc2497-core.c
> +++ b/drivers/iio/adc/ltc2497-core.c
> @@ -12,6 +12,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/units.h>
>
> #include "ltc2497.h"
>
> @@ -95,10 +96,53 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
> if (ret < 0)
> return ret;
>
> - *val = ret / 1000;
> - *val2 = ddata->chip_info->resolution + 1;
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + *val = ret / 1000;
> + *val2 = ddata->chip_info->resolution + 1;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + case IIO_TEMP:
> + if (!ddata->chip_info->has_temp_channel)
> + return -EINVAL;
> +
> + /*
> + * The datasheet formula to get Temperature in Celsius is:
> + * Temp_C = (DATAOUT * Vref_V / temp_scale) - 273
> + *
> + * To match the IIO framework's model of (raw + offset) * scale,
> + * and to get the final result in millidegrees Celsius:
> + *
> + * Temp_mC = ((DATAOUT * Vref_mV / temp_scale_mV) - 273) * 1000
> + * Temp_mC = (DATAOUT - (273 * temp_scale_mV / Vref_mv)) * 1000 *
> + * Vref_mV / temp_scale_mV
> + *
> + * This gives us:
> + * scale = Vref_mV * 1000 / temp_scale_mV
> + * offset = -273 * temp_scale / Vref_mV
> + */
Probably don't need quite so much comments here. TBH, I though just the
comment in struct ltc2497_chip_info was enough.
> + *val = ret;
> + *val2 = ddata->chip_info->temp_scale_mV;
> +
> + return IIO_VAL_FRACTIONAL;
> +
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_OFFSET:
> + if (chan->type != IIO_TEMP)
> + return -EINVAL;
> +
> + /* see the calculation above. Offset with (-273 * temp_scale / Vref) */
> + ret = regulator_get_voltage(ddata->ref);
> + if (ret < 0)
> + return ret;
>
> - return IIO_VAL_FRACTIONAL_LOG2;
> + *val = kelvin_to_celsius(0) * ddata->chip_info->temp_scale_mV;
> + *val2 = ret / 1000;
> +
> + return IIO_VAL_FRACTIONAL;
>
> default:
> return -EINVAL;
> @@ -126,39 +170,56 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
> .differential = 1, \
> }
>
> +#define LTC2497_TEMPERATURE_CHAN \
> +{ \
> + .type = IIO_TEMP, \
> + .channel = 0, \
No need for channel if indexed == 0.
> + .address = (LTC2497_ENABLE_TEMPERATURE_CONV), \
Since this can't be used the same was as the voltage address
field, not sure it makes sense to set this. Later, we can
just check the channel.type instead.
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET), \
> +}
> +
> +#define LTC2497_VOLTAGE_CHANNEL_LIST \
> + LTC2497_CHAN(0, LTC2497_SGL, "CH0"), \
> + LTC2497_CHAN(1, LTC2497_SGL, "CH1"), \
> + LTC2497_CHAN(2, LTC2497_SGL, "CH2"), \
> + LTC2497_CHAN(3, LTC2497_SGL, "CH3"), \
> + LTC2497_CHAN(4, LTC2497_SGL, "CH4"), \
> + LTC2497_CHAN(5, LTC2497_SGL, "CH5"), \
> + LTC2497_CHAN(6, LTC2497_SGL, "CH6"), \
> + LTC2497_CHAN(7, LTC2497_SGL, "CH7"), \
> + LTC2497_CHAN(8, LTC2497_SGL, "CH8"), \
> + LTC2497_CHAN(9, LTC2497_SGL, "CH9"), \
> + LTC2497_CHAN(10, LTC2497_SGL, "CH10"), \
> + LTC2497_CHAN(11, LTC2497_SGL, "CH11"), \
> + LTC2497_CHAN(12, LTC2497_SGL, "CH12"), \
> + LTC2497_CHAN(13, LTC2497_SGL, "CH13"), \
> + LTC2497_CHAN(14, LTC2497_SGL, "CH14"), \
> + LTC2497_CHAN(15, LTC2497_SGL, "CH15"), \
> + LTC2497_CHAN_DIFF(0, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(1, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(2, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(3, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(4, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(5, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(6, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(7, LTC2497_DIFF), \
> + LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN), \
> + LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN)
> +
> static const struct iio_chan_spec ltc2497core_channel[] = {
> - LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
> - LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
> - LTC2497_CHAN(2, LTC2497_SGL, "CH2"),
> - LTC2497_CHAN(3, LTC2497_SGL, "CH3"),
> - LTC2497_CHAN(4, LTC2497_SGL, "CH4"),
> - LTC2497_CHAN(5, LTC2497_SGL, "CH5"),
> - LTC2497_CHAN(6, LTC2497_SGL, "CH6"),
> - LTC2497_CHAN(7, LTC2497_SGL, "CH7"),
> - LTC2497_CHAN(8, LTC2497_SGL, "CH8"),
> - LTC2497_CHAN(9, LTC2497_SGL, "CH9"),
> - LTC2497_CHAN(10, LTC2497_SGL, "CH10"),
> - LTC2497_CHAN(11, LTC2497_SGL, "CH11"),
> - LTC2497_CHAN(12, LTC2497_SGL, "CH12"),
> - LTC2497_CHAN(13, LTC2497_SGL, "CH13"),
> - LTC2497_CHAN(14, LTC2497_SGL, "CH14"),
> - LTC2497_CHAN(15, LTC2497_SGL, "CH15"),
> - LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
> - LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
> - LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
> + LTC2497_VOLTAGE_CHANNEL_LIST,
> +};
> +
> +static const struct iio_chan_spec ltc2497core_channel_with_temperature[] = {
> + LTC2497_VOLTAGE_CHANNEL_LIST,
> + LTC2497_TEMPERATURE_CHAN,
> };
>
> static const struct iio_info ltc2497core_info = {
> @@ -182,8 +243,14 @@ int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev)
>
> indio_dev->info = <c2497core_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
> - indio_dev->channels = ltc2497core_channel;
> - indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel);
> +
> + if (ddata->chip_info->has_temp_channel) {
> + indio_dev->channels = ltc2497core_channel_with_temperature;
> + indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel_with_temperature);
> + } else {
> + indio_dev->channels = ltc2497core_channel;
> + indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel);
> + }
>
> ret = ddata->result_and_measure(ddata, LTC2497_CONFIG_DEFAULT, NULL);
> if (ret < 0)
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index 8f4665547b5b0d32084599f8557c40102c37a4ce..07fced79aeead3778964b114d479fdcb643c16df 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -86,8 +86,28 @@ static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
> return 0;
> }
>
> - ret = i2c_smbus_write_byte(st->client,
> - LTC2497_ENABLE | address);
> + /*
> + * Chips with temperature sensor support (e.g., LTC2495/LTC2499)
> + * require a two-byte command format to select any channel.
> + *
> + * To read the internal temperature, LTC2497_ENABLE_TEMPERATURE_CONV
> + * is sent as the second byte. To read a voltage channel, LTC2497_EN2
> + * is sent, which sets the default configuration: simultaneous 50/60Hz
> + * rejection, 1x speed, and gain=1.
> + *
> + * Chips without this feature use a standard single-byte command.
> + */
> + if (ddata->chip_info->has_temp_channel) {
> + if (address == LTC2497_ENABLE_TEMPERATURE_CONV)
Here we can just check the channel type.
> + ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE,
> + LTC2497_ENABLE_TEMPERATURE_CONV);
And use LTC2497_EN2 | LTC2497_IM here.
> + else
> + ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE | address,
> + LTC2497_EN2);
> + } else {
> + ret = i2c_smbus_write_byte(st->client, LTC2497_ENABLE | address);
> + }
> +
> if (ret)
> dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
> ERR_PTR(ret));
> @@ -135,6 +155,8 @@ static const struct ltc2497_chip_info ltc2497_info[] = {
> [TYPE_LTC2495] = {
> .resolution = 16,
> .name = "ltc2495",
> + .has_temp_channel = true,
> + .temp_scale_mV = 12250,
> },
> [TYPE_LTC2497] = {
> .resolution = 16,
> @@ -143,6 +165,8 @@ static const struct ltc2497_chip_info ltc2497_info[] = {
> [TYPE_LTC2499] = {
> .resolution = 24,
> .name = "ltc2499",
> + .has_temp_channel = true,
> + .temp_scale_mV = 1570000,
> },
> };
>
> diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
> index 64e81c95a3dd05911b6717c09ac0560c9f47f304..65f406bc61c24b912de4beed604a074b3ea9df91 100644
> --- a/drivers/iio/adc/ltc2497.h
> +++ b/drivers/iio/adc/ltc2497.h
> @@ -3,10 +3,25 @@
> #define LTC2497_ENABLE 0xA0
> #define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE
> #define LTC2497_CONVERSION_TIME_MS 150ULL
> +#define LTC2497_EN2 BIT(7)
> +/* Enable the internal temperature sensor */
> +#define LTC2497_IM BIT(6)
These should go directly above:
#define LTC2497_SGL BIT(4)
in the .c file. No need to have them in the header.
> +/* Second command byte value to initiate a temperature conversion */
> +#define LTC2497_ENABLE_TEMPERATURE_CONV (LTC2497_EN2 | LTC2497_IM)
This macro seems odd. We usually don't make macros for combined bits
like this. And the way it is being used in the code feels a bit odd
as well. We can probably rework the code to not need it as suggested
in some of the comments above.
>
> struct ltc2497_chip_info {
> u32 resolution;
> const char *name;
> + /*
> + * Represents the datasheet constant from the temperature formula:
> + * T_Kelvin = (DATAOUT * Vref) / temp_scale, where Vref is in Volts.
> + *
> + * To allow the driver to use Vref in millivolts for the calculation
> + * and also to avoid floating points, this stored value represents the
> + * datasheet constant scaled by 1000.
> + */
> + u32 temp_scale_mV;
> + bool has_temp_channel;
> };
>
> struct ltc2497core_driverdata {
>
Powered by blists - more mailing lists