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 to reflect previously missing chip characteristics.

Minor improvements (introduced before the new features):
- Adjusted register map range now prevents access to uninitialized local variable. [NEW PATCH]
- Sign extend sample data.
- Update include list to comply with IWYU principles.
- Serialize single-read operations.
- Run shorter transfers when CRC is disabled.

The updates in v3 are intended to tackle all applicable suggestions from reviewers and from automated tests/reviews/AI.

Change log v2 -> v3:
[device tree]
- Adjusted spi-rx-bus-width constraints.
- Dropped mux provider from offload example.
- Dropped odr-gpios from offload example.
[IIO]
- Already applied fixes patches have been dropped.
- Fixed off-by-one mistake to avoid access to uninitialized local variable. [NEW PATCH]
- Now using IIO_DECLARE_BUFFER_WITH_TS macro to set properly sized triggered buffer storage that includes room for timestamp.
- Now using SPI_MODE_X_MASK to update CPOL/CPHA bits.
- Refactored access mode locking to avoid umpaired calls to mux_state_select()/deselect().
- Dropped unreachable code on offload setup.
- Fixed sampling frequency validation check.
- Avoided division by zero when external clock is not provided.
- Follow LIFO teardown order on offload buffer disable.
- Set correct buffer size to read all channels in offload mode.
- Disallowed single-read when setup for SPI offloading.
- Corrected AD4134 IIO Doc to only show the PWM connected to ODR on offload diagrams.

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

With best regards,
Marcelo

Marcelo Schmitt (13):
iio: adc: ad4134: Adjust register map range
iio: adc: ad4134: Sign extend sample data
iio: adc: ad4134: Update include list to comply with IWYU principles
iio: adc: ad4134: Serialize single-read operations
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          | 104 ++-
 Documentation/iio/ad4134.rst                  | 128 +++
 Documentation/iio/index.rst                   |   1 +
 MAINTAINERS                                   |   1 +
 drivers/iio/adc/Kconfig                       |   8 +
 drivers/iio/adc/ad4134.c                      | 820 +++++++++++++++++-
 6 files changed, 1027 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/iio/ad4134.rst

base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68

2.53.0

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.

---
New patch in v3.

 drivers/iio/adc/ad4134.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 70f80bf1825f..e53c10b9a44a 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(ARRAY_SIZE(ad4134_chan_set) - 1),
 };
 
 static int ad4134_read_raw(struct iio_dev *indio_dev,

2.53.0

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

---
No changes in v3.

 drivers/iio/adc/ad4134.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index e53c10b9a44a..6fdeb27d7a42 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;
 	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;
 }

2.53.0

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.

---
No changes in v3.

 drivers/iio/adc/ad4134.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 6fdeb27d7a42..4c8adb8efe4e 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)

2.53.0

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.

---
Change log v2 -> v3:
- Renamed the mutex so it has a more distinguishable name from the mutex to be added on a follow up patch.

 drivers/iio/adc/ad4134.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

index 4c8adb8efe4e..db3cf69401f6 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 members the of device state, 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;

@@ -444,6 +454,10 @@ static int ad4134_probe(struct spi_device *spi)
 
 	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),

2.53.0

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.

---
No changes in v3.

 drivers/iio/adc/ad4134.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

index db3cf69401f6..ce077aef2d2a 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;

@@ -313,6 +314,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),

2.53.0

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.

---
No changes in v3.

 drivers/iio/adc/ad4134.c | 76 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

index ce077aef2d2a..14996a0d1fe8 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,12 +83,47 @@
 #define AD4134_SPI_CRC_INIT_VALUE		0xA5
 static unsigned char ad4134_spi_crc_table[CRC8_TABLE_SIZE];
 
+enum ad4134_filter_type {
+	AD4134_WIDEBAND,
+	AD4134_SINC6,
+	AD4134_SINC3,
+	AD4134_SINC3_REJ60,
+};
+
+static const char * const ad4134_filt_names[] = {
+	[AD4134_WIDEBAND] = "wideband",
+	[AD4134_SINC6] = "sinc6",
+	[AD4134_SINC3] = "sinc3",
+	[AD4134_SINC3_REJ60] = "sinc3+rej60",
+};
+
+static int ad4134_get_filter_type(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan);
+
+static int ad4134_set_filter_type(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  unsigned int 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[] = {

@@ -115,6 +153,44 @@ 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 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),

2.53.0

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 v2 -> v3:
- Now using IIO_DECLARE_BUFFER_WITH_TS macro to set properly sized triggered buffer storage that includes room for timestamp.

 drivers/iio/adc/Kconfig  |  2 ++
 drivers/iio/adc/ad4134.c | 61 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

index a802d8ee4dd9..fc7de0bb400a 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).

2.53.0

On 09/29, [email protected] wrote:
...

Yeah, debugfs is not safe to use like that. I believe claiming direct mode would solve the issue, although, the conclusion a while ago was that enforcing that for debug interface was not the way to go, IIRC.

I'll ignore complaints about issues related to debugfs unless asked otherwise.

Doesn't apply to this specific device in 4-wire operation mode since the AD4134 data interface is read with CS off.

I have no idea where it supposedly found such information.

Anyways, running a longer transfer to get data all at once can indeed be done to minimize delays. So, I may do that on a v4.

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

---
Change log v2 -> v3:
- Adjusted spi-rx-bus-width constraints.

Unfortunately, I missed SPI connection mode properties when introducing AD4134 dt-doc and the device driver defaulted to a no-CS configuration. Due to that, conventional 4-wire mode can't be made default without leading to a regression. Still, updating the SPI mode default to 4-wire is a sensible thing to do later on if acceptable.

 .../bindings/iio/adc/adi,ad4134.yaml          | 51 ++++++++++++++++---
 1 file changed, 44 insertions(+), 7 deletions(-)

index ea6d7e026419..0b2a2cbfadae 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,30 @@ 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:
+            const: 1, 1
+      - properties:
+          '#daisy-chained-devices': false
 
 unevaluatedProperties: false
 

2.53.0

On 09/29, [email protected] wrote:
...

I'll do so and further test dt_binding_check catches any invalid combination of properties and values.

Will fix that following the suggestion from Rob's review.

Will double check that and, if needed, change how the constraints are written.

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml:174:24: [warning] too few spaces after comma (commas) ./Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml:180:24: [warning] too few spaces after comma (commas)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/12e5af9d96f6c5e800ad860d3cfa6ac8006a22e2.1790719425.git.marcelo.schmitt@analog.com

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

Modulo the issue the bot flagged, I'm happy with this.

I dunno, it's probably not really a big deal to leave it as it is, it's just a default and it's probably just not worth affecting what users there may be.

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.

---
Change log v2 -> v3:
- Included the example right away with the doc update that documents it.
- Dropped mux provider from offload example.

Currently, SPI offloading is only supported in 4 wire mode. So, offloading wouldn't be usable if added before the 4-wire patch.

Detailed reasoning for the external multiplexer usage.

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

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

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         |                           |                 |
  | for register     SCLK |<--------------------------| SCLK            |
  | access             CS |<--------------------------| CS              |
  |                   SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI0            |
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI1            |
  | for ADC data    DOUT1 |-------------------------->| SDI2            |
  | read back       DOUT2 |-------------------------->| SDI3            |
  |                 DOUT3 |-------------------------->| SDI4            |
  |                 DCLK  |<--------------------------| DCLK
  |                 ODR   |<------------------+    +->| Offload Trigger |
  +-----------------------+                   |    |  +-----------------+
                                              |    +--| PWM1            |
                                              +-------| PWM0            |
                                              +-------| GPIO            |
                                                      +-----------------+

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            |
                                     |      +-------| GPIO0           |
                                     +--------------| GPIO1           |
                                                    +-----------------+

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 13).

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            |
                                              +-------| GPIO            |
                                                      +-----------------+

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(+)

index 0b2a2cbfadae..27de44719903 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:

@@ -225,4 +239,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";
+        };
+    };
 ...

2.53.0

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 v2 -> v3:
- Now using SPI_MODE_X_MASK to update CPOL/CPHA bits.
- Refactored access mode locking to avoid umpaired calls to mux_state_select()/deselect().

Currently, SPI offloading is only supported in 4 wire mode. So, offloading wouldn't be usable if added before the 4-wire patch.

Detailed reasoning for the external multiplexer usage.

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

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

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         |                           |                 |
  | for register     SCLK |<--------------------------| SCLK            |
  | access             CS |<--------------------------| CS              |
  |                   SDI |<--------------------------| SDO             |
  |                   SDO |-------------------------->| SDI0            |
  |                       |                           |                 |
  | Data interface  DOUT0 |-------------------------->| SDI1            |
  | for ADC data    DOUT1 |-------------------------->| SDI2            |
  | read back       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 13).

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 | 260 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 248 insertions(+), 13 deletions(-)

index fc7de0bb400a..21bb16f66bc9 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).

2.53.0

On 09/29, [email protected] wrote:
...

If spi_setup() fails it will be tried again on the next register or sample read. So, the device would be desynchronized only when not being used.

...

The default on device reset is to output data from all four channels serialized through DOUT0. Anyways, will make the configuration explicit in case the device is not reset.

I'm not sure about this one. The intent is to prevent sysfs regmap access while the buffer is enabled. IIO direct/buffer mode locking should provide that, no? The rationale for the two mutexes is for access_lock to avoid concurrent updates to the external multiplexer and for sync_lock to protect read/modify/write sequences. Maybe IIO direct/buffer mode locking + sync_lock is enough?

Not sure. Should it lock on a mutex before editing the regmap_config. Maybe I get rid of access_lock and make regmap_config const again.

Okay, that seems to be an issue. Will make the device request the GPIO if offload/PWM is not available.

NO_CS/minimum I/O mode doesn't need to constantly switch the external multiplexer and can leave that in reg_access state. Also, since minimum I/O always operate in SPI mode 0, nothing that access_lock protects is really used.

Technically claiming mode would be enough and if the locking is only coupled to "in buffered mode or not in buffered mode" I'm fine with that. However, if you need to serialize for other reasons we probably need to grow a more complex solution with a lock and a flag (similar to what the IIO mode stuff is doing) so as to avoid holding a lock across syscalls as this points out.

Good catch here.

You shouldn't be modifying the global. The mutex solution works because devm_regmap_init() doesn't keep any references to the config anyway but I'd just use a local copy instead. One on the stack should be fine.

Jonathan

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

---
Change log v2 -> v3:
- Dropped odr-gpios from offload example.
- Now that the 4-wire example comes earlier, the pwm is only now added to that.

 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

index 27de44719903..f585243b624a 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

@@ -251,9 +255,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";

2.53.0

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

---
Change log v2 -> v3:
- Dropped unreachable code on offload setup.
- Fixed sampling frequency validation check.
- Avoided division by zero when external clock is not provided.
- Follow LIFO teardown order on offload buffer disable.
- Set correct buffer size to read all channels in offload mode.
- Disallowed single-read when setup for SPI offloading.

 drivers/iio/adc/Kconfig  |   5 +
 drivers/iio/adc/ad4134.c | 395 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 391 insertions(+), 9 deletions(-)

index 21bb16f66bc9..c5b321b5f4d8 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

2.53.0

On 09/29, [email protected] wrote:
...

Oh, I should've done that. Will change accordingly.

For cases where the range doesn't start with 0 or 1, in_range() becomes a bit harder to read.

if (freq_Hz < AD4134_MIN_ODR_FREQ_HZ || freq_Hz > AD4134_MAX_ODR_FREQ_HZ)

	return -ERANGE;

would have been simpler, IMHO.
Anyways, will fix this and all other issues reported.

...

With Best Regards,
Andy Shevchenko

Add initial documentation for the ad4134 IIO driver.

---
Change log v2 -> v3:
- Corrected AD4134 IIO Doc to only show the PWM connected to ODR on offload diagrams.

 Documentation/iio/ad4134.rst | 128 +++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst  |   1 +
 MAINTAINERS                  |   1 +
 3 files changed, 130 insertions(+)
 create mode 100644 Documentation/iio/ad4134.rst

new file mode 100644
index 000000000000..cb16c710e1ae
@@ -0,0 +1,128 @@
+.. 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         |                         |                 |
+  | for register     SCLK |<------------------------| SCLK            |
+  | access             CS |<------------------------| CS              |
+  |                   SDI |<------------------------| SDO             |
+  |                   SDO |------->|¯¯¯¯\           |                 |
+  | Data interface        |        |MUX  >--------->| SDI             |
+  | for ADC sample  DOUT0 |------->|____/           |                 |
+  | 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>``).

index d0fb32f4b80f..3a49e08a2366 100644
@@ -23,6 +23,7 @@ Industrial I/O Kernel Drivers
    ad4000
    ad4030
    ad4062
+   ad4134
    ad4691
    ad4695
    ad7191

index ecc22d1149d1..97501bcab40a 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

2.53.0