* [PATCH v2 1/6] iio: adc: ti-ads1298: fix the data read path
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 2/6] iio: adc: ti-ads1298: fix power-up and clock handling Stephan Costa Barros
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
Fix five issues in how the driver reads samples:
- cmd_buffer serves both the register accessors, under the regmap lock,
and the commands, under the IIO device lock, and shares a cacheline
with rx_buffer and tx_buffer. A register access can then rewrite a
command on the bus, or corrupt an RDATA transfer in flight on a
platform without coherent DMA. Give each path its own buffer, each on
its own cacheline.
- After STOP, the conversion in progress still raises DRDY, so an RDATA
transfer can be in flight when the core frees the active scan mask.
Track in the driver whether samples go to the buffer, wait in
predisable for the last DRDY and its transfer, and push the sample
before marking the transfer done. update_scan_mode reset the transfer
counter, which let a new transfer be queued over one in flight; wait
for it instead.
- When a single-shot read times out, its transfer may still be in
flight, and the next read then wakes up with the old sample. On
unbind, nothing waits for the DRDY interrupt or its transfer. Wait for
the transfer before a single-shot read, and on teardown disable the
interrupt and wait for the last transfer.
- The return value of spi_async() and the status of the RDATA message
are ignored: a failure leaves the transfer counter stuck, and a failed
transfer is reported as a sample. Handle both.
- read_raw takes the sample out of rx_buffer after releasing direct
mode, so a buffer enabled in between can overwrite it. Take it while
direct mode is still held.
The first, second and last were found by review and not reproduced.
The third and fourth were found by review and reproduced on an emulated
device only.
Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
drivers/iio/adc/ti-ads1298.c | 183 +++++++++++++++++++++++++++--------
1 file changed, 144 insertions(+), 39 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 06c6353aa..1d32b1f47 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -10,6 +10,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
#include <linux/log2.h>
#include <linux/math.h>
#include <linux/module.h>
@@ -18,6 +19,7 @@
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/units.h>
+#include <linux/wait.h>
#include <linux/iio/iio.h>
#include <linux/iio/buffer.h>
@@ -119,15 +121,26 @@ struct ads1298_private {
* >2 = Multiple DRDY during transfer, lost rdata_xfer_busy - 2 samples
*/
unsigned int rdata_xfer_busy;
+ /* Samples go to the buffer; under irq_busy_lock */
+ bool buffered;
+ /* Woken when rdata_xfer_busy drops to 0 */
+ wait_queue_head_t idle_wq;
+ /* Status of the last single-shot read */
+ int rdata_status;
/* Temporary storage for demuxing data after SPI transfer */
u32 bounce_buffer[ADS1298_MAX_CHANNELS];
- /* For synchronous SPI exchanges (read/write registers) */
- u8 cmd_buffer[ADS1298_SPI_CMD_BUFFER_SIZE] __aligned(IIO_DMA_MINALIGN);
-
+ /*
+ * Each buffer below is used by one path at a time and has its own
+ * cacheline, so that no two transfers in flight share one.
+ */
+ /* Register access, under the regmap lock */
+ u8 reg_buffer[ADS1298_SPI_CMD_BUFFER_SIZE] __aligned(IIO_DMA_MINALIGN);
+ /* Commands, under the IIO device lock or in probe */
+ u8 cmd_buffer[1] __aligned(IIO_DMA_MINALIGN);
/* Buffer used for incoming SPI data */
- u8 rx_buffer[ADS1298_SPI_RDATA_BUFFER_SIZE_MAX];
+ u8 rx_buffer[ADS1298_SPI_RDATA_BUFFER_SIZE_MAX] __aligned(IIO_DMA_MINALIGN);
/* Contains the RDATA command and zeroes to clock out */
u8 tx_buffer[ADS1298_SPI_RDATA_BUFFER_SIZE_MAX];
};
@@ -185,10 +198,49 @@ static int ads1298_write_cmd(struct ads1298_private *priv, u8 command)
return spi_sync_transfer(priv->spi, &xfer, 1);
}
+/* Whether no RDATA transfer is queued or in flight */
+static bool ads1298_rdata_idle(struct ads1298_private *priv)
+{
+ guard(spinlock_irqsave)(&priv->irq_busy_lock);
+ return !priv->rdata_xfer_busy;
+}
+
+static int ads1298_wait_rdata_idle(struct ads1298_private *priv)
+{
+ if (!wait_event_timeout(priv->idle_wq, ads1298_rdata_idle(priv),
+ msecs_to_jiffies(100)))
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+/* Called with irq_busy_lock held */
+static void ads1298_rdata_start(struct ads1298_private *priv)
+{
+ int ret;
+
+ ret = spi_async(priv->spi, &priv->rdata_msg);
+ if (!ret)
+ return;
+
+ /* No completion will run for this transfer */
+ priv->rdata_xfer_busy = 0;
+ if (!priv->buffered) {
+ priv->rdata_status = ret;
+ complete(&priv->completion);
+ }
+ wake_up(&priv->idle_wq);
+}
+
static int ads1298_read_one(struct ads1298_private *priv, int chan_index)
{
int ret;
+ /* A transfer left over from an earlier timeout must finish first */
+ ret = ads1298_wait_rdata_idle(priv);
+ if (ret)
+ return ret;
+
/* Enable the channel */
ret = regmap_update_bits(priv->regmap, ADS1298_REG_CHnSET(chan_index),
ADS1298_MASK_CH_PD, 0);
@@ -217,7 +269,7 @@ static int ads1298_read_one(struct ads1298_private *priv, int chan_index)
if (!wait_for_completion_timeout(&priv->completion, msecs_to_jiffies(50)))
return -ETIMEDOUT;
- return 0;
+ return priv->rdata_status;
}
static int ads1298_get_samp_freq(struct ads1298_private *priv, int *val)
@@ -330,14 +382,17 @@ static int ads1298_read_raw(struct iio_dev *indio_dev,
return -EBUSY;
ret = ads1298_read_one(priv, chan->scan_index);
+ /* Once direct mode is released, a buffer may overwrite rx_buffer */
+ if (!ret)
+ *val = sign_extend32(get_unaligned_be24(priv->rx_buffer +
+ chan->address),
+ ADS1298_BITS_PER_SAMPLE - 1);
iio_device_release_direct(indio_dev);
if (ret)
return ret;
- *val = sign_extend32(get_unaligned_be24(priv->rx_buffer + chan->address),
- ADS1298_BITS_PER_SAMPLE - 1);
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
return ads1298_get_scale(priv, chan->channel, val, val2);
@@ -373,8 +428,8 @@ static int ads1298_reg_write(void *context, unsigned int reg, unsigned int val)
{
struct ads1298_private *priv = context;
struct spi_transfer reg_write_xfer = {
- .tx_buf = priv->cmd_buffer,
- .rx_buf = priv->cmd_buffer,
+ .tx_buf = priv->reg_buffer,
+ .rx_buf = priv->reg_buffer,
.len = 3,
.speed_hz = ADS1298_SPI_BUS_SPEED_SLOW,
.delay = {
@@ -383,9 +438,9 @@ static int ads1298_reg_write(void *context, unsigned int reg, unsigned int val)
},
};
- priv->cmd_buffer[0] = ADS1298_CMD_WREG | reg;
- priv->cmd_buffer[1] = 0; /* Number of registers to be written - 1 */
- priv->cmd_buffer[2] = val;
+ priv->reg_buffer[0] = ADS1298_CMD_WREG | reg;
+ priv->reg_buffer[1] = 0; /* Number of registers to be written - 1 */
+ priv->reg_buffer[2] = val;
return spi_sync_transfer(priv->spi, ®_write_xfer, 1);
}
@@ -394,8 +449,8 @@ static int ads1298_reg_read(void *context, unsigned int reg, unsigned int *val)
{
struct ads1298_private *priv = context;
struct spi_transfer reg_read_xfer = {
- .tx_buf = priv->cmd_buffer,
- .rx_buf = priv->cmd_buffer,
+ .tx_buf = priv->reg_buffer,
+ .rx_buf = priv->reg_buffer,
.len = 3,
.speed_hz = ADS1298_SPI_BUS_SPEED_SLOW,
.delay = {
@@ -405,15 +460,15 @@ static int ads1298_reg_read(void *context, unsigned int reg, unsigned int *val)
};
int ret;
- priv->cmd_buffer[0] = ADS1298_CMD_RREG | reg;
- priv->cmd_buffer[1] = 0; /* Number of registers to be read - 1 */
- priv->cmd_buffer[2] = 0;
+ priv->reg_buffer[0] = ADS1298_CMD_RREG | reg;
+ priv->reg_buffer[1] = 0; /* Number of registers to be read - 1 */
+ priv->reg_buffer[2] = 0;
ret = spi_sync_transfer(priv->spi, ®_read_xfer, 1);
if (ret)
return ret;
- *val = priv->cmd_buffer[2];
+ *val = priv->reg_buffer[2];
return 0;
}
@@ -429,13 +484,6 @@ static int ads1298_reg_access(struct iio_dev *indio_dev, unsigned int reg,
return regmap_write(priv->regmap, reg, writeval);
}
-static void ads1298_rdata_unmark_busy(struct ads1298_private *priv)
-{
- /* Notify we're no longer waiting for the SPI transfer to complete */
- guard(spinlock_irqsave)(&priv->irq_busy_lock);
- priv->rdata_xfer_busy = 0;
-}
-
static int ads1298_update_scan_mode(struct iio_dev *indio_dev,
const unsigned long *scan_mask)
{
@@ -444,8 +492,10 @@ static int ads1298_update_scan_mode(struct iio_dev *indio_dev,
int ret;
int i;
- /* Make the interrupt routines start with a clean slate */
- ads1298_rdata_unmark_busy(priv);
+ /* Nothing may be in flight when the buffer starts */
+ ret = ads1298_wait_rdata_idle(priv);
+ if (ret)
+ return ret;
/* Configure power-down bits to match scan mask */
for (i = 0; i < indio_dev->num_channels; i++) {
@@ -470,22 +520,22 @@ static void ads1298_rdata_release_busy_or_restart(struct ads1298_private *priv)
{
guard(spinlock_irqsave)(&priv->irq_busy_lock);
- if (priv->rdata_xfer_busy > 1) {
+ if (priv->buffered && priv->rdata_xfer_busy > 1) {
/*
* DRDY interrupt occurred before SPI completion. Start a new
* SPI transaction now to retrieve the data that wasn't latched
* into the ADS1298 chip's transfer buffer yet.
- */
- spi_async(priv->spi, &priv->rdata_msg);
- /*
+ *
* If more than one DRDY took place, there was an overrun. Since
* the sample is already lost, reset the counter to 1 so that
* we will wait for a DRDY interrupt after this SPI transaction.
*/
priv->rdata_xfer_busy = 1;
+ ads1298_rdata_start(priv);
} else {
/* No pending data, wait for DRDY */
priv->rdata_xfer_busy = 0;
+ wake_up(&priv->idle_wq);
}
}
@@ -494,19 +544,32 @@ static void ads1298_rdata_complete(void *context)
{
struct iio_dev *indio_dev = context;
struct ads1298_private *priv = iio_priv(indio_dev);
+ int status = priv->rdata_msg.status;
int scan_index;
u32 *bounce = priv->bounce_buffer;
+ bool buffered;
- if (!iio_buffer_enabled(indio_dev)) {
+ /* Only changes while no transfer is in flight */
+ scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
+ buffered = priv->buffered;
+
+ if (!buffered) {
/*
* for a single transfer mode we're kept in direct_mode until
* completion, avoiding a race with buffered IO.
*/
- ads1298_rdata_unmark_busy(priv);
+ priv->rdata_status = status;
+ ads1298_rdata_release_busy_or_restart(priv);
complete(&priv->completion);
return;
}
+ /* A failed transfer leaves no sample to push */
+ if (status) {
+ ads1298_rdata_release_busy_or_restart(priv);
+ return;
+ }
+
/* Demux the channel data into our bounce buffer */
iio_for_each_active_channel(indio_dev, scan_index) {
const struct iio_chan_spec *scan_chan =
@@ -516,10 +579,13 @@ static void ads1298_rdata_complete(void *context)
*bounce++ = get_unaligned_be24(data);
}
- /* rx_buffer can be overwritten from this point on */
- ads1298_rdata_release_busy_or_restart(priv);
-
+ /*
+ * Push before marking the transfer done: predisable waits for that,
+ * and the scan must not be torn down while it is used here.
+ */
iio_push_to_buffers(indio_dev, priv->bounce_buffer);
+
+ ads1298_rdata_release_busy_or_restart(priv);
}
static irqreturn_t ads1298_interrupt(int irq, void *dev_id)
@@ -533,7 +599,7 @@ static irqreturn_t ads1298_interrupt(int irq, void *dev_id)
wasbusy = priv->rdata_xfer_busy++;
/* When no SPI transfer in transit, start one now */
if (!wasbusy)
- spi_async(priv->spi, &priv->rdata_msg);
+ ads1298_rdata_start(priv);
return IRQ_HANDLED;
};
@@ -549,14 +615,39 @@ static int ads1298_buffer_postenable(struct iio_dev *indio_dev)
if (ret)
return ret;
- return ads1298_write_cmd(priv, ADS1298_CMD_START);
+ scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
+ priv->buffered = true;
+
+ ret = ads1298_write_cmd(priv, ADS1298_CMD_START);
+ if (ret) {
+ scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
+ priv->buffered = false;
+ }
+
+ return ret;
}
static int ads1298_buffer_predisable(struct iio_dev *indio_dev)
{
struct ads1298_private *priv = iio_priv(indio_dev);
+ int ret, ret2;
+ int rate;
+
+ ret = ads1298_write_cmd(priv, ADS1298_CMD_STOP);
+
+ /*
+ * The conversion in progress completes and raises DRDY once more.
+ * Wait for it and for its transfer before the core tears down the
+ * scan.
+ */
+ if (ads1298_get_samp_freq(priv, &rate) >= 0 && rate > 0)
+ fsleep(2 * DIV_ROUND_UP(USEC_PER_SEC, rate));
+ ret2 = ads1298_wait_rdata_idle(priv);
- return ads1298_write_cmd(priv, ADS1298_CMD_STOP);
+ scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
+ priv->buffered = false;
+
+ return ret ?: ret2;
}
static const struct iio_buffer_setup_ops ads1298_setup_ops = {
@@ -569,6 +660,15 @@ static void ads1298_reg_disable(void *reg)
regulator_disable(reg);
}
+static void ads1298_quiesce(void *data)
+{
+ struct ads1298_private *priv = data;
+
+ /* No new transfer may start, and the last one must end, before teardown */
+ disable_irq(priv->spi->irq);
+ ads1298_wait_rdata_idle(priv);
+}
+
static const struct regmap_range ads1298_regmap_volatile_range[] = {
regmap_reg_range(ADS1298_REG_LOFF_STATP, ADS1298_REG_LOFF_STATN),
};
@@ -690,6 +790,7 @@ static int ads1298_probe(struct spi_device *spi)
priv->spi = spi;
init_completion(&priv->completion);
+ init_waitqueue_head(&priv->idle_wq);
spin_lock_init(&priv->irq_busy_lock);
priv->regmap = devm_regmap_init(dev, NULL, priv, &ads1298_regmap_config);
if (IS_ERR(priv->regmap))
@@ -735,6 +836,10 @@ static int ads1298_probe(struct spi_device *spi)
if (ret)
return ret;
+ ret = devm_add_action_or_reset(dev, ads1298_quiesce, priv);
+ if (ret)
+ return ret;
+
ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &ads1298_setup_ops);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 2/6] iio: adc: ti-ads1298: fix power-up and clock handling
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 1/6] iio: adc: ti-ads1298: fix the data read path Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 3/6] iio: adc: ti-ads1298: fix scale, ID check, regmap and Kconfig Stephan Costa Barros
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
Fix three issues with the power-up sequence and the device clock:
- The datasheet powers up the supplies first, then the clock, then
waits t_POR = 2^18 clock cycles (128 ms at 2.048 MHz) before the
reset. The driver enabled an external clock and the vref supply
before AVDD, reset right away, and did not wait for the 150 ms
start-up of the internal reference. Follow the datasheet. Probe takes
about 280 ms longer.
- The binding describes one optional clock and no clock-names, but the
driver asked for the clock named "clk". With a device tree that
follows the binding, the lookup fails, the optional getter returns
NULL, and a clock given in the device tree is never enabled, while the
sampling frequency is computed from the nominal 2.048 MHz. Ask for the
first clock of the node; a device tree with clock-names = "clk" still
gets the same clock.
- A command takes 4 cycles of the device clock to decode, the minimum
reset pulse is 2 cycles, and the reset takes 18 more. These were
converted with the nominal 2.048 MHz, so a command ends after a fixed
2 us, short of 4 cycles with an external clock below 2 MHz, which the
ADS129x allows down to 1.94 MHz. Convert them with the rate of the
clock in use. At 2.048 MHz the delays do not change.
The clock lookup was seen to fail on a Raspberry Pi 4, with a
fixed-clock in the device tree. The power-up order was found by review
and reproduced on an emulated device only. The decode time was found
with an emulated device; it is not seen on hardware running at
2.048 MHz.
Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
drivers/iio/adc/ti-ads1298.c | 73 +++++++++++++++++++++++++-----------
1 file changed, 51 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 1d32b1f47..f5965b994 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -84,8 +84,12 @@
#define ADS1298_MAX_CHANNELS 8
#define ADS1298_BITS_PER_SAMPLE 24
#define ADS1298_CLK_RATE_HZ 2048000
-#define ADS1298_CLOCKS_TO_USECS(x) \
- (DIV_ROUND_UP((x) * MICROHZ_PER_HZ, ADS1298_CLK_RATE_HZ))
+/* A command takes 4 clock cycles to decode, before the next one may start */
+#define ADS1298_DECODE_CLOCKS 4
+/* Power-on reset: wait 2^18 clock cycles before the reset */
+#define ADS1298_TPOR_CLOCKS BIT(18)
+/* Start-up time of the internal reference */
+#define ADS1298_REF_START_UP_MS 150
/*
* Read/write register commands require 4 clocks to decode, for speeds above
* 2x the clock rate, this would require extra time between the command byte and
@@ -106,6 +110,7 @@ struct ads1298_private {
struct regulator *reg_avdd;
struct regulator *reg_vref;
struct clk *clk;
+ unsigned long clk_rate;
struct regmap *regmap;
struct completion completion;
struct iio_trigger *trig;
@@ -180,6 +185,12 @@ static const struct iio_chan_spec ads1298_channels[] = {
ADS1298_CHAN(7),
};
+static unsigned int ads1298_clocks_to_usecs(struct ads1298_private *priv,
+ unsigned int clocks)
+{
+ return DIV_ROUND_UP(clocks * USEC_PER_SEC, priv->clk_rate);
+}
+
static int ads1298_write_cmd(struct ads1298_private *priv, u8 command)
{
struct spi_transfer xfer = {
@@ -188,7 +199,7 @@ static int ads1298_write_cmd(struct ads1298_private *priv, u8 command)
.len = 1,
.speed_hz = ADS1298_SPI_BUS_SPEED_SLOW,
.delay = {
- .value = 2,
+ .value = ads1298_clocks_to_usecs(priv, ADS1298_DECODE_CLOCKS),
.unit = SPI_DELAY_UNIT_USECS,
},
};
@@ -433,7 +444,7 @@ static int ads1298_reg_write(void *context, unsigned int reg, unsigned int val)
.len = 3,
.speed_hz = ADS1298_SPI_BUS_SPEED_SLOW,
.delay = {
- .value = 2,
+ .value = ads1298_clocks_to_usecs(priv, ADS1298_DECODE_CLOCKS),
.unit = SPI_DELAY_UNIT_USECS,
},
};
@@ -454,7 +465,7 @@ static int ads1298_reg_read(void *context, unsigned int reg, unsigned int *val)
.len = 3,
.speed_hz = ADS1298_SPI_BUS_SPEED_SLOW,
.delay = {
- .value = 2,
+ .value = ads1298_clocks_to_usecs(priv, ADS1298_DECODE_CLOCKS),
.unit = SPI_DELAY_UNIT_USECS,
},
};
@@ -730,7 +741,14 @@ static int ads1298_init(struct iio_dev *indio_dev)
if (regulator_get_voltage(priv->reg_avdd) >= 4400000)
val |= ADS1298_MASK_CONFIG3_VREF_4V;
}
- return regmap_write(priv->regmap, ADS1298_REG_CONFIG3, val);
+ ret = regmap_write(priv->regmap, ADS1298_REG_CONFIG3, val);
+ if (ret)
+ return ret;
+
+ if (!priv->reg_vref)
+ msleep(ADS1298_REF_START_UP_MS);
+
+ return 0;
}
static int ads1298_probe(struct spi_device *spi)
@@ -739,6 +757,7 @@ static int ads1298_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct device *dev = &spi->dev;
struct gpio_desc *reset_gpio;
+ unsigned long clk_rate;
int ret;
indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
@@ -753,6 +772,19 @@ static int ads1298_probe(struct spi_device *spi)
return dev_err_probe(dev, PTR_ERR(reset_gpio),
"Cannot get reset GPIO\n");
+ priv->reg_avdd = devm_regulator_get(dev, "avdd");
+ if (IS_ERR(priv->reg_avdd))
+ return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
+ "Failed to get avdd regulator\n");
+
+ ret = regulator_enable(priv->reg_avdd);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable avdd regulator\n");
+
+ ret = devm_add_action_or_reset(dev, ads1298_reg_disable, priv->reg_avdd);
+ if (ret)
+ return ret;
+
/* VREF can be supplied externally, otherwise use internal reference */
priv->reg_vref = devm_regulator_get_optional(dev, "vref");
if (IS_ERR(priv->reg_vref)) {
@@ -771,22 +803,15 @@ static int ads1298_probe(struct spi_device *spi)
return ret;
}
- priv->clk = devm_clk_get_optional_enabled(dev, "clk");
+ /* The clock follows the supplies */
+ priv->clk = devm_clk_get_optional_enabled(dev, NULL);
if (IS_ERR(priv->clk))
return dev_err_probe(dev, PTR_ERR(priv->clk), "Failed to get clk\n");
- priv->reg_avdd = devm_regulator_get(dev, "avdd");
- if (IS_ERR(priv->reg_avdd))
- return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
- "Failed to get avdd regulator\n");
-
- ret = regulator_enable(priv->reg_avdd);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to enable avdd regulator\n");
-
- ret = devm_add_action_or_reset(dev, ads1298_reg_disable, priv->reg_avdd);
- if (ret)
- return ret;
+ clk_rate = priv->clk ? clk_get_rate(priv->clk) : 0;
+ if (!clk_rate)
+ clk_rate = ADS1298_CLK_RATE_HZ;
+ priv->clk_rate = clk_rate;
priv->spi = spi;
init_completion(&priv->completion);
@@ -800,12 +825,15 @@ static int ads1298_probe(struct spi_device *spi)
indio_dev->channels = ads1298_channels;
indio_dev->info = &ads1298_info;
+ /* Power-on reset time, counted from when the clock is valid */
+ msleep(DIV_ROUND_UP(ADS1298_TPOR_CLOCKS * MILLI, clk_rate));
+
if (reset_gpio) {
/*
* Deassert reset now that clock and power are active.
* Minimum reset pulsewidth is 2 clock cycles.
*/
- fsleep(ADS1298_CLOCKS_TO_USECS(2));
+ fsleep(ads1298_clocks_to_usecs(priv, 2));
gpiod_set_value_cansleep(reset_gpio, 0);
} else {
ret = ads1298_write_cmd(priv, ADS1298_CMD_RESET);
@@ -813,7 +841,7 @@ static int ads1298_probe(struct spi_device *spi)
return dev_err_probe(dev, ret, "RESET failed\n");
}
/* Wait 18 clock cycles for reset command to complete */
- fsleep(ADS1298_CLOCKS_TO_USECS(18));
+ fsleep(ads1298_clocks_to_usecs(priv, 18));
ret = ads1298_init(indio_dev);
if (ret)
@@ -824,7 +852,8 @@ static int ads1298_probe(struct spi_device *spi)
priv->rdata_xfer.rx_buf = priv->rx_buffer;
priv->rdata_xfer.len = ADS1298_SPI_RDATA_BUFFER_SIZE(indio_dev->num_channels);
/* Must keep CS low for 4 clocks */
- priv->rdata_xfer.delay.value = 2;
+ priv->rdata_xfer.delay.value = ads1298_clocks_to_usecs(priv,
+ ADS1298_DECODE_CLOCKS);
priv->rdata_xfer.delay.unit = SPI_DELAY_UNIT_USECS;
spi_message_init_with_transfers(&priv->rdata_msg, &priv->rdata_xfer, 1);
priv->rdata_msg.complete = &ads1298_rdata_complete;
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 3/6] iio: adc: ti-ads1298: fix scale, ID check, regmap and Kconfig
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 1/6] iio: adc: ti-ads1298: fix the data read path Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 2/6] iio: adc: ti-ads1298: fix power-up and clock handling Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 4/6] iio: adc: ti-ads1298: prepare for more chip families Stephan Costa Barros
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
Fix four smaller issues:
- The scale divides the reference, in whole millivolts, by the gain
before it is reported. With the internal 4 V reference, it is 0.099 %
low at gain 6, 0.101 % at gain 12 and 0.025 % at gain 3. Report
VREF / (gain * 2^23) as an IIO_VAL_FRACTIONAL; sysfs still shows it
with nine decimals, now without the error.
- Only the channel codes 0, 1 and 2 of the ID register are defined; an
ID of 0x93, for example, gives 10 channels. The IIO core then reads
past ads1298_channels, and the RDATA transfer writes past rx_buffer.
Refuse the reserved codes as an unknown ID.
- The driver uses regmap but does not select REGMAP, and fails to build
where nothing else selects it, such as tinyconfig with SPI, IIO and
TI_ADS1298 on x86_64.
- Bit 0 of CONFIG3 is a read-only status bit and the GPIO register
reads the pins, but both are cached, so a read through debugfs returns
a stale value; debugfs can also write ID, LOFF_STATP and LOFF_STATN.
Mark CONFIG3 and GPIO volatile, and the other three not writeable.
The ID check was found by review and reproduced on an emulated device
only, where KASAN reports the read past ads1298_channels.
Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/ti-ads1298.c | 29 ++++++++++++++++++++++++-----
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f32f678f7..a29ed8f45 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1924,6 +1924,7 @@ config TI_ADS1298
depends on SPI
select IIO_BUFFER
select IIO_KFIFO_BUF
+ select REGMAP
help
If you say yes here you get support for Texas Instruments ADS1298
medical ADC chips
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index f5965b994..b390c13ae 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -75,6 +75,7 @@
#define ADS1298_REG_LOFF_STATP 0x12
#define ADS1298_REG_LOFF_STATN 0x13
+#define ADS1298_REG_GPIO 0x14
#define ADS1298_REG_CONFIG4 0x17
#define ADS1298_MASK_CONFIG4_SINGLE_SHOT BIT(3)
@@ -373,11 +374,10 @@ static int ads1298_get_scale(struct ads1298_private *priv,
return -EINVAL;
gain = ads1298_pga_settings[pga_idx];
- *val /= gain; /* Full scale is VREF / gain */
+ /* Full scale is VREF / gain, signed, hence the -1 */
+ *val2 = gain << (ADS1298_BITS_PER_SAMPLE - 1);
- *val2 = ADS1298_BITS_PER_SAMPLE - 1; /* Signed, hence the -1 */
-
- return IIO_VAL_FRACTIONAL_LOG2;
+ return IIO_VAL_FRACTIONAL;
}
static int ads1298_read_raw(struct iio_dev *indio_dev,
@@ -680,8 +680,11 @@ static void ads1298_quiesce(void *data)
ads1298_wait_rdata_idle(priv);
}
+/* CONFIG3 holds the read-only RLD_STAT; GPIO reads the pins */
static const struct regmap_range ads1298_regmap_volatile_range[] = {
+ regmap_reg_range(ADS1298_REG_CONFIG3, ADS1298_REG_CONFIG3),
regmap_reg_range(ADS1298_REG_LOFF_STATP, ADS1298_REG_LOFF_STATN),
+ regmap_reg_range(ADS1298_REG_GPIO, ADS1298_REG_GPIO),
};
static const struct regmap_access_table ads1298_regmap_volatile = {
@@ -689,12 +692,23 @@ static const struct regmap_access_table ads1298_regmap_volatile = {
.n_yes_ranges = ARRAY_SIZE(ads1298_regmap_volatile_range),
};
+static const struct regmap_range ads1298_regmap_read_only_range[] = {
+ regmap_reg_range(ADS1298_REG_ID, ADS1298_REG_ID),
+ regmap_reg_range(ADS1298_REG_LOFF_STATP, ADS1298_REG_LOFF_STATN),
+};
+
+static const struct regmap_access_table ads1298_regmap_writeable = {
+ .no_ranges = ads1298_regmap_read_only_range,
+ .n_no_ranges = ARRAY_SIZE(ads1298_regmap_read_only_range),
+};
+
static const struct regmap_config ads1298_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
.reg_read = ads1298_reg_read,
.reg_write = ads1298_reg_write,
.max_register = ADS1298_REG_WCT2,
+ .wr_table = &ads1298_regmap_writeable,
.volatile_table = &ads1298_regmap_volatile,
.cache_type = REGCACHE_MAPLE,
};
@@ -704,6 +718,7 @@ static int ads1298_init(struct iio_dev *indio_dev)
struct ads1298_private *priv = iio_priv(indio_dev);
struct device *dev = &priv->spi->dev;
const char *suffix;
+ unsigned int nu_ch;
unsigned int val;
int ret;
@@ -717,7 +732,11 @@ static int ads1298_init(struct iio_dev *indio_dev)
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);
+ nu_ch = FIELD_GET(ADS1298_MASK_ID_CHANNELS, val);
+ /* 0, 1 and 2 are 4, 6 and 8 channels; the other codes are reserved */
+ if (nu_ch > 2)
+ return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+ indio_dev->num_channels = 4 + 2 * nu_ch;
switch (val & ADS1298_MASK_ID_FAMILY) {
case ADS1298_ID_FAMILY_ADS129X:
suffix = "";
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 4/6] iio: adc: ti-ads1298: prepare for more chip families
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
` (2 preceding siblings ...)
2026-10-24 23:13 ` [PATCH v2 3/6] iio: adc: ti-ads1298: fix scale, ID check, regmap and Kconfig Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 5/6] dt-bindings: iio: adc: ti,ads1298: add ADS1299 Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 6/6] iio: adc: ti-ads1298: add ADS1299 support Stephan Costa Barros
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
The ADS1299 has the register map of the ADS129x, with other gains, data
rates, reference and ID layout. To support it in this driver, collect
what differs between families in a chip info structure, given as match
data. The private structure has carried an unused chip_info pointer
since the driver was added.
The chip info holds:
- the regmap configuration and the PGA gain table;
- the data rate rules: the base shift, whether CONFIG1 has an HR bit,
and the oversampling ratio at DR = 0;
- the internal reference, whether CONFIG3 has the VREF_4V bit, and the
reserved bits of CONFIG3 that must be written as 1;
- a callback that checks the ID register and sets the channel count and
the name, as the ID layouts differ; bit 3 alone tells them apart.
No functional change.
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
drivers/iio/adc/ti-ads1298.c | 132 +++++++++++++++++++++++++----------
1 file changed, 96 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index b390c13ae..5ad543bdf 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -49,8 +49,6 @@
#define ADS1298_REG_CONFIG1 0x01
#define ADS1298_MASK_CONFIG1_HR BIT(7)
#define ADS1298_MASK_CONFIG1_DR GENMASK(2, 0)
-#define ADS1298_SHIFT_DR_HR 6
-#define ADS1298_SHIFT_DR_LP 7
#define ADS1298_LOWEST_DR 0x06
#define ADS1298_REG_CONFIG2 0x02
@@ -66,6 +64,7 @@
#define ADS1298_MASK_CONFIG3_PWR_REFBUF BIT(7)
#define ADS1298_MASK_CONFIG3_RESERVED BIT(6)
#define ADS1298_MASK_CONFIG3_VREF_4V BIT(5)
+#define ADS1298_VREF_4V_MV 4000
#define ADS1298_REG_LOFF 0x04
#define ADS1298_REG_CHnSET(n) (0x05 + (n))
@@ -105,6 +104,31 @@
#define ADS1298_SPI_RDATA_BUFFER_SIZE_MAX \
ADS1298_SPI_RDATA_BUFFER_SIZE(ADS1298_MAX_CHANNELS)
+/* What differs between the chip families the driver supports */
+struct ads1298_chip_info {
+ /* Checks the ID register, and sets the channel count and the name */
+ int (*parse_id)(struct iio_dev *indio_dev, unsigned int id);
+ const struct regmap_config *regmap_config;
+ /* PGA gain for each value of the GAIN field of CHnSET */
+ const u8 *gains;
+ unsigned int num_gains;
+ /*
+ * The data rate is f_CLK >> (dr_shift + DR). With an HR bit in
+ * CONFIG1, dr_shift is that of the high-resolution mode, and the
+ * low-power mode shifts one more.
+ */
+ unsigned int dr_shift;
+ bool has_hr;
+ /* Oversampling ratio at DR = 0 */
+ unsigned int osr_base;
+ /* Internal reference, in millivolts */
+ unsigned int vref_mv;
+ /* CONFIG3 has the VREF_4V bit, which selects a 4 V internal reference */
+ bool has_vref_4v;
+ /* Reserved bits of CONFIG3 that must be written as 1 */
+ unsigned int config3_reserved;
+};
+
struct ads1298_private {
const struct ads1298_chip_info *chip_info;
struct spi_device *spi;
@@ -286,6 +310,7 @@ static int ads1298_read_one(struct ads1298_private *priv, int chan_index)
static int ads1298_get_samp_freq(struct ads1298_private *priv, int *val)
{
+ const struct ads1298_chip_info *info = priv->chip_info;
unsigned long rate;
unsigned int cfg;
int ret;
@@ -302,10 +327,10 @@ static int ads1298_get_samp_freq(struct ads1298_private *priv, int *val)
return -EINVAL;
/* Data rate shift depends on HR/LP mode */
- if (cfg & ADS1298_MASK_CONFIG1_HR)
- rate >>= ADS1298_SHIFT_DR_HR;
+ if (info->has_hr && !(cfg & ADS1298_MASK_CONFIG1_HR))
+ rate >>= info->dr_shift + 1;
else
- rate >>= ADS1298_SHIFT_DR_LP;
+ rate >>= info->dr_shift;
*val = rate >> (cfg & ADS1298_MASK_CONFIG1_DR);
@@ -314,6 +339,7 @@ static int ads1298_get_samp_freq(struct ads1298_private *priv, int *val)
static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
{
+ const struct ads1298_chip_info *info = priv->chip_info;
unsigned long rate;
unsigned int factor;
unsigned int cfg;
@@ -327,9 +353,9 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
if (val <= 0)
return -EINVAL;
- factor = (rate >> ADS1298_SHIFT_DR_HR) / val;
- if (factor >= BIT(ADS1298_SHIFT_DR_LP))
- cfg = ADS1298_LOWEST_DR;
+ factor = (rate >> info->dr_shift) / val;
+ if (factor >= BIT(ADS1298_LOWEST_DR + 1))
+ cfg = ADS1298_LOWEST_DR; /* Lowest rate, in LP mode */
else if (factor)
cfg = ADS1298_MASK_CONFIG1_HR | ilog2(factor); /* Use HR mode */
else
@@ -340,11 +366,10 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
cfg);
}
-static const u8 ads1298_pga_settings[] = { 6, 1, 2, 3, 4, 8, 12 };
-
static int ads1298_get_scale(struct ads1298_private *priv,
int channel, int *val, int *val2)
{
+ const struct ads1298_chip_info *info = priv->chip_info;
unsigned int pga_idx;
int ret;
unsigned int regval;
@@ -356,13 +381,16 @@ static int ads1298_get_scale(struct ads1298_private *priv,
return ret;
*val = ret / MILLI; /* Convert to millivolts */
- } else {
+ } else if (info->has_vref_4v) {
ret = regmap_read(priv->regmap, ADS1298_REG_CONFIG3, ®val);
if (ret)
return ret;
/* Reference in millivolts */
- *val = regval & ADS1298_MASK_CONFIG3_VREF_4V ? 4000 : 2400;
+ *val = regval & ADS1298_MASK_CONFIG3_VREF_4V ?
+ ADS1298_VREF_4V_MV : info->vref_mv;
+ } else {
+ *val = info->vref_mv;
}
ret = regmap_read(priv->regmap, ADS1298_REG_CHnSET(channel), ®val);
@@ -370,10 +398,10 @@ static int ads1298_get_scale(struct ads1298_private *priv,
return ret;
pga_idx = FIELD_GET(ADS1298_MASK_CH_PGA, regval);
- if (pga_idx >= ARRAY_SIZE(ads1298_pga_settings))
+ if (pga_idx >= info->num_gains)
return -EINVAL;
- gain = ads1298_pga_settings[pga_idx];
+ gain = info->gains[pga_idx];
/* Full scale is VREF / gain, signed, hence the -1 */
*val2 = gain << (ADS1298_BITS_PER_SAMPLE - 1);
@@ -414,7 +442,7 @@ static int ads1298_read_raw(struct iio_dev *indio_dev,
if (ret)
return ret;
- *val = 16 << (*val & ADS1298_MASK_CONFIG1_DR);
+ *val = priv->chip_info->osr_base << (*val & ADS1298_MASK_CONFIG1_DR);
return IIO_VAL_INT;
default:
return -EINVAL;
@@ -713,31 +741,20 @@ static const struct regmap_config ads1298_regmap_config = {
.cache_type = REGCACHE_MAPLE,
};
-static int ads1298_init(struct iio_dev *indio_dev)
+static int ads1298_parse_id(struct iio_dev *indio_dev, unsigned int id)
{
struct ads1298_private *priv = iio_priv(indio_dev);
struct device *dev = &priv->spi->dev;
const char *suffix;
unsigned int nu_ch;
- unsigned int val;
- int ret;
-
- /* Device initializes into RDATAC mode, which we don't want */
- ret = ads1298_write_cmd(priv, ADS1298_CMD_SDATAC);
- if (ret)
- return ret;
-
- ret = regmap_read(priv->regmap, ADS1298_REG_ID, &val);
- if (ret)
- return ret;
/* Fill in name and channel count based on what the chip told us */
- nu_ch = FIELD_GET(ADS1298_MASK_ID_CHANNELS, val);
+ nu_ch = FIELD_GET(ADS1298_MASK_ID_CHANNELS, id);
/* 0, 1 and 2 are 4, 6 and 8 channels; the other codes are reserved */
if (nu_ch > 2)
- return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+ return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", id);
indio_dev->num_channels = 4 + 2 * nu_ch;
- switch (val & ADS1298_MASK_ID_FAMILY) {
+ switch (id & ADS1298_MASK_ID_FAMILY) {
case ADS1298_ID_FAMILY_ADS129X:
suffix = "";
break;
@@ -745,19 +762,57 @@ static int ads1298_init(struct iio_dev *indio_dev)
suffix = "r";
break;
default:
- return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+ return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", id);
}
indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads129%u%s",
indio_dev->num_channels, suffix);
if (!indio_dev->name)
return -ENOMEM;
- val = ADS1298_MASK_CONFIG3_RESERVED; /* Must write 1 always */
+ return 0;
+}
+
+static const u8 ads1298_gains[] = { 6, 1, 2, 3, 4, 8, 12 };
+
+static const struct ads1298_chip_info ads1298_chip_info = {
+ .parse_id = ads1298_parse_id,
+ .regmap_config = &ads1298_regmap_config,
+ .gains = ads1298_gains,
+ .num_gains = ARRAY_SIZE(ads1298_gains),
+ .dr_shift = 6,
+ .has_hr = true,
+ .osr_base = 16,
+ .vref_mv = 2400,
+ .has_vref_4v = true,
+ .config3_reserved = ADS1298_MASK_CONFIG3_RESERVED,
+};
+
+static int ads1298_init(struct iio_dev *indio_dev)
+{
+ struct ads1298_private *priv = iio_priv(indio_dev);
+ unsigned int val;
+ int ret;
+
+ /* Device initializes into RDATAC mode, which we don't want */
+ ret = ads1298_write_cmd(priv, ADS1298_CMD_SDATAC);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(priv->regmap, ADS1298_REG_ID, &val);
+ if (ret)
+ return ret;
+
+ ret = priv->chip_info->parse_id(indio_dev, val);
+ if (ret)
+ return ret;
+
+ val = priv->chip_info->config3_reserved; /* Must write 1 always */
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)
+ if (priv->chip_info->has_vref_4v &&
+ regulator_get_voltage(priv->reg_avdd) >= 4400000)
val |= ADS1298_MASK_CONFIG3_VREF_4V;
}
ret = regmap_write(priv->regmap, ADS1298_REG_CONFIG3, val);
@@ -785,6 +840,10 @@ static int ads1298_probe(struct spi_device *spi)
priv = iio_priv(indio_dev);
+ priv->chip_info = spi_get_device_match_data(spi);
+ if (!priv->chip_info)
+ return -ENODEV;
+
/* Reset to be asserted before enabling clock and power */
reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset_gpio))
@@ -836,7 +895,8 @@ static int ads1298_probe(struct spi_device *spi)
init_completion(&priv->completion);
init_waitqueue_head(&priv->idle_wq);
spin_lock_init(&priv->irq_busy_lock);
- priv->regmap = devm_regmap_init(dev, NULL, priv, &ads1298_regmap_config);
+ priv->regmap = devm_regmap_init(dev, NULL, priv,
+ priv->chip_info->regmap_config);
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
@@ -896,13 +956,13 @@ static int ads1298_probe(struct spi_device *spi)
}
static const struct spi_device_id ads1298_id[] = {
- { .name = "ads1298" },
+ { .name = "ads1298", .driver_data = (kernel_ulong_t)&ads1298_chip_info },
{ }
};
MODULE_DEVICE_TABLE(spi, ads1298_id);
static const struct of_device_id ads1298_of_table[] = {
- { .compatible = "ti,ads1298" },
+ { .compatible = "ti,ads1298", .data = &ads1298_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, ads1298_of_table);
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 5/6] dt-bindings: iio: adc: ti,ads1298: add ADS1299
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
` (3 preceding siblings ...)
2026-10-24 23:13 ` [PATCH v2 4/6] iio: adc: ti-ads1298: prepare for more chip families Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
2026-10-24 23:13 ` [PATCH v2 6/6] iio: adc: ti-ads1298: add ADS1299 support Stephan Costa Barros
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
The ADS1299 is an 8-channel, 24-bit biopotential ADC for EEG with the
register map of the ADS129x. It differs in its gains, data rates and
ID layout, and has a fixed 4.5 V internal reference. The ADS1299-4 and
ADS1299-6 are the same part with four and six channels.
TI sells the 8-channel part as plain ADS1299, so ti,ads1299 names it,
and it is the fallback for ti,ads1299-4 and ti,ads1299-6: the channel
count can be read from the ID register. The ADS1299 is not compatible
with the ADS1298, so ti,ads1298 is not a fallback for it.
In the same way, give each ADS129x part a compatible, with ti,ads1298 as
the fallback: the ID register tells the parts apart, by family and
channel count. ti,ads1298 alone remains valid, so existing device trees
are not affected.
Both families have a digital supply, DVDD, and an active-low power-down
pin, PWDN, which the binding did not describe. Add them, both optional.
Add myself as a maintainer of the binding, next to Mike.
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
.../bindings/iio/adc/ti,ads1298.yaml | 41 +++++++++++++++----
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
index afcc683db..c88f0dd95 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1298.yaml
@@ -4,19 +4,36 @@
$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 and ads1299 medical ADC chips
description: |
- Datasheet at: https://www.ti.com/product/ADS1298
- Bindings for this chip aren't complete.
+ Datasheets at: https://www.ti.com/product/ADS1298
+ and https://www.ti.com/product/ADS1299
+ Bindings for these chips aren't complete.
maintainers:
- Mike Looijmans <mike.looijmans@topic.nl>
+ - Stephan Costa Barros <stephanbrrs8@gmail.com>
properties:
compatible:
- enum:
- - ti,ads1298
+ oneOf:
+ - enum:
+ - ti,ads1298
+ - ti,ads1299
+ - items:
+ - enum:
+ - ti,ads1294
+ - ti,ads1294r
+ - ti,ads1296
+ - ti,ads1296r
+ - ti,ads1298r
+ - const: ti,ads1298
+ - items:
+ - enum:
+ - ti,ads1299-4
+ - ti,ads1299-6
+ - const: ti,ads1299
reg:
maxItems: 1
@@ -26,15 +43,25 @@ properties:
reset-gpios:
maxItems: 1
+ powerdown-gpios:
+ description:
+ Active low GPIO connected to the PWDN pin. While it is asserted, the
+ device is powered down.
+ maxItems: 1
+
avdd-supply:
description:
Analog power supply, voltage between AVDD and AVSS. When providing a
symmetric +/- 2.5V, the regulator should report 5V.
+ dvdd-supply:
+ description: Digital power supply, voltage between DVDD and DGND.
+
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.
+ On the ads129x, it is 2.4V when analog supply is below 4.4V, 4V
+ otherwise. On the ads1299, it is 4.5V.
clocks:
description: Optional 2.048 MHz external source clock on CLK pin
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 6/6] iio: adc: ti-ads1298: add ADS1299 support
2026-10-24 23:13 [PATCH v2 0/6] iio: adc: ti-ads1298: fixes, and ADS1299 support Stephan Costa Barros
` (4 preceding siblings ...)
2026-10-24 23:13 ` [PATCH v2 5/6] dt-bindings: iio: adc: ti,ads1298: add ADS1299 Stephan Costa Barros
@ 2026-10-24 23:13 ` Stephan Costa Barros
5 siblings, 0 replies; 7+ messages in thread
From: Stephan Costa Barros @ 2026-10-24 23:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mike Looijmans, Andy Shevchenko, Uwe Kleine-König,
Krzysztof Kozlowski, linux-iio, devicetree, linux-kernel
The ADS1299, ADS1299-6 and ADS1299-4 are 8, 6 and 4-channel, 24-bit
biopotential ADCs for EEG. They share the register map, the commands
and the data format of the ADS129x, and differ in:
- the PGA gains, 1, 2, 4, 6, 8, 12 and 24;
- the data rates, f_CLK / 2^(7 + DR) with no HR bit, from 16 kSPS to
250 SPS, and the oversampling ratio, 64 * 2^DR;
- the internal reference, a fixed 4.5 V, and the reserved bits 6:5 of
CONFIG3;
- the ID register, which holds the channel count in bits 1:0;
- the register map, which ends at CONFIG4;
- the inputs, which are shorted out of reset.
Add the chip info for the ADS1299, and connect the inputs of every
channel at init, which changes nothing on the ADS129x, whose inputs are
already connected out of reset. The channel count is read from the ID
register, so ti,ads1299 also serves the ADS1299-4 and ADS1299-6 through
their fallback.
For both families, enable the DVDD supply when the device tree gives
one, and hold PWDN asserted, like RESET, until the supplies and the
clock are up. Nothing changes for a device tree that has neither.
Tested on an ADS1299 with a Raspberry Pi 4.
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
drivers/iio/adc/Kconfig | 6 +--
drivers/iio/adc/ti-ads1298.c | 100 +++++++++++++++++++++++++++++++++--
2 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a29ed8f45..724ae6123 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1920,14 +1920,14 @@ config TI_ADS1262
called ti-ads1262.
config TI_ADS1298
- tristate "Texas Instruments ADS1298"
+ tristate "Texas Instruments ADS1298 and ADS1299"
depends on SPI
select IIO_BUFFER
select IIO_KFIFO_BUF
select REGMAP
help
- If you say yes here you get support for Texas Instruments ADS1298
- medical ADC chips
+ If you say yes here you get support for Texas Instruments ADS1294,
+ ADS1296, ADS1298 and ADS1299 medical ADC chips, and their variants
This driver can also be built as a module. If so, the module will be
called ti-ads1298.
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index 5ad543bdf..e0cea23b1 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 and ADS1299 chip family driver
* Copyright (C) 2023 - 2024 Topic Embedded Products
+ * Copyright (C) 2026 Stephan Costa Barros <stephanbrrs8@gmail.com>
*/
#include <linux/bitfield.h>
@@ -45,6 +46,10 @@
#define ADS1298_MASK_ID_CHANNELS GENMASK(2, 0)
#define ADS1298_ID_FAMILY_ADS129X 0x90
#define ADS1298_ID_FAMILY_ADS129XR 0xd0
+/* ADS1299: bit 4 reads 1 and DEV_ID[3:2] is 11; bits 7:5 are the revision */
+#define ADS1299_MASK_ID_DEVICE GENMASK(4, 2)
+#define ADS1299_MASK_ID_CHANNELS GENMASK(1, 0)
+#define ADS1299_ID_DEVICE 0x7
#define ADS1298_REG_CONFIG1 0x01
#define ADS1298_MASK_CONFIG1_HR BIT(7)
@@ -65,12 +70,14 @@
#define ADS1298_MASK_CONFIG3_RESERVED BIT(6)
#define ADS1298_MASK_CONFIG3_VREF_4V BIT(5)
#define ADS1298_VREF_4V_MV 4000
+#define ADS1299_MASK_CONFIG3_RESERVED GENMASK(6, 5)
#define ADS1298_REG_LOFF 0x04
#define ADS1298_REG_CHnSET(n) (0x05 + (n))
#define ADS1298_MASK_CH_PD BIT(7)
#define ADS1298_MASK_CH_PGA GENMASK(6, 4)
#define ADS1298_MASK_CH_MUX GENMASK(2, 0)
+#define ADS1298_MUX_NORMAL 0
#define ADS1298_REG_LOFF_STATP 0x12
#define ADS1298_REG_LOFF_STATN 0x13
@@ -354,6 +361,15 @@ static int ads1298_set_samp_freq(struct ads1298_private *priv, int val)
return -EINVAL;
factor = (rate >> info->dr_shift) / val;
+
+ if (!info->has_hr) {
+ /* The slowest rate at or above val, or the lowest of all */
+ cfg = factor ? min(ilog2(factor), ADS1298_LOWEST_DR) : 0;
+
+ return regmap_update_bits(priv->regmap, ADS1298_REG_CONFIG1,
+ ADS1298_MASK_CONFIG1_DR, cfg);
+ }
+
if (factor >= BIT(ADS1298_LOWEST_DR + 1))
cfg = ADS1298_LOWEST_DR; /* Lowest rate, in LP mode */
else if (factor)
@@ -772,6 +788,43 @@ static int ads1298_parse_id(struct iio_dev *indio_dev, unsigned int id)
return 0;
}
+/* The ADS1299 has no WCT registers */
+static const struct regmap_config ads1299_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .reg_read = ads1298_reg_read,
+ .reg_write = ads1298_reg_write,
+ .max_register = ADS1298_REG_CONFIG4,
+ .wr_table = &ads1298_regmap_writeable,
+ .volatile_table = &ads1298_regmap_volatile,
+ .cache_type = REGCACHE_MAPLE,
+};
+
+static int ads1299_parse_id(struct iio_dev *indio_dev, unsigned int id)
+{
+ struct ads1298_private *priv = iio_priv(indio_dev);
+ struct device *dev = &priv->spi->dev;
+ unsigned int nu_ch;
+
+ nu_ch = FIELD_GET(ADS1299_MASK_ID_CHANNELS, id);
+ /* 0, 1 and 2 are 4, 6 and 8 channels; 3 is reserved */
+ if (FIELD_GET(ADS1299_MASK_ID_DEVICE, id) != ADS1299_ID_DEVICE ||
+ nu_ch > 2)
+ return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", id);
+ indio_dev->num_channels = 4 + 2 * nu_ch;
+
+ /* TI sells the 8-channel part as plain ADS1299 */
+ if (indio_dev->num_channels == ADS1298_MAX_CHANNELS)
+ indio_dev->name = "ads1299";
+ else
+ indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads1299-%u",
+ indio_dev->num_channels);
+ if (!indio_dev->name)
+ return -ENOMEM;
+
+ return 0;
+}
+
static const u8 ads1298_gains[] = { 6, 1, 2, 3, 4, 8, 12 };
static const struct ads1298_chip_info ads1298_chip_info = {
@@ -787,10 +840,24 @@ static const struct ads1298_chip_info ads1298_chip_info = {
.config3_reserved = ADS1298_MASK_CONFIG3_RESERVED,
};
+static const u8 ads1299_gains[] = { 1, 2, 4, 6, 8, 12, 24 };
+
+static const struct ads1298_chip_info ads1299_chip_info = {
+ .parse_id = ads1299_parse_id,
+ .regmap_config = &ads1299_regmap_config,
+ .gains = ads1299_gains,
+ .num_gains = ARRAY_SIZE(ads1299_gains),
+ .dr_shift = 7,
+ .osr_base = 64,
+ .vref_mv = 4500,
+ .config3_reserved = ADS1299_MASK_CONFIG3_RESERVED,
+};
+
static int ads1298_init(struct iio_dev *indio_dev)
{
struct ads1298_private *priv = iio_priv(indio_dev);
unsigned int val;
+ unsigned int i;
int ret;
/* Device initializes into RDATAC mode, which we don't want */
@@ -822,6 +889,16 @@ static int ads1298_init(struct iio_dev *indio_dev)
if (!priv->reg_vref)
msleep(ADS1298_REF_START_UP_MS);
+ /* The ADS1299 comes out of reset with its inputs shorted */
+ for (i = 0; i < indio_dev->num_channels; i++) {
+ ret = regmap_update_bits(priv->regmap, ADS1298_REG_CHnSET(i),
+ ADS1298_MASK_CH_MUX,
+ FIELD_PREP(ADS1298_MASK_CH_MUX,
+ ADS1298_MUX_NORMAL));
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -830,6 +907,7 @@ static int ads1298_probe(struct spi_device *spi)
struct ads1298_private *priv;
struct iio_dev *indio_dev;
struct device *dev = &spi->dev;
+ struct gpio_desc *powerdown_gpio;
struct gpio_desc *reset_gpio;
unsigned long clk_rate;
int ret;
@@ -844,12 +922,18 @@ static int ads1298_probe(struct spi_device *spi)
if (!priv->chip_info)
return -ENODEV;
- /* Reset to be asserted before enabling clock and power */
+ /* Reset and power-down to be asserted before enabling clock and power */
reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset_gpio))
return dev_err_probe(dev, PTR_ERR(reset_gpio),
"Cannot get reset GPIO\n");
+ powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(powerdown_gpio))
+ return dev_err_probe(dev, PTR_ERR(powerdown_gpio),
+ "Cannot get powerdown GPIO\n");
+
priv->reg_avdd = devm_regulator_get(dev, "avdd");
if (IS_ERR(priv->reg_avdd))
return dev_err_probe(dev, PTR_ERR(priv->reg_avdd),
@@ -863,6 +947,10 @@ static int ads1298_probe(struct spi_device *spi)
if (ret)
return ret;
+ ret = devm_regulator_get_enable_optional(dev, "dvdd");
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "Failed to enable dvdd regulator\n");
+
/* VREF can be supplied externally, otherwise use internal reference */
priv->reg_vref = devm_regulator_get_optional(dev, "vref");
if (IS_ERR(priv->reg_vref)) {
@@ -904,6 +992,9 @@ static int ads1298_probe(struct spi_device *spi)
indio_dev->channels = ads1298_channels;
indio_dev->info = &ads1298_info;
+ /* Out of power-down, now that the supplies and the clock are up */
+ gpiod_set_value_cansleep(powerdown_gpio, 0);
+
/* Power-on reset time, counted from when the clock is valid */
msleep(DIV_ROUND_UP(ADS1298_TPOR_CLOCKS * MILLI, clk_rate));
@@ -957,12 +1048,14 @@ static int ads1298_probe(struct spi_device *spi)
static const struct spi_device_id ads1298_id[] = {
{ .name = "ads1298", .driver_data = (kernel_ulong_t)&ads1298_chip_info },
+ { .name = "ads1299", .driver_data = (kernel_ulong_t)&ads1299_chip_info },
{ }
};
MODULE_DEVICE_TABLE(spi, ads1298_id);
static const struct of_device_id ads1298_of_table[] = {
{ .compatible = "ti,ads1298", .data = &ads1298_chip_info },
+ { .compatible = "ti,ads1299", .data = &ads1299_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, ads1298_of_table);
@@ -978,5 +1071,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("Stephan Costa Barros <stephanbrrs8@gmail.com>");
+MODULE_DESCRIPTION("TI ADS1298 and ADS1299 ADC");
MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread