Skip to contentSkip to conversation

dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants

PATCH Superseded Md Shofiqul Islam 6 participants 9 messages Jun 30 – Jul 1

Subsystems (2)
Device tree bindings IIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Md Shofiqul Islam

author
#0 lore raw

PATCH 1/2 dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants

The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface and register map as the ADS1298 ECG ADC family but are designed for EEG and biopotential measurement. Key differences from ADS1298:
- PGA gain up to 24x (vs 12x for ADS1298)
- Internal reference is always 2.4V (no 4V option)

Add compatible strings and update the vref-supply description to document the ADS1299 reference voltage behaviour.

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +26 -5
index 71f9f9b745cb..82c5181126df 100644
@@ -4,11 +4,16 @@
 $id: http://devicetree.org/schemas/iio/adc/ti,ads1298.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Texas Instruments' ads1298 medical ADC chips
+title: Texas Instruments ADS1298/ADS1299 biopotential ADC chips
 
 description: |
-  Datasheet at: https://www.ti.com/product/ADS1298
-  Bindings for this chip aren't complete.
+  ADS1298/ADS1298R: 8-channel, 24-bit ECG ADC.
+  Datasheet: https://www.ti.com/product/ADS1298
+
+  ADS1299/ADS1299-4/ADS1299-6: 8/4/6-channel, 24-bit EEG/biopotential ADC.
+  Identical SPI interface and register map to the ADS1298 family, but with
+  a higher maximum PGA gain (24x vs 12x) and fixed 2.4V internal reference.
+  Datasheet: https://www.ti.com/product/ADS1299
 
 maintainers:
   - Mike Looijmans <[email protected]>

@@ -17,6 +22,9 @@ properties:
   compatible:
     enum:
       - ti,ads1298
+      - ti,ads1299
+      - ti,ads1299-4
+      - ti,ads1299-6
 
   reg:
     maxItems: 1

@@ -33,8 +41,10 @@ properties:
 
   vref-supply:
     description:
-      Optional reference voltage. If omitted, internal reference is used,
-      which is 2.4V when analog supply is below 4.4V, 4V otherwise.
+      Optional reference voltage. If omitted, internal reference is used.
+      For ADS1298, the internal reference is 2.4V when analog supply is
+      below 4.4V, 4V otherwise. For ADS1299, the internal reference is
+      always 2.4V.
 
   clocks:
     description: Optional 2.048 MHz external source clock on CLK pin

@@ -75,5 +85,16 @@ examples:
           spi-max-frequency = <20000000>;
           spi-cpha;
         };
+
+        adc@2 {
+          reg = <2>;
+          compatible = "ti,ads1299";
+          label = "ads1299-1-eeg";
+          avdd-supply = <&reg_iso_5v_a>;
+          interrupt-parent = <&gpio0>;
+          interrupts = <79 IRQ_TYPE_EDGE_FALLING>;
+          spi-max-frequency = <20000000>;
+          spi-cpha;
+        };
     };
 ...
Signature · 1 line
2.51.1
author
#1 lore raw

PATCH 2/2 iio: adc: ti-ads1298: add ADS1299 EEG ADC family support

The ADS1299, ADS1299-4 and ADS1299-6 are 8/4/6-channel, 24-bit biopotential (EEG) ADCs from Texas Instruments. They share the same SPI interface, command set and register map as the ADS1298 ECG family, making it straightforward to support them in this driver.

Key differences from the ADS1298:
- PGA gain range is 1/2/4/6/8/12/24 (vs 6/1/2/3/4/8/12 for ADS1298)
- Internal voltage reference is always 2.4V (no 4V option)
- Channel count is encoded differently in the ID register: bits [4:3] identify the ADS1299 family (both bits set), bits [1:0] encode the channel count as (bits + 2) * 2

Detect the chip family in ads1298_init() by checking bits [4:3] of the ID register. Store the result in the is_ads1299 flag and use it to select the correct PGA table in ads1298_get_scale() and to skip the 4V reference option in the CONFIG3 initialisation.

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 drivers/iio/adc/ti-ads1298.c | 88 ++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 19 deletions(-)
drivers/iio/adc/ti-ads1298.c +69 -19
index 579200e06cbd..8c48a72e6c61 100644
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
-/* TI ADS1298 chip family driver
+/* TI ADS1298/ADS1299 biopotential ADC driver
  * Copyright (C) 2023 - 2024 Topic Embedded Products
+ * Copyright (C) 2026 Md Shofiqul Islam <[email protected]>
  */
 
 #include <linux/bitfield.h>

@@ -43,6 +44,20 @@
 #define ADS1298_MASK_ID_CHANNELS		GENMASK(2, 0)
 #define ADS1298_ID_FAMILY_ADS129X		0x90
 #define ADS1298_ID_FAMILY_ADS129XR		0xd0
+/*
+ * ADS1299 family is identified by bits [4:3] = 0b11.  Bits [7:5] encode
+ * the silicon revision and vary between production lots, so only bits [4:3]
+ * are checked for family identification.
+ */
+#define ADS1299_MASK_ID_FAMILY			GENMASK(4, 3)
+#define ADS1299_ID_FAMILY_EEG			GENMASK(4, 3)
+/*
+ * ADS1299 channel count is encoded in bits [1:0] of the ID register:
+ * 00 -> 4 channels (ADS1299-4)
+ * 01 -> 6 channels (ADS1299-6)
+ * 10 -> 8 channels (ADS1299)
+ */
+#define ADS1299_MASK_ID_CHANNELS		GENMASK(1, 0)
 
 #define ADS1298_REG_CONFIG1	0x01
 #define ADS1298_MASK_CONFIG1_HR			BIT(7)

@@ -101,6 +116,7 @@
 struct ads1298_private {
 	const struct ads1298_chip_info *chip_info;
 	struct spi_device *spi;
+	bool is_ads1299;
 	struct regulator *reg_avdd;
 	struct regulator *reg_vref;
 	struct clk *clk;

@@ -276,7 +292,10 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
 				  cfg);
 }
 
+/* ADS1298 PGA: register bits [6:4] -> gain (000=6, 001=1, 010=2, ...) */
 static const u8 ads1298_pga_settings[] = { 6, 1, 2, 3, 4, 8, 12 };
+/* ADS1299 PGA: register bits [6:4] -> gain (000=1, 001=2, 010=4, ...) */
+static const u8 ads1299_pga_settings[] = { 1, 2, 4, 6, 8, 12, 24 };
 
 static int ads1298_get_scale(struct ads1298_private *priv,
 			     int channel, int *val, int *val2)

@@ -291,12 +310,15 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 			return ret;
 
 		*val = ret / MILLI; /* Convert to millivolts */
+	} else if (priv->is_ads1299) {
+		/* ADS1299 internal reference is always 2.4V */
+		*val = 2400;
 	} else {
 		ret = regmap_read(priv->regmap, ADS1298_REG_CONFIG3, &regval);
 		if (ret)
 			return ret;
 
-		/* Reference in millivolts */
+		/* ADS1298 reference in millivolts: 2.4V or 4V */
 		*val = regval & ADS1298_MASK_CONFIG3_VREF_4V ? 4000 : 2400;
 	}
 

@@ -304,7 +326,8 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 	if (ret)
 		return ret;
 
-	gain = ads1298_pga_settings[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];
+	gain = (priv->is_ads1299 ? ads1299_pga_settings : ads1298_pga_settings)
+		[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];
 	*val /= gain; /* Full scale is VREF / gain */
 
 	*val2 = ADS1298_BITS_PER_SAMPLE - 1; /* Signed, hence the -1 */

@@ -600,20 +623,39 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	/* Fill in name and channel count based on what the chip told us */
-	indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
-	switch (val & ADS1298_MASK_ID_FAMILY) {
-	case ADS1298_ID_FAMILY_ADS129X:
-		suffix = "";
-		break;
-	case ADS1298_ID_FAMILY_ADS129XR:
-		suffix = "r";
-		break;
-	default:
-		return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+	/*
+	 * Detect chip family from the ID register.  The ADS1299 EEG family
+	 * is identified by bits [4:3] = 0b11; the ADS1298 ECG family uses
+	 * bits [7:3] for family identification.
+	 */
+	if (FIELD_GET(ADS1299_MASK_ID_FAMILY, val) == ADS1299_ID_FAMILY_EEG) {
+		/*
+		 * ADS1299 family: channel count from bits [1:0].
+		 * 00 -> 4ch, 01 -> 6ch, 10 -> 8ch.
+		 */
+		priv->is_ads1299 = true;
+		indio_dev->num_channels =
+			(FIELD_GET(ADS1299_MASK_ID_CHANNELS, val) + 2) * 2;
+		indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads1299%s",
+			indio_dev->num_channels == 4 ? "-4" :
+			indio_dev->num_channels == 6 ? "-6" : "");
+	} else {
+		/* ADS1298 family: channel count from bits [2:0], name from family */
+		indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
+		switch (val & ADS1298_MASK_ID_FAMILY) {
+		case ADS1298_ID_FAMILY_ADS129X:
+			suffix = "";
+			break;
+		case ADS1298_ID_FAMILY_ADS129XR:
+			suffix = "r";
+			break;
+		default:
+			return dev_err_probe(dev, -ENODEV,
+					     "Unknown ID: 0x%x\n", val);
+		}
+		indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads129%u%s",
+						 indio_dev->num_channels, suffix);
 	}
-	indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads129%u%s",
-					 indio_dev->num_channels, suffix);
 	if (!indio_dev->name)
 		return -ENOMEM;
 

@@ -621,8 +663,9 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	if (!priv->reg_vref) {
 		/* Enable internal reference */
 		val |= ADS1298_MASK_CONFIG3_PWR_REFBUF;
-		/* Use 4V VREF when power supply is at least 4.4V */
-		if (regulator_get_voltage(priv->reg_avdd) >= 4400000)
+		/* ADS1298 only: use 4V VREF when supply is at least 4.4V */
+		if (!priv->is_ads1299 &&
+		    regulator_get_voltage(priv->reg_avdd) >= 4400000)
 			val |= ADS1298_MASK_CONFIG3_VREF_4V;
 	}
 	return regmap_write(priv->regmap, ADS1298_REG_CONFIG3, val);

@@ -739,12 +782,18 @@ static int ads1298_probe(struct spi_device *spi)
 
 static const struct spi_device_id ads1298_id[] = {
 	{ "ads1298" },
+	{ "ads1299" },
+	{ "ads1299-4" },
+	{ "ads1299-6" },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ads1298_id);
 
 static const struct of_device_id ads1298_of_table[] = {
 	{ .compatible = "ti,ads1298" },
+	{ .compatible = "ti,ads1299" },
+	{ .compatible = "ti,ads1299-4" },
+	{ .compatible = "ti,ads1299-6" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ads1298_of_table);

@@ -760,5 +809,6 @@ static struct spi_driver ads1298_driver = {
 module_spi_driver(ads1298_driver);
 
 MODULE_AUTHOR("Mike Looijmans <[email protected]>");
-MODULE_DESCRIPTION("TI ADS1298 ADC");
+MODULE_AUTHOR("Md Shofiqul Islam <[email protected]>");
+MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");
 MODULE_LICENSE("GPL");
Signature · 1 line
2.51.1
AI review
#2 lore raw

Re: [PATCH 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The ADS1299 chip family detection condition will always evaluate to false, rendering the ADS1299 support unreachable dead code.

Pre-existing issues:
- [High] Lack of upper bound validation on the device-provided channel count leads to array out-of-bounds reads and heap buffer overflows.
- [High] The driver reads PGA gain settings from static arrays using an unvalidated hardware-provided index, leading to out-of-bounds memory reads and potential division by zero panics.
- [High] The driver fails to cancel in-flight asynchronous SPI transfers on device unbind, leading to a Use-After-Free.
--

commit bae6bfa79577c0e4b004da3b94f146ed2ea3105d
Author: Md Shofiqul Islam <[email protected]>

iio: adc: ti-ads1298: add ADS1299 EEG ADC family support

This patch adds support for the Texas Instruments ADS1299 EEG ADC family. It adds the necessary device IDs and detection logic to distinguish the ADS1299 from the ADS1298 family and configure the appropriate PGA settings and voltage references.

Quoted 4 lines
drivers/iio/adc/ti-ads1298.c +0 -0
index ae30b47e45142..5aae67a60519e 100644

[ ... ]

drivers/iio/adc/ti-ads1298.c line 53

Quoted 11 lines
drivers/iio/adc/ti-ads1298.c +7 -0
@@ -43,6 +44,20 @@
 #define ADS1298_MASK_ID_CHANNELS		GENMASK(2, 0)
 #define ADS1298_ID_FAMILY_ADS129X		0x90
 #define ADS1298_ID_FAMILY_ADS129XR		0xd0
+/*
+ * ADS1299 family is identified by bits [4:3] = 0b11.  Bits [7:5] encode
+ * the silicon revision and vary between production lots, so only bits [4:3]
+ * are checked for family identification.
+ */
+#define ADS1299_MASK_ID_FAMILY			GENMASK(4, 3)
+#define ADS1299_ID_FAMILY_EEG			GENMASK(4, 3)

[Severity: High]
Does this define the comparison constant as an unshifted bitmask, which makes the equality check impossible to satisfy?

[ ... ]

Quoted 18 lines
drivers/iio/adc/ti-ads1298.c +4 -1
@@ -289,12 +308,15 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 			return ret;
 
 		*val = ret / MILLI; /* Convert to millivolts */
+	} else if (priv->is_ads1299) {
+		/* ADS1299 internal reference is always 2.4V */
+		*val = 2400;
 	} else {
 		ret = regmap_read(priv->regmap, ADS1298_REG_CONFIG3, &regval);
 		if (ret)
 			return ret;
 
-		/* Reference in millivolts */
+		/* ADS1298 reference in millivolts: 2.4V or 4V */
 		*val = regval & ADS1298_MASK_CONFIG3_VREF_4V ? 4000 : 2400;
 	}

drivers/iio/adc/ti-ads1298.c static int ads1298_get_scale(struct ads1298_private *priv, · line 328

Quoted 6 lines
drivers/iio/adc/ti-ads1298.c +2 -1
@@ -302,7 +324,8 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 	if (ret)
 		return ret;
 
-	gain = ads1298_pga_settings[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];
+	gain = (priv->is_ads1299 ? ads1299_pga_settings : ads1298_pga_settings)
+		[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];

[Severity: High]
This isn't a bug introduced by this patch, but can this lead to an out-of-bounds array read and potential division by zero?

The ads1298_pga_settings and ads1299_pga_settings arrays both contain 7 elements. FIELD_GET(ADS1298_MASK_CH_PGA, regval) extracts the PGA index from the hardware register, yielding a value from 0 to 7. If the hardware register value is 7, it reads past the end of the array.

The read out-of-bounds byte is then used as a divisor. If the byte happens to be 0, it can trigger a kernel panic via division by zero.

[ ... ]

Quoted 7 lines
drivers/iio/adc/ti-ads1298.c +0 -3
@@ -598,20 +621,39 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	/* Fill in name and channel count based on what the chip told us */
-	indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
-	switch (val & ADS1298_MASK_ID_FAMILY) {

[ ... ]

drivers/iio/adc/ti-ads1298.c static int ads1298_init(struct iio_dev *indio_dev) · line 631

Quoted 6 lines
drivers/iio/adc/ti-ads1298.c +6 -0
+	/*
+	 * Detect chip family from the ID register.  The ADS1299 EEG family
+	 * is identified by bits [4:3] = 0b11; the ADS1298 ECG family uses
+	 * bits [7:3] for family identification.
+	 */
+	if (FIELD_GET(ADS1299_MASK_ID_FAMILY, val) == ADS1299_ID_FAMILY_EEG) {

[Severity: High]
Will this condition always evaluate to false?

FIELD_GET() extracts bits 4 and 3 and shifts them right, yielding a value between 0 and 3. However, ADS1299_ID_FAMILY_EEG is defined as GENMASK(4, 3) which evaluates to the unshifted integer value 24 (0x18).

Comparing a shifted value to 24 will prevent ADS1299 detection, causing valid devices to fall through to the ADS1298 logic and fail to probe with an Unknown ID error.

drivers/iio/adc/ti-ads1298.c static int ads1298_init(struct iio_dev *indio_dev) · line 638

Quoted 7 lines
drivers/iio/adc/ti-ads1298.c +7 -0
+		/*
+		 * ADS1299 family: channel count from bits [1:0].
+		 * 00 -> 4ch, 01 -> 6ch, 10 -> 8ch.
+		 */
+		priv->is_ads1299 = true;
+		indio_dev->num_channels =
+			(FIELD_GET(ADS1299_MASK_ID_CHANNELS, val) + 2) * 2;

[Severity: High]
This is a pre-existing issue, but does calculating num_channels directly from the hardware ID register without upper bounds validation lead to overflows?

If a faulty SPI device returns an unexpected ID, num_channels could exceed ADS1298_MAX_CHANNELS (8). This unbounded value can cause priv->rdata_xfer.len to exceed the fixed sizes of tx_buffer and rx_buffer configured in ads1298_probe(), resulting in out-of-bounds DMA transfers and heap buffer overflows when demuxing data.

[ ... ]

[Severity: High]
This is another pre-existing issue, but does the driver fail to cancel in-flight asynchronous SPI transfers on device unbind?

Looking at ads1298_rdata_release_busy_or_restart() in drivers/iio/adc/ti-ads1298.c:

    spi_async(priv->spi, &priv->rdata_msg);

If the driver is unbound and resources are released without cancelling the transfer, the pending SPI transfer can outlive the unbind. The SPI core would then invoke the completion callback:

    ads1298_rdata_complete(void *context) {
        struct iio_dev *indio_dev = context;
        ...

This would dereference the freed indio_dev pointer, leading to a use-after-free memory corruption.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
reviewer
#3 lore raw

Re: [PATCH 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support

In reply to Md Shofiqul Islam

Please add a cover letter for multi-patch series.

Quoted 57 lines · Md Shofiqul Islam

The ADS1299, ADS1299-4 and ADS1299-6 are 8/4/6-channel, 24-bit biopotential (EEG) ADCs from Texas Instruments. They share the same SPI interface, command set and register map as the ADS1298 ECG family, making it straightforward to support them in this driver.

Key differences from the ADS1298:
- PGA gain range is 1/2/4/6/8/12/24 (vs 6/1/2/3/4/8/12 for ADS1298)
- Internal voltage reference is always 2.4V (no 4V option)
- Channel count is encoded differently in the ID register: bits [4:3] identify the ADS1299 family (both bits set), bits [1:0] encode the channel count as (bits + 2) * 2

Detect the chip family in ads1298_init() by checking bits [4:3] of the ID register. Store the result in the is_ads1299 flag and use it to select the correct PGA table in ads1298_get_scale() and to skip the 4V reference option in the CONFIG3 initialisation.

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 drivers/iio/adc/ti-ads1298.c | 88 ++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 19 deletions(-)
drivers/iio/adc/ti-ads1298.c +16 -1
index 579200e06cbd..8c48a72e6c61 100644
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
-/* TI ADS1298 chip family driver
+/* TI ADS1298/ADS1299 biopotential ADC driver
  * Copyright (C) 2023 - 2024 Topic Embedded Products
+ * Copyright (C) 2026 Md Shofiqul Islam <[email protected]>
  */
 
 #include <linux/bitfield.h>

@@ -43,6 +44,20 @@
 #define ADS1298_MASK_ID_CHANNELS		GENMASK(2, 0)
 #define ADS1298_ID_FAMILY_ADS129X		0x90
 #define ADS1298_ID_FAMILY_ADS129XR		0xd0
+/*
+ * ADS1299 family is identified by bits [4:3] = 0b11.  Bits [7:5] encode
+ * the silicon revision and vary between production lots, so only bits [4:3]
+ * are checked for family identification.
+ */
+#define ADS1299_MASK_ID_FAMILY			GENMASK(4, 3)
+#define ADS1299_ID_FAMILY_EEG			GENMASK(4, 3)
+/*
+ * ADS1299 channel count is encoded in bits [1:0] of the ID register:
+ * 00 -> 4 channels (ADS1299-4)
+ * 01 -> 6 channels (ADS1299-6)
+ * 10 -> 8 channels (ADS1299)
+ */
+#define ADS1299_MASK_ID_CHANNELS		GENMASK(1, 0)
 
 #define ADS1298_REG_CONFIG1	0x01
 #define ADS1298_MASK_CONFIG1_HR			BIT(7)

drivers/iio/adc/ti-ads1298.c line 119

Quoted 4 lines
drivers/iio/adc/ti-ads1298.c +1 -0
@@ -101,6 +116,7 @@
 struct ads1298_private {
 	const struct ads1298_chip_info *chip_info;
 	struct spi_device *spi;
+	bool is_ads1299;

This is not how we handle chip-specific differences in IIO. Instead we should add a new chip info struct and pass that as the data in the module device tables.

This new struct should have a flag, e.g. bool is_fixed_internal_ref and const u8 *pga_settings that is a pointer to the appropriate array.

Quoted 15 lines
drivers/iio/adc/ti-ads1298.c +3 -0
 	struct regulator *reg_avdd;
 	struct regulator *reg_vref;
 	struct clk *clk;

@@ -276,7 +292,10 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
 				  cfg);
 }
 
+/* ADS1298 PGA: register bits [6:4] -> gain (000=6, 001=1, 010=2, ...) */
 static const u8 ads1298_pga_settings[] = { 6, 1, 2, 3, 4, 8, 12 };
+/* ADS1299 PGA: register bits [6:4] -> gain (000=1, 001=2, 010=4, ...) */
+static const u8 ads1299_pga_settings[] = { 1, 2, 4, 6, 8, 12, 24 };
 
 static int ads1298_get_scale(struct ads1298_private *priv,
 			     int channel, int *val, int *val2)

drivers/iio/adc/ti-ads1298.c static int ads1298_get_scale(struct ads1298_private *priv, · line 313

Quoted 4 lines
drivers/iio/adc/ti-ads1298.c +1 -0
@@ -291,12 +310,15 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 			return ret;
 
 		*val = ret / MILLI; /* Convert to millivolts */
+	} else if (priv->is_ads1299) {

So this would change to:

	} else if (priv->info->is_fixed_internal_ref) {
Quoted 13 lines
drivers/iio/adc/ti-ads1298.c +3 -1
+		/* ADS1299 internal reference is always 2.4V */
+		*val = 2400;
 	} else {
 		ret = regmap_read(priv->regmap, ADS1298_REG_CONFIG3, &regval);
 		if (ret)
 			return ret;
 
-		/* Reference in millivolts */
+		/* ADS1298 reference in millivolts: 2.4V or 4V */
 		*val = regval & ADS1298_MASK_CONFIG3_VREF_4V ? 4000 : 2400;
 	}

drivers/iio/adc/ti-ads1298.c static int ads1298_get_scale(struct ads1298_private *priv, · line 330

Quoted 6 lines
drivers/iio/adc/ti-ads1298.c +2 -1
@@ -304,7 +326,8 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 	if (ret)
 		return ret;
 
-	gain = ads1298_pga_settings[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];
+	gain = (priv->is_ads1299 ? ads1299_pga_settings : ads1298_pga_settings)
+		[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];

And this would be:

	gain = priv->info->pga_settings[FIELD_GET(ADS1298_MASK_CH_PGA, regval)];
Quoted 21 lines
drivers/iio/adc/ti-ads1298.c +3 -11
 	*val /= gain; /* Full scale is VREF / gain */
 
 	*val2 = ADS1298_BITS_PER_SAMPLE - 1; /* Signed, hence the -1 */

@@ -600,20 +623,39 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	/* Fill in name and channel count based on what the chip told us */
-	indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
-	switch (val & ADS1298_MASK_ID_FAMILY) {
-	case ADS1298_ID_FAMILY_ADS129X:
-		suffix = "";
-		break;
-	case ADS1298_ID_FAMILY_ADS129XR:
-		suffix = "r";
-		break;
-	default:
-		return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+	/*
+	 * Detect chip family from the ID register.  The ADS1299 EEG family
+	 * is identified by bits [4:3] = 0b11; the ADS1298 ECG family uses

drivers/iio/adc/ti-ads1298.c static int ads1298_init(struct iio_dev *indio_dev) · line 638

Quoted 10 lines
drivers/iio/adc/ti-ads1298.c +10 -0
+	 * bits [7:3] for family identification.
+	 */
+	if (FIELD_GET(ADS1299_MASK_ID_FAMILY, val) == ADS1299_ID_FAMILY_EEG) {
+		/*
+		 * ADS1299 family: channel count from bits [1:0].
+		 * 00 -> 4ch, 01 -> 6ch, 10 -> 8ch.
+		 */
+		priv->is_ads1299 = true;
+		indio_dev->num_channels =
+			(FIELD_GET(ADS1299_MASK_ID_CHANNELS, val) + 2) * 2;

Number of channels should come from the new chip info based on the compatible string as well.

drivers/iio/adc/ti-ads1298.c static int ads1298_init(struct iio_dev *indio_dev) · line 641

Quoted 3 lines
drivers/iio/adc/ti-ads1298.c +3 -0
+		indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads1299%s",
+			indio_dev->num_channels == 4 ? "-4" :
+			indio_dev->num_channels == 6 ? "-6" : "");

Generally, we put the chip name in the chip info as well.

Quoted 23 lines
drivers/iio/adc/ti-ads1298.c +16 -2
+	} else {
+		/* ADS1298 family: channel count from bits [2:0], name from family */
+		indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
+		switch (val & ADS1298_MASK_ID_FAMILY) {
+		case ADS1298_ID_FAMILY_ADS129X:
+			suffix = "";
+			break;
+		case ADS1298_ID_FAMILY_ADS129XR:
+			suffix = "r";
+			break;
+		default:
+			return dev_err_probe(dev, -ENODEV,
+					     "Unknown ID: 0x%x\n", val);
+		}
+		indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads129%u%s",
+						 indio_dev->num_channels, suffix);
 	}
-	indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads129%u%s",
-					 indio_dev->num_channels, suffix);
 	if (!indio_dev->name)
 		return -ENOMEM;

drivers/iio/adc/ti-ads1298.c static int ads1298_init(struct iio_dev *indio_dev) · line 667

Quoted 7 lines
drivers/iio/adc/ti-ads1298.c +2 -2
@@ -621,8 +663,9 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	if (!priv->reg_vref) {
 		/* Enable internal reference */
 		val |= ADS1298_MASK_CONFIG3_PWR_REFBUF;
-		/* Use 4V VREF when power supply is at least 4.4V */
-		if (regulator_get_voltage(priv->reg_avdd) >= 4400000)
+		/* ADS1298 only: use 4V VREF when supply is at least 4.4V */
+		if (!priv->is_ads1299 &&

And of course use priv->info->is_fixed_internal_ref here too.

Quoted 31 lines
drivers/iio/adc/ti-ads1298.c +9 -1
+		    regulator_get_voltage(priv->reg_avdd) >= 4400000)
 			val |= ADS1298_MASK_CONFIG3_VREF_4V;
 	}
 	return regmap_write(priv->regmap, ADS1298_REG_CONFIG3, val);

@@ -739,12 +782,18 @@ static int ads1298_probe(struct spi_device *spi)
 
 static const struct spi_device_id ads1298_id[] = {
 	{ "ads1298" },
+	{ "ads1299" },
+	{ "ads1299-4" },
+	{ "ads1299-6" },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ads1298_id);
 
 static const struct of_device_id ads1298_of_table[] = {
 	{ .compatible = "ti,ads1298" },
+	{ .compatible = "ti,ads1299" },
+	{ .compatible = "ti,ads1299-4" },
+	{ .compatible = "ti,ads1299-6" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ads1298_of_table);

@@ -760,5 +809,6 @@ static struct spi_driver ads1298_driver = {
 module_spi_driver(ads1298_driver);
 
 MODULE_AUTHOR("Mike Looijmans <[email protected]>");
-MODULE_DESCRIPTION("TI ADS1298 ADC");
+MODULE_AUTHOR("Md Shofiqul Islam <[email protected]>");
+MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");
 MODULE_LICENSE("GPL");
#4 lore raw

Re: [PATCH 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support

In reply to Md Shofiqul Islam
Quoted 16 lines · Md Shofiqul Islam

The ADS1299, ADS1299-4 and ADS1299-6 are 8/4/6-channel, 24-bit biopotential (EEG) ADCs from Texas Instruments. They share the same SPI interface, command set and register map as the ADS1298 ECG family, making it straightforward to support them in this driver.

Key differences from the ADS1298:
- PGA gain range is 1/2/4/6/8/12/24 (vs 6/1/2/3/4/8/12 for ADS1298)
- Internal voltage reference is always 2.4V (no 4V option)
- Channel count is encoded differently in the ID register: bits [4:3] identify the ADS1299 family (both bits set), bits [1:0] encode the channel count as (bits + 2) * 2

Detect the chip family in ads1298_init() by checking bits [4:3] of the ID register. Store the result in the is_ads1299 flag and use it to select the correct PGA table in ads1298_get_scale() and to skip the 4V reference option in the CONFIG3 initialisation.

...

drivers/iio/adc/ti-ads1298.c line 2

Quoted 2 lines
drivers/iio/adc/ti-ads1298.c +1 -1
-/* TI ADS1298 chip family driver
+/* TI ADS1298/ADS1299 biopotential ADC driver

Consider updating Kconfig help text as well.

...

drivers/iio/adc/ti-ads1298.c line 122

Quoted 7 lines

struct ads1298_private {

drivers/iio/adc/ti-ads1298.c +1 -0
 	const struct ads1298_chip_info *chip_info;
 	struct spi_device *spi;
+	bool is_ads1299;
 	struct regulator *reg_avdd;
 	struct regulator *reg_vref;
 	struct clk *clk;

When add a new member or create a new data structure always consult with pahole tool on the layout.

Quoted 1 line

}

...

drivers/iio/adc/ti-ads1298.c static struct spi_driver ads1298_driver = { · line 813

Quoted 2 lines
drivers/iio/adc/ti-ads1298.c +1 -1
-MODULE_DESCRIPTION("TI ADS1298 ADC");
+MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");

This doesn't scale, rephrase here and in top comment in the file (and probably in the Kconfig title) to make it stretchy. See the history of the last few months of IIO subsystem development, you will find examples on how to achieve that,

Signature · 2 lines
With Best Regards,
Andy Shevchenko
reviewer
#5 lore raw
In reply to Md Shofiqul Islam
Quoted 49 lines · Md Shofiqul Islam

The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface and register map as the ADS1298 ECG ADC family but are designed for EEG and biopotential measurement. Key differences from ADS1298:
- PGA gain up to 24x (vs 12x for ADS1298)
- Internal reference is always 2.4V (no 4V option)

Add compatible strings and update the vref-supply description to document the ADS1299 reference voltage behaviour.

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +11 -3
index 71f9f9b745cb..82c5181126df 100644
@@ -4,11 +4,16 @@
 $id: http://devicetree.org/schemas/iio/adc/ti,ads1298.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Texas Instruments' ads1298 medical ADC chips
+title: Texas Instruments ADS1298/ADS1299 biopotential ADC chips
 
 description: |
-  Datasheet at: https://www.ti.com/product/ADS1298
-  Bindings for this chip aren't complete.
+  ADS1298/ADS1298R: 8-channel, 24-bit ECG ADC.
+  Datasheet: https://www.ti.com/product/ADS1298
+
+  ADS1299/ADS1299-4/ADS1299-6: 8/4/6-channel, 24-bit EEG/biopotential ADC.
+  Identical SPI interface and register map to the ADS1298 family, but with
+  a higher maximum PGA gain (24x vs 12x) and fixed 2.4V internal reference.
+  Datasheet: https://www.ti.com/product/ADS1299
 
 maintainers:
   - Mike Looijmans <[email protected]>

@@ -17,6 +22,9 @@ properties:
   compatible:
     enum:
       - ti,ads1298
+      - ti,ads1299
+      - ti,ads1299-4
+      - ti,ads1299-6
 
   reg:
     maxItems: 1

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml properties: · line 47

Quoted 9 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +4 -2
@@ -33,8 +41,10 @@ properties:
 
   vref-supply:
     description:
-      Optional reference voltage. If omitted, internal reference is used,
-      which is 2.4V when analog supply is below 4.4V, 4V otherwise.
+      Optional reference voltage. If omitted, internal reference is used.
+      For ADS1298, the internal reference is 2.4V when analog supply is
+      below 4.4V, 4V otherwise. For ADS1299, the internal reference is
+      always 2.4V.

Not sure the dt-bindings care about the internal reference. It should just be a driver implementation detail.

So I would just trim this down to:

Optional reference voltage. If omitted, internal reference is used.

Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +1 -0
   clocks:
     description: Optional 2.048 MHz external source clock on CLK pin

@@ -75,5 +85,16 @@ examples:
           spi-max-frequency = <20000000>;
           spi-cpha;
         };
+

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml examples: · line 98

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +10 -0
+        adc@2 {
+          reg = <2>;
+          compatible = "ti,ads1299";
+          label = "ads1299-1-eeg";
+          avdd-supply = <&reg_iso_5v_a>;
+          interrupt-parent = <&gpio0>;
+          interrupts = <79 IRQ_TYPE_EDGE_FALLING>;
+          spi-max-frequency = <20000000>;
+          spi-cpha;
+        };

Since the chips are nearly identical, not sure a 2nd example is particularly useful.

Quoted 2 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +0 -0
     };
...
maintainer
#6 lore raw
Quoted 26 lines · On Tue, 30 Jun 2026 10:24:19 -0500 David Lechner <[email protected]> wrote:
Quoted 25 lines · Md Shofiqul Islam

The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface and register map as the ADS1298 ECG ADC family but are designed for EEG and biopotential measurement. Key differences from ADS1298:
- PGA gain up to 24x (vs 12x for ADS1298)
- Internal reference is always 2.4V (no 4V option)

Add compatible strings and update the vref-supply description to document the ADS1299 reference voltage behaviour.

Signed-off-by Md Shofiqul Islam <[email protected]>
---
 .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +1 -1
index 71f9f9b745cb..82c5181126df 100644
@@ -4,11 +4,16 @@
 $id: http://devicetree.org/schemas/iio/adc/ti,ads1298.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Texas Instruments' ads1298 medical ADC chips
+title: Texas Instruments ADS1298/ADS1299 biopotential ADC chips
Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml line 16

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +7 -2
 description: |
-  Datasheet at: https://www.ti.com/product/ADS1298
-  Bindings for this chip aren't complete.
+  ADS1298/ADS1298R: 8-channel, 24-bit ECG ADC.
+  Datasheet: https://www.ti.com/product/ADS1298
+
+  ADS1299/ADS1299-4/ADS1299-6: 8/4/6-channel, 24-bit EEG/biopotential ADC.
+  Identical SPI interface and register map to the ADS1298 family, but with
+  a higher maximum PGA gain (24x vs 12x) and fixed 2.4V internal reference.
+  Datasheet: https://www.ti.com/product/ADS1299

Further to David's comment below about not caring about the internal reference details, I'd drop that bit from here as well.

If you want to differentiate the parts (I'm not sure it's necessary) then say what each one does as that will be more extensible than starting from a base then expanding.

Something like:
ADS1298/ADS1298R: 8-channel, 24-bit ECG ADC. Max 12x PGA gain.
Datasheet: https://www.ti.com/product/ADS1298

ADS1299/ADS1299-4/ADS1299-6: 8/4/6-channel, 24-bit EEG/biopotential ADC. Max 24x PGA gain.
Datasheet: https://www.ti.com/product/ADS1299

(check how that gets formatted and edit as appropriate)

Quoted 56 lines
Quoted 14 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +3 -0
 maintainers:
   - Mike Looijmans <[email protected]>

@@ -17,6 +22,9 @@ properties:
   compatible:
     enum:
       - ti,ads1298
+      - ti,ads1299
+      - ti,ads1299-4
+      - ti,ads1299-6
 
   reg:
     maxItems: 1

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml properties: · line 47

Quoted 9 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +4 -2
@@ -33,8 +41,10 @@ properties:
 
   vref-supply:
     description:
-      Optional reference voltage. If omitted, internal reference is used,
-      which is 2.4V when analog supply is below 4.4V, 4V otherwise.
+      Optional reference voltage. If omitted, internal reference is used.
+      For ADS1298, the internal reference is 2.4V when analog supply is
+      below 4.4V, 4V otherwise. For ADS1299, the internal reference is
+      always 2.4V.

Not sure the dt-bindings care about the internal reference. It should just be a driver implementation detail.

So I would just trim this down to:

Optional reference voltage. If omitted, internal reference is used.

Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +1 -0
   clocks:
     description: Optional 2.048 MHz external source clock on CLK pin

@@ -75,5 +85,16 @@ examples:
           spi-max-frequency = <20000000>;
           spi-cpha;
         };
+

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml examples: · line 98

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +10 -0
+        adc@2 {
+          reg = <2>;
+          compatible = "ti,ads1299";
+          label = "ads1299-1-eeg";
+          avdd-supply = <&reg_iso_5v_a>;
+          interrupt-parent = <&gpio0>;
+          interrupts = <79 IRQ_TYPE_EDGE_FALLING>;
+          spi-max-frequency = <20000000>;
+          spi-cpha;
+        };

Since the chips are nearly identical, not sure a 2nd example is particularly useful.

Quoted 2 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +0 -0
     };
...
maintainer
#7 lore raw
In reply to Md Shofiqul Islam
Quoted 15 lines · On Tue, 30 Jun 2026 17:03:10 +0300 Md Shofiqul Islam <[email protected]>…

The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface and register map as the ADS1298 ECG ADC family but are designed for EEG and biopotential measurement. Key differences from ADS1298:
- PGA gain up to 24x (vs 12x for ADS1298)
- Internal reference is always 2.4V (no 4V option)

Add compatible strings and update the vref-supply description to document the ADS1299 reference voltage behaviour.

Signed-off-by Md Shofiqul Islam <[email protected]>

maintainers:
- Mike Looijmans <[email protected]>

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml properties: · line 27

Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +3 -0
@@ -17,6 +22,9 @@ properties:
   compatible:
     enum:
       - ti,ads1298
+      - ti,ads1299
+      - ti,ads1299-4
+      - ti,ads1299-6

As the channel count is a discoverable feature (encoded as a field in the ID Register, I think we can use just the one compatible). FWIW it is nice to see a manufacturer making this discoverable! I'd expect any future drop in parts to vary the other part of the ID, but this field should be respected - as such this doesn't fall into the stuff I've been discussing with DT maintainers about fallback compatibles needing to be specific (I'm way behind so not sure where that thread got to!)

Jonathan

#8 lore raw
Quoted 25 lines · Jonathan Cameron
Quoted 15 lines · On Tue, 30 Jun 2026 17:03:10 +0300 Md Shofiqul Islam <[email protected]>…

The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface and register map as the ADS1298 ECG ADC family but are designed for EEG and biopotential measurement. Key differences from ADS1298:
- PGA gain up to 24x (vs 12x for ADS1298)
- Internal reference is always 2.4V (no 4V option)

Add compatible strings and update the vref-supply description to document the ADS1299 reference voltage behaviour.

Signed-off-by Md Shofiqul Islam <[email protected]>

maintainers:
- Mike Looijmans <[email protected]>

Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml properties: · line 27

Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml +3 -0
@@ -17,6 +22,9 @@ properties:
   compatible:
     enum:
       - ti,ads1298
+      - ti,ads1299
+      - ti,ads1299-4
+      - ti,ads1299-6
Quoted 7 lines

As the channel count is a discoverable feature (encoded as a field in the ID Register, I think we can use just the one compatible). FWIW it is nice to see a manufacturer making this discoverable! I'd expect any future drop in parts to vary the other part of the ID, but this field should be respected - as such this doesn't fall into the stuff I've been discussing with DT maintainers about fallback compatibles needing to be specific (I'm way behind so not sure where that thread got to!)

I forget that thread too, but I think the difference is discoverable ID versus discoverable capabilities. If you match on a generic compatible with a new discoverable ID you've got no idea how to handle it. Matching on an existing specific compatible with a new ID, you treat it as the existing device. Matching on a generic compatible with the ability to discover the channel count, you know what to do.