* [PATCH v2 0/2] iio: adc: ti-ads1298: add ADS1298/ADS1299 driver
@ 2026-07-06 11:26 Md Shofiqul Islam
2026-07-06 11:26 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants Md Shofiqul Islam
2026-07-06 11:26 ` [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support Md Shofiqul Islam
0 siblings, 2 replies; 6+ messages in thread
From: Md Shofiqul Islam @ 2026-07-06 11:26 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, lars, linux-kernel, Md Shofiqul Islam
The ADS1299 is an 8-channel, 24-bit SPI ADC from Texas Instruments
designed for EEG and biopotential measurement. It shares the same SPI
interface and register map as the ADS1298 ECG ADC family, differing in
PGA gain range (24x vs 12x) and internal reference voltage (always 2.4V).
This series adds ADS1299 support to the existing ti-ads1298 driver by
extending the channel configuration and adding the compatible string to
the device tree binding.
Changes in v2:
- Remove ads1299-4 and ads1299-6 variants; only ads1299 (8-channel)
is supported. The -4 and -6 variants have not been tested on hardware
and their channel-count difference is better handled at runtime via
the ID register rather than separate compatibles.
- Remove second DT binding example (ads1299 node); one example is
sufficient per binding documentation guidelines.
- Simplify vref-supply description; remove ADS1298 vs ADS1299 specifics
that belong in the datasheet rather than the binding.
Md Shofiqul Islam (2):
dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants
iio: adc: ti-ads1298: add ADS1299 EEG ADC family support
.../bindings/iio/adc/ti,ads1298.yaml | 15 ++--
drivers/iio/adc/ti-ads1298.c | 84 ++++++++++++++-----
2 files changed, 75 insertions(+), 24 deletions(-)
--
2.51.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants
2026-07-06 11:26 [PATCH v2 0/2] iio: adc: ti-ads1298: add ADS1298/ADS1299 driver Md Shofiqul Islam
@ 2026-07-06 11:26 ` Md Shofiqul Islam
2026-07-07 6:21 ` Krzysztof Kozlowski
2026-07-11 23:21 ` Jonathan Cameron
2026-07-06 11:26 ` [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support Md Shofiqul Islam
1 sibling, 2 replies; 6+ messages in thread
From: Md Shofiqul Islam @ 2026-07-06 11:26 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, lars, linux-kernel, 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>
---
.../devicetree/bindings/iio/adc/ti,ads1298.yaml | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
index 71f9f9b745cbc..202776cee73ff 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
@@ -4,11 +4,15 @@
$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. Max 12x PGA gain.
+ Datasheet: https://www.ti.com/product/ADS1298
+
+ ADS1299: 4/6/8-channel, 24-bit EEG/biopotential ADC. Max 24x PGA gain.
+ Channel count is discoverable from the ID register at runtime.
+ Datasheet: https://www.ti.com/product/ADS1299
maintainers:
- Mike Looijmans <mike.looijmans@topic.nl>
@@ -17,6 +21,7 @@ properties:
compatible:
enum:
- ti,ads1298
+ - ti,ads1299
reg:
maxItems: 1
@@ -33,8 +38,7 @@ 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.
clocks:
description: Optional 2.048 MHz external source clock on CLK pin
@@ -75,5 +79,6 @@ examples:
spi-max-frequency = <20000000>;
spi-cpha;
};
+
};
...
--
2.51.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support
2026-07-06 11:26 [PATCH v2 0/2] iio: adc: ti-ads1298: add ADS1298/ADS1299 driver Md Shofiqul Islam
2026-07-06 11:26 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants Md Shofiqul Islam
@ 2026-07-06 11:26 ` Md Shofiqul Islam
2026-07-11 23:52 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Md Shofiqul Islam @ 2026-07-06 11:26 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, lars, linux-kernel, 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 | 84 ++++++++++++++++++++++++++++--------
1 file changed, 65 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 579200e06cbd6..7cff51d23d12f 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, ®val);
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,14 @@ static int ads1298_probe(struct spi_device *spi)
static const struct spi_device_id ads1298_id[] = {
{ "ads1298" },
+ { "ads1299" },
{ }
};
MODULE_DEVICE_TABLE(spi, ads1298_id);
static const struct of_device_id ads1298_of_table[] = {
{ .compatible = "ti,ads1298" },
+ { .compatible = "ti,ads1299" },
{ }
};
MODULE_DEVICE_TABLE(of, ads1298_of_table);
@@ -760,5 +805,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants
2026-07-06 11:26 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants Md Shofiqul Islam
@ 2026-07-07 6:21 ` Krzysztof Kozlowski
2026-07-11 23:21 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-07 6:21 UTC (permalink / raw)
To: Md Shofiqul Islam, linux-iio; +Cc: jic23, lars, linux-kernel
On 06/07/2026 13:26, 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.
You already received below comment, so please do not expect us to repeat:
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.
Please kindly resend and include all necessary To/Cc entries.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants
2026-07-06 11:26 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants Md Shofiqul Islam
2026-07-07 6:21 ` Krzysztof Kozlowski
@ 2026-07-11 23:21 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2026-07-11 23:21 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: linux-iio, lars, linux-kernel, Mike Looijmans
On Mon, 6 Jul 2026 14:26:13 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> wrote:
> The ADS1299, ADS1299-4, and ADS1299-6 share the same SPI interface
Given mention of 3 parts, this description should say why only
one compatible is needed.
> 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.
That comment needs an update seeing as now it is removing description
rather than documenting the ADS1299 description.
Note Sashiko identified something was wrong around both of these -
glad it reads patch descriptions more closely than I do!
>
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
> .../devicetree/bindings/iio/adc/ti,ads1298.yaml | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> index 71f9f9b745cbc..202776cee73ff 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
> @@ -4,11 +4,15 @@
> $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. Max 12x PGA gain.
> + Datasheet: https://www.ti.com/product/ADS1298
> +
> + ADS1299: 4/6/8-channel, 24-bit EEG/biopotential ADC. Max 24x PGA gain.
> + Channel count is discoverable from the ID register at runtime.
> + Datasheet: https://www.ti.com/product/ADS1299
>
> maintainers:
> - Mike Looijmans <mike.looijmans@topic.nl>
Make sure to +CC Mike (added)
> @@ -17,6 +21,7 @@ properties:
> compatible:
> enum:
> - ti,ads1298
> + - ti,ads1299
>
> reg:
> maxItems: 1
> @@ -33,8 +38,7 @@ 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.
>
> clocks:
> description: Optional 2.048 MHz external source clock on CLK pin
> @@ -75,5 +79,6 @@ examples:
> spi-max-frequency = <20000000>;
> spi-cpha;
> };
> +
Unrelated change. Make sure to clean these out before sending a series.
Jonathan
> };
> ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support
2026-07-06 11:26 ` [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support Md Shofiqul Islam
@ 2026-07-11 23:52 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2026-07-11 23:52 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: linux-iio, lars, linux-kernel
On Mon, 6 Jul 2026 14:26:14 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> 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>
These sort of special purpose ADC chips are normally non trivial to test.
I'm curious, what are you testing with? Basic testing with a dev board
or actual capture of ECG signals? Or is there a device out there that has
one of these integrated and runs Linux?
> ---
> drivers/iio/adc/ti-ads1298.c | 84 ++++++++++++++++++++++++++++--------
> 1 file changed, 65 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index 579200e06cbd6..7cff51d23d12f 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)
So the above wild cards, ADS129X covers this which makes me wonder if we need
to rename that. I think we should - despite that naming being on the
datasheet. If anyone wondered why we don't trust device manufacturers
to keep to their numbering / naming schemes and hence generally resist wild
cards in IIO, then this is a wonderful example.
Also, the datasheet google fed me has
REV_ID: 7:5
Reserved: 4 - always 1.
DEV_ID: 3:2
NU_CH: 1:0
So I have no idea what this is...
> +#define ADS1299_ID_FAMILY_EEG GENMASK(4, 3)
Have the values of the ID field as the values that will come out of
FIELD_GET(ADS1299_MASK_ID_FAMILY, val)
so, here that would be 3. I'm not sure representing it as a bit
mask makes sense. It is just a number. However it's also called
DEV_ID on the datat sheet, so follow that
> +/*
> + * 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 */
It is? Give as specific reference to which table, section that is in on the datasheet.
(which is to say I checked and am seeing a different value - though the wiring
is non trivial so maybe this is the effective value in some sense?)
> + *val = 2400;
> } else {
> ret = regmap_read(priv->regmap, ADS1298_REG_CONFIG3, ®val);
> if (ret)
> return ret;
>
> - /* Reference in millivolts */
> + /* ADS1298 reference in millivolts: 2.4V or 4V */
Number is right there in the code, so not sure the comment adds much.
> *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)];
Too nasty to read. Use a local variable for the ternary output then
index into that.
> *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;
This reflects the question above. Is it? I'm seeing that in [3:2] from
Table 12: ID Control Register Field Descriptions
Now the guarantee that bit 3 for the ADS1298 etc is 0 and for the ADS1299 is 1.
(upper bit of actual Id location) + the bit 4 is reserved on the ADS1299 and 1
makes this work but not for any of the reasons stated. To me detecting between
these two annoyingly inconsistent families is going to be fragile and rely
on those reserved bits. So either it needs to do as many sanity checks as possible
on each type, or (and I'd go this way) given up and use the firmware description
as a lead on where to look. So add match data to the arrays and use a structure
in that with fields to identify which sub part each is.
> 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);
That's fine on one line. We get a bit flexible on 90 chars for cases like this,
just don't go too long.
> + }
> + 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,14 @@ static int ads1298_probe(struct spi_device *spi)
>
> static const struct spi_device_id ads1298_id[] = {
> { "ads1298" },
> + { "ads1299" },
> { }
> };
> MODULE_DEVICE_TABLE(spi, ads1298_id);
>
> static const struct of_device_id ads1298_of_table[] = {
> { .compatible = "ti,ads1298" },
> + { .compatible = "ti,ads1299" },
> { }
> };
> MODULE_DEVICE_TABLE(of, ads1298_of_table);
> @@ -760,5 +805,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>");
Hmm. Maybe Mike has a view on this, but I'd take that as a stretch
given the relatively small amount of code needed to add the new part.
The copyright is fine for reflecting that I think.
> +MODULE_DESCRIPTION("TI ADS1298/ADS1299 biopotential ADC");
> MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-11 23:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 11:26 [PATCH v2 0/2] iio: adc: ti-ads1298: add ADS1298/ADS1299 driver Md Shofiqul Islam
2026-07-06 11:26 ` [PATCH v2 1/2] dt-bindings: iio: adc: ti,ads1298: add ADS1299 EEG ADC variants Md Shofiqul Islam
2026-07-07 6:21 ` Krzysztof Kozlowski
2026-07-11 23:21 ` Jonathan Cameron
2026-07-06 11:26 ` [PATCH v2 2/2] iio: adc: ti-ads1298: add ADS1299 EEG ADC family support Md Shofiqul Islam
2026-07-11 23:52 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox