[PATCH v4 0/3] Add LTC2495 support

[PATCH v4 0/3] Add LTC2495 support

[Yusuf Alper Bilgin]

Hi All,

This is the v4 of the patch series to add support for the LTC2495 ADC
and to enable the internal temperature channel for the LTC2495 and
LTC2499.

Thanks to Andy Shevchenko for his helpful reviews on earlier versions,
and to David Lechner and Krzysztof Kozlowski for their feedbacks on
v3. This version addresses all feedback from v3.

Best Regards,

Alper

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
Changes in v4:
  - Removed the verbose formula derivation comment from `read_raw`.
  - Reworked the temperature channel definition to omit the redundant
    `.address` and `.channel` fields.
  - Moved I2C-specific bit definitions from the shared header into
    `ltc2497.c` and removed the pre-combined command macro.
  - Simplified I2C logic to check the channel type instead of a
    non-standard address.
  - Combined the basic device support (#2) and temperature sensor feature (#3)
    patches into a single patch (#2).
  - Link to v3: https://lore.kernel.org/r/20250814-ltc2495-v3-0-c2a6cecd6b99@gmail.com

Changes in v3:
  - Used the standard `kelvin_to_celsius()` helper instead of a custom
    define.
  - Corrected macro definition style.
  - Renamed `LTC2497_CHANNELS` and `LTC_T_CHAN` for clarity.
  - Combined all struct layout optimizations into a single patch.
  - Link to v2: https://lore.kernel.org/r/20250813-ltc2495-v2-0-bbaf20f6ba07@gmail.com

Changes in v2:
  - Rewrote all commit messages to use the imperative mood.
  - Added a justification for the new compatible string to the device
    tree binding commit message.
  - Removed all unrelated whitespace and formatting changes.
  - Removed redundant explicit `false` initializers from structs.
  - Replaced the magic number for Kelvin conversion with a define.
  - Improved comments for defines and temperature scaling constants.
  - Renamed confusing macros and struct fields to be more descriptive.
  - Replaced dynamic channel allocation with a static array approach
    using a shared macro to improve readability.
  - Optimized data structure layouts based on pahole output to remove
    memory holes.
  - Link to v1: https://lore.kernel.org/r/20250812-ltc2495-v1-0-7bf4c6feec2e@gmail.com
---
Implementation Notes
 - checkpatch warning: The new static array approach uses a shared macro
   for the common channels (`LTC2497_CHANNELS`), which triggers a
   checkpatch.pl warning: "Macros with complex values should be enclosed
   in parentheses". However, this will cause a compilitaion error, as an
   initializer list cannot be parenthesized.
 - uV to mV conversion: I could not find a standard macro, so a manual
   division is used for now. This could be a point of future
   improvement.

---
Yusuf Alper Bilgin (3):
      dt-bindings: iio: adc: ltc2497: add lltc,ltc2495 bindings
      iio: adc: ltc2497: add support for LTC2495
      iio: adc: ltc2497: reorder struct members to fix memory holes

 .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml  |   3 +
 drivers/iio/adc/ltc2497-core.c                     | 132 ++++++++++++++-------
 drivers/iio/adc/ltc2497.c                          |  39 +++++-
 drivers/iio/adc/ltc2497.h                          |  19 ++-
 4 files changed, 147 insertions(+), 46 deletions(-)
---
base-commit: acbbb5a20971089064ca6b271dd251e629be8d4d
change-id: 20250811-ltc2495-572817c13fd3

Best regards,
-- 
Yusuf Alper Bilgin <y.alperbilgin@gmail.com>

[PATCH v4 1/3] dt-bindings: iio: adc: ltc2497: add lltc,ltc2495 bindings

[Yusuf Alper Bilgin]

Update the binding documentation for LTC2497 to include LTC2495 to
enable support for its internal temperature sensor, which requires a
different I2C command format and a new IIO channel.

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
 Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
index 5cc6a96840778473895f436b7e2627d6240b254b..2a3e3dcc6ca7a48a0fccb88d8d42fee34efcff73 100644
--- a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
@@ -17,11 +17,13 @@ description: |
 
     https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
 
+  LTC2495:
   LTC2497:
   LTC2499:
     16bit ADC supporting up to 16 single ended or 8 differential inputs.
     I2C interface.
 
+    https://www.analog.com/media/en/technical-documentation/data-sheets/2495fe.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/2497fb.pdf
     https://www.analog.com/media/en/technical-documentation/data-sheets/2499fe.pdf
 
@@ -29,6 +31,7 @@ properties:
   compatible:
     enum:
       - lltc,ltc2309
+      - lltc,ltc2495
       - lltc,ltc2497
       - lltc,ltc2499
 

-- 
2.43.0

Re: [PATCH v4 1/3] dt-bindings: iio: adc: ltc2497: add lltc,ltc2495 bindings

[Jonathan Cameron]

On Fri, 15 Aug 2025 12:02:02 +0200
Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:

> Update the binding documentation for LTC2497 to include LTC2495 to
> enable support for its internal temperature sensor, which requires a
> different I2C command format and a new IIO channel.
> 
> Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
See reply to v1.  

At least at first look it appears this should have a fallback compatible
as Krzysztof called out.

> ---
>  Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> index 5cc6a96840778473895f436b7e2627d6240b254b..2a3e3dcc6ca7a48a0fccb88d8d42fee34efcff73 100644
> --- a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> @@ -17,11 +17,13 @@ description: |
>  
>      https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
>  
> +  LTC2495:
>    LTC2497:
>    LTC2499:
>      16bit ADC supporting up to 16 single ended or 8 differential inputs.
>      I2C interface.
>  
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/2495fe.pdf
>      https://www.analog.com/media/en/technical-documentation/data-sheets/2497fb.pdf
>      https://www.analog.com/media/en/technical-documentation/data-sheets/2499fe.pdf
>  
> @@ -29,6 +31,7 @@ properties:
>    compatible:
>      enum:
>        - lltc,ltc2309
> +      - lltc,ltc2495
>        - lltc,ltc2497
>        - lltc,ltc2499
>  
> 

[PATCH v4 2/3] iio: adc: ltc2497: add support for LTC2495

[Yusuf Alper Bilgin]

Update the LTC2497 driver to also support the LTC2495.

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@gmail.com>
---
 drivers/iio/adc/ltc2497-core.c | 132 ++++++++++++++++++++++++++++-------------
 drivers/iio/adc/ltc2497.c      |  39 +++++++++++-
 drivers/iio/adc/ltc2497.h      |  11 ++++
 3 files changed, 140 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
index 2dc5c704426949a4ec62c42591d6c2c40ffb79cc..9f3509c5974dd7a0489145c591630e1ff9dc333e 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"
 
@@ -46,7 +47,8 @@ static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata)
 	return 1;
 }
 
-static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, int *val)
+static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address,
+			    enum iio_chan_type type, int *val)
 {
 	int ret;
 
@@ -54,11 +56,13 @@ static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, in
 	if (ret < 0)
 		return ret;
 
-	if (ret || ddata->addr_prev != address) {
+	if (ret || ddata->addr_prev != address ||
+	    ddata->chan_type_prev != type) {
 		ret = ddata->result_and_measure(ddata, address, NULL);
 		if (ret < 0)
 			return ret;
 		ddata->addr_prev = address;
+		ddata->chan_type_prev = type;
 
 		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
 			return -ERESTARTSYS;
@@ -83,7 +87,7 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&ddata->lock);
-		ret = ltc2497core_read(ddata, chan->address, val);
+		ret = ltc2497core_read(ddata, chan->address, chan->type, val);
 		mutex_unlock(&ddata->lock);
 		if (ret < 0)
 			return ret;
@@ -95,10 +99,37 @@ 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;
+			return IIO_VAL_FRACTIONAL_LOG2;
+
+		case IIO_TEMP:
+			if (!ddata->chip_info->has_temp_channel)
+				return -EINVAL;
+
+			*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;
+
+		ret = regulator_get_voltage(ddata->ref);
+		if (ret < 0)
+			return ret;
+
+		*val = kelvin_to_celsius(0) * ddata->chip_info->temp_scale_mV;
+		*val2 = ret / 1000;
+
+		return IIO_VAL_FRACTIONAL;
 
 	default:
 		return -EINVAL;
@@ -126,39 +157,54 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
 	.differential = 1, \
 }
 
+#define LTC2497_TEMPERATURE_CHAN \
+{												\
+	.type = IIO_TEMP,									\
+	.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 +228,14 @@ int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev)
 
 	indio_dev->info = &ltc2497core_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 eb9d521e86e54def0493ea0e81f63b37900c56a5..cd3d00a8ffcb44107c9ad948913c97db9412b5f5 100644
--- a/drivers/iio/adc/ltc2497.c
+++ b/drivers/iio/adc/ltc2497.c
@@ -18,7 +18,12 @@
 
 #include "ltc2497.h"
 
+/* Optional second byte fields for LTC2495 and LTC2499 */
+#define LTC2497_EN2	BIT(7)
+#define LTC2497_IM	BIT(6)
+
 enum ltc2497_chip_type {
+	TYPE_LTC2495,
 	TYPE_LTC2497,
 	TYPE_LTC2499,
 };
@@ -85,8 +90,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_EN2 and LTC2497_IM
+	 * are 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 (ddata->chan_type_prev == IIO_TEMP)
+			ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE,
+							LTC2497_EN2 | LTC2497_IM);
+		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));
@@ -131,6 +156,12 @@ static void ltc2497_remove(struct i2c_client *client)
 }
 
 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,
 		.name = NULL,
@@ -138,10 +169,13 @@ static const struct ltc2497_chip_info ltc2497_info[] = {
 	[TYPE_LTC2499] = {
 		.resolution = 24,
 		.name = "ltc2499",
+		.has_temp_channel = true,
+		.temp_scale_mV = 1570000,
 	},
 };
 
 static const struct i2c_device_id ltc2497_id[] = {
+	{ "ltc2495", (kernel_ulong_t)&ltc2497_info[TYPE_LTC2495] },
 	{ "ltc2497", (kernel_ulong_t)&ltc2497_info[TYPE_LTC2497] },
 	{ "ltc2499", (kernel_ulong_t)&ltc2497_info[TYPE_LTC2499] },
 	{ }
@@ -149,6 +183,7 @@ static const struct i2c_device_id ltc2497_id[] = {
 MODULE_DEVICE_TABLE(i2c, ltc2497_id);
 
 static const struct of_device_id ltc2497_of_match[] = {
+	{ .compatible = "lltc,ltc2495", .data = &ltc2497_info[TYPE_LTC2495] },
 	{ .compatible = "lltc,ltc2497", .data = &ltc2497_info[TYPE_LTC2497] },
 	{ .compatible = "lltc,ltc2499", .data = &ltc2497_info[TYPE_LTC2499] },
 	{ }
diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
index 64e81c95a3dd05911b6717c09ac0560c9f47f304..dfe2d5c30017adeb3f17e57fc5bf1e0e792ff30f 100644
--- a/drivers/iio/adc/ltc2497.h
+++ b/drivers/iio/adc/ltc2497.h
@@ -7,6 +7,16 @@
 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 {
@@ -18,6 +28,7 @@ struct ltc2497core_driverdata {
 	u8 addr_prev;
 	int (*result_and_measure)(struct ltc2497core_driverdata *ddata,
 				  u8 address, int *val);
+	enum iio_chan_type chan_type_prev;
 };
 
 int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev);

-- 
2.43.0

Re: [PATCH v4 2/3] iio: adc: ltc2497: add support for LTC2495

[Jonathan Cameron]

On Fri, 15 Aug 2025 12:02:03 +0200
Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:

> Update the LTC2497 driver to also support the LTC2495.
> 
> 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@gmail.com>

Hi Alper,

Various comments inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ltc2497-core.c | 132 ++++++++++++++++++++++++++++-------------
>  drivers/iio/adc/ltc2497.c      |  39 +++++++++++-
>  drivers/iio/adc/ltc2497.h      |  11 ++++
>  3 files changed, 140 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> index 2dc5c704426949a4ec62c42591d6c2c40ffb79cc..9f3509c5974dd7a0489145c591630e1ff9dc333e 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>
I'm not seeing this used in this patch.  If it is used in driver already and the include
was missing, add it as a separate patch.

> @@ -95,10 +99,37 @@ 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;
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +
> +		case IIO_TEMP:
> +			if (!ddata->chip_info->has_temp_channel)
> +				return -EINVAL;
> +
> +			*val = ret;
> +			*val2 = ddata->chip_info->temp_scale_mV;

Uou are outputting via _RAW but doing part of the scaling in here.
Why not figure out the full scale with the vref part and output
that as _scale? It's linear so that should be easy to do.

> +
> +			return IIO_VAL_FRACTIONAL;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (chan->type != IIO_TEMP)
> +			return -EINVAL;
> +
> +		ret = regulator_get_voltage(ddata->ref);

Reference regulators very rarely change in practice (once enabled) so it's
common option to just read them in probe as then can use the devm_ calls that
exist to turn them on and read the voltage and only keep a visible copy of
the voltage.

It might be worth a precursor patch to do that for existing scaling measurement.
 
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = kelvin_to_celsius(0) * ddata->chip_info->temp_scale_mV;
> +		*val2 = ret / 1000;
> +
> +		return IIO_VAL_FRACTIONAL;
>  
>  	default:
>  		return -EINVAL;
> @@ -126,39 +157,54 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
>  	.differential = 1, \
>  }

>  static const struct iio_info ltc2497core_info = {
> @@ -182,8 +228,14 @@ int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev)
>  
>  	indio_dev->info = &ltc2497core_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 eb9d521e86e54def0493ea0e81f63b37900c56a5..cd3d00a8ffcb44107c9ad948913c97db9412b5f5 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -18,7 +18,12 @@
>  
>  #include "ltc2497.h"
>  
> +/* Optional second byte fields for LTC2495 and LTC2499 */
> +#define LTC2497_EN2	BIT(7)
If they only exist on parts other than LTC2497, name the define

after one of those rather than the general driver prefix.
LTC2499_EN2 probably

> +#define LTC2497_IM	BIT(6)
> +
>  enum ltc2497_chip_type {
> +	TYPE_LTC2495,
>  	TYPE_LTC2497,
>  	TYPE_LTC2499,
>  };
> @@ -85,8 +90,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)
Given one of these parts was supported prior to this patch I would suggest some reorganization.

Patch 1.  Add this temperature channel support for the LTC2499
Patch 2.  Binding for the LTC2495
Patch 3.  Device support for the LTC2945 (which should just be a few lines).

Alternative would be

Patch 1. Binding with fallback compatible so it 'works'.
Patch 2. Basic matching support for new part.
Patch 3. Temperature support for 9 and 5 parts.

but to me that is a little less elegant.


> +	 * require a two-byte command format to select any channel.
> +	 *
> +	 * To read the internal temperature, LTC2497_EN2 and LTC2497_IM
> +	 * are 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) {

Given your description there is other stuff in byte 2.
Maybe better to gate this on something called has_byte_2

If there are devices with byte 2 and no temp sensor, then two flags probably needed.

> +		if (ddata->chan_type_prev == IIO_TEMP)
> +			ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE,
> +							LTC2497_EN2 | LTC2497_IM);
> +		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));

> diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
> index 64e81c95a3dd05911b6717c09ac0560c9f47f304..dfe2d5c30017adeb3f17e57fc5bf1e0e792ff30f 100644
> --- a/drivers/iio/adc/ltc2497.h
> +++ b/drivers/iio/adc/ltc2497.h
> @@ -7,6 +7,16 @@
>  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;
You've lost me on the untis here. If this temp_scale in the formula then a
bit of dimensional analysis says it's units must be derived as

	kevlin = (1 * volts) / temp_scale.
	
	tempscale = volts/ kelvin.

So I think it is probably mV_per_kelvin

I might have the logic wrong there but it's definitely not in mV.

Jonathan





> +	bool has_temp_channel;
>  };

[PATCH v4 3/3] iio: adc: ltc2497: reorder struct members to fix memory holes

[Yusuf Alper Bilgin]

Reorder members in the `ltc2497_chip_info` and `ltc2497core_driverdata`
structs to eliminate memory holes identified by the `pahole` tool.

Confirm via the `bloat-o-meter` that this change has no significant
impact on the final code size:

| Object File     | Total Size Change |
|-----------------|-------------------|
| ltc2497-core.o  | 0 (0.00%)         |
| ltc2497.o       | +2 (+0.10%)       |
| ltc2496.o       | 0 (0.00%)         |

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
 drivers/iio/adc/ltc2497.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
index dfe2d5c30017adeb3f17e57fc5bf1e0e792ff30f..48e9f74870ab489b5df6e69a39446610c6a72b93 100644
--- a/drivers/iio/adc/ltc2497.h
+++ b/drivers/iio/adc/ltc2497.h
@@ -5,8 +5,8 @@
 #define LTC2497_CONVERSION_TIME_MS	150ULL
 
 struct ltc2497_chip_info {
-	u32 resolution;
 	const char *name;
+	u32 resolution;
 	/*
 	 * Represents the datasheet constant from the temperature formula:
 	 * T_Kelvin = (DATAOUT * Vref) / temp_scale, where Vref is in Volts.
@@ -20,15 +20,15 @@ struct ltc2497_chip_info {
 };
 
 struct ltc2497core_driverdata {
-	struct regulator *ref;
-	ktime_t	time_prev;
 	/* lock to protect against multiple access to the device */
 	struct mutex lock;
+	struct regulator *ref;
+	ktime_t	time_prev;
 	const struct ltc2497_chip_info	*chip_info;
-	u8 addr_prev;
 	int (*result_and_measure)(struct ltc2497core_driverdata *ddata,
 				  u8 address, int *val);
 	enum iio_chan_type chan_type_prev;
+	u8 addr_prev;
 };
 
 int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev);

-- 
2.43.0

Re: [PATCH v4 3/3] iio: adc: ltc2497: reorder struct members to fix memory holes

[Jonathan Cameron]

On Fri, 15 Aug 2025 12:02:04 +0200
Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:

> Reorder members in the `ltc2497_chip_info` and `ltc2497core_driverdata`
> structs to eliminate memory holes identified by the `pahole` tool.
> 
> Confirm via the `bloat-o-meter` that this change has no significant
> impact on the final code size:
> 
> | Object File     | Total Size Change |
> |-----------------|-------------------|
> | ltc2497-core.o  | 0 (0.00%)         |
> | ltc2497.o       | +2 (+0.10%)       |
> | ltc2496.o       | 0 (0.00%)         |
> 
> Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>

whilst I know Andy is a fan of this stuff, I'm not convinced it is worth
the churn in this particular case. 

The driverdata is allocated via iio_priv() so has a bunch of additional
alignment rules applied and is on the end of another larger allocation.
I suspect that completely hides the advantages in closing the holes up.

The chip_info one is a bit more convincing as that's static const stuff and
maybe it ends up packing a little better.

Anyhow, let us see what Andy thinks.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ltc2497.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
> index dfe2d5c30017adeb3f17e57fc5bf1e0e792ff30f..48e9f74870ab489b5df6e69a39446610c6a72b93 100644
> --- a/drivers/iio/adc/ltc2497.h
> +++ b/drivers/iio/adc/ltc2497.h
> @@ -5,8 +5,8 @@
>  #define LTC2497_CONVERSION_TIME_MS	150ULL
>  
>  struct ltc2497_chip_info {
> -	u32 resolution;
>  	const char *name;
> +	u32 resolution;
>  	/*
>  	 * Represents the datasheet constant from the temperature formula:
>  	 * T_Kelvin = (DATAOUT * Vref) / temp_scale, where Vref is in Volts.
> @@ -20,15 +20,15 @@ struct ltc2497_chip_info {
>  };
>  
>  struct ltc2497core_driverdata {
> -	struct regulator *ref;
> -	ktime_t	time_prev;
>  	/* lock to protect against multiple access to the device */
>  	struct mutex lock;
> +	struct regulator *ref;
> +	ktime_t	time_prev;
>  	const struct ltc2497_chip_info	*chip_info;
> -	u8 addr_prev;
>  	int (*result_and_measure)(struct ltc2497core_driverdata *ddata,
>  				  u8 address, int *val);
>  	enum iio_chan_type chan_type_prev;
> +	u8 addr_prev;
>  };
>  
>  int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev);
> 

Re: [PATCH v4 0/3] Add LTC2495 support

[Jonathan Cameron]

On Fri, 15 Aug 2025 12:02:01 +0200
Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:

> Hi All,
> 
> This is the v4 of the patch series to add support for the LTC2495 ADC
> and to enable the internal temperature channel for the LTC2495 and
> LTC2499.
> 
> Thanks to Andy Shevchenko for his helpful reviews on earlier versions,
> and to David Lechner and Krzysztof Kozlowski for their feedbacks on
> v3. This version addresses all feedback from v3.
> 
> Best Regards,
> 
> Alper

Hi Alper,

A small process thing.  Wait a little longer between versions
as it tends to save time for both the submitter and reviewers.

At least a few days is appropriate typically and for first versions
I'd generally advise a week. Many reviewers only get to the list
once a week or so (some less than that!)

Thanks,

Jonathan

> 
> Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
> ---
> Changes in v4:
>   - Removed the verbose formula derivation comment from `read_raw`.
>   - Reworked the temperature channel definition to omit the redundant
>     `.address` and `.channel` fields.
>   - Moved I2C-specific bit definitions from the shared header into
>     `ltc2497.c` and removed the pre-combined command macro.
>   - Simplified I2C logic to check the channel type instead of a
>     non-standard address.
>   - Combined the basic device support (#2) and temperature sensor feature (#3)
>     patches into a single patch (#2).
>   - Link to v3: https://lore.kernel.org/r/20250814-ltc2495-v3-0-c2a6cecd6b99@gmail.com
> 
> Changes in v3:
>   - Used the standard `kelvin_to_celsius()` helper instead of a custom
>     define.
>   - Corrected macro definition style.
>   - Renamed `LTC2497_CHANNELS` and `LTC_T_CHAN` for clarity.
>   - Combined all struct layout optimizations into a single patch.
>   - Link to v2: https://lore.kernel.org/r/20250813-ltc2495-v2-0-bbaf20f6ba07@gmail.com
> 
> Changes in v2:
>   - Rewrote all commit messages to use the imperative mood.
>   - Added a justification for the new compatible string to the device
>     tree binding commit message.
>   - Removed all unrelated whitespace and formatting changes.
>   - Removed redundant explicit `false` initializers from structs.
>   - Replaced the magic number for Kelvin conversion with a define.
>   - Improved comments for defines and temperature scaling constants.
>   - Renamed confusing macros and struct fields to be more descriptive.
>   - Replaced dynamic channel allocation with a static array approach
>     using a shared macro to improve readability.
>   - Optimized data structure layouts based on pahole output to remove
>     memory holes.
>   - Link to v1: https://lore.kernel.org/r/20250812-ltc2495-v1-0-7bf4c6feec2e@gmail.com
> ---
> Implementation Notes
>  - checkpatch warning: The new static array approach uses a shared macro
>    for the common channels (`LTC2497_CHANNELS`), which triggers a
>    checkpatch.pl warning: "Macros with complex values should be enclosed
>    in parentheses". However, this will cause a compilitaion error, as an
>    initializer list cannot be parenthesized.
>  - uV to mV conversion: I could not find a standard macro, so a manual
>    division is used for now. This could be a point of future
>    improvement.
> 
> ---
> Yusuf Alper Bilgin (3):
>       dt-bindings: iio: adc: ltc2497: add lltc,ltc2495 bindings
>       iio: adc: ltc2497: add support for LTC2495
>       iio: adc: ltc2497: reorder struct members to fix memory holes
> 
>  .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml  |   3 +
>  drivers/iio/adc/ltc2497-core.c                     | 132 ++++++++++++++-------
>  drivers/iio/adc/ltc2497.c                          |  39 +++++-
>  drivers/iio/adc/ltc2497.h                          |  19 ++-
>  4 files changed, 147 insertions(+), 46 deletions(-)
> ---
> base-commit: acbbb5a20971089064ca6b271dd251e629be8d4d
> change-id: 20250811-ltc2495-572817c13fd3
> 
> Best regards,

Re: [PATCH v4 0/3] Add LTC2495 support

[Alper Bilgin]

Hi Jonathan,

Thank you for this guidance on the review process, that makes perfect
sense. I will wait longer for feedback on future versions and
contributions.

Best regards,

Alper

On Sat, Aug 16, 2025 at 12:19 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 15 Aug 2025 12:02:01 +0200
> Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:
>
> > Hi All,
> >
> > This is the v4 of the patch series to add support for the LTC2495 ADC
> > and to enable the internal temperature channel for the LTC2495 and
> > LTC2499.
> >
> > Thanks to Andy Shevchenko for his helpful reviews on earlier versions,
> > and to David Lechner and Krzysztof Kozlowski for their feedbacks on
> > v3. This version addresses all feedback from v3.
> >
> > Best Regards,
> >
> > Alper
>
> Hi Alper,
>
> A small process thing.  Wait a little longer between versions
> as it tends to save time for both the submitter and reviewers.
>
> At least a few days is appropriate typically and for first versions
> I'd generally advise a week. Many reviewers only get to the list
> once a week or so (some less than that!)
>
> Thanks,
>
> Jonathan
>
> >
> > Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
> > ---
> > Changes in v4:
> >   - Removed the verbose formula derivation comment from `read_raw`.
> >   - Reworked the temperature channel definition to omit the redundant
> >     `.address` and `.channel` fields.
> >   - Moved I2C-specific bit definitions from the shared header into
> >     `ltc2497.c` and removed the pre-combined command macro.
> >   - Simplified I2C logic to check the channel type instead of a
> >     non-standard address.
> >   - Combined the basic device support (#2) and temperature sensor feature (#3)
> >     patches into a single patch (#2).
> >   - Link to v3: https://lore.kernel.org/r/20250814-ltc2495-v3-0-c2a6cecd6b99@gmail.com
> >
> > Changes in v3:
> >   - Used the standard `kelvin_to_celsius()` helper instead of a custom
> >     define.
> >   - Corrected macro definition style.
> >   - Renamed `LTC2497_CHANNELS` and `LTC_T_CHAN` for clarity.
> >   - Combined all struct layout optimizations into a single patch.
> >   - Link to v2: https://lore.kernel.org/r/20250813-ltc2495-v2-0-bbaf20f6ba07@gmail.com
> >
> > Changes in v2:
> >   - Rewrote all commit messages to use the imperative mood.
> >   - Added a justification for the new compatible string to the device
> >     tree binding commit message.
> >   - Removed all unrelated whitespace and formatting changes.
> >   - Removed redundant explicit `false` initializers from structs.
> >   - Replaced the magic number for Kelvin conversion with a define.
> >   - Improved comments for defines and temperature scaling constants.
> >   - Renamed confusing macros and struct fields to be more descriptive.
> >   - Replaced dynamic channel allocation with a static array approach
> >     using a shared macro to improve readability.
> >   - Optimized data structure layouts based on pahole output to remove
> >     memory holes.
> >   - Link to v1: https://lore.kernel.org/r/20250812-ltc2495-v1-0-7bf4c6feec2e@gmail.com
> > ---
> > Implementation Notes
> >  - checkpatch warning: The new static array approach uses a shared macro
> >    for the common channels (`LTC2497_CHANNELS`), which triggers a
> >    checkpatch.pl warning: "Macros with complex values should be enclosed
> >    in parentheses". However, this will cause a compilitaion error, as an
> >    initializer list cannot be parenthesized.
> >  - uV to mV conversion: I could not find a standard macro, so a manual
> >    division is used for now. This could be a point of future
> >    improvement.
> >
> > ---
> > Yusuf Alper Bilgin (3):
> >       dt-bindings: iio: adc: ltc2497: add lltc,ltc2495 bindings
> >       iio: adc: ltc2497: add support for LTC2495
> >       iio: adc: ltc2497: reorder struct members to fix memory holes
> >
> >  .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml  |   3 +
> >  drivers/iio/adc/ltc2497-core.c                     | 132 ++++++++++++++-------
> >  drivers/iio/adc/ltc2497.c                          |  39 +++++-
> >  drivers/iio/adc/ltc2497.h                          |  19 ++-
> >  4 files changed, 147 insertions(+), 46 deletions(-)
> > ---
> > base-commit: acbbb5a20971089064ca6b271dd251e629be8d4d
> > change-id: 20250811-ltc2495-572817c13fd3
> >
> > Best regards,
>