* [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
* 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
* [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 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