iio: adc: ad4134: Enable greater sample rate data capture
Subsystems (5)
Marcelo Schmitt
maintainerv6 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 v5 -> v6:
[device tree]
- No changes in dt-doc.
[IIO]
- Correct reset time to microseconds.
- Added sinc6 filter_type to IIO ABI [NEW PATCH]
- Added Fixes tag to iio: adc: ad4134: Serialize single-read operations
- Better explanation for requiring debugfs synchronization.
- Lock sync_lock mutex to avoid filter type read interleaving with read/write/modify sequences.
- Now running bus preparation routines on IIO buffer preenable/postdisable.
- Put dev_err_probe() in a single line.
- Use mux_state local vaiable to improve readability.
- Restore SPI mode if spi_setup() fails.
- Use cleanup macros to handle claim/release pairs.
- Check the ODR to SCLK offset aginst the maximum ODR high time.
- Removed defensive offload support check on read_raw().
- Added comment explaining why single-shot reads are not supported in offload mode.
- No error out when pwm_round_waveform_might_sleep() return > 0.
- Adjusted intial ODR PWM target duty cycle to odr_high_time_ns.
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]/
v5: https://lore.kernel.org/linux-iio/[email protected]/
Marcelo Schmitt (16):
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: ABI: Add sinc6 filter_type
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
Documentation/ABI/testing/sysfs-bus-iio | 2 + .../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 | 781 +++++++++++++++++- 7 files changed, 985 insertions(+), 45 deletions(-) create mode 100644 Documentation/iio/ad4134.rst
base-commit: 87e28db23d0384f3995c43ecc5ae2118b371edb5
Signature · 1 line
2.53.0
Marcelo Schmitt
maintainerPATCH v6 1/16 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.
---
No changes in v6.
drivers/iio/adc/ad4134.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/adc/ad4134.c +2 -2
@@ -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
Marcelo Schmitt
maintainerPATCH v6 2/16 iio: adc: ad4134: Sign extend sample data
In reply to Marcelo SchmittSign extend ADC sample data for correct delivery of negative ADC input difference.
---
No changes in v6.
drivers/iio/adc/ad4134.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +3 -1
@@ -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
Marcelo Schmitt
maintainerPATCH v6 3/16 iio: adc: ad4134: Apply reset recovery wait time
In reply to Marcelo SchmittAdd a reset recovery wait time to avoid any SPI transfer from running too soon during device probe.
---
Change log v5 -> v6:
- Correct reset time to microseconds.
drivers/iio/adc/ad4134.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +2 -1
@@ -26,7 +26,7 @@#include <linux/unaligned.h> #include <linux/units.h> -#define AD4134_RESET_TIME_US (10 * USEC_PER_SEC) +#define AD4134_RESET_TIME_US 10#define AD4134_REG_READ_MASK BIT(7)#define AD4134_SPI_MAX_XFER_LEN 3
@@ -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
Marcelo Schmitt
maintainerPATCH v6 4/16 iio: adc: ad4134: Update include list to comply with IWYU principles
In reply to Marcelo SchmittAdd 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.
---
No changes in v6.
drivers/iio/adc/ad4134.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/iio/adc/ad4134.c +3 -3
@@ -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#define AD4134_REG_READ_MASK BIT(7)
Signature · 1 line
2.53.0
Marcelo Schmitt
maintainerPATCH v6 5/16 iio: adc: ad4134: Serialize single-read operations
In reply to Marcelo SchmittProtect 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.
---
Change log v5 -> v6:
- Added Fixes tag.
drivers/iio/adc/ad4134.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/iio/adc/ad4134.c +15 -1
@@ -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
Marcelo Schmitt
maintainerPATCH v6 6/16 iio: adc: ad4134: Protect conventional device access from debug accesses
In reply to Marcelo SchmittAD4134 sample read operations may more than one transfer to complete and are expected to run atomically to avoid triggering any DCLK count error. Though, register access made from debugfs interface may scramble data if they are run amidst sample read operations and read/write/modify cycles.
Require debug access to acquire device synchronization locks before running any transfer requested from debugfs interface.
---
Change log v5 -> v6:
- Better explanation for requiring debugfs synchronization.
drivers/iio/adc/ad4134.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/iio/adc/ad4134.c +6 -0
@@ -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
Jonathan Cameron
maintainerRe: [PATCH v6 06/16] iio: adc: ad4134: Protect conventional device access from debug accesses
Quoted 1 line · On Fri, 23 Oct 2026 19:34:50 -0300 Marcelo Schmitt…
AD4134 sample read operations may more than one transfer to complete and
may require more than one?
Quoted 32 lines
are expected to run atomically to avoid triggering any DCLK count error. Though, register access made from debugfs interface may scramble data if they are run amidst sample read operations and read/write/modify cycles.
Require debug access to acquire device synchronization locks before running any transfer requested from debugfs interface.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v5 -> v6:
- Better explanation for requiring debugfs synchronization.drivers/iio/adc/ad4134.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/iio/adc/ad4134.c +6
-0@@ -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);
Marcelo Schmitt
maintainerPATCH v6 7/16 iio: adc: ad4134: Run shorter transfers when CRC is disabled
In reply to Marcelo SchmittWhen 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.
---
No change in v6.
drivers/iio/adc/ad4134.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/iio/adc/ad4134.c +6 -4
@@ -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
Marcelo Schmitt
maintainerPATCH v6 8/16 iio: ABI: Add sinc6 filter_type
In reply to Marcelo SchmittDocument "sinc6" as a valid value for the filter_type attributes (used by the ad4134 device driver).
---
New patch.
Documentation/ABI/testing/sysfs-bus-iio | 2 ++ 1 file changed, 2 insertions(+)
Documentation/ABI/testing/sysfs-bus-iio +2 -0
@@ -2479,6 +2479,8 @@ Description: * "sinc5+sinc1+pf2" - Sinc5 + Sinc1 + device specific Post Filter 2. * "sinc5+sinc1+pf3" - Sinc5 + Sinc1 + device specific Post Filter 3. * "sinc5+sinc1+pf4" - Sinc5 + Sinc1 + device specific Post Filter 4. + * "sinc6" - The digital sinc6 filter. balance between noise + rejection and settling time. * "wideband" - filter with wideband low ripple passband and sharp transition band.
Signature · 1 line
2.53.0
Marcelo Schmitt
maintainerPATCH v6 9/16 iio: adc: ad4134: Add support for digital filter type selection
In reply to Marcelo SchmittAD4134 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 v5 -> v6:
- Lock sync_lock mutex to avoid filter type read interleaving with read/write/modify sequences.
drivers/iio/adc/ad4134.c | 95 +++++++++++++++++++++++++++++++++++----- 1 file changed, 83 insertions(+), 12 deletions(-)
drivers/iio/adc/ad4134.c +83 -12
@@ -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 0xA5static 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,75 @@ 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 ; + +guard (mutex)(&st->sync_lock); + + ret =regmap_read (st->regmap,AD4134_CHAN_DIG_FILTER_SEL_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
Marcelo Schmitt
maintainerPATCH v6 10/16 iio: adc: ad4134: Support buffered data read
In reply to Marcelo SchmittEnable 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 v5 -> v6:
- Now running bus preparation routines on IIO buffer preenable/postdisable.
drivers/iio/adc/Kconfig | 2 ++ drivers/iio/adc/ad4134.c | 61 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
drivers/iio/adc/Kconfig +2 -0
@@ -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
Marcelo Schmitt
maintainerPATCH v6 11/16 dt-bindings: iio: adc: adi,ad4134: Document SPI connection mode
In reply to Marcelo SchmittDocument how AD4134 chips are connected to the host SPI controller according to different wiring configurations.
---
No change in v6.
.../bindings/iio/adc/adi,ad4134.yaml | 57 ++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml +50 -7
@@ -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
Marcelo Schmitt
maintainerPATCH v6 12/16 dt-bindings: iio: adc: adi,ad4134: Document external multiplexer usage
In reply to Marcelo SchmittThe 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 change in v6.
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
@@ -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
Marcelo Schmitt
maintainerPATCH v6 13/16 iio: adc: ad4134: Support SPI 4-wire mode
In reply to Marcelo SchmittAD4134 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 v5 -> v6:
- Put dev_err_probe() in a single line.
- Use mux_state local vaiable to improve readability.
- Restore SPI mode if spi_setup() fails.
- Use cleanup macros to handle claim/release pairs.
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 | 206 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 198 insertions(+), 9 deletions(-)
drivers/iio/adc/Kconfig +1 -0
@@ -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 +197 -9
278 lines not on this page · raw message
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH v6 13/16] iio: adc: ad4134: Support SPI 4-wire mode
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] iio: adc: ad4134: broken ACQUIRE() and DEFINE_GUARD_COND() for access claims - [Medium] iio: adc…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iio: adc: ad4134: broken ACQUIRE() and DEFINE_GUARD_COND() for access claims
- [Medium] iio: adc: ad4134: duplicate devm_mux_state_get() call in ad4134_probe() --
commit 2e7d5e9974efe0835378f504b67e632ce2d53ac2
Author: Marcelo Schmitt <[email protected]>
iio: adc: ad4134: Support SPI 4-wire mode
This commit adds support for interfacing with the AD4134 ADC in SPI 4-wire mode using an external multiplexer to switch between register access on SDO and ADC sample data reads on DOUT0.
Quoted 4 lines
drivers/iio/adc/ad4134.c +0
-0
[ ... ]
Quoted 123 lines
drivers/iio/adc/ad4134.c +101
-0@@ -221,6 +239,99 @@ 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 spi_mode = st->spi->mode &SPI_MODE_X_MASK ; +int ret; + +if (st->spi_mode !=AD4134_SPI_MODE_NO_CS && spi_mode !=SPI_MODE_0 ) { + st->spi->mode &= ~SPI_MODE_X_MASK ; + st->spi->mode |=SPI_MODE_0 ; + ret =spi_setup (st->spi); +if (ret) { + st->spi->mode &= ~SPI_MODE_X_MASK ; + st->spi->mode |= spi_mode; +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 ; +} + +/* +* Must be balanced with successful calls to ad4134_claim_reg_access(). +*/ +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 ; +} + +DEFINE_GUARD(__ad4134_reg_access_mode, struct ad4134_state *, +ad4134_claim_reg_access (_T),ad4134_release_reg_access (_T)); +DEFINE_GUARD_COND (__ad4134_reg_access_mode, _try,ad4134_claim_reg_access (_T)); + +/* +* When operating in modes other than minimum I/O mode, AD4134 ADC sample data +* on the DOUT lines is sampled on SCLK falling edge (SPI mode 1). 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 DOUT0 to the SPI +* controller. With both SPI mode and multiplexer state properly set, ADC +* samples can be read normally. Successful call to this function must be paired +* with a call to ad4134_release_sample_access(). +*/ +static int ad4134_claim_sample_access (struct ad4134_state *st) +{ +unsigned int spi_mode = st->spi->mode &SPI_MODE_X_MASK ; +int ret; + +if (st->spi_mode !=AD4134_SPI_MODE_NO_CS && spi_mode !=SPI_MODE_1 ) { + st->spi->mode &= ~SPI_MODE_X_MASK ; + st->spi->mode |=SPI_MODE_1 ; + ret =spi_setup (st->spi); +if (ret) { + st->spi->mode &= ~SPI_MODE_X_MASK ; + st->spi->mode |= spi_mode; +return ret; + } + } + +if (st->spi_mode !=AD4134_SPI_MODE_NO_CS && st->mux_st[AD4134_DOUT0_INPUT ]) +return mux_state_select (st->mux_st[AD4134_DOUT0_INPUT ]); + +return 0 ; +} + +/* +* Must be balanced with successful calls to ad4134_claim_sample_access(). +*/ +static int ad4134_release_sample_access (struct ad4134_state *st) +{ +if (st->spi_mode !=AD4134_SPI_MODE_NO_CS && st->mux_st[AD4134_DOUT0_INPUT ]) +return mux_state_deselect (st->mux_st[AD4134_DOUT0_INPUT ]); + +return 0 ; +} + +DEFINE_GUARD(__ad4134_sample_access_mode, struct ad4134_state *, +ad4134_claim_sample_access (_T),ad4134_release_sample_access (_T)); +DEFINE_GUARD_COND (__ad4134_sample_access_mode, _try,ad4134_claim_sample_access (_T)); +static int ad4134_calc_spi_crc (u8 inst,u8 data) {u8 buf[] = { inst, data };@@ -246,6 +357,10 @@ static int ad4134_reg_write(void *context, unsigned int reg, unsigned int val) };int ret; +ACQUIRE (__ad4134_reg_access_mode, claim)(st); +if (ACQUIRE_ERR (__ad4134_reg_access_mode, &claim)) +return -EBUSY ; +ad4134_prepare_spi_tx_buf (reg, val, st->tx_buf); ret =spi_sync_transfer (st->spi, &xfer,1 );@@ -265,6 +380,10 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,u32 sample =0 ;int ret; +ACQUIRE (__ad4134_sample_access_mode, claim)(st); +if (ACQUIRE_ERR (__ad4134_sample_access_mode, &claim)) +return -EBUSY ; +/* * To be able to read data from all 4 channels through a single line, we * set DOUTx output format to 0 in the digital interface config register
drivers/iio/adc/ad4134.c static int ad4134_register_read(struct ad4134_state *st, unsigned int reg, · line 429
Quoted 6 lines
drivers/iio/adc/ad4134.c +3
-0@@ -305,6 +424,10 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,unsigned int inst;int ret; +ACQUIRE (__ad4134_reg_access_mode, claim)(st); +if (ACQUIRE_ERR (__ad4134_reg_access_mode, &claim)) +return -EBUSY ;
[Severity: High]
Are these guard definitions and ACQUIRE() calls handling claim failures as intended?
In ad4134_reg_write(), ad4134_data_read(), and ad4134_register_read(), ACQUIRE() and ACQUIRE_ERR() are called with the unconditional guard classes __ad4134_reg_access_mode and __ad4134_sample_access_mode rather than the conditional _try variants (__ad4134_reg_access_mode_try and __ad4134_sample_access_mode_try).
DEFINE_GUARD() defines a constructor of the form ({ _lock; _T; }), which discards the return value of ad4134_claim_reg_access(st) or ad4134_claim_sample_access(st) and unconditionally initializes claim to the valid pointer st. As a result, ACQUIRE_ERR(..., &claim) always evaluates to 0. If spi_setup() or mux_state_select() fails, the error is ignored, the SPI transfer runs with the wrong SPI mode or mux routing, and scope exit runs ad4134_release_*_access() -> mux_state_deselect(), calling up(&mux->lock) on a semaphore that was never acquired.
Additionally, the conditional guards use the 3-argument DEFINE_GUARD_COND() macro:
include/linux/cleanup.h:
#define DEFINE_GUARD_COND_3(_name, _ext, _condlock) \ DEFINE_GUARD_COND_4(_name, _ext, _condlock, _RET)
Because ad4134_claim_reg_access() and ad4134_claim_sample_access() return 0 on success and a negative errno on failure, the default _RET condition treats 0 as failure and negative error codes as success. Should DEFINE_GUARD_COND() pass _RET == 0 as the fourth argument, and should the callers use __ad4134_reg_access_mode_try and __ad4134_sample_access_mode_try?
[ ... ]
Quoted 1 line
drivers/iio/adc/ad4134.c +0
-0@@ -593,10 +727,64 @@ static int ad4134_probe(struct spi_device *spi)
[ ... ]
Quoted 9 lines
drivers/iio/adc/ad4134.c +9
-0+ }else { +/* +* This controls a multiplexer OUTSIDE OF AD4134 SILICON. +* See AD4134 IIO documentation for details. +*/ + mux_st =devm_mux_state_get (dev,"reg_access" ); +if (IS_ERR (mux_st)) +return dev_err_probe (dev,PTR_ERR (mux_st), +"failed to get reg_access mux-state\n" );
drivers/iio/adc/ad4134.c static int ad4134_probe(struct spi_device *spi) · line 780
Quoted 10 lines
drivers/iio/adc/ad4134.c +10
-0+ + st->mux_st[AD4134_SDO_INPUT ] = mux_st; + + mux_st =devm_mux_state_get (dev,"data_read" ); + st->mux_st[AD4134_DOUT0_INPUT ] =devm_mux_state_get (dev,"data_read" ); +if (IS_ERR (mux_st)) +return dev_err_probe (dev,PTR_ERR (mux_st), +"failed to get data_read mux-state\n" ); + + st->mux_st[AD4134_DOUT0_INPUT ] = mux_st;
[Severity: Medium]
In ad4134_probe(), when st->spi_mode is not AD4134_SPI_MODE_NO_CS, devm_mux_state_get(dev, "data_read") is called twice in succession, and the second call's result in st->mux_st[AD4134_DOUT0_INPUT] is later overwritten by mux_st without checking for errors.
Was the second devm_mux_state_get(dev, "data_read") call left in by mistake when introducing the mux_st local variable, leaving an extra struct mux_state allocation and mux_chip reference held for the lifetime of the device?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
Marcelo Schmitt
maintainerPATCH v6 14/16 dt-bindings: iio: adc: adi,ad4134: Document PWM usage
In reply to Marcelo SchmittAdvanced setups may connect a PWM signal to AD4134 ODR pin for faster output data rates.
---
No change in v6.
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml +6 -0
@@ -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
Marcelo Schmitt
maintainerPATCH v6 15/16 iio: adc: ad4134: Support high-speed data capture
In reply to Marcelo SchmittMake use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 1.496 MSPS). In this operation mode, a PWM connected to ADC ODR pin and single-shot reads can't be supported due to that.
---
Change log v5 -> v6:
- Check the ODR to SCLK offset aginst the maximum ODR high time.
- Removed defensive offload support check on read_raw().
- Added comment explaining why single-shot reads are not supported in offload mode.
- No error out when pwm_round_waveform_might_sleep() return > 0.
- Adjusted intial ODR PWM target duty cycle to odr_high_time_ns.
drivers/iio/adc/Kconfig | 5 + drivers/iio/adc/ad4134.c | 390 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 382 insertions(+), 13 deletions(-)
drivers/iio/adc/Kconfig +5 -0
@@ -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 +377 -13
481 lines not on this page · raw message
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH v6 15/16] iio: adc: ad4134: Support high-speed data capture
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] iio: adc: ad4134: premature -EDOM in ad4134_update_conversion_rate() offset loop - [Medium] iio…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iio: adc: ad4134: premature -EDOM in ad4134_update_conversion_rate() offset loop
- [Medium] iio: adc: ad4134: st->odr_pwm left enabled on error, buffer disable, and unbind --
commit 7220c3687cb1811a6679e1fe3b919243558ed540
Author: Marcelo Schmitt <[email protected]>
iio: adc: ad4134: Support high-speed data capture
This commit adds SPI offload support to the ad4134 ADC driver to allow high-speed data capture up to 1.496 MSPS using a PWM-driven ODR signal and DMA-backed IIO buffers.
Quoted 4 lines
drivers/iio/adc/ad4134.c +0
-0
[ ... ]
Quoted 65 lines
drivers/iio/adc/ad4134.c +61
-0@@ -462,6 +516,94 @@ 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 max_odr_high_time_ns; +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. +*/ + odr_high_time_ns =div64_ul (6ULL *NSEC_PER_SEC , st->sys_clk_hz); + target_ns = odr_high_time_ns; + count =100 ; +do { + odr_wf.duty_length_ns = target_ns; + ret =pwm_round_waveform_might_sleep (st->odr_pwm, &odr_wf); +if (ret <0 ) +return ret; + target_ns +=10 ;/* Increment by PWM duty cycle period */ + }while (--count && odr_wf.duty_length_ns < odr_high_time_ns); + +/* 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); + if (ret)
drivers/iio/adc/ad4134.c static const struct regmap_config ad4134_regmap_config = { · line 589
Quoted 10 lines
drivers/iio/adc/ad4134.c +10
-0+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 */ + max_odr_high_time_ns =max_t (u64, odr_high_time_ns, odr_wf.duty_length_ns); +if (config.periodic.offset_ns < max_odr_high_time_ns +AD4134_DCLK_RISING_OFFSET_NS ) +return -EDOM ;
[Severity: High]
Can this loop in ad4134_update_conversion_rate() exit early and cause the subsequent check to fail with -EDOM?
After the ODR PWM rounding loop finishes, odr_wf.duty_length_ns is guaranteed to be at least odr_high_time_ns (for example, 125 ns at a 48 MHz sys_clk_hz rounded up to 130 ns or 140 ns depending on the PWM clock period), so max_odr_high_time_ns is equal to odr_wf.duty_length_ns.
However, the SPI offload trigger validation loop initializes offload_offset_ns to odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS and stops as soon as config.periodic.offset_ns reaches odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS.
When odr_wf.duty_length_ns was rounded above odr_high_time_ns and spi_offload_trigger_validate() rounds config.periodic.offset_ns to a value in [odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS, max_odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS), the loop terminates on the first iteration and the check against max_odr_high_time_ns + AD4134_DCLK_RISING_OFFSET_NS immediately returns -EDOM, failing both ad4134_probe() -> ad4134_offload_setup() and ad4134_write_raw().
Should max_odr_high_time_ns be computed before the loop and used for both the initial offload_offset_ns and the loop termination condition (and should config.periodic.frequency_hz be re-initialized on each iteration in case spi_offload_trigger_validate() modifies it in place)?
Quoted 16 lines
drivers/iio/adc/ad4134.c +16
-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 ; + + 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 1 line
drivers/iio/adc/ad4134.c +0
-0@@ -603,6 +783,166 @@ static const struct iio_info ad4134_info = {
[ ... ]
Quoted 16 lines
drivers/iio/adc/ad4134.c +16
-0+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: Medium]
Does this leave st->odr_pwm enabled when the buffer is disabled, if probe fails after ad4134_update_conversion_rate(), or when the driver is unbound?
In ad4134_probe() -> ad4134_offload_setup() -> ad4134_update_conversion_rate(), pwm_set_waveform_might_sleep(st->odr_pwm, &odr_wf, false) is called with a non-zero duty cycle and period, which immediately enables the PWM output in hardware. It is also saved to st->odr_wf, which is never read anywhere else in the driver.
Because devm_pwm_get() only releases the PWM handle on teardown without disabling the hardware output, and neither a devm_add_action_or_reset() callback nor ad4134_offload_buffer_predisable() disables st->odr_pwm, the PWM continues toggling the AD4134 ODR pin while the IIO buffer is idle, if a subsequent step in ad4134_offload_setup() or ad4134_probe() fails, or after driver unbind when the ADC regulators and clocks have been disabled.
Should st->odr_pwm be disabled via a devm action (or only enabled between ad4134_offload_buffer_postenable() and ad4134_offload_buffer_predisable() using the saved st->odr_wf)?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
Marcelo Schmitt
maintainerPATCH v6 16/16 Docs: iio: Add AD4134
In reply to Marcelo SchmittAdd 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
@@ -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
@@ -23,6 +23,7 @@ Industrial I/O Kernel Drivers ad4000 ad4030 ad4062 + ad4134 ad4691 ad4695 ad7191
MAINTAINERS +1 -0
@@ -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
Jonathan Cameron
maintainerQuoted 7 lines · On Fri, 23 Oct 2026 19:32:58 -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.
Hi Marcelo,
Series looks fine to me. I'll not pick it up just yet though as want to give some time for others to take a look.
Thanks
Jonathan
Quoted 11 lines · On Sat, 24 Oct 2026 02:54:21 +0100 Jonathan Cameron <[email protected]> wrote:
Quoted 7 lines · On Fri, 23 Oct 2026 19:32:58 -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.
Quoted 4 lines
Hi Marcelo,
Series looks fine to me. I'll not pick it up just yet though as want to give some time for others to take a look.
Applied. Still a narrow window for last minute feedback if anyone is looking at this given I need to rebase on rc1 anyway and then let 0-day have a few days of fun with it.
thanks,
Jonathan
Quoted 5 lines
Thanks
Jonathan