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

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

[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 <shofiqtest@gmail.com>
---
 .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
index 71f9f9b745cb..82c5181126df 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
@@ -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 <mike.looijmans@topic.nl>
@@ -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;
+        };
     };
 ...
-- 
2.51.1

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

[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 <shofiqtest@gmail.com>
---
 drivers/iio/adc/ti-ads1298.c | 88 ++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 579200e06cbd..8c48a72e6c61 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -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 <shofiqtest@gmail.com>
  */
 
 #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 <mike.looijmans@topic.nl>");
-MODULE_DESCRIPTION("TI ADS1298 ADC");
+MODULE_AUTHOR("Md Shofiqul Islam <shofiqtest@gmail.com>");
+MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");
 MODULE_LICENSE("GPL");
-- 
2.51.1

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

[sashiko-bot]

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 <shofiqtest@gmail.com>

    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.

> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index ae30b47e45142..5aae67a60519e 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
[ ... ]
> @@ -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?

[ ... ]
> @@ -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;
>  	}
>  
> @@ -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.

[ ... ]
> @@ -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) {
[ ... ]
> +	/*
> +	 * 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.

> +		/*
> +		 * 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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630140311.1473031-1-shofiqtest@gmail.com?part=2

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

[David Lechner]

On 6/30/26 9:03 AM, Md Shofiqul Islam wrote:
> 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 <shofiqtest@gmail.com>
> ---
>  .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> index 71f9f9b745cb..82c5181126df 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> @@ -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 <mike.looijmans@topic.nl>
> @@ -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.

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.

>  
>    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;
> +        };

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

>      };
>  ...

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

[David Lechner]

Please add a cover letter for multi-patch series.

On 6/30/26 9:03 AM, Md Shofiqul Islam wrote:
> 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 <shofiqtest@gmail.com>
> ---
>  drivers/iio/adc/ti-ads1298.c | 88 ++++++++++++++++++++++++++++--------
>  1 file changed, 69 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index 579200e06cbd..8c48a72e6c61 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
> @@ -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 <shofiqtest@gmail.com>
>   */
>  
>  #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;

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.

>  	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) {

So this would change to:

	} else if (priv->info->is_fixed_internal_ref) {

> +		/* 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)];

And this would be:

	gain = priv->info->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;

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

> +		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.

> +	} 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 &&

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

> +		    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 <mike.looijmans@topic.nl>");
> -MODULE_DESCRIPTION("TI ADS1298 ADC");
> +MODULE_AUTHOR("Md Shofiqul Islam <shofiqtest@gmail.com>");
> +MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");
>  MODULE_LICENSE("GPL");

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

[Jonathan Cameron]

On Tue, 30 Jun 2026 10:24:19 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 6/30/26 9:03 AM, Md Shofiqul Islam wrote:
> > 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 <shofiqtest@gmail.com>
> > ---
> >  .../bindings/iio/adc/ti,ads1298.yaml          | 31 ++++++++++++++++---
> >  1 file changed, 26 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> > index 71f9f9b745cb..82c5181126df 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> > @@ -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
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)

> >  
> >  maintainers:
> >    - Mike Looijmans <mike.looijmans@topic.nl>
> > @@ -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.  
> 
> 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.
> 
> >  
> >    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;
> > +        };  
> 
> Since the chips are nearly identical, not sure a 2nd example is
> particularly useful.
> 
> >      };
> >  ...  
> 

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

[Jonathan Cameron]

On Tue, 30 Jun 2026 17:03:10 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> wrote:

> 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 <shofiqtest@gmail.com>

>  
>  maintainers:
>    - Mike Looijmans <mike.looijmans@topic.nl>
> @@ -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

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

[Andy Shevchenko]

On Tue, Jun 30, 2026 at 05:03:11PM +0300, Md Shofiqul Islam wrote:
> 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.

...

> -/* TI ADS1298 chip family driver
> +/* TI ADS1298/ADS1299 biopotential ADC driver

Consider updating Kconfig help text as well.

...

>  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;

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

>  }

...

> -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,

-- 
With Best Regards,
Andy Shevchenko

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

[Conor Dooley]

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

On Wed, Jul 01, 2026 at 12:14:28AM +0100, Jonathan Cameron wrote:
> On Tue, 30 Jun 2026 17:03:10 +0300
> Md Shofiqul Islam <shofiqtest@gmail.com> wrote:
> 
> > 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 <shofiqtest@gmail.com>
> 
> >  
> >  maintainers:
> >    - Mike Looijmans <mike.looijmans@topic.nl>
> > @@ -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!)

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.


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