linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add LTC2495 support
@ 2025-08-12 17:04 Yusuf Alper Bilgin
  2025-08-12 17:04 ` [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495 Yusuf Alper Bilgin
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Yusuf Alper Bilgin @ 2025-08-12 17:04 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin
  Cc: Michael Hennerich, linux-iio, devicetree, linux-kernel,
	Yusuf Alper Bilgin

Hi Maintainers,

This patch series extends the existing LTC2497 driver to add support for
LTC2495 ADC.

In addition to supporting the LTC2495, this series also enables the
reading of the internal temperature sensor channel for both the newly
added LTC2495 and the already supported LTC2499.

Please consider these patches for inclusion.

Thanks and best regards,

Yusuf Alper Bilgin

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
Yusuf Alper Bilgin (3):
      dt-bindings: iio: adc: ltc2497: add docs for LTC2495
      iio: adc: ltc2497: add support for LTC2495
      iio: adc: ltc2497: add temperature sensor support

 .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml  |   3 +
 drivers/iio/adc/ltc2496.c                          |   1 +
 drivers/iio/adc/ltc2497-core.c                     | 100 +++++++++++++++++----
 drivers/iio/adc/ltc2497.c                          |  40 ++++++++-
 drivers/iio/adc/ltc2497.h                          |   9 +-
 5 files changed, 128 insertions(+), 25 deletions(-)
---
base-commit: acbbb5a20971089064ca6b271dd251e629be8d4d
change-id: 20250811-ltc2495-572817c13fd3

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-12 17:04 [PATCH 0/3] Add LTC2495 support Yusuf Alper Bilgin
@ 2025-08-12 17:04 ` Yusuf Alper Bilgin
  2025-08-12 18:00   ` Krzysztof Kozlowski
  2025-08-12 17:04 ` [PATCH 2/3] iio: adc: ltc2497: add support " Yusuf Alper Bilgin
  2025-08-12 17:04 ` [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support Yusuf Alper Bilgin
  2 siblings, 1 reply; 11+ messages in thread
From: Yusuf Alper Bilgin @ 2025-08-12 17:04 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin
  Cc: Michael Hennerich, linux-iio, devicetree, linux-kernel,
	Yusuf Alper Bilgin

This updates the binding documentation for LTC2497 to include LTC2495.

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] iio: adc: ltc2497: add support for LTC2495
  2025-08-12 17:04 [PATCH 0/3] Add LTC2495 support Yusuf Alper Bilgin
  2025-08-12 17:04 ` [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495 Yusuf Alper Bilgin
@ 2025-08-12 17:04 ` Yusuf Alper Bilgin
  2025-08-13 10:18   ` Andy Shevchenko
  2025-08-12 17:04 ` [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support Yusuf Alper Bilgin
  2 siblings, 1 reply; 11+ messages in thread
From: Yusuf Alper Bilgin @ 2025-08-12 17:04 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin
  Cc: Michael Hennerich, linux-iio, devicetree, linux-kernel,
	Yusuf Alper Bilgin

This updates the LTC2497 driver to also support the LTC2495.

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
 drivers/iio/adc/ltc2497-core.c | 20 +++++++++-----------
 drivers/iio/adc/ltc2497.c      |  7 +++++++
 drivers/iio/adc/ltc2497.h      |  4 +---
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
index 2dc5c704426949a4ec62c42591d6c2c40ffb79cc..400f4fe5af30e8e16b75506726235f10f2a4237f 100644
--- a/drivers/iio/adc/ltc2497-core.c
+++ b/drivers/iio/adc/ltc2497-core.c
@@ -15,9 +15,12 @@
 
 #include "ltc2497.h"
 
-#define LTC2497_SGL			BIT(4)
-#define LTC2497_DIFF			0
-#define LTC2497_SIGN			BIT(3)
+#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
+#define LTC2497_CONVERSION_TIME_MS	150ULL
+
+#define LTC2497_SGL	BIT(4)
+#define LTC2497_DIFF	0
+#define LTC2497_SIGN	BIT(3)
 
 static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata)
 {
@@ -26,20 +29,15 @@ static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata)
 	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
 
 	if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
-		/* delay if conversion time not passed
-		 * since last read or write
-		 */
-		if (msleep_interruptible(
-		    LTC2497_CONVERSION_TIME_MS - time_elapsed))
+		/* delay if conversion time not passed since last read or write */
+		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS - time_elapsed))
 			return -ERESTARTSYS;
 
 		return 0;
 	}
 
 	if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
-		/* We're in automatic mode -
-		 * so the last reading is still not outdated
-		 */
+		/* We're in automatic mode - so the last reading is still not outdated */
 		return 0;
 	}
 
diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
index eb9d521e86e54def0493ea0e81f63b37900c56a5..8f4665547b5b0d32084599f8557c40102c37a4ce 100644
--- a/drivers/iio/adc/ltc2497.c
+++ b/drivers/iio/adc/ltc2497.c
@@ -19,6 +19,7 @@
 #include "ltc2497.h"
 
 enum ltc2497_chip_type {
+	TYPE_LTC2495,
 	TYPE_LTC2497,
 	TYPE_LTC2499,
 };
@@ -131,6 +132,10 @@ static void ltc2497_remove(struct i2c_client *client)
 }
 
 static const struct ltc2497_chip_info ltc2497_info[] = {
+	[TYPE_LTC2495] = {
+		.resolution = 16,
+		.name = "ltc2495",
+	},
 	[TYPE_LTC2497] = {
 		.resolution = 16,
 		.name = NULL,
@@ -142,6 +147,7 @@ static const struct ltc2497_chip_info ltc2497_info[] = {
 };
 
 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 +155,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..f2139f260c3fe4e8772c6db9c46331de775dcd5c 100644
--- a/drivers/iio/adc/ltc2497.h
+++ b/drivers/iio/adc/ltc2497.h
@@ -1,8 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
-#define LTC2497_ENABLE			0xA0
-#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
-#define LTC2497_CONVERSION_TIME_MS	150ULL
+#define LTC2497_ENABLE	0xA0
 
 struct ltc2497_chip_info {
 	u32 resolution;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support
  2025-08-12 17:04 [PATCH 0/3] Add LTC2495 support Yusuf Alper Bilgin
  2025-08-12 17:04 ` [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495 Yusuf Alper Bilgin
  2025-08-12 17:04 ` [PATCH 2/3] iio: adc: ltc2497: add support " Yusuf Alper Bilgin
@ 2025-08-12 17:04 ` Yusuf Alper Bilgin
  2025-08-13 10:32   ` Andy Shevchenko
  2 siblings, 1 reply; 11+ messages in thread
From: Yusuf Alper Bilgin @ 2025-08-12 17:04 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin
  Cc: Michael Hennerich, linux-iio, devicetree, linux-kernel,
	Yusuf Alper Bilgin

The LTC2495 and LTC2499 include an internal temperature sensor. This
patch adds support for reading it via a standard IIO temperature
channel.

Signed-off-by: Yusuf Alper Bilgin <y.alperbilgin@gmail.com>
---
 drivers/iio/adc/ltc2496.c      |  1 +
 drivers/iio/adc/ltc2497-core.c | 80 ++++++++++++++++++++++++++++++++++++++----
 drivers/iio/adc/ltc2497.c      | 33 ++++++++++++++---
 drivers/iio/adc/ltc2497.h      |  7 +++-
 4 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c
index f06dd0b9a85819935abea6496abe2a808ac549c6..909dbd5f9aee877aafe423b47581ff4f36a76c52 100644
--- a/drivers/iio/adc/ltc2496.c
+++ b/drivers/iio/adc/ltc2496.c
@@ -90,6 +90,7 @@ static void ltc2496_remove(struct spi_device *spi)
 static const struct ltc2497_chip_info ltc2496_info = {
 	.resolution = 16,
 	.name = NULL,
+	.has_temp_channel = false,
 };
 
 static const struct of_device_id ltc2496_of_match[] = {
diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
index 400f4fe5af30e8e16b75506726235f10f2a4237f..938259df4d27e9e26dea2be4112bef355adaaafb 100644
--- a/drivers/iio/adc/ltc2497-core.c
+++ b/drivers/iio/adc/ltc2497-core.c
@@ -72,8 +72,8 @@ static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, in
 }
 
 static int ltc2497core_read_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int *val, int *val2, long mask)
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
 {
 	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
 	int ret;
@@ -93,10 +93,52 @@ 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 = (Conversion * Vref / temp_scale) - 273
+			 *
+			 * To match the IIO framework's model of (raw + offset) * scale,
+			 * and to get the final result in millidegrees Celsius:
+			 *
+			 * = ((Conversion * Vref / temp_scale) - 273) * 1000
+			 * = (Conversion - (273 * temp_scale / Vref)) * 1000 * Vref / temp_scale
+			 *
+			 * This gives us if the Vref is in mV:
+			 * scale  = Vref * 1000 / temp_scale
+			 * offset = -273 * temp_scale / Vref
+			 */
+			*val = ret;
+			*val2 = ddata->chip_info->temp_scale;
+
+			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 = -273 * ddata->chip_info->temp_scale;
+		*val2 = ret / 1000;
+
+		return IIO_VAL_FRACTIONAL;
 
 	default:
 		return -EINVAL;
@@ -124,6 +166,14 @@ static int ltc2497core_read_raw(struct iio_dev *indio_dev,
 	.differential = 1, \
 }
 
+#define LTC2497_T_CHAN() {									\
+	.type = IIO_TEMP,									\
+	.channel = 0,										\
+	.address = (LTC2497_TEMP_CMD_ADDR),							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),						\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),	\
+}
+
 static const struct iio_chan_spec ltc2497core_channel[] = {
 	LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
 	LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
@@ -159,6 +209,8 @@ static const struct iio_chan_spec ltc2497core_channel[] = {
 	LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
 };
 
+static const struct iio_chan_spec ltc2497_temp_channel = LTC2497_T_CHAN();
+
 static const struct iio_info ltc2497core_info = {
 	.read_raw = ltc2497core_read_raw,
 };
@@ -180,8 +232,22 @@ 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);
+
+	/* Dynamically create the channel list */
+	if (ddata->chip_info->has_temp_channel) {
+		/* Allocate space for common channels + 1 temp channel */
+		indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel) + 1;
+		indio_dev->channels = devm_kmemdup(dev, ltc2497core_channel,
+						   sizeof(ltc2497core_channel), GFP_KERNEL);
+		if (!indio_dev->channels)
+			return -ENOMEM;
+
+		memcpy((void *)&indio_dev->channels[ARRAY_SIZE(ltc2497core_channel)],
+		       &ltc2497_temp_channel, sizeof(ltc2497_temp_channel));
+	} 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..3f8515ef465d3fda7572d63ef3a265dec89079ea 100644
--- a/drivers/iio/adc/ltc2497.c
+++ b/drivers/iio/adc/ltc2497.c
@@ -86,11 +86,31 @@ 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_TEMP_CMD_ADDR 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_TEMP_CMD_ADDR)
+			ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE,
+							LTC2497_TEMP_CMD_ADDR);
+		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));
+		dev_err(&st->client->dev, "i2c transfer failed: %pe\n", ERR_PTR(ret));
 	return ret;
 }
 
@@ -135,14 +155,19 @@ static const struct ltc2497_chip_info ltc2497_info[] = {
 	[TYPE_LTC2495] = {
 		.resolution = 16,
 		.name = "ltc2495",
+		.has_temp_channel = true,
+		.temp_scale = 12250, /* 12.250 V per degree C -> 12250 mV */
 	},
 	[TYPE_LTC2497] = {
 		.resolution = 16,
 		.name = NULL,
+		.has_temp_channel = false,
 	},
 	[TYPE_LTC2499] = {
 		.resolution = 24,
 		.name = "ltc2499",
+		.has_temp_channel = true,
+		.temp_scale = 1570000, /* 1570 V per degree C -> 1570000 mV */
 	},
 };
 
diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
index f2139f260c3fe4e8772c6db9c46331de775dcd5c..c133a4fb8092b1c6a66a7336016dd361783f531c 100644
--- a/drivers/iio/adc/ltc2497.h
+++ b/drivers/iio/adc/ltc2497.h
@@ -1,10 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
-#define LTC2497_ENABLE	0xA0
+#define LTC2497_ENABLE		0xA0
+#define LTC2497_EN2		0x80
+#define LTC2497_IM		0x40
+#define LTC2497_TEMP_CMD_ADDR	(LTC2497_EN2 | LTC2497_IM)
 
 struct ltc2497_chip_info {
 	u32 resolution;
 	const char *name;
+	bool has_temp_channel;
+	u32 temp_scale; /* in mV, for temp conversion */
 };
 
 struct ltc2497core_driverdata {

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-12 17:04 ` [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495 Yusuf Alper Bilgin
@ 2025-08-12 18:00   ` Krzysztof Kozlowski
  2025-08-13  8:44     ` Yusuf Alper Bilgin
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-12 18:00 UTC (permalink / raw)
  To: Yusuf Alper Bilgin, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Beguin
  Cc: linux-iio, devicetree, linux-kernel

On 12/08/2025 19:04, Yusuf Alper Bilgin wrote:
> This updates the binding documentation for LTC2497 to include LTC2495.

What are the differences, why it cannot be made compatible with 2497
(fallback)?

If there is going to be any new version:
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

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



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-12 18:00   ` Krzysztof Kozlowski
@ 2025-08-13  8:44     ` Yusuf Alper Bilgin
  2025-08-16 10:23       ` Jonathan Cameron
  2025-08-17  7:11       ` Krzysztof Kozlowski
  0 siblings, 2 replies; 11+ messages in thread
From: Yusuf Alper Bilgin @ 2025-08-13  8:44 UTC (permalink / raw)
  To: krzk
  Cc: Michael.Hennerich, andy, conor+dt, devicetree, dlechner, jic23,
	krzk+dt, lars, liambeguin, linux-iio, linux-kernel, nuno.sa, robh,
	y.alperbilgin

Hi Krzysztof,

Thank you for the review and guidance.

On Tue, Aug 12, 2025 at 07:04:00PM +0200, Krzysztof Kozlowski wrote:
> What are the differences, why it cannot be made compatible with 2497
> (fallback)?

The LTC2495 offers a more advanced feature set compared to the LTC2497,
including:

- Adjustable input gain
- A selectable 50Hz/60Hz lowpass filter to reject line frequency noise
- Selectable speed modes
- An internal temperature sensor

All of these features are configured via a second I2C command byte
(listed in Table 4 of:
https://www.analog.com/media/en/technical-documentation/data-sheets/2495fe.pdf),
which changes the driver's communication protocol compared to the
single-byte commands of the LTC2497.

This patch series begins to support reading the internal temperature
sensor by implementing driver logic for the two-byte I2C command format
and exposing the IIO temperature channel. Therefore, I added a new
binding. Without the support for the temperature sensor and this
different command structure, a simple fallback would be sufficient.

Let me know if you agree with the reasoning to add the binding. If so, I
will update the commit messages in v2 to include this justification and
ensure they follow the imperative mood convention.

Best regards,

Alper

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] iio: adc: ltc2497: add support for LTC2495
  2025-08-12 17:04 ` [PATCH 2/3] iio: adc: ltc2497: add support " Yusuf Alper Bilgin
@ 2025-08-13 10:18   ` Andy Shevchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-08-13 10:18 UTC (permalink / raw)
  To: Yusuf Alper Bilgin
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin, linux-iio,
	devicetree, linux-kernel

On Tue, Aug 12, 2025 at 7:09 PM Yusuf Alper Bilgin
<y.alperbilgin@gmail.com> wrote:
>
> This updates the LTC2497 driver to also support the LTC2495.

...

> +#define LTC2497_CONVERSION_TIME_MS     150ULL

Why ULL? (Mainly why the 'L'/'LL'?)

...

>         if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
> -               /* delay if conversion time not passed
> -                * since last read or write
> -                */
> -               if (msleep_interruptible(
> -                   LTC2497_CONVERSION_TIME_MS - time_elapsed))
> +               /* delay if conversion time not passed since last read or write */
> +               if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS - time_elapsed))
>                         return -ERESTARTSYS;
>
>                 return 0;
>         }
>
>         if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
> -               /* We're in automatic mode -
> -                * so the last reading is still not outdated
> -                */
> +               /* We're in automatic mode - so the last reading is still not outdated */
>                 return 0;
>         }

AFAICS these are unrelated changes. Please, strip them and if you
wish, create a new patch.

...

> -#define LTC2497_ENABLE                 0xA0

> +#define LTC2497_ENABLE 0xA0

Why?!

> -#define LTC2497_CONFIG_DEFAULT         LTC2497_ENABLE
> -#define LTC2497_CONVERSION_TIME_MS     150ULL

Unrelated.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support
  2025-08-12 17:04 ` [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support Yusuf Alper Bilgin
@ 2025-08-13 10:32   ` Andy Shevchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-08-13 10:32 UTC (permalink / raw)
  To: Yusuf Alper Bilgin
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Beguin, linux-iio,
	devicetree, linux-kernel

On Tue, Aug 12, 2025 at 7:09 PM Yusuf Alper Bilgin
<y.alperbilgin@gmail.com> wrote:
>
> The LTC2495 and LTC2499 include an internal temperature sensor. This
> patch adds support for reading it via a standard IIO temperature
> channel.

..

>  static const struct ltc2497_chip_info ltc2496_info = {
>         .resolution = 16,
>         .name = NULL,
> +       .has_temp_channel = false,
>  };

Unneeded change.

...

>  static int ltc2497core_read_raw(struct iio_dev *indio_dev,
> -                           struct iio_chan_spec const *chan,
> -                           int *val, int *val2, long mask)
> +                               struct iio_chan_spec const *chan,
> +                               int *val, int *val2, long mask)

Unrelated change.

...

> +               switch (chan->type) {
> +               case IIO_VOLTAGE:
> +                       *val = ret / 1000;

Don't remember if we have something like MICROVOLT_PER_MILLIVOLT.

> +                       *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 = (Conversion * Vref / temp_scale) - 273
> +                        *
> +                        * To match the IIO framework's model of (raw + offset) * scale,
> +                        * and to get the final result in millidegrees Celsius:
> +                        *
> +                        * = ((Conversion * Vref / temp_scale) - 273) * 1000
> +                        * = (Conversion - (273 * temp_scale / Vref)) * 1000 * Vref / temp_scale

 * Temp_mC = ... =
 *                      ...

I.o.w. make it more explicit, otherwise those dangling equal signs are
not helpful.

> +                        *
> +                        * This gives us if the Vref is in mV:
> +                        * scale  = Vref * 1000 / temp_scale
> +                        * offset = -273 * temp_scale / Vref
> +                        */
> +                       *val = ret;
> +                       *val2 = ddata->chip_info->temp_scale;
> +
> +                       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 = -273 * ddata->chip_info->temp_scale;
> +               *val2 = ret / 1000;

I remember we have some constants in units.h for degrees, but don't
remember if they are for Kelvin only or Celsius also included. Please,
check and use, if available.

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

...

> +#define LTC2497_T_CHAN() {                                                                     \

Why parentheses?

> +       .type = IIO_TEMP,                                                                       \
> +       .channel = 0,                                                                           \
> +       .address = (LTC2497_TEMP_CMD_ADDR),                                                     \
> +       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),                                           \
> +       .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),       \
> +}

...

> +       /* Dynamically create the channel list */
> +       if (ddata->chip_info->has_temp_channel) {
> +               /* Allocate space for common channels + 1 temp channel */
> +               indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel) + 1;
> +               indio_dev->channels = devm_kmemdup(dev, ltc2497core_channel,

Why not devm_kmemdup_array() ?

> +                                                  sizeof(ltc2497core_channel), GFP_KERNEL);

sizeof(*...)

> +               if (!indio_dev->channels)
> +                       return -ENOMEM;
> +
> +               memcpy((void *)&indio_dev->channels[ARRAY_SIZE(ltc2497core_channel)],

This is a strange style, can you improve it?

> +                      &ltc2497_temp_channel, sizeof(ltc2497_temp_channel));
> +       } else {
> +               indio_dev->channels = ltc2497core_channel;
> +               indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel);
> +       }

...

> +       if (ddata->chip_info->has_temp_channel) {
> +               if (address == LTC2497_TEMP_CMD_ADDR)
> +                       ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE,
> +                                                       LTC2497_TEMP_CMD_ADDR);
> +               else
> +                       ret = i2c_smbus_write_byte_data(st->client, LTC2497_ENABLE | address,
> +                                                       LTC2497_EN2);

> +

Stray blank line.

> +       } 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));
> +               dev_err(&st->client->dev, "i2c transfer failed: %pe\n", ERR_PTR(ret));
>         return ret;

...

>         [TYPE_LTC2497] = {
>                 .resolution = 16,
>                 .name = NULL,
> +               .has_temp_channel = false,
>         },

Unneeded change.

...

>         [TYPE_LTC2499] = {
>                 .resolution = 24,
>                 .name = "ltc2499",
> +               .has_temp_channel = true,
> +               .temp_scale = 1570000, /* 1570 V per degree C -> 1570000 mV */

1570V ?! Hmm...

>         },

...

> +#define LTC2497_ENABLE         0xA0
> +#define LTC2497_EN2            0x80
> +#define LTC2497_IM             0x40

Are those bit masks / bits in the register? Offsets?

> +#define LTC2497_TEMP_CMD_ADDR  (LTC2497_EN2 | LTC2497_IM)

This is really strange naming for... what exactly? Can a comment be added?


...

>  struct ltc2497_chip_info {
>         u32 resolution;
>         const char *name;
> +       bool has_temp_channel;
> +       u32 temp_scale; /* in mV, for temp conversion */

Then simply add a (mV, yes, with capital V) suffix to the field name.

>  };

Have you run `pahole`? Does it agree with the proposed layout?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-13  8:44     ` Yusuf Alper Bilgin
@ 2025-08-16 10:23       ` Jonathan Cameron
  2025-08-18 16:26         ` Alper Bilgin
  2025-08-17  7:11       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2025-08-16 10:23 UTC (permalink / raw)
  To: Yusuf Alper Bilgin
  Cc: krzk, Michael.Hennerich, andy, conor+dt, devicetree, dlechner,
	krzk+dt, lars, liambeguin, linux-iio, linux-kernel, nuno.sa, robh

On Wed, 13 Aug 2025 10:44:44 +0200
Yusuf Alper Bilgin <y.alperbilgin@gmail.com> wrote:

> Hi Krzysztof,
> 
> Thank you for the review and guidance.
> 
> On Tue, Aug 12, 2025 at 07:04:00PM +0200, Krzysztof Kozlowski wrote:
> > What are the differences, why it cannot be made compatible with 2497
> > (fallback)?  
> 
> The LTC2495 offers a more advanced feature set compared to the LTC2497,
> including:
> 
> - Adjustable input gain
> - A selectable 50Hz/60Hz lowpass filter to reject line frequency noise
> - Selectable speed modes
> - An internal temperature sensor
> 
> All of these features are configured via a second I2C command byte
> (listed in Table 4 of:
> https://www.analog.com/media/en/technical-documentation/data-sheets/2495fe.pdf),
> which changes the driver's communication protocol compared to the
> single-byte commands of the LTC2497.
> 
Is that second byte optional?  Figure 3b seems to suggest so but I haven't
taken the time to read the rest of the data sheet.
If it is never written does this new device function as backwards compatible
with the LTC2497?

If so a fallback compatible is appropriate.  This is used when we have
new newer DT against an older driver that doesn't support new features.

A newer kernel will match on the new ID and hence provide these extended
features you have here.

Note this discussion should have happened before you posted v2, let alone v3 and v4!

Jonathan

> This patch series begins to support reading the internal temperature
> sensor by implementing driver logic for the two-byte I2C command format
> and exposing the IIO temperature channel. Therefore, I added a new
> binding. Without the support for the temperature sensor and this
> different command structure, a simple fallback would be sufficient.
> 
> Let me know if you agree with the reasoning to add the binding. If so, I
> will update the commit messages in v2 to include this justification and
> ensure they follow the imperative mood convention.
> 
> Best regards,
> 
> Alper


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-13  8:44     ` Yusuf Alper Bilgin
  2025-08-16 10:23       ` Jonathan Cameron
@ 2025-08-17  7:11       ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-17  7:11 UTC (permalink / raw)
  To: Yusuf Alper Bilgin
  Cc: Michael.Hennerich, andy, conor+dt, devicetree, dlechner, jic23,
	krzk+dt, lars, liambeguin, linux-iio, linux-kernel, nuno.sa, robh

On 13/08/2025 10:44, Yusuf Alper Bilgin wrote:
> Hi Krzysztof,
> 
> Thank you for the review and guidance.
> 
> On Tue, Aug 12, 2025 at 07:04:00PM +0200, Krzysztof Kozlowski wrote:
>> What are the differences, why it cannot be made compatible with 2497
>> (fallback)?
> 
> The LTC2495 offers a more advanced feature set compared to the LTC2497,
> including:
> 
> - Adjustable input gain
> - A selectable 50Hz/60Hz lowpass filter to reject line frequency noise
> - Selectable speed modes
> - An internal temperature sensor

"More advanced" does not mean it is not compatible. If old device (less
advanced) features are working the same, it is the exact meaning of
compatible devices.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495
  2025-08-16 10:23       ` Jonathan Cameron
@ 2025-08-18 16:26         ` Alper Bilgin
  0 siblings, 0 replies; 11+ messages in thread
From: Alper Bilgin @ 2025-08-18 16:26 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: krzk, Michael.Hennerich, andy, conor+dt, devicetree, dlechner,
	krzk+dt, lars, liambeguin, linux-iio, linux-kernel, nuno.sa, robh

Hi Jonathan and Krzysztof,

Thank you for the detailed guidance on the fallback compatible.

On Sat, Aug 16, 2025 at 12:23 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> Is that second byte optional?  Figure 3b seems to suggest so but I haven't
> taken the time to read the rest of the data sheet.
> If it is never written does this new device function as backwards compatible
> with the LTC2497?
>
> If so a fallback compatible is appropriate.  This is used when we have
> new newer DT against an older driver that doesn't support new features.
>
> A newer kernel will match on the new ID and hence provide these extended
> features you have here.
>
The second I2C command byte is indeed optional. If it is not sent, the
LTC2495 defaults to its standard mode and functions as
backwards-compatible with the LTC2497 for basic voltage readings.

The new "lltc,ltc2495" compatible is therefore needed only to allow
the updated driver to identify the chip and enable the temperature
channel.

I will rework the patch series to reflect this.

> Note this discussion should have happened before you posted v2, let alone v3 and v4!

My apologies for the extra revisions and discussion on this topic.

Best regards,

Alper

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-18 16:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-12 17:04 [PATCH 0/3] Add LTC2495 support Yusuf Alper Bilgin
2025-08-12 17:04 ` [PATCH 1/3] dt-bindings: iio: adc: ltc2497: add docs for LTC2495 Yusuf Alper Bilgin
2025-08-12 18:00   ` Krzysztof Kozlowski
2025-08-13  8:44     ` Yusuf Alper Bilgin
2025-08-16 10:23       ` Jonathan Cameron
2025-08-18 16:26         ` Alper Bilgin
2025-08-17  7:11       ` Krzysztof Kozlowski
2025-08-12 17:04 ` [PATCH 2/3] iio: adc: ltc2497: add support " Yusuf Alper Bilgin
2025-08-13 10:18   ` Andy Shevchenko
2025-08-12 17:04 ` [PATCH 3/3] iio: adc: ltc2497: add temperature sensor support Yusuf Alper Bilgin
2025-08-13 10:32   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).