[PATCH v2 0/2] Add driver for the TI TLA2528 i2c ADC

[PATCH v2 0/2] Add driver for the TI TLA2528 i2c ADC

[Maxime Chevallier]

Hi everyone,

This is V2 for the TLA2528 ADC support, V1 was sent almost a year ago
and was already resurecting yet another older series from Rodolfo
Giometti [1]. Sorry for the slow iteration.

This V2 addresses the various comments that were made on V1, thanks
again to everyone involved in the first review round (Jonathan, Andy,
Matti and David)

This was tested on a custom board based on imx93, with a 3v3 ref supply.

Maxime  

[1] : https://lore.kernel.org/linux-iio/20210802130441.972978-1-giometti@enneenne.com/

V2:
  - Describe AVDD and DVDD in the bindings instead of vref.
  - Add gpio-controller
  - Allow describing individual ADC channels
  - Switch to regmap with caching
  - Reset the chip at probe time
  - Small cleanups following all the reviews
  - Moved MAINTAINERS to patch 1

V1: https://lore.kernel.org/linux-iio/20251223155534.220504-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (2):
  dt-bindings: iio: adc: add Texas Instruments TLA2528
  iio: adc: add driver for Texas Instruments TLA2528 adc

 .../bindings/iio/adc/ti,tla2528.yaml          |  91 +++++++
 MAINTAINERS                                   |   7 +
 drivers/iio/adc/Kconfig                       |  12 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ti-tla2528.c                  | 245 ++++++++++++++++++
 5 files changed, 356 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
 create mode 100644 drivers/iio/adc/ti-tla2528.c

-- 
2.56.0

[PATCH v2 1/2] dt-bindings: iio: adc: add Texas Instruments TLA2528

[Maxime Chevallier]

The TI TLA2528 is a simple 8-channel, 12-bit ADC and GPIO chip
accessible over i2c. The ADC part has an AVDD supply that doubles as a
reference.

As this chip can be used both for its ADC and GPIOs, let's allow
describing each ADC channel individually. If no ADC chan description is
represented, all channels will be exposed as analog inputs.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 .../bindings/iio/adc/ti,tla2528.yaml          | 91 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml b/Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
new file mode 100644
index 000000000000..7a50a5026c1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,tla2528.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TLA2528 8-channel 12-bit I2C ADC
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+description:
+  Each of the 8 input pins can be configured as an analog input, a digital
+  input or a digital output.
+
+properties:
+  compatible:
+    const: ti,tla2528
+
+  reg:
+    maxItems: 1
+
+  avdd-supply:
+    description: Analog supply, also used as the ADC reference voltage.
+
+  dvdd-supply: true
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  "#io-channel-cells":
+    const: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^channel@[0-7]$":
+    type: object
+    $ref: adc.yaml
+    description: Represents an analog input.
+
+    properties:
+      reg:
+        description: AIN pin number
+        minimum: 0
+        maximum: 7
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - dvdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@17 {
+            compatible = "ti,tla2528";
+            reg = <0x17>;
+            avdd-supply = <&reg_adc_supply>;
+            dvdd-supply = <&reg_adc_supply>;
+            #io-channel-cells = <1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 {
+                reg = <0>;
+            };
+
+            channel@3 {
+                reg = <3>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a162e9bcab1..471585b29c19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27715,6 +27715,12 @@ F:	include/dt-bindings/soc/ti,sci_pm_domain.h
 F:	include/linux/soc/ti/ti_sci_inta_msi.h
 F:	include/linux/soc/ti/ti_sci_protocol.h
 
+TEXAS INSTRUMENTS' TLA2528 ADC DRIVER
+M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
+
 TEXAS INSTRUMENTS' TMP117 TEMPERATURE SENSOR DRIVER
 M:	Puranjay Mohan <puranjay@kernel.org>
 L:	linux-iio@vger.kernel.org
-- 
2.56.0

Re: [PATCH v2 1/2] dt-bindings: iio: adc: add Texas Instruments TLA2528

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

[Maxime Chevallier]

Add a new driver for the TI TLA2528 ADC chip. It has 8 12-bit
channels, that can also be configured as 16-bit averaging channels.

This is a very simple driver, allowing reading raw values for each
channel. GPIO mode isn't supported for now, as well as buffered mode and
the 16-bit averaging mode.

Co-developed-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 MAINTAINERS                  |   1 +
 drivers/iio/adc/Kconfig      |  12 ++
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-tla2528.c | 245 +++++++++++++++++++++++++++++++++++
 4 files changed, 259 insertions(+)
 create mode 100644 drivers/iio/adc/ti-tla2528.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 471585b29c19..1ebb6418ab1f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27720,6 +27720,7 @@ M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
+F:	drivers/iio/adc/ti-tla2528.c
 
 TEXAS INSTRUMENTS' TMP117 TEMPERATURE SENSOR DRIVER
 M:	Puranjay Mohan <puranjay@kernel.org>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a802d8ee4dd9..8633267d6056 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -2016,6 +2016,18 @@ config TI_LMP92064
 	  This driver can also be built as a module. If so, the module will be called
 	  ti-lmp92064.
 
+config TI_TLA2528
+	tristate "Texas Instruments TLA2528 ADC driver"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_ADC_HELPER
+	help
+	  Say yes here to build support for Texas Instruments TLA2528
+	  12-Bit 8-Channel ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ti-tla2528.
+
 config TI_TLC4541
 	tristate "Texas Instruments TLC4541 ADC driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9e82c21dbd37..b98cded4d0c7 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -171,6 +171,7 @@ obj-$(CONFIG_TI_ADS8344) += ti-ads8344.o
 obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_TI_LMP92064) += ti-lmp92064.o
+obj-$(CONFIG_TI_TLA2528) += ti-tla2528.o
 obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
 obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
 obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
diff --git a/drivers/iio/adc/ti-tla2528.c b/drivers/iio/adc/ti-tla2528.c
new file mode 100644
index 000000000000..287e52dc9d6d
--- /dev/null
+++ b/drivers/iio/adc/ti-tla2528.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Texas Instruments TLA2528 ADC
+ *
+ * Copyright (C) 2020-2021 Rodolfo Giometti <giometti@enneenne.com>
+ * Copyright (C) 2025 Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/adc-helpers.h>
+#include <linux/iio/iio.h>
+
+#define TLA2528_OP_WRITE_REG		0x08
+#define TLA2528_OP_READ_REG		0x10
+
+#define TLA2528_SYSTEM_STATUS_ADR	0x00
+#define TLA2528_GENERAL_CFG_ADR		0x01
+#define TLA2528_GENERAL_CFG_RST		BIT(0)
+#define TLA2528_GPI_VALUE_ADR		0x0d
+#define TLA2528_CHANNEL_SEL_ADR		0x11
+#define TLA2528_CHANNEL_SEL_MANUAL_CHID	GENMASK(3, 0)
+#define TLA2528_AUTO_SEQ_CH_SEL_ADR	0x12
+
+#define TLA2528_NUM_CHANNELS		8
+
+struct tla2528 {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	int vref_mv;
+
+	/* Protects manual channel selection */
+	struct mutex lock;
+};
+
+static bool tla2528_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TLA2528_SYSTEM_STATUS_ADR:
+	case TLA2528_GENERAL_CFG_ADR:
+	case TLA2528_GPI_VALUE_ADR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config tla2528_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.read_flag_mask = TLA2528_OP_READ_REG,
+	.write_flag_mask = TLA2528_OP_WRITE_REG,
+	.max_register = TLA2528_AUTO_SEQ_CH_SEL_ADR,
+	.volatile_reg = tla2528_volatile_reg,
+	.cache_type = REGCACHE_MAPLE,
+};
+
+static int tla2528_read_sample(const struct i2c_client *client)
+{
+	__be16 data;
+	int ret;
+
+	ret = i2c_master_recv(client, (char *)&data, sizeof(data));
+	if (ret < 0)
+		return ret;
+
+	return be16_to_cpu(data) >> 4;
+}
+
+static int tla2528_read(struct tla2528 *tla2528, u8 channel, int *val)
+{
+	int ret;
+
+	ret = regmap_update_bits(tla2528->regmap, TLA2528_CHANNEL_SEL_ADR,
+				 TLA2528_CHANNEL_SEL_MANUAL_CHID, channel);
+	if (ret)
+		return ret;
+
+	ret = tla2528_read_sample(tla2528->client);
+	if (ret < 0)
+		return ret;
+
+	*val = ret;
+
+	return 0;
+}
+
+static int tla2528_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct tla2528 *tla2528 = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&tla2528->lock);
+		ret = tla2528_read(tla2528, chan->channel, val);
+		mutex_unlock(&tla2528->lock);
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = tla2528->vref_mv;
+		*val2 = 12;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+#define TLA2528_CHAN(chan) {					\
+	.type = IIO_VOLTAGE,					\
+	.channel = (chan),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+	.indexed = 1,						\
+}
+
+static const struct iio_chan_spec tla2528_chan_template = TLA2528_CHAN(0);
+
+static const struct iio_chan_spec tla2528_channel[] = {
+	TLA2528_CHAN(0),
+	TLA2528_CHAN(1),
+	TLA2528_CHAN(2),
+	TLA2528_CHAN(3),
+	TLA2528_CHAN(4),
+	TLA2528_CHAN(5),
+	TLA2528_CHAN(6),
+	TLA2528_CHAN(7),
+};
+
+static const struct iio_info tla2528_info = {
+	.read_raw = tla2528_read_raw,
+};
+
+static int tla2528_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct iio_chan_spec *channels;
+	struct iio_dev *indio_dev;
+	struct tla2528 *tla2528;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	if (i2c_check_quirks(client->adapter, I2C_AQ_NO_CLK_STRETCH))
+		return dev_err_probe(dev, -EOPNOTSUPP,
+				     "adapter does not support clock stretching\n");
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*tla2528));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	tla2528 = iio_priv(indio_dev);
+	tla2528->client = client;
+
+	tla2528->regmap = devm_regmap_init_i2c(client, &tla2528_regmap_config);
+	if (IS_ERR(tla2528->regmap))
+		return PTR_ERR(tla2528->regmap);
+
+	ret = devm_mutex_init(dev, &tla2528->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(dev, "dvdd");
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
+	if (ret < 0)
+		return ret;
+
+	tla2528->vref_mv = ret / (MICRO / MILLI);
+
+	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &tla2528_chan_template,
+						    TLA2528_NUM_CHANNELS - 1,
+						    &channels);
+	if (ret < 0 && ret != -ENOENT)
+		return ret;
+
+	if (ret == -ENOENT) {
+		indio_dev->channels = tla2528_channel;
+		indio_dev->num_channels = ARRAY_SIZE(tla2528_channel);
+	} else {
+		indio_dev->channels = channels;
+		indio_dev->num_channels = ret;
+	}
+
+	indio_dev->name = "tla2528";
+	indio_dev->info = &tla2528_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = regmap_write(tla2528->regmap, TLA2528_GENERAL_CFG_ADR,
+			   TLA2528_GENERAL_CFG_RST);
+	if (ret)
+		return ret;
+
+	fsleep(5 * USEC_PER_MSEC);
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct i2c_device_id tla2528_id[] = {
+	{ "tla2528" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tla2528_id);
+
+static const struct of_device_id tla2528_of_match[] = {
+	{ .compatible = "ti,tla2528" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tla2528_of_match);
+
+static struct i2c_driver tla2528_driver = {
+	.driver = {
+		.name = "tla2528",
+		.of_match_table = tla2528_of_match,
+	},
+	.probe = tla2528_probe,
+	.id_table = tla2528_id,
+};
+module_i2c_driver(tla2528_driver);
+
+MODULE_AUTHOR("Maxime Chevallier <maxime.chevallier@bootlin.com>");
+MODULE_AUTHOR("Rodolfo Giometti <giometti@enneenne.com>");
+MODULE_DESCRIPTION("Texas Instruments TLA2528 ADC driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_DRIVER");
-- 
2.56.0

Re: [PATCH v2 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

[Victor Chernyakin]

+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Texas Instruments TLA2528 ADC
+ *
+ * Copyright (C) 2020-2021 Rodolfo Giometti <giometti@enneenne.com>
+ * Copyright (C) 2025 Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */

Add a "Datasheet: https://www.ti.com/lit/gpn/tla2528" line here.

Re: [PATCH v2 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

[Jonathan Cameron]

On Fri, 30 Oct 2026 17:47:04 +0100
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> Add a new driver for the TI TLA2528 ADC chip. It has 8 12-bit
> channels, that can also be configured as 16-bit averaging channels.
> 
> This is a very simple driver, allowing reading raw values for each
> channel. GPIO mode isn't supported for now, as well as buffered mode and
> the 16-bit averaging mode.
> 
> Co-developed-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Hi Maxime

A couple of minor things inline.  At least one due to cleanup
that happened tree wide in the last year.

I'd also like to avoid propagating the fallback to all channels
if none are specified. In this particular case it is problematic
because if you add the GPIO mode, for chips like this, we tend
to expose all pins that aren't in use for anything else as GPIOs.
With the fallback scheme you have no way to tell if you want all
pins as GPIOs.

That hack went in a few drivers were we added per channel properties
after the initial driver was added added and had to paper over
the backwards compatibility issue.  Hopefully it didn't make it
into fresh drivers whilst I wasn't paying attention!

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/ti-tla2528.c b/drivers/iio/adc/ti-tla2528.c
> new file mode 100644
> index 000000000000..287e52dc9d6d
> --- /dev/null
> +++ b/drivers/iio/adc/ti-tla2528.c

> +
> +static int tla2528_read_sample(const struct i2c_client *client)

Benefit of this helper seems marginal.  Maybe squash into where
it is currently called?  I don't care either way on this one.


> +{
> +	__be16 data;
> +	int ret;
> +
> +	ret = i2c_master_recv(client, (char *)&data, sizeof(data));
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(data) >> 4;
> +}
> +
> +static int tla2528_read(struct tla2528 *tla2528, u8 channel, int *val)
> +{
> +	int ret;

As below. Use guard() in here. That makes it really obvious why
the mutex is needed.

> +
> +	ret = regmap_update_bits(tla2528->regmap, TLA2528_CHANNEL_SEL_ADR,
> +				 TLA2528_CHANNEL_SEL_MANUAL_CHID, channel);
> +	if (ret)
> +		return ret;
> +
> +	ret = tla2528_read_sample(tla2528->client);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = ret;
> +
> +	return 0;
> +}
> +
> +static int tla2528_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct tla2528 *tla2528 = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&tla2528->lock);
> +		ret = tla2528_read(tla2528, chan->channel, val);
> +		mutex_unlock(&tla2528->lock);
> +		if (ret < 0)
> +			return ret;
> +
> +		return IIO_VAL_INT;
	case IIO_CHAN_INFO_RAW: {
		guard(mutex)(&tla2528->lock);

		ret = tla2528_read(tla2528, chan->channel, val);
		if (ret < 0)
			return ret;

		return IIO_VAL_INT;
	}
Small gain here in keeping the error check right next to the thing
that set it, but might as well have that given it is simple to
use a guard.

Or better still: Move the guard into tla2528_read() 
There it will be more obvious that it is the two step nature
of the read that requires the lock.

> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = tla2528->vref_mv;
> +		*val2 = 12;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +static int tla2528_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_chan_spec *channels;
> +	struct iio_dev *indio_dev;
> +	struct tla2528 *tla2528;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))

Add a comment that the data reads directly use i2c with
no register address.  FWIW I briefly wondered why the checks in
the regmap init weren't enough.  They aren't because for this particular
mix of reg and val sizes can match others I2C protocol options in
regmap_get_i2c_bus().  With this present though we know they will match
the first and simplest one.

> +		return -EOPNOTSUPP;
> +
> +	if (i2c_check_quirks(client->adapter, I2C_AQ_NO_CLK_STRETCH))
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "adapter does not support clock stretching\n");
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*tla2528));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	tla2528 = iio_priv(indio_dev);
> +	tla2528->client = client;
> +
> +	tla2528->regmap = devm_regmap_init_i2c(client, &tla2528_regmap_config);
> +	if (IS_ERR(tla2528->regmap))
> +		return PTR_ERR(tla2528->regmap);
> +
> +	ret = devm_mutex_init(dev, &tla2528->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable(dev, "dvdd");
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
> +	if (ret < 0)
> +		return ret;
> +
> +	tla2528->vref_mv = ret / (MICRO / MILLI);
> +
> +	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &tla2528_chan_template,
> +						    TLA2528_NUM_CHANNELS - 1,
> +						    &channels);
> +	if (ret < 0 && ret != -ENOENT)
> +		return ret;
> +
> +	if (ret == -ENOENT) {

We do this trick for old drivers where we only added per channel properties
later.  For a new driver, why not always require the binding to be explicit
and list the channels (even if that is all of them)?

> +		indio_dev->channels = tla2528_channel;
> +		indio_dev->num_channels = ARRAY_SIZE(tla2528_channel);
> +	} else {
> +		indio_dev->channels = channels;
> +		indio_dev->num_channels = ret;
> +	}
> +
> +	indio_dev->name = "tla2528";
> +	indio_dev->info = &tla2528_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = regmap_write(tla2528->regmap, TLA2528_GENERAL_CFG_ADR,
> +			   TLA2528_GENERAL_CFG_RST);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(5 * USEC_PER_MSEC);
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id tla2528_id[] = {
> +	{ "tla2528" },

This is something that happened in last year, but for these
we now always use named initializers.  There is work ongoing
for a change in the structure definition and that only works
if they are named.  + It is consistent with what we always
did for the of_device_id tables.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, tla2528_id);

Re: [PATCH v2 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

[Rodolfo Giometti]

On 01/11/2026 00:48, Jonathan Cameron wrote:
> I'd also like to avoid propagating the fallback to all channels
> if none are specified. In this particular case it is problematic
> because if you add the GPIO mode, for chips like this, we tend
> to expose all pins that aren't in use for anything else as GPIOs.
> With the fallback scheme you have no way to tell if you want all
> pins as GPIOs.

Agreed. With only 8 pins, listing the channels explicitly in the DT
costs little, and it keeps the door open for the GPIO support.

Maxime, if you drop the fallback, I think the commit message of patch
1/2 should lose its last sentence too ("If no ADC chan description is
represented, all channels will be exposed as analog inputs."), since
it would no longer describe what the driver does.

Thanks,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

Re: [PATCH v2 2/2] iio: adc: add driver for Texas Instruments TLA2528 adc

[Matti Vaittinen]

On 30/10/2026 18:47, Maxime Chevallier wrote:
> Add a new driver for the TI TLA2528 ADC chip. It has 8 12-bit
> channels, that can also be configured as 16-bit averaging channels.
> 
> This is a very simple driver, allowing reading raw values for each
> channel. GPIO mode isn't supported for now, as well as buffered mode and
> the 16-bit averaging mode.
> 
> Co-developed-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Looks very nice and clean to me. I made couple of remarks below but they 
are very minor and I do not require them to be addressed. Nice job!

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

> ---
>   MAINTAINERS                  |   1 +
>   drivers/iio/adc/Kconfig      |  12 ++
>   drivers/iio/adc/Makefile     |   1 +
>   drivers/iio/adc/ti-tla2528.c | 245 +++++++++++++++++++++++++++++++++++
>   4 files changed, 259 insertions(+)
>   create mode 100644 drivers/iio/adc/ti-tla2528.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 471585b29c19..1ebb6418ab1f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27720,6 +27720,7 @@ M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
>   L:	linux-iio@vger.kernel.org
>   S:	Supported
>   F:	Documentation/devicetree/bindings/iio/adc/ti,tla2528.yaml
> +F:	drivers/iio/adc/ti-tla2528.c
>   
>   TEXAS INSTRUMENTS' TMP117 TEMPERATURE SENSOR DRIVER
>   M:	Puranjay Mohan <puranjay@kernel.org>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a802d8ee4dd9..8633267d6056 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -2016,6 +2016,18 @@ config TI_LMP92064
>   	  This driver can also be built as a module. If so, the module will be called
>   	  ti-lmp92064.
>   
> +config TI_TLA2528
> +	tristate "Texas Instruments TLA2528 ADC driver"
> +	depends on I2C
> +	select REGMAP_I2C
> +	select IIO_ADC_HELPER
> +	help
> +	  Say yes here to build support for Texas Instruments TLA2528
> +	  12-Bit 8-Channel ADC.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ti-tla2528.

I deeply dislike repetitive boilerplate text, which often, in my 
opinion, provides no real value. This last chapter (which is also 
present in many other drivers, and which is preferred by some 
developers) is for me an example of wasted lines. I believe writing the 
"choose M here: ..." is guidance, specific to menuconfig tool. The 
purpose of this file is not to explain menuconfig tool usage, but 
explain what the configuration option enables. Perhaps we some day have 
different config tool, with no option labeled as 'M' at all.

Furthermore, I do dislike also the "module will be called "foo", which I 
think is just unnecessary noise. You probably want to still keep it, to 
avoid comments from other reviewers, who claim someone (if not them) has 
once used this information for something. ;)

> +
>   config TI_TLC4541
>   	tristate "Texas Instruments TLC4541 ADC driver"
>   	depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9e82c21dbd37..b98cded4d0c7 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -171,6 +171,7 @@ obj-$(CONFIG_TI_ADS8344) += ti-ads8344.o
>   obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>   obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>   obj-$(CONFIG_TI_LMP92064) += ti-lmp92064.o
> +obj-$(CONFIG_TI_TLA2528) += ti-tla2528.o
>   obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
>   obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
>   obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> diff --git a/drivers/iio/adc/ti-tla2528.c b/drivers/iio/adc/ti-tla2528.c
> new file mode 100644
> index 000000000000..287e52dc9d6d
> --- /dev/null
> +++ b/drivers/iio/adc/ti-tla2528.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Texas Instruments TLA2528 ADC
> + *
> + * Copyright (C) 2020-2021 Rodolfo Giometti <giometti@enneenne.com>
> + * Copyright (C) 2025 Maxime Chevallier <maxime.chevallier@bootlin.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/adc-helpers.h>
> +#include <linux/iio/iio.h>
> +
> +#define TLA2528_OP_WRITE_REG		0x08
> +#define TLA2528_OP_READ_REG		0x10
> +
> +#define TLA2528_SYSTEM_STATUS_ADR	0x00
> +#define TLA2528_GENERAL_CFG_ADR		0x01
> +#define TLA2528_GENERAL_CFG_RST		BIT(0)
> +#define TLA2528_GPI_VALUE_ADR		0x0d

Not sure having the GPI_VALUE_ADR or SYSTEM_STATUS_ADR here is worth it. 
They seem to be unused.

> +#define TLA2528_CHANNEL_SEL_ADR		0x11
> +#define TLA2528_CHANNEL_SEL_MANUAL_CHID	GENMASK(3, 0)
> +#define TLA2528_AUTO_SEQ_CH_SEL_ADR	0x12
> +
> +#define TLA2528_NUM_CHANNELS		8
> +
> +struct tla2528 {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	int vref_mv;
> +
> +	/* Protects manual channel selection */
> +	struct mutex lock;
> +};
> +
> +static bool tla2528_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case TLA2528_SYSTEM_STATUS_ADR:
> +	case TLA2528_GENERAL_CFG_ADR:
> +	case TLA2528_GPI_VALUE_ADR:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

My personal preference for static volatile/read-only/write-only register 
information, is to use range tables. Seeing a function makes me always 
think there is some runtime logic involved.

This is still nice and clean function, with not much room for confusion 
so I suppose it is fine.

> +static const struct regmap_config tla2528_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.read_flag_mask = TLA2528_OP_READ_REG,
> +	.write_flag_mask = TLA2528_OP_WRITE_REG,
> +	.max_register = TLA2528_AUTO_SEQ_CH_SEL_ADR,
> +	.volatile_reg = tla2528_volatile_reg,
> +	.cache_type = REGCACHE_MAPLE,
> +};
> +
> +static int tla2528_read_sample(const struct i2c_client *client)
> +{
> +	__be16 data;
> +	int ret;
> +
> +	ret = i2c_master_recv(client, (char *)&data, sizeof(data));
> +	if (ret < 0)
> +		return ret;
> +
> +	return be16_to_cpu(data) >> 4;
> +}
> +
> +static int tla2528_read(struct tla2528 *tla2528, u8 channel, int *val)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(tla2528->regmap, TLA2528_CHANNEL_SEL_ADR,
> +				 TLA2528_CHANNEL_SEL_MANUAL_CHID, channel);
> +	if (ret)
> +		return ret;
> +
> +	ret = tla2528_read_sample(tla2528->client);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = ret;
> +
> +	return 0;
> +}
> +
> +static int tla2528_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct tla2528 *tla2528 = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&tla2528->lock);
> +		ret = tla2528_read(tla2528, chan->channel, val);
> +		mutex_unlock(&tla2528->lock);
> +		if (ret < 0)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = tla2528->vref_mv;
> +		*val2 = 12;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +#define TLA2528_CHAN(chan) {					\
> +	.type = IIO_VOLTAGE,					\
> +	.channel = (chan),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +	.indexed = 1,						\
> +}
> +
> +static const struct iio_chan_spec tla2528_chan_template = TLA2528_CHAN(0);
> +
> +static const struct iio_chan_spec tla2528_channel[] = {
> +	TLA2528_CHAN(0),
> +	TLA2528_CHAN(1),
> +	TLA2528_CHAN(2),
> +	TLA2528_CHAN(3),
> +	TLA2528_CHAN(4),
> +	TLA2528_CHAN(5),
> +	TLA2528_CHAN(6),
> +	TLA2528_CHAN(7),
> +};
> +
> +static const struct iio_info tla2528_info = {
> +	.read_raw = tla2528_read_raw,
> +};
> +
> +static int tla2528_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_chan_spec *channels;
> +	struct iio_dev *indio_dev;
> +	struct tla2528 *tla2528;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -EOPNOTSUPP;
> +
> +	if (i2c_check_quirks(client->adapter, I2C_AQ_NO_CLK_STRETCH))
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "adapter does not support clock stretching\n");
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*tla2528));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	tla2528 = iio_priv(indio_dev);
> +	tla2528->client = client;
> +
> +	tla2528->regmap = devm_regmap_init_i2c(client, &tla2528_regmap_config);
> +	if (IS_ERR(tla2528->regmap))
> +		return PTR_ERR(tla2528->regmap);
> +
> +	ret = devm_mutex_init(dev, &tla2528->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable(dev, "dvdd");
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
> +	if (ret < 0)
> +		return ret;
> +
> +	tla2528->vref_mv = ret / (MICRO / MILLI);
> +
> +	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &tla2528_chan_template,
> +						    TLA2528_NUM_CHANNELS - 1,
> +						    &channels);
> +	if (ret < 0 && ret != -ENOENT)
> +		return ret;
> +
> +	if (ret == -ENOENT) {
> +		indio_dev->channels = tla2528_channel;
> +		indio_dev->num_channels = ARRAY_SIZE(tla2528_channel);

I think just requiring the channels to be there would be simpler. It's 
only 8 chans so it's not that huge description - and this would make it 
simpler to support "all GPIOs" -case later.

> +	} else {
> +		indio_dev->channels = channels;
> +		indio_dev->num_channels = ret;
> +	}
> +
> +	indio_dev->name = "tla2528";
> +	indio_dev->info = &tla2528_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = regmap_write(tla2528->regmap, TLA2528_GENERAL_CFG_ADR,
> +			   TLA2528_GENERAL_CFG_RST);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(5 * USEC_PER_MSEC);
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id tla2528_id[] = {
> +	{ "tla2528" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, tla2528_id);
> +
> +static const struct of_device_id tla2528_of_match[] = {
> +	{ .compatible = "ti,tla2528" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tla2528_of_match);
> +
> +static struct i2c_driver tla2528_driver = {
> +	.driver = {
> +		.name = "tla2528",
> +		.of_match_table = tla2528_of_match,
> +	},
> +	.probe = tla2528_probe,
> +	.id_table = tla2528_id,
> +};
> +module_i2c_driver(tla2528_driver);
> +
> +MODULE_AUTHOR("Maxime Chevallier <maxime.chevallier@bootlin.com>");
> +MODULE_AUTHOR("Rodolfo Giometti <giometti@enneenne.com>");
> +MODULE_DESCRIPTION("Texas Instruments TLA2528 ADC driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_DRIVER");


-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~