Skip to contentSkip to conversation

iio: adc: ad4134: Enable greater sample rate data capture

PATCH Superseded Marcelo Schmitt 5 participants 32 messages Oct 17 – Oct 21

Subsystems (5)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Marcelo Schmitt

author maintainer
#0 lore raw

v5 cover iio: adc: ad4134: Enable greater sample rate data capture

Extend AD4134 support to enable greater sample rate data capture. To achieve that, provide control of digital filter type selection. The default filter type on device power up only supports sample rates up to 374 kSPS. In addition to that, enable interfacing with AD4134 in SPI 4-wire mode that can support reading ADC sample data through multiple wires. Finally, use SPI offloading to speed up data transfers at up to 1.49 MSPS.

Features being added:
- Data filter type selection.
- Triggered buffer data read.
- SPI 4-wire mode.
- High speed data capture with SPI offloading.
- Device tree doc updates.

Minor improvements (introduced before the new features):
- Sign extend sample data.
- Apply reset recovery wait time.
- Update include list to comply with IWYU principles.
- Serialize single-read operations.
- Run shorter transfers when CRC is disabled.
- Protect conventional device access from debug accesses [NEW PATCH]

Change log v4 -> v5:
[device tree]
- No changes in dt-doc.
[IIO]
- Initialized local variable with 0 to avoid build warns.
- Set max_register according to AD4134_NUM_CHANNELS.
- Apply reset recovery wait time. [NEW FIX PATCH]
- Protect conventional device access from debug accesses. [NEW PATCH]
- Re-arranged code so early function prototype declaration is no longer needed.
- Now using iio_for_each_active_channel() to iterate over IIO channels.
- Updated adi,spi-mode property handling.
- No longer selecting/deselecting multiplexer states when in minimum I/O mode.

Previous submissions:
v1: https://lore.kernel.org/linux-iio/[email protected]/
v2: https://lore.kernel.org/linux-iio/[email protected]/
v3: https://lore.kernel.org/linux-iio/[email protected]/
v4: https://lore.kernel.org/linux-iio/[email protected]/

Marcelo Schmitt (15):
iio: adc: ad4134: Adjust register map range
iio: adc: ad4134: Sign extend sample data
iio: adc: ad4134: Apply reset recovery wait time
iio: adc: ad4134: Update include list to comply with IWYU principles
iio: adc: ad4134: Serialize single-read operations
iio: adc: ad4134: Protect conventional device access from debug
accesses
iio: adc: ad4134: Run shorter transfers when CRC is disabled
iio: adc: ad4134: Add support for digital filter type selection
iio: adc: ad4134: Support buffered data read
dt-bindings: iio: adc: adi,ad4134: Document SPI connection mode
dt-bindings: iio: adc: adi,ad4134: Document external multiplexer usage
iio: adc: ad4134: Support SPI 4-wire mode
dt-bindings: iio: adc: adi,ad4134: Document PWM usage
iio: adc: ad4134: Support high-speed data capture
Docs: iio: Add AD4134

 .../bindings/iio/adc/adi,ad4134.yaml          | 110 ++-
 Documentation/iio/ad4134.rst                  | 127 +++
 Documentation/iio/index.rst                   |   1 +
 MAINTAINERS                                   |   1 +
 drivers/iio/adc/Kconfig                       |   8 +
 drivers/iio/adc/ad4134.c                      | 793 +++++++++++++++++-
 6 files changed, 987 insertions(+), 53 deletions(-)
 create mode 100644 Documentation/iio/ad4134.rst

base-commit: a22dc7ebb21a6bd4c8b63bc2c1dd46a24945aae7

Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH v5 1/15 iio: adc: ad4134: Adjust register map range

Due to a off-by-one mistake, privileged users would be able to read from an unsupported register address when reading device registers through a debug interface. Correct the off-by-one mistake at the high register map range to prevent access of unsupported addresses.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
Change log v4 -> v5
- Set max_register according to AD4134_NUM_CHANNELS.

 drivers/iio/adc/ad4134.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/adc/ad4134.c +2 -2
index 70f80bf1825f..8400c3934925 100644
@@ -111,7 +111,7 @@ static const struct regmap_range ad4134_regmap_rd_range[] = {
 	regmap_reg_range(AD4134_IFACE_CONFIG_A_REG, AD4134_SILICON_REV_REG),
 	regmap_reg_range(AD4134_SCRATCH_PAD_REG, AD4134_PW_DOWN_CTRL_REG),
 	regmap_reg_range(AD4134_DEVICE_STATUS_REG, AD4134_AIN_OR_ERROR_REG),
-	regmap_reg_range(AD4134_CH_VREG(0), AD4134_CH_VREG(AD4134_NUM_CHANNELS)),
+	regmap_reg_range(AD4134_CH_VREG(0), AD4134_CH_VREG(AD4134_NUM_CHANNELS - 1)),
 };
 
 static const struct regmap_range ad4134_regmap_wr_range[] = {
@@ -244,7 +244,7 @@ static const struct regmap_config ad4134_regmap_config = {
 	.reg_write = ad4134_reg_write,
 	.rd_table = &ad4134_regmap_rd_table,
 	.wr_table = &ad4134_regmap_wr_table,
-	.max_register = AD4134_CH_VREG(ARRAY_SIZE(ad4134_chan_set)),
+	.max_register = AD4134_CH_VREG(AD4134_NUM_CHANNELS - 1),
 };
 
 static int ad4134_read_raw(struct iio_dev *indio_dev,
Signature · 1 line
2.53.0
author maintainer
#2 lore raw

PATCH v5 2/15 iio: adc: ad4134: Sign extend sample data

In reply to Marcelo Schmitt

Sign extend ADC sample data for correct delivery of negative ADC input difference.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
Change log v5
- Initialized local variable with 0 to avoid build warns.

 drivers/iio/adc/ad4134.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +3 -1
index 8400c3934925..40b27b01f126 100644
@@ -172,6 +172,7 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
 			    unsigned int *val)
 {
 	unsigned int i;
+	u32 sample = 0;
 	int ret;
 
 	/*
@@ -196,8 +197,9 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
 		 * Clock out data from all channels to avoid that.
 		 */
 		if (i == AD4134_VREG_CH(reg))
-			*val = get_unaligned_be24(st->rx_buf);
+			sample = get_unaligned_be24(st->rx_buf);
 	}
+	*val = sign_extend32(sample, AD4134_CHAN_PRECISION_BITS - 1);
 
 	return 0;
 }
Signature · 1 line
2.53.0
author maintainer
#3 lore raw

PATCH v5 3/15 iio: adc: ad4134: Apply reset recovery wait time

In reply to Marcelo Schmitt

Add a reset recovery wait time to avoid any SPI transfer from running too soon during device probe.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
New patch in v5.

 drivers/iio/adc/ad4134.c | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/adc/ad4134.c +1 -0
index 40b27b01f126..d555874b4959 100644
@@ -442,6 +442,7 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(rst),
 				     "failed to get and deassert reset\n");
 
+	fsleep(AD4134_RESET_TIME_US);
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);
Signature · 1 line
2.53.0
#4 lore raw

Re: [PATCH v5 03/15] iio: adc: ad4134: Apply reset recovery wait time

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] iio: adc: ad4134: 10-second sleep in ad4134_probe() due to USEC_PER_SEC --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iio: adc: ad4134: 10-second sleep in ad4134_probe() due to USEC_PER_SEC --

commit 3488776492f80838a5517d1bec747527ca957473
Author: Marcelo Schmitt <[email protected]>

iio: adc: ad4134: Apply reset recovery wait time

This commit adds an fsleep() call after deasserting reset in ad4134_probe() to wait for reset recovery before starting SPI transfers.

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 448

Quoted 12 lines
drivers/iio/adc/ad4134.c +1 -0
index 40b27b01f1260..d555874b49597 100644
@@ -442,6 +442,7 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(rst),
 				     "failed to get and deassert reset\n");
 
+	fsleep(AD4134_RESET_TIME_US);
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);

[Severity: Medium]
Does calling fsleep(AD4134_RESET_TIME_US) here in ad4134_probe() cause the probe path to stall for 10 seconds?

Earlier in drivers/iio/adc/ad4134.c, AD4134_RESET_TIME_US is defined using USEC_PER_SEC:

    #define AD4134_RESET_TIME_US			(10 * USEC_PER_SEC)

This evaluates to 10,000,000 microseconds (10 seconds), causing fsleep() to invoke msleep(10000) every time devm_reset_control_get_optional_exclusive_deasserted() succeeds during probe. Should AD4134_RESET_TIME_US be defined with USEC_PER_MSEC for a 10 millisecond recovery delay instead?

титься

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Reviewed-by on v5 3/15
#5 lore raw

Re: [PATCH v5 03/15] iio: adc: ad4134: Apply reset recovery wait time

In reply to Marcelo Schmitt
Quoted 16 lines · On Fri, 16 Oct 2026 22:57:08 -0300 Marcelo Schmitt…

Add a reset recovery wait time to avoid any SPI transfer from running too soon during device probe.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
New patch in v5.

 drivers/iio/adc/ad4134.c | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/adc/ad4134.c +0 -0
index 40b27b01f126..d555874b4959 100644

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 448

Quoted 7 lines
drivers/iio/adc/ad4134.c +1 -0
@@ -442,6 +442,7 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(rst),
 				     "failed to get and deassert reset\n");
 
+	fsleep(AD4134_RESET_TIME_US);
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);
Signature · 2 lines
Kind regards,
Joshua Crofts
maintainer
#6 lore raw

Re: [PATCH v5 03/15] iio: adc: ad4134: Apply reset recovery wait time

Quoted 22 lines · On Sat, 17 Oct 2026 07:55:13 +0200 Joshua Crofts <[email protected]>…
Quoted 19 lines · On Fri, 16 Oct 2026 22:57:08 -0300 Marcelo Schmitt…

Add a reset recovery wait time to avoid any SPI transfer from running too soon during device probe.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
New patch in v5.

 drivers/iio/adc/ad4134.c | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/adc/ad4134.c +0 -0
index 40b27b01f126..d555874b4959 100644
@@ -442,6 +442,7 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(rst),
 				     "failed to get and deassert reset\n");
 
Quoted 1 line

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 445

Quoted 1 line
drivers/iio/adc/ad4134.c +1 -0
+	fsleep(AD4134_RESET_TIME_US);

Sashiko calls out that this is currently 10 seconds, which seems unlikely!

Quoted 6 lines

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 448

Quoted 3 lines
drivers/iio/adc/ad4134.c +0 -0
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);  
#7 lore raw

Re: [PATCH v5 03/15] iio: adc: ad4134: Apply reset recovery wait time

Quoted 25 lines · Jonathan Cameron
Quoted 22 lines · On Sat, 17 Oct 2026 07:55:13 +0200 Joshua Crofts <[email protected]>…
Quoted 19 lines · On Fri, 16 Oct 2026 22:57:08 -0300 Marcelo Schmitt…

Add a reset recovery wait time to avoid any SPI transfer from running too soon during device probe.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")

---
New patch in v5.

 drivers/iio/adc/ad4134.c | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/adc/ad4134.c +0 -0
index 40b27b01f126..d555874b4959 100644
@@ -442,6 +442,7 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(rst),
 				     "failed to get and deassert reset\n");
 
Quoted 3 lines
Quoted 1 line

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 445

Quoted 1 line
drivers/iio/adc/ad4134.c +1 -0
+	fsleep(AD4134_RESET_TIME_US);

Sashiko calls out that this is currently 10 seconds, which seems unlikely!

It was actually correct. AD4134_RESET_TIME_US was misset. Fixed that for v6.

author maintainer
#8 lore raw

PATCH v5 4/15 iio: adc: ad4134: Update include list to comply with IWYU principles

In reply to Marcelo Schmitt

Add includes that were missing (according to include-what-you-use tool) to comply with include what is used principles. Namely, add time64.h for USEC_PER_SEC.

Remove time.h which is covered by time64.h and iio/types.h which is covered by iio/iio.h.

While updating the list, organize it to follow current IIO include list convention.

---
 drivers/iio/adc/ad4134.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/iio/adc/ad4134.c +3 -3
index d555874b4959..dc2b32c73a3a 100644
@@ -14,18 +14,18 @@
 #include <linux/err.h>
 #include <linux/export.h>
 #include <linux/gpio/consumer.h>
-#include <linux/iio/iio.h>
-#include <linux/iio/types.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
-#include <linux/time.h>
+#include <linux/time64.h>
 #include <linux/types.h>
 #include <linux/unaligned.h>
 #include <linux/units.h>
 
+#include <linux/iio/iio.h>
+
 #define AD4134_RESET_TIME_US			(10 * USEC_PER_SEC)
 
 #define AD4134_REG_READ_MASK			BIT(7)
Signature · 1 line
2.53.0
author maintainer
#9 lore raw

PATCH v5 5/15 iio: adc: ad4134: Serialize single-read operations

In reply to Marcelo Schmitt

Protect single-sample read operations from going wrong if two or more of them run concurrently. Serialize the data read operation so to avoid single-sample read requests disrupting each other.

---
 drivers/iio/adc/ad4134.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +15 -1
index dc2b32c73a3a..dc4d801490db 100644
@@ -7,6 +7,7 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/crc8.h>
 #include <linux/delay.h>
@@ -15,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
@@ -98,6 +100,11 @@ struct ad4134_state {
 	struct regmap *regmap;
 	unsigned long sys_clk_hz;
 	struct gpio_desc *odr_gpio;
+	/*
+	 * Synchronize access to device state fields and ensure atomicity of
+	 * consecutive register access operations.
+	 */
+	struct mutex sync_lock;
 	int refin_mv;
 	/*
 	 * DMA (thus cache coherency maintenance) requires the transfer buffers
@@ -257,7 +264,9 @@ static int ad4134_read_raw(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (info) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->sync_lock);
+
 		gpiod_set_value_cansleep(st->odr_gpio, 1);
 		/*
 		 * For slave mode gated DCLK (data sheet page 11), the minimum
@@ -276,6 +285,7 @@ static int ad4134_read_raw(struct iio_dev *indio_dev,
 			return ret;
 
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->refin_mv;
 		*val2 = AD4134_CHAN_PRECISION_BITS - 1;
@@ -445,6 +455,10 @@ static int ad4134_probe(struct spi_device *spi)
 	fsleep(AD4134_RESET_TIME_US);
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
+	ret = devm_mutex_init(dev, &st->sync_lock);
+	if (ret)
+		return ret;
+
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);
 	if (IS_ERR(st->regmap))
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
Signature · 1 line
2.53.0
maintainer
#10 lore raw

Re: [PATCH v5 05/15] iio: adc: ad4134: Serialize single-read operations

Quoted 3 lines · On Fri, 16 Oct 2026 22:57:47 -0300 Marcelo Schmitt…

Protect single-sample read operations from going wrong if two or more of them run concurrently. Serialize the data read operation so to avoid single-sample read requests disrupting each other.

Why isn't this a fix (with appropriate fixes tag)? If nothing else comes up and you agree it should have one just reply to this email with it.

Otherwise LGTM

Quoted 70 lines
---
 drivers/iio/adc/ad4134.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +15 -1
index dc2b32c73a3a..dc4d801490db 100644
@@ -7,6 +7,7 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/crc8.h>
 #include <linux/delay.h>
@@ -15,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
@@ -98,6 +100,11 @@ struct ad4134_state {
 	struct regmap *regmap;
 	unsigned long sys_clk_hz;
 	struct gpio_desc *odr_gpio;
+	/*
+	 * Synchronize access to device state fields and ensure atomicity of
+	 * consecutive register access operations.
+	 */
+	struct mutex sync_lock;
 	int refin_mv;
 	/*
 	 * DMA (thus cache coherency maintenance) requires the transfer buffers
@@ -257,7 +264,9 @@ static int ad4134_read_raw(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (info) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->sync_lock);
+
 		gpiod_set_value_cansleep(st->odr_gpio, 1);
 		/*
 		 * For slave mode gated DCLK (data sheet page 11), the minimum
@@ -276,6 +285,7 @@ static int ad4134_read_raw(struct iio_dev *indio_dev,
 			return ret;
 
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->refin_mv;
 		*val2 = AD4134_CHAN_PRECISION_BITS - 1;
@@ -445,6 +455,10 @@ static int ad4134_probe(struct spi_device *spi)
 	fsleep(AD4134_RESET_TIME_US);
 	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
 
+	ret = devm_mutex_init(dev, &st->sync_lock);
+	if (ret)
+		return ret;
+
 	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);
 	if (IS_ERR(st->regmap))
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
#11 lore raw

Re: [PATCH v5 05/15] iio: adc: ad4134: Serialize single-read operations

Quoted 10 lines · Jonathan Cameron
Quoted 3 lines · On Fri, 16 Oct 2026 22:57:47 -0300 Marcelo Schmitt…

Protect single-sample read operations from going wrong if two or more of them run concurrently. Serialize the data read operation so to avoid single-sample read requests disrupting each other.

Why isn't this a fix (with appropriate fixes tag)? If nothing else comes up and you agree it should have one just reply to this email with it.

I seem to have forgotten to add that.

Fixes e0bc6d7e2584 ("iio: adc: Initial support for AD4134")
Quoted 7 lines

Otherwise LGTM

Quoted 4 lines

Thanks,
Marcelo

author maintainer
#12 lore raw

PATCH v5 6/15 iio: adc: ad4134: Protect conventional device access from debug accesses

In reply to Marcelo Schmitt

Require debug access to acquire device synchronization locks before allowing any transfer from the debug interface so to avoid disrupting operations issued from usual (non-debug) interfaces.

---
New patch in v5.

I'm not sure about this one and have no strong opinion about it. Providing it according to suggestion from Nuno.
https://lore.kernel.org/linux-iio/179199692826.658576.4841754843069240356.b4-review@b4/
From previous discussions I guess this patch might not be welcome.
https://lore.kernel.org/linux-iio/20250421142359.11fb483f@jic23-huawei/
In that case, completely fine with it being dropped.

 drivers/iio/adc/ad4134.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/iio/adc/ad4134.c +6 -0
index dc4d801490db..42a1272c2894 100644
@@ -302,6 +302,12 @@ static int ad4134_debugfs_reg_access(struct iio_dev *indio_dev,
 {
 	struct ad4134_state *st = iio_priv(indio_dev);
 
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	guard(mutex)(&st->sync_lock);
+
 	if (readval)
 		return regmap_read(st->regmap, reg, readval);
 
Signature · 1 line
2.53.0
maintainer
#13 lore raw

Re: [PATCH v5 06/15] iio: adc: ad4134: Protect conventional device access from debug accesses

Quoted 3 lines · On Fri, 16 Oct 2026 22:58:05 -0300 Marcelo Schmitt…

Require debug access to acquire device synchronization locks before allowing any transfer from the debug interface so to avoid disrupting operations issued from usual (non-debug) interfaces.

These needs more. What sort of disruption are we talking?

A deliberate write to change state that breaks normal operation is a debug mess up that we don't guard against, but if a read is enough to break things, or there is an inherent disruption of other operations by using the bus at all then state that.

What isn't regmap locking covering?

If this is all about the mux stuff and backend usage, then move it to nearer that in the series and refer to that.

Jonathan

Quoted 32 lines

---
New patch in v5.

I'm not sure about this one and have no strong opinion about it. Providing it according to suggestion from Nuno.
https://lore.kernel.org/linux-iio/179199692826.658576.4841754843069240356.b4-review@b4/
From previous discussions I guess this patch might not be welcome.
https://lore.kernel.org/linux-iio/20250421142359.11fb483f@jic23-huawei/
In that case, completely fine with it being dropped.

 drivers/iio/adc/ad4134.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/iio/adc/ad4134.c +6 -0
index dc4d801490db..42a1272c2894 100644
@@ -302,6 +302,12 @@ static int ad4134_debugfs_reg_access(struct iio_dev *indio_dev,
 {
 	struct ad4134_state *st = iio_priv(indio_dev);
 
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	guard(mutex)(&st->sync_lock);
+
 	if (readval)
 		return regmap_read(st->regmap, reg, readval);
 
#14 lore raw

Re: [PATCH v5 06/15] iio: adc: ad4134: Protect conventional device access from debug accesses

Quoted 7 lines · Jonathan Cameron
Quoted 3 lines · On Fri, 16 Oct 2026 22:58:05 -0300 Marcelo Schmitt…

Require debug access to acquire device synchronization locks before allowing any transfer from the debug interface so to avoid disrupting operations issued from usual (non-debug) interfaces.

Quoted 6 lines

These needs more. What sort of disruption are we talking?

A deliberate write to change state that breaks normal operation is a debug mess up that we don't guard against, but if a read is enough to break things, or there is an inherent disruption of other operations by using the bus at all then state that.

A read is enough to break any ongoing buffered read because register reads will cause the bus configuration to change to a state that invalidates any sample read that happens afterwards.

Quoted 2 lines

What isn't regmap locking covering?

regmap doens't cover IIO device direct/buffered modes.

Quoted 3 lines

If this is all about the mux stuff and backend usage, then move it to nearer that in the series and refer to that.

We also need the mux usage to be synchronized which is why debugfs_reg_access now also locks on device state sync_lock.

In practice, because every device access will select then deselect the mux state, we are not able to have two operations going on simultaneously (not even two register read can run concurrently due to the needed mux state care).

On v3, there was a dedicated mutex for mux access synchronization. In an atempt to make things more simple, that second mutex was removed and sync_lock is now also utilized to prevent multiple threads from changing the mux state simultaneously. It shall work properly as long as any device access first
claims IIO direct + get sync_lock or claims IIO buffered mode.
Is it okay to have it like that?

Quoted 36 lines

Jonathan

Quoted 32 lines

---
New patch in v5.

I'm not sure about this one and have no strong opinion about it. Providing it according to suggestion from Nuno.
https://lore.kernel.org/linux-iio/179199692826.658576.4841754843069240356.b4-review@b4/
From previous discussions I guess this patch might not be welcome.
https://lore.kernel.org/linux-iio/20250421142359.11fb483f@jic23-huawei/
In that case, completely fine with it being dropped.

 drivers/iio/adc/ad4134.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/iio/adc/ad4134.c +6 -0
index dc4d801490db..42a1272c2894 100644
@@ -302,6 +302,12 @@ static int ad4134_debugfs_reg_access(struct iio_dev *indio_dev,
 {
 	struct ad4134_state *st = iio_priv(indio_dev);
 
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	guard(mutex)(&st->sync_lock);
+
 	if (readval)
 		return regmap_read(st->regmap, reg, readval);
 
maintainer
#15 lore raw

Re: [PATCH v5 06/15] iio: adc: ad4134: Protect conventional device access from debug accesses

Quoted 10 lines · On Mon, 19 Oct 2026 18:04:28 -0300 Marcelo Schmitt…
Quoted 9 lines · Jonathan Cameron
Quoted 3 lines · On Fri, 16 Oct 2026 22:58:05 -0300 Marcelo Schmitt…

Require debug access to acquire device synchronization locks before allowing any transfer from the debug interface so to avoid disrupting operations issued from usual (non-debug) interfaces.

These needs more. What sort of disruption are we talking?

Quoted 8 lines
Quoted 4 lines

A deliberate write to change state that breaks normal operation is a debug mess up that we don't guard against, but if a read is enough to break things, or there is an inherent disruption of other operations by using the bus at all then state that.

A read is enough to break any ongoing buffered read because register reads will cause the bus configuration to change to a state that invalidates any sample read that happens afterwards.

Capture that in the commit message.

Quoted 17 lines
Quoted 3 lines

What isn't regmap locking covering?

regmap doens't cover IIO device direct/buffered modes.

Quoted 3 lines

If this is all about the mux stuff and backend usage, then move it to nearer that in the series and refer to that.

We also need the mux usage to be synchronized which is why debugfs_reg_access now also locks on device state sync_lock.

In practice, because every device access will select then deselect the mux state, we are not able to have two operations going on simultaneously (not even two register read can run concurrently due to the needed mux state care).

Quoted 6 lines

On v3, there was a dedicated mutex for mux access synchronization. In an atempt to make things more simple, that second mutex was removed and sync_lock is now also utilized to prevent multiple threads from changing the mux state simultaneously. It shall work properly as long as any device access first
claims IIO direct + get sync_lock or claims IIO buffered mode.
Is it okay to have it like that?

That seems fine. Just add some documentation on the requirements in appropriate places.

Jonathan

author maintainer
#16 lore raw

PATCH v5 7/15 iio: adc: ad4134: Run shorter transfers when CRC is disabled

In reply to Marcelo Schmitt

When AD4134 has its SPI CRC feature enabled, one extra byte (the CRC code) is sent through SPI for both ends to verify the integrity of data transfers. Though, when CRC is not enabled, SPI transfers can be shorter.

---
 drivers/iio/adc/ad4134.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/iio/adc/ad4134.c +6 -4
index 42a1272c2894..91a7c2107770 100644
@@ -106,6 +106,7 @@ struct ad4134_state {
 	 */
 	struct mutex sync_lock;
 	int refin_mv;
+	bool crc_en;
 	/*
 	 * DMA (thus cache coherency maintenance) requires the transfer buffers
 	 * to live in their own cache lines.
@@ -159,7 +160,7 @@ static int ad4134_reg_write(void *context, unsigned int reg, unsigned int val)
 	struct spi_transfer xfer = {
 		.tx_buf = st->tx_buf,
 		.rx_buf = st->rx_buf,
-		.len = AD4134_SPI_MAX_XFER_LEN,
+		.len = st->crc_en ? AD4134_SPI_MAX_XFER_LEN : 2,
 	};
 	int ret;
 
@@ -169,7 +170,7 @@ static int ad4134_reg_write(void *context, unsigned int reg, unsigned int val)
 	if (ret)
 		return ret;
 
-	if (st->rx_buf[2] != st->tx_buf[2])
+	if (st->crc_en && st->rx_buf[2] != st->tx_buf[2])
 		dev_dbg(&st->spi->dev, "reg write CRC check failed\n");
 
 	return 0;
@@ -217,7 +218,7 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
 	struct spi_transfer xfer = {
 		.tx_buf = st->tx_buf,
 		.rx_buf = st->rx_buf,
-		.len = AD4134_SPI_MAX_XFER_LEN,
+		.len = st->crc_en ? AD4134_SPI_MAX_XFER_LEN : 2,
 	};
 	unsigned int inst;
 	int ret;
@@ -232,7 +233,7 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
 	*val = st->rx_buf[1];
 
 	/* Check CRC */
-	if (st->rx_buf[2] != st->tx_buf[2])
+	if (st->crc_en && st->rx_buf[2] != st->tx_buf[2])
 		dev_dbg(&st->spi->dev, "reg read CRC check failed\n");
 
 	return 0;
@@ -319,6 +320,7 @@ static int ad4134_min_io_mode_setup(struct ad4134_state *st)
 	struct device *dev = &st->spi->dev;
 	int ret;
 
+	st->crc_en = true; /* In minimum I/O mode CRC cannot be disabled */
 	st->odr_gpio = devm_gpiod_get(dev, "odr", GPIOD_OUT_LOW);
 	if (IS_ERR(st->odr_gpio))
 		return dev_err_probe(dev, PTR_ERR(st->odr_gpio),
Signature · 1 line
2.53.0
author maintainer
#17 lore raw

PATCH v5 8/15 iio: adc: ad4134: Add support for digital filter type selection

In reply to Marcelo Schmitt

AD4134 supports four types of programmable digital filters that allow users to choose between different output data rates and input bandwidths. Enable basic digital filter configuration by providing a way to select which filter type is activated for each channel.

---
Change log v4 -> v5
- Re-arranged code so early function prototype declaration is no longer needed.

 drivers/iio/adc/ad4134.c | 93 ++++++++++++++++++++++++++++++++++------
 1 file changed, 81 insertions(+), 12 deletions(-)
drivers/iio/adc/ad4134.c +81 -12
index 91a7c2107770..b245001d4551 100644
@@ -63,6 +63,9 @@
 #define AD4134_PW_DOWN_CTRL_REG			0x13
 #define AD4134_DEVICE_STATUS_REG		0x15
 #define AD4134_ODR_VAL_INT_LSB_REG		0x16
+#define AD4134_CHAN_DIG_FILTER_SEL_REG		0x1E
+#define AD4134_CHAN_DIG_FILTER_SEL_CH_MASK(ch)	(GENMASK(1, 0) << 2 * (ch))
+
 #define AD4134_CH3_OFFSET_MSB_REG		0x3E
 #define AD4134_AIN_OR_ERROR_REG			0x48
 
@@ -80,19 +83,18 @@
 #define AD4134_SPI_CRC_INIT_VALUE		0xA5
 static unsigned char ad4134_spi_crc_table[CRC8_TABLE_SIZE];
 
-#define AD4134_CHANNEL(_index) {						\
-	.type = IIO_VOLTAGE,							\
-	.indexed = 1,								\
-	.channel = (_index),							\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
-}
+enum ad4134_filter_type {
+	AD4134_WIDEBAND,
+	AD4134_SINC6,
+	AD4134_SINC3,
+	AD4134_SINC3_REJ60,
+};
 
-static const struct iio_chan_spec ad4134_chan_set[] = {
-	AD4134_CHANNEL(0),
-	AD4134_CHANNEL(1),
-	AD4134_CHANNEL(2),
-	AD4134_CHANNEL(3),
+static const char * const ad4134_filt_names[] = {
+	[AD4134_WIDEBAND] = "wideband",
+	[AD4134_SINC6] = "sinc6",
+	[AD4134_SINC3] = "sinc3",
+	[AD4134_SINC3_REJ60] = "sinc3+rej60",
 };
 
 struct ad4134_state {
@@ -115,6 +117,73 @@ struct ad4134_state {
 	u8 tx_buf[AD4134_SPI_MAX_XFER_LEN];
 };
 
+static int ad4134_set_filter_type(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  unsigned int val)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+	unsigned int mask, reg_val;
+
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	guard(mutex)(&st->sync_lock);
+
+	mask = AD4134_CHAN_DIG_FILTER_SEL_CH_MASK(chan->channel);
+	reg_val = field_prep(mask, val);
+	return regmap_update_bits(st->regmap, AD4134_CHAN_DIG_FILTER_SEL_REG,
+				  mask, reg_val);
+}
+
+static int ad4134_get_filter_type(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+	unsigned int mask, reg_val;
+	int ret;
+
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	ret = regmap_read(st->regmap, AD4134_CHAN_DIG_FILTER_SEL_REG, &reg_val);
+	if (ret)
+		return ret;
+
+	mask = AD4134_CHAN_DIG_FILTER_SEL_CH_MASK(chan->channel);
+	return field_get(mask, reg_val);
+}
+
+static const struct iio_enum ad4134_filter_type_enum = {
+	.items = ad4134_filt_names,
+	.num_items = ARRAY_SIZE(ad4134_filt_names),
+	.get = ad4134_get_filter_type,
+	.set = ad4134_set_filter_type,
+};
+
+static const struct iio_chan_spec_ext_info ad4134_filter_type_ext_info[] = {
+	IIO_ENUM("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
+	{ }
+};
+
+#define AD4134_CHANNEL(_index) {						\
+	.type = IIO_VOLTAGE,							\
+	.indexed = 1,								\
+	.channel = (_index),							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
+	.ext_info = ad4134_filter_type_ext_info,				\
+}
+
+static const struct iio_chan_spec ad4134_chan_set[] = {
+	AD4134_CHANNEL(0),
+	AD4134_CHANNEL(1),
+	AD4134_CHANNEL(2),
+	AD4134_CHANNEL(3),
+};
+
 static const struct regmap_range ad4134_regmap_rd_range[] = {
 	regmap_reg_range(AD4134_IFACE_CONFIG_A_REG, AD4134_SILICON_REV_REG),
 	regmap_reg_range(AD4134_SCRATCH_PAD_REG, AD4134_PW_DOWN_CTRL_REG),
Signature · 1 line
2.53.0
#18 lore raw

Re: [PATCH v5 08/15] iio: adc: ad4134: Add support for digital filter type selection

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] iio: adc: ad4134: undocumented sinc6 filter_type and sysfs ABI attributes --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: adc: ad4134: undocumented sinc6 filter_type and sysfs ABI attributes --

commit f7364f2a84e232ee97a6956a54b434e35fd7f79d
Author: Marcelo Schmitt <[email protected]>

iio: adc: ad4134: Add support for digital filter type selection

This commit adds per-channel digital filter type configuration for the AD4134 ADC by exposing filter_type and filter_type_available sysfs attributes via IIO_ENUM.

Quoted 4 lines
drivers/iio/adc/ad4134.c +0 -0
index 91a7c21077709..b245001d45517 100644

[ ... ]

Quoted 1 line
drivers/iio/adc/ad4134.c +0 -0
@@ -80,19 +83,18 @@

[ ... ]

Quoted 24 lines
drivers/iio/adc/ad4134.c +11 -5
+enum ad4134_filter_type {
+	AD4134_WIDEBAND,
+	AD4134_SINC6,
+	AD4134_SINC3,
+	AD4134_SINC3_REJ60,
+};
 
-static const struct iio_chan_spec ad4134_chan_set[] = {
-	AD4134_CHANNEL(0),
-	AD4134_CHANNEL(1),
-	AD4134_CHANNEL(2),
-	AD4134_CHANNEL(3),
+static const char * const ad4134_filt_names[] = {
+	[AD4134_WIDEBAND] = "wideband",
+	[AD4134_SINC6] = "sinc6",
+	[AD4134_SINC3] = "sinc3",
+	[AD4134_SINC3_REJ60] = "sinc3+rej60",
 };
 
 struct ad4134_state {
@@ -115,6 +117,73 @@ struct ad4134_state {
 	u8 tx_buf[AD4134_SPI_MAX_XFER_LEN];
 };
 

[ ... ]

Quoted 11 lines
drivers/iio/adc/ad4134.c +11 -0
+static const struct iio_enum ad4134_filter_type_enum = {
+	.items = ad4134_filt_names,
+	.num_items = ARRAY_SIZE(ad4134_filt_names),
+	.get = ad4134_get_filter_type,
+	.set = ad4134_set_filter_type,
+};
+
+static const struct iio_chan_spec_ext_info ad4134_filter_type_ext_info[] = {
+	IIO_ENUM("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SEPARATE, &ad4134_filter_type_enum),
+	{ }

drivers/iio/adc/ad4134.c struct ad4134_state { · line 178

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+};
+
+#define AD4134_CHANNEL(_index) {						\
+	.type = IIO_VOLTAGE,							\
+	.indexed = 1,								\
+	.channel = (_index),							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
+	.ext_info = ad4134_filter_type_ext_info,				\
+}

[Severity: Low]
In drivers/iio/adc/ad4134.c, ad4134_filt_names[] introduces "sinc6" as a new filter_type string option when the driver probes and registers ad4134_chan_set[].

In addition, attaching ad4134_filter_type_ext_info[] with IIO_SEPARATE to indexed non-differential IIO_VOLTAGE channels in AD4134_CHANNEL() exposes /sys/bus/iio/devices/iio:deviceX/in_voltageY_filter_type and /sys/bus/iio/devices/iio:deviceX/in_voltageY_filter_type_available to userspace.

Currently, Documentation/ABI/testing/sysfs-bus-iio only lists What entries for filter_type, in_voltageY-voltageZ_filter_type, filter_type_available, and in_voltage-voltage_filter_type_available, and its list of valid filter types does not include "sinc6".

Should Documentation/ABI/testing/sysfs-bus-iio be updated to document the "sinc6" filter option and the in_voltageY_filter_type and in_voltageY_filter_type_available attributes?

Quoted 7 lines
drivers/iio/adc/ad4134.c +7 -0
+
+static const struct iio_chan_spec ad4134_chan_set[] = {
+	AD4134_CHANNEL(0),
+	AD4134_CHANNEL(1),
+	AD4134_CHANNEL(2),
+	AD4134_CHANNEL(3),
+};
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
author maintainer
#19 lore raw

PATCH v5 9/15 iio: adc: ad4134: Support buffered data read

In reply to Marcelo Schmitt

Enable users to run buffered data captures triggered by IIO trigger device. Add an IIO timestamp channel so each data scan is provided with measurement time information. Require single-read operations to be in IIO device direct access mode to prevent buffered and single-shot data captures to disrupt each other.

---
Change log v4 -> v5
- Now using iio_for_each_active_channel() to iterate over IIO channels.

 drivers/iio/adc/Kconfig  |  2 ++
 drivers/iio/adc/ad4134.c | 61 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 61 insertions(+), 2 deletions(-)
drivers/iio/adc/Kconfig +2 -0
index 02de4809ae31..d625f36a0283 100644
@@ -125,6 +125,8 @@ config AD4134
 	depends on SPI
 	select REGMAP_SPI
 	select CRC8
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Analog Devices AD4134 SPI analog to
 	  digital converters (ADC).
drivers/iio/adc/ad4134.c +59 -2

110 lines not on this page · raw message

Signature · 1 line
2.53.0
Reviewed-by on v5 9/15
#20 lore raw

Re: [PATCH v5 09/15] iio: adc: ad4134: Support buffered data read

Quoted 8 lines · On Fri, 16 Oct 2026 22:59:00 -0300 Marcelo Schmitt…

Enable users to run buffered data captures triggered by IIO trigger device. Add an IIO timestamp channel so each data scan is provided with measurement time information. Require single-read operations to be in IIO device direct access mode to prevent buffered and single-shot data captures to disrupt each other.

---

Signature · 2 lines
Kind regards,
Joshua Crofts
author maintainer
#21 lore raw

PATCH v5 10/15 dt-bindings: iio: adc: adi,ad4134: Document SPI connection mode

In reply to Marcelo Schmitt

Document how AD4134 chips are connected to the host SPI controller according to different wiring configurations.

---
No changes in v5.

 .../bindings/iio/adc/adi,ad4134.yaml          | 57 ++++++++++++++++---
 1 file changed, 50 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml +50 -7
index ea6d7e026419..970e7adb2405 100644
@@ -131,6 +131,26 @@ properties:
     enum: [ free-running, gated ]
     default: gated
 
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ no-cs, 4-wire, one-channel-chain, two-channel-chain ]
+    default: no-cs
+    description: |
+      This property indicates the SPI wiring configuration.
+      * no-cs: The datasheet calls this "minimum I/O mode". In this mode, the CS
+        line is not used (tied to GND).
+      * 4-wire: Specifies a conventional SPI wiring configuration.
+      * one-channel-chain: In this mode, the DOUT0 line of one chip is connected
+        to DOUT2 of the next chip upstream in the chain. Only the data interface
+        of the upmost device in the stream is in direct connection with the
+        digital host.
+      * two-channel-chain: In this mode, the DOUT0 and DOUT1 lines of one chip
+        are respectively connected to DOUT2 and DOUT3 of the next chip upstream
+        in the chain. Only the data interface of the upmost device in the stream
+        is in direct connection with the digital host.
+
+  '#daisy-chained-devices': true
+
 required:
   - compatible
   - reg
@@ -141,13 +161,36 @@ required:
   - clocks
   - clock-names
 
-oneOf:
-  - required:
-      - ldoin-supply
-  - required:
-      - avdd1v8-supply
-      - dvdd1v8-supply
-      - clkvdd-supply
+allOf:
+  - oneOf:
+      - required:
+          - ldoin-supply
+      - required:
+          - avdd1v8-supply
+          - dvdd1v8-supply
+          - clkvdd-supply
+
+  - oneOf:
+      - required: ['adi,spi-mode', '#daisy-chained-devices']
+        properties:
+          adi,spi-mode:
+            const: one-channel-chain
+          spi-rx-bus-width:
+            const: 1
+      - required: ['adi,spi-mode', '#daisy-chained-devices']
+        properties:
+          adi,spi-mode:
+            const: two-channel-chain
+          spi-rx-bus-width:
+            items:
+              - const: 1
+              - const: 1
+      - properties:
+          '#daisy-chained-devices': false
+          adi,spi-mode:
+            enum:
+              - no-cs
+              - 4-wire
 
 unevaluatedProperties: false
 
Signature · 1 line
2.53.0
author maintainer
#22 lore raw

PATCH v5 11/15 dt-bindings: iio: adc: adi,ad4134: Document external multiplexer usage

In reply to Marcelo Schmitt

The AD4134 design has two data interfaces. One interface allows register access for device configuration while the other (separate interface) provides ADC sample data. One way of handling both peripheral interfaces is to merge them into a single SPI interface by switching between register access and sample access according to device user requests. Though, such solution requires extra hardware, external to the ADC chip. The access mode switch can be done with an external multiplexer selecting either AD4134 SDO or AD4134 DOUT0 to connect to the controller. The external multiplexer becomes part of hardware requested for AD4134 device operation and thus must be provided for operating the peripheral in such merged interface schema. Still, there are alternative ways of handling the two AD4134 data interfaces so the multiplexer is not always required.

---
No changes in v5.

Detailed reasoning for the external multiplexer usage.

Before coming to the current solution, the following configuration was tried.

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller  |
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |---+                       |                 |
  |                       |   |                       |                 |
  | Data interface  DOUT0 |---+---------------------->| SDI0            |
  | for ADC sample  DOUT1 |-------------------------->| SDI1            |
  | data read       DOUT2 |-------------------------->| SDI2            |
  |                 DOUT3 |-------------------------->| SDI3            |
  |                 DCLK  |<--------------------------| DCLK
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

Though, because DOUT0 never goes high-Z, the DOUT0 pin keeps driving the data line, causing register reads to fail.

Alternatively, we could have something like

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller  |
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI0            |
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI1            |
  | for ADC sample  DOUT1 |-------------------------->| SDI2            |
  | data read       DOUT2 |-------------------------->| SDI3            |
  |                 DOUT3 |-------------------------->| SDI4            |
  |                 DCLK  |<--------------------------| DCLK
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

The downside of the above is the peripheral would need fine-grained config of controller SDI lines to only read SDI0 for register access, and only read SDI1, SDI2, SDI3, SDI4 for ADC sample data (currently available SPI_MULTI_LANE_MODE_STRIPE reads from all SDI lines).

The currently proposed solution looks like the following

  +-----------------------+                         +-----------------+
  |         AD4134        |                         | SPI Controller  |
  |                       |                         |                 |
  | SPI interface    SCLK |<------------------------| SCLK            |
  | for register       CS |<------------------------| CS              |
  | access            SDI |<------------------------| SDO             |
  |                   SDO |------->|¯¯¯¯\           |                 |
  |                       |        |MUX  >--------->| SDI0            |
  | Data interface  DOUT0 |------->|____/           |                 |
  | for ADC sample        |          ^              |                 |
  | data read       DOUT1 |------------------------>| SDI1            |
  |                 DOUT2 |------------------------>| SDI2            |
  |                 DOUT3 |------------------------>| SDI3            |
  |                  DCLK |<------------------------| DCLK            |
  |                   ODR |<----------------+    +->| Offload Trigger |
  +-----------------------+          |      |    |  +-----------------+
                                     |      |    +--| PWM1            |
                                     |      +-------| PWM0            |
                                     +--------------| GPIO            |
                                                    +-----------------+

By being able to mux between AD4134 SDO and AD4134 DOUT0, the peripheral can be connected to a single bus such that controllers able to read from multiple lines will be able to gather ADC sample data from all SDI lines (SPI_MULTI_LANE_MODE_STRIPE). With that, AD4134 maximum data throughput can be supported with what is already available from the SPI core. See additional details on the AD4134 IIO documentation (patch 15).

Yet another possibility would be to have the peripheral sitting in two buses.

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller A|
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI             |
  |                       |                           +-----------------+
  |                       |                           | SPI Controller B|
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI0            |
  | for ADC sample  DOUT1 |-------------------------->| SDI1            |
  | data read       DOUT2 |-------------------------->| SDI2            |
  |                 DOUT3 |-------------------------->| SDI3            |
  |                 DCLK  |<--------------------------| DCLK            |
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

That may be a fallback option if what's currently being proposed fails to comply to Linux code standards.

 .../bindings/iio/adc/adi,ad4134.yaml          | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml +47 -0
index 970e7adb2405..c70b9113cd67 100644
@@ -104,6 +104,20 @@ properties:
       GPIO connected to ODR pin. Used to sample ADC data in minimum I/O mode.
     maxItems: 1
 
+  mux-states:
+    description:
+      mux controller node to route SDO and DOUT0 signals from peripheral to
+      controller.
+    maxItems: 2
+
+  mux-state-names:
+    description:
+      mux state names for routing SDO and DOUT0 signals from peripheral to
+      controller.
+    items:
+      - const: data_read
+      - const: reg_access
+
   adi,asrc-mode:
     $ref: /schemas/types.yaml#/definitions/string
     description:
@@ -231,4 +245,37 @@ examples:
 
         };
     };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4134";
+            reg = <0>;
+
+            spi-max-frequency = <1000000>;
+
+            reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
+            powerdown-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
+
+            clocks = <&sys_clk>;
+            clock-names = "clkin";
+
+            avdd5-supply = <&avdd5>;
+            dvdd5-supply = <&dvdd5>;
+            iovdd-supply = <&iovdd>;
+            refin-supply = <&refin>;
+            avdd1v8-supply = <&avdd1v8>;
+            dvdd1v8-supply = <&dvdd1v8>;
+            clkvdd-supply = <&clkvdd>;
+
+            mux-states = <&ad4134_sdo_dout0_mux 0>, <&ad4134_sdo_dout0_mux 1>;
+            mux-state-names = "data_read", "reg_access";
+
+            adi,spi-mode = "4-wire";
+        };
+    };
 ...
Signature · 1 line
2.53.0
author maintainer
#23 lore raw

PATCH v5 12/15 iio: adc: ad4134: Support SPI 4-wire mode

In reply to Marcelo Schmitt

AD4134 devices can be wired in a few different ways. So far, only minimum I/O mode was supported. While minimum I/O mode allows interfacing with AD4134 with a reduced number of wires, that wiring configuration is not suitable for high-throughput data acquisition.

Extend AD4134 support to enable interfacing in SPI 4-wire configuration.

---
Change log v4 -> v5
- Updated adi,spi-mode property handling.
- No longer selecting/deselecting multiplexer states when in minimum I/O mode.

Detailed reasoning for the external multiplexer usage.

Before coming to the current solution, the following configuration was tried.

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller  |
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |---+                       |                 |
  |                       |   |                       |                 |
  | Data interface  DOUT0 |---+---------------------->| SDI0            |
  | for ADC sample  DOUT1 |-------------------------->| SDI1            |
  | data read       DOUT2 |-------------------------->| SDI2            |
  |                 DOUT3 |-------------------------->| SDI3            |
  |                 DCLK  |<--------------------------| DCLK
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

Though, because DOUT0 never goes high-Z, the DOUT0 pin keeps driving the data line, causing register reads to fail.

Alternatively, we could have something like

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller  |
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI0            |
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI1            |
  | for ADC sample  DOUT1 |-------------------------->| SDI2            |
  | data read       DOUT2 |-------------------------->| SDI3            |
  |                 DOUT3 |-------------------------->| SDI4            |
  |                 DCLK  |<--------------------------| DCLK
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

The downside of the above is the peripheral would need fine-grained config of controller SDI lines to only read SDI0 for register access, and only read SDI1, SDI2, SDI3, SDI4 for ADC sample data (currently available SPI_MULTI_LANE_MODE_STRIPE reads from all SDI lines).

The currently proposed solution looks like the following

  +-----------------------+                         +-----------------+
  |         AD4134        |                         | SPI Controller  |
  |                       |                         |                 |
  | SPI interface    SCLK |<------------------------| SCLK            |
  | for register       CS |<------------------------| CS              |
  | access            SDI |<------------------------| SDO             |
  |                   SDO |------->|¯¯¯¯\           |                 |
  |                       |        |MUX  >--------->| SDI0            |
  | Data interface  DOUT0 |------->|____/           |                 |
  | for ADC sample        |          ^              |                 |
  | data read       DOUT1 |------------------------>| SDI1            |
  |                 DOUT2 |------------------------>| SDI2            |
  |                 DOUT3 |------------------------>| SDI3            |
  |                  DCLK |<------------------------| DCLK            |
  |                   ODR |<----------------+    +->| Offload Trigger |
  +-----------------------+          |      |    |  +-----------------+
                                     |      |    +--| PWM1            |
                                     |      +-------| PWM0            |
                                     +--------------| GPIO            |
                                                    +-----------------+

By being able to mux between AD4134 SDO and AD4134 DOUT0, the peripheral can be connected to a single bus such that controllers able to read from multiple lines will be able to gather ADC sample data from all SDI lines (SPI_MULTI_LANE_MODE_STRIPE). With that, AD4134 maximum data throughput can be supported with what is already available from the SPI core. See additional details on the AD4134 IIO documentation (patch 15).

Yet another possibility would be to have the peripheral sitting in two buses.

  +-----------------------+                           +-----------------+
  |       AD4134          |                           | SPI Controller A|
  |                       |                           |                 |
  | SPI interface    SCLK |<--------------------------| SCLK            |
  | for register       CS |<--------------------------| CS              |
  | access            SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI             |
  |                       |                           +-----------------+
  |                       |                           | SPI Controller B|
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI0            |
  | for ADC sample  DOUT1 |-------------------------->| SDI1            |
  | data read       DOUT2 |-------------------------->| SDI2            |
  |                 DOUT3 |-------------------------->| SDI3            |
  |                 DCLK  |<--------------------------| DCLK            |
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                                      +-----------------+

That may be a fallback option if what's currently being proposed fails to comply to Linux code standards.

 drivers/iio/adc/Kconfig  |   1 +
 drivers/iio/adc/ad4134.c | 221 +++++++++++++++++++++++++++++++++++----
 2 files changed, 204 insertions(+), 18 deletions(-)
drivers/iio/adc/Kconfig +1 -0
index d625f36a0283..10f3c804df9a 100644
@@ -127,6 +127,7 @@ config AD4134
 	select CRC8
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select MULTIPLEXER
 	help
 	  Say yes here to build support for Analog Devices AD4134 SPI analog to
 	  digital converters (ADC).
drivers/iio/adc/ad4134.c +203 -18

320 lines not on this page · raw message

Signature · 1 line
2.53.0
#24 lore raw

Re: [PATCH v5 12/15] iio: adc: ad4134: Support SPI 4-wire mode

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] iio: adc: ad4134: race with trigger handler in ad4134_buffer_setup_ops - [Medium] iio: adc…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] iio: adc: ad4134: race with trigger handler in ad4134_buffer_setup_ops
- [Medium] iio: adc: ad4134: missing DOUT0 format config and cs_off in 4-wire mode
- [Medium] iio: adc: ad4134: spi_setup() ordering and mode desync in claim helpers --

commit 8eb548670148a37e8ad1ee608a1ea7bc67bedb0c
Author: Marcelo Schmitt <[email protected]>

iio: adc: ad4134: Support SPI 4-wire mode

This commit extends the AD4134 ADC driver to support SPI 4-wire mode in addition to minimum I/O mode by switching SPI clock phase and controlling an external SDO/DOUT0 multiplexer between register and sample accesses.

Quoted 4 lines
drivers/iio/adc/ad4134.c +0 -0
index eaf5bb55f25ba..6b05e40f7aeb7 100644

[ ... ]

Quoted 22 lines
drivers/iio/adc/ad4134.c +18 -0
@@ -219,6 +237,85 @@ static const struct regmap_access_table ad4134_regmap_wr_table = {
 	.n_yes_ranges = ARRAY_SIZE(ad4134_regmap_wr_range),
 };
 
+/*
+ * When operating in modes other than minimum I/O mode, AD4134 register data on
+ * the SDO line is sampled on SCLK rising edge (SPI mode 0). In addition to
+ * that, when AD4134 SDO and DOUT0 pins are multiplexed by hardware OUTSIDE
+ * AD4134 SILICON, the multiplexer state must be set to route SDO to the SPI
+ * controller. With both SPI mode and multiplexer state properly set, register
+ * access runs as usual. Successful calls to this function must be balanced with
+ * calls to ad4134_release_reg_access() so to release the mux state lock. See
+ * AD4134 IIO documentation for details.
+ */
+static int ad4134_claim_reg_access(struct ad4134_state *st)
+{
+	unsigned int cur_mode = st->spi->mode & SPI_MODE_X_MASK;
+	int ret;
+
+	if (st->spi_mode != AD4134_SPI_MODE_NO_CS && cur_mode != SPI_MODE_0) {
+		st->spi->mode &= ~SPI_MODE_X_MASK;
+		st->spi->mode |= SPI_MODE_0;

drivers/iio/adc/ad4134.c static const struct regmap_access_table ad4134_regmap_wr_table = { · line 267

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+		ret = spi_setup(st->spi);
+		if (ret)
+			return ret;
+	}
+
+	if (st->spi_mode != AD4134_SPI_MODE_NO_CS && st->mux_st[AD4134_SDO_INPUT])
+		return mux_state_select(st->mux_st[AD4134_SDO_INPUT]);
+
+	return 0;
+}

[Severity: Medium]
In ad4134_claim_reg_access() and ad4134_claim_sample_access(), can mutating st->spi->mode and calling spi_setup() before mux_state_select() leave the SPI mode out of sync or reconfigure the controller during an active capture?

If spi_setup(st->spi) fails, st->spi->mode is not restored to cur_mode. On the next call to the same claim helper, cur_mode already matches the target mode, so spi_setup() is skipped while the SPI controller remains configured in the previous mode.

Similarly, because spi_setup() runs before mux_state_select() acquires the mux lock, if mux_state_select() fails (for example with -EINTR while waiting in down_killable()), st->spi->mode and the SPI controller remain in the new mode on an error return. In addition, if a register read occurs via regmap debugfs while buffered capture holds the mux lock in SPI_MODE_1, ad4134_claim_reg_access() will call spi_setup() to switch the controller to SPI_MODE_0 before blocking on mux_state_select().

Should mux_state_select() be called before changing st->spi->mode and calling spi_setup(), with st->spi->mode restored and the mux deselected if spi_setup() fails?

[ ... ]

Quoted 19 lines
drivers/iio/adc/ad4134.c +6 -2
@@ -261,7 +367,11 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
 {
 	unsigned int i;
 	u32 sample = 0;
-	int ret;
+	int access_ret, ret;
+
+	ret = ad4134_claim_sample_access(st);
+	if (ret)
+		return ret;
 
 	/*
 	 * To be able to read data from all 4 channels through a single line, we
@@ -277,7 +387,7 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
 		ret = spi_write_then_read(st->spi, NULL, 0, st->rx_buf,
 					  BITS_TO_BYTES(AD4134_CHAN_PRECISION_BITS));
 		if (ret)
-			return ret;
+			goto out_data_read_release;

[ ... ]

Quoted 8 lines
drivers/iio/adc/ad4134.c +4 -0
@@ -372,6 +496,21 @@ static irqreturn_t ad4134_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static int ad4134_buffer_postenable(struct iio_dev *indio_dev)
+{
+	return ad4134_claim_sample_access(iio_priv(indio_dev));
+}

drivers/iio/adc/ad4134.c static irqreturn_t ad4134_trigger_handler(int irq, void *p) · line 512

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+
+static int ad4134_buffer_predisable(struct iio_dev *indio_dev)
+{
+	return ad4134_release_sample_access(iio_priv(indio_dev));
+}
+
+static const struct iio_buffer_setup_ops ad4134_buffer_setup_ops = {
+	.postenable = &ad4134_buffer_postenable,
+	.predisable = &ad4134_buffer_predisable,
+};

[Severity: Medium]
Can using .postenable and .predisable in ad4134_buffer_setup_ops race with ad4134_trigger_handler() in 4-wire mode?

In drivers/iio/industrialio-buffer.c, iio_enable_buffers() attaches the trigger pollfunc before running .postenable:

	if (iio_dev_opaque->currentmode == INDIO_BUFFER_TRIGGERED) {
		ret = iio_trigger_attach_poll_func(indio_dev->trig,
						   indio_dev->pollfunc);
		if (ret)
			goto err_disable_buffers;
	}

	if (indio_dev->setup_ops->postenable) {
		ret = indio_dev->setup_ops->postenable(indio_dev);

Because iio_trigger_attach_poll_func() requests the threaded IRQ and enables the external trigger, ad4134_trigger_handler() can start running spi_sync_transfer() before or concurrently with ad4134_buffer_postenable() calling ad4134_claim_sample_access() to update st->spi->mode, call spi_setup(), and select st->mux_st[AD4134_DOUT0_INPUT].

Symmetrically, iio_disable_buffers() runs .predisable before detaching the trigger pollfunc:

	if (indio_dev->setup_ops->predisable) {
		ret2 = indio_dev->setup_ops->predisable(indio_dev);
		if (ret2 && !ret)
			ret = ret2;
	}

	if (iio_dev_opaque->currentmode == INDIO_BUFFER_TRIGGERED) {
		iio_trigger_detach_poll_func(indio_dev->trig,
					     indio_dev->pollfunc);
	}

This allows ad4134_buffer_predisable() to deselect the DOUT0 mux state before iio_trigger_detach_poll_func() disables the trigger and frees the pollfunc IRQ.

(Note also that in this commit, indio_dev->setup_ops is overwritten with NULL by the subsequent devm_iio_triggered_buffer_setup(..., NULL) call in ad4134_probe() until a later commit in the series passes &ad4134_buffer_setup_ops to devm_iio_triggered_buffer_setup().)

Should ad4134_buffer_setup_ops use .preenable and .postdisable instead of .postenable and .predisable?

[ ... ]

Quoted 48 lines
drivers/iio/adc/ad4134.c +40 -4
@@ -591,10 +725,61 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
 				     "failed to initialize regmap");
 
-	ret = ad4134_min_io_mode_setup(st);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to setup minimum I/O mode\n");
+	if (device_property_present(dev, "adi,spi-mode")) {
+		ret = device_property_match_property_string(dev, "adi,spi-mode",
+							    ad4134_spi_modes,
+							    ARRAY_SIZE(ad4134_spi_modes));
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "getting adi,spi-mode property failed\n");
+
+		st->spi_mode = ret;
+	} else {
+		/* Default to no-cs mode if adi,spi-mode is not specified */
+		st->spi_mode = AD4134_SPI_MODE_NO_CS;
+	}
+
+	if (st->spi_mode == AD4134_SPI_MODE_NO_CS) {
+		/*
+		 * The support hardware for AD4134 may have a multiplexer for
+		 * selecting between AD4134 SDO and AD4134 DOUT0. If that mux
+		 * is set but the user still wants to run AD4134 in minimum I/O
+		 * mode, then DOUT0 is not used and the multiplexer OUTSIDE OF
+		 * AD4134 SILICON must be set to select AD4134 SDO. See AD4134
+		 * IIO documentation for details.
+		 */
+		st->mux_st[AD4134_SDO_INPUT] =
+			devm_mux_state_get_optional_selected(dev, "reg_access");
+		if (IS_ERR(st->mux_st[AD4134_SDO_INPUT]))
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_SDO_INPUT]),
+					     "failed to get reg_access mux-state\n");
+
+		ret = ad4134_min_io_mode_setup(st);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to setup minimum I/O mode\n");
+	} else {
+		/*
+		 * This controls a multiplexer OUTSIDE OF AD4134 SILICON.
+		 * See AD4134 IIO documentation for details.
+		 */
+		st->mux_st[AD4134_SDO_INPUT] = devm_mux_state_get(dev, "reg_access");
+		if (IS_ERR(st->mux_st[AD4134_SDO_INPUT]))

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 777

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_SDO_INPUT]),
+					     "failed to get reg_access mux-state\n");
+
+		st->mux_st[AD4134_DOUT0_INPUT] = devm_mux_state_get(dev, "data_read");
+		if (IS_ERR(st->mux_st[AD4134_DOUT0_INPUT]))
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_DOUT0_INPUT]),
+					     "failed to get data_read mux-state\n");
+
+		indio_dev->setup_ops = &ad4134_buffer_setup_ops;
+	}

[Severity: Medium]
When st->spi_mode is AD4134_SPI_MODE_4_WIRE (and without SPI offload in the later high-speed data capture commit), how does sample readout work in ad4134_data_read() and ad4134_trigger_handler()?

Both ad4134_data_read() and ad4134_trigger_handler() read all 4 channels sequentially over DOUT0, which requires AD4134_DIG_IF_CFG_REG to be set to AD4134_DATA_FORMAT_SINGLE_CH_MODE. However, that register update is only performed inside ad4134_min_io_mode_setup() when st->spi_mode is AD4134_SPI_MODE_NO_CS (and in ad4134_offload_setup() for 1-lane offload in the later commit), leaving AD4134_DIG_IF_CFG_REG unconfigured in non-offload 4-wire mode.

In addition, while ad4134_trigger_handler() sets .cs_off = true on its spi_transfer, ad4134_data_read() uses spi_write_then_read(), which asserts CS during each 3-byte read while transmitting 0x00 on SDI. In 4-wire mode, CS is wired to the register SPI interface and st->crc_en is false, so won't asserting CS while clocking SCLK perform an unintended write to AD4134_IFACE_CONFIG_A_REG (0x00)?

Should AD4134_DIG_IF_CFG_REG also be configured for single-channel DOUT0 output in 4-wire mode, and should ad4134_data_read() use a spi_transfer with .cs_off = true?

Quoted 8 lines
drivers/iio/adc/ad4134.c +5 -0
+
+	st->odr_gpio = devm_gpiod_get(dev, "odr", GPIOD_OUT_LOW);
+	if (IS_ERR(st->odr_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->odr_gpio),
+				     "failed to get ODR GPIO\n");
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,

[ ... ]

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

Re: [PATCH v5 12/15] iio: adc: ad4134: Support SPI 4-wire mode

In reply to Marcelo Schmitt
Quoted 8 lines · On Fri, 16 Oct 2026 22:59:56 -0300 Marcelo Schmitt…

AD4134 devices can be wired in a few different ways. So far, only minimum I/O mode was supported. While minimum I/O mode allows interfacing with AD4134 with a reduced number of wires, that wiring configuration is not suitable for high-throughput data acquisition.

Extend AD4134 support to enable interfacing in SPI 4-wire configuration.

I think a bit of ACQUIRE() magic sprinkled in here to handle the claim / release pairs would make this more readable.

Otherwise, just a few little style suggestions.

Quoted 4 lines
drivers/iio/adc/ad4134.c +0 -0
index eaf5bb55f25b..6b05e40f7aeb 100644

...

drivers/iio/adc/ad4134.c static const struct regmap_access_table ad4134_regmap_wr_table = { · line 270

Quoted 3 lines
drivers/iio/adc/ad4134.c +3 -0
+
+/*
+ * Must be balanced with successful calls to ad4134_claim_reg_access().

If you follow the ACQUIRE() suggestion below, probably don't need this comment.

Quoted 33 lines

+ */

drivers/iio/adc/ad4134.c +13 -2
+static int ad4134_release_reg_access(struct ad4134_state *st)
+{
+	if (st->spi_mode != AD4134_SPI_MODE_NO_CS && st->mux_st[AD4134_SDO_INPUT])
+		return mux_state_deselect(st->mux_st[AD4134_SDO_INPUT]);
+
+	return 0;
+}
 
 
 static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
@@ -301,14 +416,18 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
 		.len = st->crc_en ? AD4134_SPI_MAX_XFER_LEN : 2,
 	};
 	unsigned int inst;
-	int ret;
+	int access_ret, ret;
+
+	ret = ad4134_claim_reg_access(st);
+	if (ret)
+		return ret;
 
 	inst = AD4134_REG_READ_MASK | reg;
 	ad4134_prepare_spi_tx_buf(inst, 0, st->tx_buf);
 
 	ret = spi_sync_transfer(st->spi, &xfer, 1);
 	if (ret)
-		return ret;
+		goto out_read_release;
 
 	*val = st->rx_buf[1];
 

drivers/iio/adc/ad4134.c static int ad4134_register_read(struct ad4134_state *st, unsigned int reg, · line 441

Quoted 8 lines
drivers/iio/adc/ad4134.c +4 -1
@@ -316,7 +435,12 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
 	if (st->crc_en && st->rx_buf[2] != st->tx_buf[2])
 		dev_dbg(&st->spi->dev, "reg read CRC check failed\n");
 
-	return 0;
+out_read_release:
+	access_ret = ad4134_release_reg_access(st);
+	if (access_ret)
+		dev_err(&st->spi->dev, "error on access release: %d\n", access_ret);

If you are going to not return access_ret anyway (which is common simplifying approach for cases like this where we are stuck anyway), then how about wrapping all this in an ACQUIRE() and letting the cleanup.h magic call the release for you?

Quoted 45 lines
drivers/iio/adc/ad4134.c +2 -0
+
+	return ret;
 }
 
 static int ad4134_read_raw(struct iio_dev *indio_dev,
drivers/iio/adc/ad4134.c +18 -9
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long info)
@@ -440,11 +579,6 @@ static int ad4134_min_io_mode_setup(struct ad4134_state *st)
 	int ret;
 
 	st->crc_en = true; /* In minimum I/O mode CRC cannot be disabled */
-	st->odr_gpio = devm_gpiod_get(dev, "odr", GPIOD_OUT_LOW);
-	if (IS_ERR(st->odr_gpio))
-		return dev_err_probe(dev, PTR_ERR(st->odr_gpio),
-				     "failed to get ODR GPIO\n");
-
 	ret = regmap_update_bits(st->regmap, AD4134_DIG_IF_CFG_REG,
 				 AD4134_DIF_IF_CFG_FORMAT_MASK,
 				 FIELD_PREP(AD4134_DIF_IF_CFG_FORMAT_MASK,
@@ -591,10 +725,61 @@ static int ad4134_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
 				     "failed to initialize regmap");
 
-	ret = ad4134_min_io_mode_setup(st);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to setup minimum I/O mode\n");
+	if (device_property_present(dev, "adi,spi-mode")) {
+		ret = device_property_match_property_string(dev, "adi,spi-mode",
+							    ad4134_spi_modes,
+							    ARRAY_SIZE(ad4134_spi_modes));
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "getting adi,spi-mode property failed\n");
+
+		st->spi_mode = ret;
+	} else {
+		/* Default to no-cs mode if adi,spi-mode is not specified */
+		st->spi_mode = AD4134_SPI_MODE_NO_CS;
+	}
+
+	if (st->spi_mode == AD4134_SPI_MODE_NO_CS) {
+		/*
+		 * The support hardware for AD4134 may have a multiplexer for
+		 * selecting between AD4134 SDO and AD4134 DOUT0. If that mux

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 755

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+		 * is set but the user still wants to run AD4134 in minimum I/O
+		 * mode, then DOUT0 is not used and the multiplexer OUTSIDE OF
+		 * AD4134 SILICON must be set to select AD4134 SDO. See AD4134
+		 * IIO documentation for details.
+		 */
+		st->mux_st[AD4134_SDO_INPUT] =
+			devm_mux_state_get_optional_selected(dev, "reg_access");
+		if (IS_ERR(st->mux_st[AD4134_SDO_INPUT]))
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_SDO_INPUT]),
+					     "failed to get reg_access mux-state\n");

Maybe use a local variable for the return of devm_mux_state_get_optional_selected and only sets st->mux_st[] after we know it's not an error. Should make this a little more readable.

Similar applies in other places.

drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 760

Quoted 5 lines
drivers/iio/adc/ad4134.c +5 -0
+
+		ret = ad4134_min_io_mode_setup(st);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to setup minimum I/O mode\n");

When it's 80-90 chars, feel free to put dev_err_probe() on one line.

Quoted 25 lines
drivers/iio/adc/ad4134.c +22 -0
+	} else {
+		/*
+		 * This controls a multiplexer OUTSIDE OF AD4134 SILICON.
+		 * See AD4134 IIO documentation for details.
+		 */
+		st->mux_st[AD4134_SDO_INPUT] = devm_mux_state_get(dev, "reg_access");
+		if (IS_ERR(st->mux_st[AD4134_SDO_INPUT]))
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_SDO_INPUT]),
+					     "failed to get reg_access mux-state\n");
+
+		st->mux_st[AD4134_DOUT0_INPUT] = devm_mux_state_get(dev, "data_read");
+		if (IS_ERR(st->mux_st[AD4134_DOUT0_INPUT]))
+			return dev_err_probe(dev, PTR_ERR(st->mux_st[AD4134_DOUT0_INPUT]),
+					     "failed to get data_read mux-state\n");
+
+		indio_dev->setup_ops = &ad4134_buffer_setup_ops;
+	}
+
+	st->odr_gpio = devm_gpiod_get(dev, "odr", GPIOD_OUT_LOW);
+	if (IS_ERR(st->odr_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->odr_gpio),
+				     "failed to get ODR GPIO\n");
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,
author maintainer
#26 lore raw

PATCH v5 13/15 dt-bindings: iio: adc: adi,ad4134: Document PWM usage

In reply to Marcelo Schmitt

Advanced setups may connect a PWM signal to AD4134 ODR pin for faster output data rates.

---
 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml +6 -0
index c70b9113cd67..6a190b7a9311 100644
@@ -104,6 +104,10 @@ properties:
       GPIO connected to ODR pin. Used to sample ADC data in minimum I/O mode.
     maxItems: 1
 
+  pwms:
+    description: PWM signal connected to the ODR pin.
+    maxItems: 1
+
   mux-states:
     description:
       mux controller node to route SDO and DOUT0 signals from peripheral to
@@ -257,9 +261,11 @@ examples:
             reg = <0>;
 
             spi-max-frequency = <1000000>;
+            spi-rx-bus-width = <1>, <1>, <1>, <1>; /* 4 lanes of 1 bit each */
 
             reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
             powerdown-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
+            pwms = <&ad4134_odr_generator 1 1000000 0>;
 
             clocks = <&sys_clk>;
             clock-names = "clkin";
Signature · 1 line
2.53.0
author maintainer
#27 lore raw

PATCH v5 14/15 iio: adc: ad4134: Support high-speed data capture

In reply to Marcelo Schmitt

Make use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 1.496 MSPS).

---
 drivers/iio/adc/Kconfig  |   5 +
 drivers/iio/adc/ad4134.c | 391 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 383 insertions(+), 13 deletions(-)
drivers/iio/adc/Kconfig +5 -0
index 10f3c804df9a..f2bbaef88a5a 100644
@@ -123,9 +123,14 @@ config AD4130
 config AD4134
 	tristate "Analog Devices AD4134 ADC Driver"
 	depends on SPI
+	depends on PWM
 	select REGMAP_SPI
+	select SPI_OFFLOAD
+	select SPI_OFFLOAD_TRIGGER_PWM
 	select CRC8
 	select IIO_BUFFER
+	select IIO_BUFFER_DMA
+	select IIO_BUFFER_DMAENGINE
 	select IIO_TRIGGERED_BUFFER
 	select MULTIPLEXER
 	help
drivers/iio/adc/ad4134.c +378 -13

488 lines not on this page · raw message

Signature · 1 line
2.53.0
#28 lore raw

Re: [PATCH v5 14/15] iio: adc: ad4134: Support high-speed data capture

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] iio: adc: ad4134: positive return from pwm_round_waveform_might_sleep() - [High] iio: adc: ad4134…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] iio: adc: ad4134: positive return from pwm_round_waveform_might_sleep()
- [High] iio: adc: ad4134: ODR PWM enabled in probe and never disabled
- [Medium] iio: adc: ad4134: period rounding drift in ad4134_update_conversion_rate()
- [Medium] iio: adc: ad4134: trigger offset uses unrounded ODR high time
--

commit 08608ae1b3f8617876d1b47ec67b7654d5a033e9
Author: Marcelo Schmitt <[email protected]>

iio: adc: ad4134: Support high-speed data capture

This commit adds SPI transfer offload support to the ad4134 ADC driver, using a periodic PWM trigger and DMA engine buffer to capture samples at rates up to 1.496 MSPS.

Quoted 4 lines
drivers/iio/adc/ad4134.c +0 -0
index 6b05e40f7aeb7..21e8039b34116 100644

[ ... ]

Quoted 27 lines
drivers/iio/adc/ad4134.c +23 -0
@@ -461,6 +515,92 @@ static const struct regmap_config ad4134_regmap_config = {
 	.max_register = AD4134_CH_VREG(AD4134_NUM_CHANNELS - 1),
 };
 
+static int ad4134_update_conversion_rate(struct ad4134_state *st,
+					 unsigned int freq_Hz)
+{
+	struct spi_offload_trigger_config config = st->offload_trigger_config;
+	struct pwm_waveform odr_wf = { };
+	u64 offload_period_ns;
+	u64 offload_offset_ns;
+	u64 odr_high_time_ns;
+	unsigned int count;
+	u64 target_ns;
+	int ret;
+
+	if (!in_range(freq_Hz, AD4134_MIN_ODR_FREQ_HZ,
+		      AD4134_MAX_ODR_FREQ_HZ - AD4134_MIN_ODR_FREQ_HZ + 1))
+		return -ERANGE;
+
+	odr_wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, freq_Hz);
+	/*
+	 * Set the PWM duty cycle to keep ODR high for at least minimum required
+	 * time. If the rounded PWM's value is less than the minimum required,
+	 * increase the target value by 10 and attempt to round the waveform
+	 * again, until the minimum (or try count limit) is reached.
+	 */

drivers/iio/adc/ad4134.c static const struct regmap_config ad4134_regmap_config = { · line 550

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+	odr_high_time_ns = div64_ul(6ULL * NSEC_PER_SEC, st->sys_clk_hz);
+	target_ns = 0;
+	count = 100;
+	do {
+		target_ns += 10; /* Increment by PWM duty cycle period */
+		odr_wf.duty_length_ns = target_ns;
+		ret = pwm_round_waveform_might_sleep(st->odr_pwm, &odr_wf);
+		if (ret)
+			return ret;
+	} while (--count && odr_wf.duty_length_ns < odr_high_time_ns);

[Severity: High]
In ad4134_update_conversion_rate(), pwm_round_waveform_might_sleep() returns 0 on exact or downward rounding, 1 when a waveform value had to be rounded up, or a negative errno on failure:

drivers/pwm/core.c:
* Returns: 0 on success, 1 if at least one value had to be rounded up or a
* negative errno.
* Context: May sleep.
*/
int pwm_round_waveform_might_sleep(struct pwm_device *pwm, struct pwm_waveform *wf)

Because target_ns starts at 0 (requesting 10 ns on the first iteration) and the return value is checked with if (ret) instead of if (ret < 0), won't any upward rounding cause ad4134_update_conversion_rate() to abort and return +1 to ad4134_offload_setup() -> ad4134_probe() or ad4134_write_raw()?

Also, since target_ns starts at 0 and only increments up to 1000 ns across 100 iterations rather than starting at odr_high_time_ns, this performs unnecessary rounding calls at the nominal 48 MHz clock (where odr_high_time_ns is 125 ns) and will always exhaust count and fail with -EDOM if st->sys_clk_hz < 6 MHz. Should target_ns start at odr_high_time_ns and check if (ret < 0)?

Quoted 26 lines
drivers/iio/adc/ad4134.c +26 -0
+
+	/* Check the minimum ODR high time is met */
+	if (odr_wf.duty_length_ns < odr_high_time_ns)
+		return -EDOM;
+
+	if (odr_wf.period_length_ns < 2 * odr_high_time_ns)
+		return -EDOM;
+
+	/*
+	 * Configure SPI offload PWM trigger.
+	 * For gated DCLK, the minimum required time between ODR rising edge
+	 * and DCLK rising edge is the sum of ODR high time and ODR falling
+	 * edge to DCLK rising edge time. Delay the offload trigger for at least
+	 * that amount of time so the ADC sample data will be available when the
+	 * SPI transfer begin.
+	 *
+	 * Use the same period as ODR PWM to avoid timing issues.
+	 * Convert back from period to frequency for the SPI offload API.
+	 */
+	offload_period_ns = odr_wf.period_length_ns;
+	config.periodic.frequency_hz = DIV_ROUND_UP_ULL(HZ_PER_GHZ, offload_period_ns);
+	offload_offset_ns = odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS;
+	count = 100;
+	do {
+		config.periodic.offset_ns = offload_offset_ns;
+		ret = spi_offload_trigger_validate(st->offload_trigger, &config);

drivers/iio/adc/ad4134.c static const struct regmap_config ad4134_regmap_config = { · line 586

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+		if (ret)
+			return ret;
+
+		offload_offset_ns += 10;
+	} while (--count && config.periodic.offset_ns < odr_high_time_ns +
+							AD4134_DCLK_RISING_OFFSET_NS);
+
+	/* Check the minimum ODR to DCLK delay is met */
+	if (config.periodic.offset_ns < odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS)
+		return -EDOM;

[Severity: Medium]
Here in ad4134_update_conversion_rate(), offload_offset_ns and the loop termination checks use the theoretical minimum odr_high_time_ns rather than the actual rounded ODR pulse width odr_wf.duty_length_ns.

If pwm_round_waveform_might_sleep() rounds odr_wf.duty_length_ns up above odr_high_time_ns (for example, to 140 ns or 150 ns when odr_high_time_ns is 125 ns), the ODR falling edge occurs at odr_wf.duty_length_ns. Validating config.periodic.offset_ns against odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS (133 ns) allows the SPI offload trigger to fire less than 8 ns after (or simultaneously with) the actual ODR falling edge.

Should offload_offset_ns and the post-validation checks use odr_wf.duty_length_ns + AD4134_DCLK_RISING_OFFSET_NS instead?

drivers/iio/adc/ad4134.c static const struct regmap_config ad4134_regmap_config = { · line 591

Quoted 5 lines
drivers/iio/adc/ad4134.c +5 -0
+
+	/* Check the PWM periods remain the same */
+	offload_period_ns = DIV_ROUND_UP_ULL(HZ_PER_GHZ, config.periodic.frequency_hz);
+	if (odr_wf.period_length_ns != offload_period_ns)
+		return -EDOM;

[Severity: Medium]
Can this period equality check in ad4134_update_conversion_rate() reject valid sampling frequencies in the [10, 1496000] Hz range with -EDOM?

In spi_offload_trigger_pwm_validate(), config.periodic.frequency_hz is converted back to a period via DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz), rounded down by the PWM hardware, and written back to periodic->frequency_hz:

drivers/spi/spi-offload-trigger-pwm.c:spi_offload_trigger_pwm_validate() {
	wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic->frequency_hz);
	...
	ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
	if (ret < 0)
		return ret;

	periodic->frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wf.period_length_ns);
	periodic->offset_ns = wf.duty_offset_ns;
}

When odr_wf.period_length_ns is converted to an integer frequency_hz via DIV_ROUND_UP_ULL(HZ_PER_GHZ, offload_period_ns), any period where ceil(10^9 / ceil(10^9 / P)) != P (such as almost all frequencies below 31.6 kHz, e.g., 60 Hz or 19200 Hz) produces a smaller wf.period_length_ns in spi_offload_trigger_pwm_validate().

In addition, because config.periodic.frequency_hz is set outside the do-while loop and modified in-place by spi_offload_trigger_validate(), each loop iteration feeds the rounded-up frequency_hz back into spi_offload_trigger_validate(), causing the trigger period to shrink further across iterations and fail the odr_wf.period_length_ns != offload_period_ns check.

Quoted 11 lines
drivers/iio/adc/ad4134.c +11 -0
+
+	ret = pwm_set_waveform_might_sleep(st->odr_pwm, &odr_wf, false);
+	if (ret)
+		return ret;
+
+	st->offload_trigger_config = config;
+	st->odr_wf = odr_wf;
+	st->odr_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC, odr_wf.period_length_ns);
+
+	return 0;
+}

[ ... ]

Quoted 3 lines
drivers/iio/adc/ad4134.c +0 -0
@@ -602,6 +783,166 @@ static const struct iio_info ad4134_info = {
 	.debugfs_reg_access = ad4134_debugfs_reg_access,
 };

[ ... ]

Quoted 49 lines
drivers/iio/adc/ad4134.c +49 -0
+static int ad4134_offload_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+	int ret, access_ret;
+
+	ret = ad4134_claim_sample_access(st);
+	if (ret)
+		return ret;
+
+	ad4134_prepare_offload_msg(indio_dev);
+	st->msg.offload = st->offload;
+	ret = spi_optimize_message(st->spi, &st->msg);
+	if (ret)
+		goto out_offload_release;
+
+	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
+					 &st->offload_trigger_config);
+	if (ret)
+		goto out_unoptimize;
+
+	return 0;
+
+out_unoptimize:
+	spi_unoptimize_message(&st->msg);
+
+out_offload_release:
+	access_ret = ad4134_release_sample_access(st);
+	if (access_ret)
+		dev_err(&st->spi->dev, "reg input select error: %d\n", access_ret);
+
+	return ret;
+}
+
+static int ad4134_offload_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+
+	spi_offload_trigger_disable(st->offload, st->offload_trigger);
+	spi_unoptimize_message(&st->msg);
+
+	return ad4134_release_sample_access(st);
+}
+
+static const struct iio_buffer_setup_ops ad4134_offload_buffer_setup_ops = {
+	.postenable = &ad4134_offload_buffer_postenable,
+	.predisable = &ad4134_offload_buffer_predisable,
+};
+
+static int ad4134_pwm_get(struct ad4134_state *st)

drivers/iio/adc/ad4134.c static const struct iio_info ad4134_info = { · line 867

Quoted 10 lines
drivers/iio/adc/ad4134.c +10 -0
+{
+	struct device *dev = &st->spi->dev;
+
+	st->odr_pwm = devm_pwm_get(dev, NULL);
+	if (IS_ERR(st->odr_pwm))
+		return dev_err_probe(dev, PTR_ERR(st->odr_pwm),
+				     "failed to get ODR PWM\n");
+
+	return 0;
+}

[Severity: High]
How is the lifecycle of st->odr_pwm managed?

During probe, ad4134_offload_setup() calls ad4134_update_conversion_rate(), which calls pwm_set_waveform_might_sleep(st->odr_pwm, &odr_wf, false) with a non-zero duty cycle and period. This immediately enables the ODR PWM output before the AD4134 registers are configured.

Furthermore, st->odr_wf is saved in ad4134_update_conversion_rate() but never used, and neither ad4134_offload_buffer_predisable() nor any devm cleanup action ever disables st->odr_pwm. Because devm_pwm_get() only releases the PWM reference without stopping hardware output, won't st->odr_pwm remain running continuously when the buffer is disabled, or after late probe failure and driver unbind (driving the ODR pin after the ADC's clocks and regulators have been disabled)?

[ ... ]

Quoted 21 lines
drivers/iio/adc/ad4134.c +21 -0
+static int ad4134_offload_setup(struct iio_dev *indio_dev, struct ad4134_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	int ret;
+
+	ret = ad4134_pwm_get(st);
+	if (ret)
+		return ret;
+
+	ret = ad4134_offload_buffer_setup(indio_dev, st->spi);
+	if (ret)
+		return ret;
+
+	/*
+	 * Start with a sampling rate slower than 374 kSPS because that is the
+	 * maximum rate supported with wideband filter (default on power up).
+	 */
+	st->odr_hz = 250 * HZ_PER_KHZ;
+	ret = ad4134_update_conversion_rate(st, st->odr_hz);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set sampling freq\n");

[ ... ]

Quoted 1 line

+}

[ ... ]

Quoted 1 line
drivers/iio/adc/ad4134.c +0 -0
@@ -776,17 +1117,40 @@ static int ad4134_probe(struct spi_device *spi)

[ ... ]

Quoted 4 lines
drivers/iio/adc/ad4134.c +4 -0
+	st->offload = devm_spi_offload_get(dev, spi, &ad4134_offload_config);
+	ret = PTR_ERR_OR_ZERO(st->offload);
+	/* Fall back to low speed usage when no SPI offload is available. */
+	if (ret == -ENODEV) {

[ ... ]

Quoted 15 lines
drivers/iio/adc/ad4134.c +15 -0
+	} else if (ret) {
+		return dev_err_probe(dev, ret, "failed to get offload\n");
+	} else {
+		if (st->spi_mode == AD4134_SPI_MODE_NO_CS)
+			return dev_err_probe(dev, -EPROTONOSUPPORT,
+					     "Unsupported min I/O + offload config\n");
+
+		indio_dev->info = &ad4134_offload_info;
+		indio_dev->channels = ad4134_offload_chan_set;
+		indio_dev->num_channels = ARRAY_SIZE(ad4134_offload_chan_set);
+		ret = ad4134_offload_setup(indio_dev, st);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to setup SPI offload\n");
+	}

[ ... ]

Quoted 2 lines

return devm_iio_device_register(dev, indio_dev);
}

[ ... ]

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

Re: [PATCH v5 14/15] iio: adc: ad4134: Support high-speed data capture

In reply to Marcelo Schmitt
Quoted 3 lines · On Fri, 16 Oct 2026 23:00:34 -0300 Marcelo Schmitt…

Make use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 1.496 MSPS).

I'd call out here that in this mode, you can't uses the sysfs interfaces to read channels. Even when offloading is involved, no access at all is unusual. Is there a path to add that, if only for debug?

Quoted 12 lines
drivers/iio/adc/ad4134.c +0 -0
index 6b05e40f7aeb..21e8039b3411 100644
 
 static irqreturn_t ad4134_trigger_handler(int irq, void *p)
 {
 	IIO_DECLARE_BUFFER_WITH_TS(u32, channels, AD4134_NUM_CHANNELS) = { };

drivers/iio/adc/ad4134.c static int ad4134_read_raw(struct iio_dev *indio_dev, · line 664

Quoted 5 lines
drivers/iio/adc/ad4134.c +2 -0
@@ -520,6 +660,9 @@ static int ad4134_read_raw(struct iio_dev *indio_dev,
 
 	switch (info) {
 	case IIO_CHAN_INFO_RAW: {
+		if (!IS_ERR_OR_NULL(st->offload))
+			return -EPROTONOSUPPORT;

If direct access is not supported when offload is in play then we shouldn't have an interface to get here (i.e. no IIO_CHAN_INFO_RAW bit set in the info mask bitmaps. As such is this just defensive coding?

If so add a comment to say that, or just drop it.

Quoted 4 lines
drivers/iio/adc/ad4134.c +1 -0
+
 		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
 		if (IIO_DEV_ACQUIRE_FAILED(claim))
 			return -EBUSY;
author maintainer
#30 lore raw

PATCH v5 15/15 Docs: iio: Add AD4134

In reply to Marcelo Schmitt

Add initial documentation for the ad4134 IIO driver.

---
 Documentation/iio/ad4134.rst | 127 +++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst  |   1 +
 MAINTAINERS                  |   1 +
 3 files changed, 129 insertions(+)
 create mode 100644 Documentation/iio/ad4134.rst
Documentation/iio/ad4134.rst new file +127 -0
new file mode 100644
index 000000000000..c0889a21cd77
@@ -0,0 +1,127 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+AD4134 driver
+=============
+
+Device driver for Analog Devices Inc. AD4134 and similar ADCs.
+
+Supported devices
+=================
+
+* `AD4134 <https://www.analog.com/AD4134>`_
+* `AD7134 <https://www.analog.com/AD7134>`_
+
+Wiring connections
+------------------
+
+AD4134 and similar ADCs can operate in a few different wiring configurations.
+Because AD4134 also supports control modes other than through SPI protocol,
+the following pins must be statically set for all of the wiring modes described
+below.
+
++----------------------------+----------------------+--------------------+
+| Pin Function               |         Level        | Description        |
++============================+======================+====================+
+| PIN/SPI                    |         High         | SPI control mode   |
++----------------------------+----------------------+--------------------+
+| MODE                       |         Low          | ASRC slave mode    |
++----------------------------+----------------------+--------------------+
+| DEC0/DCLKI                 |         Low          | DCLK input         |
++----------------------------+----------------------+--------------------+
+| DEC1/DCLKMODE              |         Low          | Gated DCLK         |
++----------------------------+----------------------+--------------------+
+
+Minimum I/O mode
+^^^^^^^^^^^^^^^^
+
+The minimum I/O mode wiring allows AD4134 register and data access with the
+conventional set of SPI bus lines. In this mode, the peripheral CS pin must be
+tied to GND. A possible connection schema that sets AD4134 digital interface for
+minimum I/O mode is::
+
+      +------------+         +-------------+
+      |  AD4134    |         | SPI         |
+      |            |         | Controller  |
+      |       DCLK |<--+     |             |
+      |       SCLK |<--+---- | SCLK        |
+      |        SDI |<--------| SDO         |
+      |        SDO |-------->| SDI         |
+      |        ODR |<--------| GPIO        |
+      |         CS |----+    |             |
+      +------------+    |    +-------------+
+                        V
+                       GND
+
+Set ``adi,spi-mode = "no-cs"`` in device tree to indicate the device is
+connected in minimum I/O wiring configuration.
+
+SPI 4-wire mode - single lane
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The conventional SPI 4-wire configuration except a multiplexer is used to
+select between SDO (for register data) and DOUT0 (for ADC sample data).::
+
+  +-----------------------+                         +-----------------+
+  |         AD4134        |                         | SPI Controller  |
+  |                       |                         |                 |
+  | SPI interface    SCLK |<------------------------| SCLK            |
+  | for register       CS |<------------------------| CS              |
+  | access            SDI |<------------------------| SDO             |
+  |                   SDO |------->|¯¯¯¯\           |                 |
+  |                       |        |MUX  >--------->| SDI             |
+  | Data interface DOUT0  |------->|____/           |                 |
+  | for ADC sample        |          ^              |                 |
+  | data read             |          |              |                 |
+  |                  DCLK |<------------------------| DCLK            |
+  |                   ODR |<----------------+    +->| Offload Trigger |
+  +-----------------------+          |      |    |  +-----------------+
+                                     |      |    +--| PWM1            |
+                                     |      +-------| PWM0            |
+                                     +--------------| GPIO            |
+                                                    +-----------------+
+
+Set ``adi,spi-mode = "4-wire"`` in device tree to indicate the device is
+connected in SPI 4-wire configuration.
+
+For SPI offload operation, a PWM is connected to the ADC ODR pin. The ``pwms``
+firmware property specifies the PWM that is connected to the ADC. The SPI
+offload will have a ``trigger-sources`` property to indicate the SPI offload
+(PWM) trigger source.
+
+To achieve high sample rates while meeting ADC timing requirements, an offset
+must be added between the rising edges of PWM0 and PWM1 to delay the SPI
+transfer until ADC sample data is available. This requires a specialized PWM
+controller that can provide such an offset.  The `AD4134-FMC HDL project`_, for
+example, can be configured to sample AD4134 at high speed sample rates.
+
+.. _AD4134-FMC HDL project: https://analogdevicesinc.github.io/hdl/projects/ad4134_fmc/index.html
+
+SPI 4-wire mode - multi-lane
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Maximum throughput rate can be achieved by reading 4 input lines simultaneously.::
+
+  +-----------------------+                         +-----------------+
+  |         AD4134        |                         | SPI Controller  |
+  |                       |                         |                 |
+  | SPI interface    SCLK |<------------------------| SCLK            |
+  | for register       CS |<------------------------| CS              |
+  | access            SDI |<------------------------| SDO             |
+  |                   SDO |------->|¯¯¯¯\           |                 |
+  |                       |        |MUX  >--------->| SDI0            |
+  | Data interface  DOUT0 |------->|____/           |                 |
+  | for ADC sample        |          ^              |                 |
+  | data read       DOUT1 |------------------------>| SDI1            |
+  |                 DOUT2 |------------------------>| SDI2            |
+  |                 DOUT3 |------------------------>| SDI3            |
+  |                  DCLK |<------------------------| DCLK            |
+  |                   ODR |<----------------+    +->| Offload Trigger |
+  +-----------------------+          |      |    |  +-----------------+
+                                     |      |    +--| PWM1            |
+                                     |      +-------| PWM0            |
+                                     +--------------| GPIO            |
+                                                    +-----------------+
+
+Each of SDI0 to SDI3 connect to a different SPI controller deserializer
+(``spi-rx-bus-width = <1>, <1>, <1>, <1>``).
Documentation/iio/index.rst +1 -0
index d0fb32f4b80f..3a49e08a2366 100644
@@ -23,6 +23,7 @@ Industrial I/O Kernel Drivers
    ad4000
    ad4030
    ad4062
+   ad4134
    ad4691
    ad4695
    ad7191
MAINTAINERS +1 -0
index 3a2dfd1a4fd8..82a810ac788e 100644
@@ -1533,6 +1533,7 @@ L:	[email protected]
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
+F:	Documentation/iio/ad4134.rst
 F:	drivers/iio/adc/ad4134.c
 
 ANALOG DEVICES INC AD4170-4 DRIVER
Signature · 1 line
2.53.0
maintainer
#31 lore raw
In reply to Marcelo Schmitt
Quoted 14 lines · On Fri, 16 Oct 2026 22:56:00 -0300 Marcelo Schmitt…

Extend AD4134 support to enable greater sample rate data capture. To achieve that, provide control of digital filter type selection. The default filter type on device power up only supports sample rates up to 374 kSPS. In addition to that, enable interfacing with AD4134 in SPI 4-wire mode that can support reading ADC sample data through multiple wires. Finally, use SPI offloading to speed up data transfers at up to 1.49 MSPS.

Features being added:
- Data filter type selection.
- Triggered buffer data read.
- SPI 4-wire mode.
- High speed data capture with SPI offloading.
- Device tree doc updates.

Quoted 8 lines

Minor improvements (introduced before the new features):
- Sign extend sample data.
- Apply reset recovery wait time.
- Update include list to comply with IWYU principles.
- Serialize single-read operations.
- Run shorter transfers when CRC is disabled.
- Protect conventional device access from debug accesses [NEW PATCH]

Mostly looks fine to me. I'll assume you will take a look at sashiko feedback before doing v6.
I may well start picking up parts of this series if it looks like they are ready and the tail of the series needs more work.

I gave a couple of late replies to your earlier postings before catching up with this one.

Jonathan

Quoted 51 lines

Change log v4 -> v5:
[device tree]
- No changes in dt-doc.
[IIO]
- Initialized local variable with 0 to avoid build warns.
- Set max_register according to AD4134_NUM_CHANNELS.
- Apply reset recovery wait time. [NEW FIX PATCH]
- Protect conventional device access from debug accesses. [NEW PATCH]
- Re-arranged code so early function prototype declaration is no longer needed.
- Now using iio_for_each_active_channel() to iterate over IIO channels.
- Updated adi,spi-mode property handling.
- No longer selecting/deselecting multiplexer states when in minimum I/O mode.

Previous submissions:
v1: https://lore.kernel.org/linux-iio/[email protected]/
v2: https://lore.kernel.org/linux-iio/[email protected]/
v3: https://lore.kernel.org/linux-iio/[email protected]/
v4: https://lore.kernel.org/linux-iio/[email protected]/

Marcelo Schmitt (15):
iio: adc: ad4134: Adjust register map range
iio: adc: ad4134: Sign extend sample data
iio: adc: ad4134: Apply reset recovery wait time
iio: adc: ad4134: Update include list to comply with IWYU principles
iio: adc: ad4134: Serialize single-read operations
iio: adc: ad4134: Protect conventional device access from debug
accesses
iio: adc: ad4134: Run shorter transfers when CRC is disabled
iio: adc: ad4134: Add support for digital filter type selection
iio: adc: ad4134: Support buffered data read
dt-bindings: iio: adc: adi,ad4134: Document SPI connection mode
dt-bindings: iio: adc: adi,ad4134: Document external multiplexer usage
iio: adc: ad4134: Support SPI 4-wire mode
dt-bindings: iio: adc: adi,ad4134: Document PWM usage
iio: adc: ad4134: Support high-speed data capture
Docs: iio: Add AD4134

 .../bindings/iio/adc/adi,ad4134.yaml          | 110 ++-
 Documentation/iio/ad4134.rst                  | 127 +++
 Documentation/iio/index.rst                   |   1 +
 MAINTAINERS                                   |   1 +
 drivers/iio/adc/Kconfig                       |   8 +
 drivers/iio/adc/ad4134.c                      | 793 +++++++++++++++++-
 6 files changed, 987 insertions(+), 53 deletions(-)
 create mode 100644 Documentation/iio/ad4134.rst

base-commit: a22dc7ebb21a6bd4c8b63bc2c1dd46a24945aae7