public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family
@ 2026-03-20 11:03 Radu Sabau via B4 Relay
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-03-20 11:03 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	Radu Sabau

This series adds support for the Analog Devices AD4691 family of
high-speed, low-power multichannel successive approximation register
(SAR) ADCs with an SPI-compatible serial interface.

The family includes:
  - AD4691: 16-channel, 500 kSPS
  - AD4692: 16-channel, 1 MSPS
  - AD4693: 8-channel, 500 kSPS
  - AD4694: 8-channel, 1 MSPS

The devices support two operating modes, auto-detected from the device
tree:
  - CNV Burst Mode: external PWM drives CNV independently of SPI;
                    DATA_READY on a GP pin signals end of conversion
  - Manual Mode: CNV tied to SPI CS; each SPI transfer reads
                 the previous conversion result and starts the
                 next (pipelined N+1 scheme)

A new driver is warranted rather than extending ad4695: the AD4691
data path uses an accumulator-register model — results are read from
AVG_IN registers, with ACC_MASK, ADC_SETUP, DEVICE_SETUP, and
GPIO_MODE registers controlling the sequencer — none of which exist
in AD4695. CNV Burst Mode (PWM drives CNV independently of SPI) and
Manual Mode (pipelined N+1 transfers) also have no equivalent in
AD4695's command-embedded single-cycle protocol.

The series is structured as follows:
  1/4 - DT bindings (YAML schema) and MAINTAINERS entry
  2/4 - Initial driver: register map via custom regmap callbacks,
        IIO read_raw/write_raw, both operating modes, single-channel
        reads via internal oscillator (Autonomous Mode)
  3/4 - Triggered buffer support: IRQ-driven (DATA_READY on a GP pin
        selected via interrupt-names) for CNV Burst Mode; external IIO
        trigger for Manual Mode to handle the pipelined N+1 SPI protocol
  4/4 - SPI Engine offload support: DMA-backed high-throughput
        capture path using the SPI offload subsystem

Datasheets:
  https://www.analog.com/en/products/ad4691.html
  https://www.analog.com/en/products/ad4692.html
  https://www.analog.com/en/products/ad4693.html
  https://www.analog.com/en/products/ad4694.html

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
Changes in v4:
- dt-bindings: add avdd-supply (required) and ldo-in-supply (optional);
  rename vref-supply → ref-supply, vrefin-supply → refin-supply;
  corrected reset-gpios polarity (active-high → active-low); remove
  clocks and pwm-names; extend interrupts to up to 4 GP pins with
  interrupt-names "gp0".."gp3"; reduce #trigger-source-cells to
  const: 1 (GP pin number); add gpio-controller / #gpio-cells = <2>;
  drop adi,ad4691.h header; update binding examples
- driver: rename CNV Clock Mode → CNV Burst Mode throughout
- driver: add avdd-supply (required) and ldo-in-supply; track ref vs.
  refin supply for REFBUF_EN; set LDO_EN in DEVICE_SETUP when ldo-in
  is present; add software reset fallback via SPI_CONFIG_A register
- driver: merge ACC_MASK1_REG / ACC_MASK2_REG into ACC_MASK_REG with
  a single ADDR_DESCENDING 16-bit SPI write
- driver: remove clocks usage; set PWM rate directly without ref clock
- driver: rename chip info structs (ad4691_chip_info etc.); rename
  *chip → *info in state struct; replace adc_mode enum with manual_mode
  bool; replace ktime sampling_period with u32 cnv_period_ns
- driver: move IIO_CHAN_INFO_SAMP_FREQ to info_mask_separate with an
  available list for the internal oscillator frequency
- driver: use regcache MAPLE instead of RBTREE
- triggered buffer: derive DATA_READY GP pin from interrupt-names in
  firmware ("gp0".."gp3") instead of assuming GP0
- triggered buffer: use regmap_update_bits for DEVICE_SETUP mode toggle
  to avoid clobbering LDO_EN when toggling MANUAL_MODE bit
- triggered buffer: split buffer setup ops into separate Manual and
  CNV Burst variants (mirrors offload path structure)
- SPI offload: promote channel storagebits from 16 to 32 to match DMA
  word size; introduce ad4691_manual_channels[] with shift=16 (data in
  upper 16 bits of the 32-bit word); update triggered-buffer paths to
  the same layout for consistency
- SPI offload: derive GP pin from trigger-source args[0] instead of
  hardcoding GP0; split offload buffer setup ops per mode
- replace put_unaligned_be32() + FIELD_PREP() with cpu_to_be32() and
  plain bit-shift ops for SPI offload message construction
- multiple reviewer-requested code style and correctness fixes
  (Andy Shevchenko, Nuno Sá, Uwe Kleine-König, David Lechner)
- Link to v3: https://lore.kernel.org/r/20260313-ad4692-multichannel-sar-adc-driver-v3-0-b4d14d81a181@analog.com

Changes in v3:
- Replace GPIO reset handling with reset controller framework
- Replace two regmap_write() calls for ACC_MASK1/ACC_MASK2 with regmap_bulk_write()
- Move conv_us declaration closer to its first use
- Derive spi_device/dev from regmap instead of storing st->spi
- ad4691_trigger_handler(): use guard(mutex)() and iio_for_each_active_channel()
- ad4691_setup_triggered_buffer(): return -ENOMEM/-ENOENT directly instead of
  wrapping in dev_err_probe(); fix fwnode_irq_get() check (irq <= 0 → irq < 0)
- Add GENMASK defines for SPI offload 32-bit message layout; replace manual
  bit-shifts with put_unaligned_be32() + FIELD_PREP()
- Use DIV_ROUND_CLOSEST_ULL() instead of div64_u64()
- ad4691_set_sampling_freq(): fix indentation; drop unnecessary else after return
- ad4691_probe(): use PTR_ERR_OR_ZERO() for devm_spi_offload_get()
- Link to v2: https://lore.kernel.org/r/20260310-ad4692-multichannel-sar-adc-driver-v2-0-d9bb8aeb5e17@analog.com

Changes in v2:
- Drop adi,spi-mode DT property; operating mode now auto-detected
  from pwms presence (CNV Clock Mode if present, Manual Mode if not)
- Reduce from 5 operating modes to 2 (CNV Clock Mode, Manual Mode);
  Autonomous, SPI Burst and CNV Burst modes removed as user-selectable
  modes; Autonomous Mode is now the internal idle/single-shot state
- Single-shot read_raw always uses internal oscillator (Autonomous
  Mode), independent of the configured buffer mode
- Replace bulk regulator API with devm_regulator_get_enable() and
  devm_regulator_get_enable_read_voltage()
- Use guard(mutex) and IIO_DEV_ACQUIRE_DIRECT_MODE scoped helpers
- Replace enum + indexed chip_info array with named chip_info structs
- Remove product_id field and hardware ID check from probe
- Factor IIO_CHAN_INFO_RAW body into ad4691_single_shot_read() helper
- Use fwnode_irq_get(dev_fwnode(dev), 0); drop interrupt-names from
  DT binding
- Use devm_clk_get_enabled(dev, NULL); drop clock-names from DT
  binding
- Use spi_write_then_read() for DMA-safe register writes
- Use put_unaligned_be16() for SPI header construction
- fsleep() instead of usleep_range() in single-shot path
- storagebits 24->32 for manual-mode channels (uniform DMA layout)
- Collect full scan into vals[16], single iio_push_to_buffers_with_ts()
- Use pf->timestamp instead of iio_get_time_ns() in trigger handler
- Remove IRQF_TRIGGER_FALLING (comes from firmware/DT)
- Fix offload xfer array size ([17]: N channels + 1 state reset)
- Drop third DT binding example per reviewer request
- Link to v1: https://lore.kernel.org/r/20260305-ad4692-multichannel-sar-adc-driver-v1-0-336229a8dcc7@analog.com

---
Radu Sabau (4):
      dt-bindings: iio: adc: add AD4691 family
      iio: adc: ad4691: add initial driver for AD4691 family
      iio: adc: ad4691: add triggered buffer support
      iio: adc: ad4691: add SPI offload support

 .../devicetree/bindings/iio/adc/adi,ad4691.yaml    |  173 +++
 MAINTAINERS                                        |    8 +
 drivers/iio/adc/Kconfig                            |   14 +
 drivers/iio/adc/Makefile                           |    1 +
 drivers/iio/adc/ad4691.c                           | 1638 ++++++++++++++++++++
 5 files changed, 1834 insertions(+)
---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20260302-ad4692-multichannel-sar-adc-driver-78e4d44d24b2

Best regards,
-- 
Radu Sabau <radu.sabau@analog.com>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family
  2026-03-20 11:03 [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
@ 2026-03-20 11:03 ` Radu Sabau via B4 Relay
  2026-03-20 12:26   ` Rob Herring (Arm)
                     ` (2 more replies)
  2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 22+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-03-20 11:03 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	Radu Sabau

From: Radu Sabau <radu.sabau@analog.com>

Add DT bindings for the Analog Devices AD4691 family of multichannel
SAR ADCs (AD4691, AD4692, AD4693, AD4694).

The binding describes the hardware connections:

- Power domains: avdd-supply (required), vio-supply, ref-supply or
  refin-supply (external reference; the REFIN path enables the
  internal reference buffer), and an optional ldo-in-supply, that if
  absent, means the on-chip internal LDO will be used.

- Optional PWM on the CNV pin selects CNV Burst Mode; when absent,
  Manual Mode is assumed with CNV tied to SPI CS.

- An optional reset GPIO (reset-gpios) for hardware reset.

- Up to four GP pins (gp0..gp3) usable as interrupt sources,
  identified in firmware via interrupt-names "gp0".."gp3".

- gpio-controller with #gpio-cells = <2> for GP pin GPIO usage.

- #trigger-source-cells = <1>: one cell selecting the GP pin number
  (0-3) used as the SPI offload trigger source.

Two binding examples are provided: CNV Burst Mode with SPI offload
(DMA data acquisition driven by DATA_READY on a GP pin), and Manual
Mode for CPU-driven triggered-buffer or single-shot capture.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 .../devicetree/bindings/iio/adc/adi,ad4691.yaml    | 173 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 2 files changed, 180 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
new file mode 100644
index 000000000000..def9f32c78af
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
@@ -0,0 +1,173 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4691.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4691 Family Multichannel SAR ADCs
+
+maintainers:
+  - Radu Sabau <radu.sabau@analog.com>
+
+description: |
+  The AD4691 family are high-speed, low-power, multichannel successive
+  approximation register (SAR) analog-to-digital converters (ADCs) with
+  an SPI-compatible serial interface. The ADC supports CNV Burst Mode,
+  where an external PWM drives the CNV pin, and Manual Mode, where CNV
+  is directly tied to the SPI chip-select.
+
+  Datasheets:
+    * https://www.analog.com/en/products/ad4692.html
+    * https://www.analog.com/en/products/ad4691.html
+    * https://www.analog.com/en/products/ad4694.html
+    * https://www.analog.com/en/products/ad4693.html
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4691
+      - adi,ad4692
+      - adi,ad4693
+      - adi,ad4694
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 40000000
+
+  spi-cpol: true
+  spi-cpha: true
+
+  avdd-supply:
+    description: Analog power supply (4.5V to 5.5V).
+
+  ldo-in-supply:
+    description: LDO input supply. When absent, the internal LDO is used.
+
+  vio-supply:
+    description: I/O voltage supply (1.71V to 1.89V or VDD).
+
+  ref-supply:
+    description: External reference voltage supply (2.4V to 5.25V).
+
+  refin-supply:
+    description: Internal reference buffer input supply.
+
+  reset-gpios:
+    description:
+      GPIO line controlling the hardware reset pin (active-low).
+    maxItems: 1
+
+  pwms:
+    description:
+      PWM connected to the CNV pin. When present, selects CNV Burst Mode where
+      the PWM drives the conversion rate. When absent, Manual Mode is used
+      (CNV tied to SPI CS).
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt lines connected to the ADC GP pins. Each GP pin can be
+      physically wired to an interrupt-capable input on the SoC.
+    maxItems: 4
+
+  interrupt-names:
+    description: Names of the interrupt lines, matching the GP pin names.
+    minItems: 1
+    maxItems: 4
+    items:
+      - const: gp0
+      - const: gp1
+      - const: gp2
+      - const: gp3
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  '#trigger-source-cells':
+    description:
+      This node can act as a trigger source. The single cell in a consumer
+      reference specifies the GP pin number (0-3) used as the trigger output.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - vio-supply
+
+allOf:
+  # ref-supply and refin-supply are mutually exclusive, one is required
+  - oneOf:
+      - required:
+          - ref-supply
+      - required:
+          - refin-supply
+
+  # CNV Burst Mode (pwms present) without SPI offload requires a DRDY interrupt.
+  # Offload configurations expose '#trigger-source-cells' instead.
+  - if:
+      required:
+        - pwms
+      not:
+        required:
+          - '#trigger-source-cells'
+    then:
+      required:
+        - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    /* AD4692 in CNV Burst Mode with SPI offload */
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4692";
+            reg = <0>;
+            spi-cpol;
+            spi-cpha;
+            spi-max-frequency = <40000000>;
+
+            avdd-supply = <&avdd_supply>;
+            vio-supply = <&vio_supply>;
+            ref-supply = <&ref_5v>;
+
+            reset-gpios = <&gpio0 15 GPIO_ACTIVE_LOW>;
+
+            pwms = <&pwm_gen 0 0>;
+
+            #trigger-source-cells = <1>;
+        };
+    };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    /* AD4692 in Manual Mode (CNV tied to SPI CS) */
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4692";
+            reg = <0>;
+            spi-cpol;
+            spi-cpha;
+            spi-max-frequency = <31250000>;
+
+            avdd-supply = <&avdd_supply>;
+            vio-supply = <&vio_supply>;
+            refin-supply = <&refin_supply>;
+
+            reset-gpios = <&gpio0 15 GPIO_ACTIVE_LOW>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 61bf550fd37c..438ca850fa1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1484,6 +1484,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4170-4.yaml
 F:	drivers/iio/adc/ad4170-4.c
 
+ANALOG DEVICES INC AD4691 DRIVER
+M:	Radu Sabau <radu.sabau@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
+
 ANALOG DEVICES INC AD4695 DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 M:	Nuno Sá <nuno.sa@analog.com>

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 2/4] iio: adc: ad4691: add initial driver for AD4691 family
  2026-03-20 11:03 [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
@ 2026-03-20 11:03 ` Radu Sabau via B4 Relay
  2026-03-20 15:16   ` Andy Shevchenko
                     ` (2 more replies)
  2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
  3 siblings, 3 replies; 22+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-03-20 11:03 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	Radu Sabau

From: Radu Sabau <radu.sabau@analog.com>

Add support for the Analog Devices AD4691 family of high-speed,
low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
AD4694 (8-ch, 1 MSPS).

The driver implements a custom regmap layer over raw SPI to handle the
device's mixed 1/2/3/4-byte register widths and uses the standard IIO
read_raw/write_raw interface for single-channel reads.

The chip idles in Autonomous Mode so that single-shot read_raw can use
the internal oscillator without disturbing the hardware configuration.

Three voltage supply domains are managed: avdd (required), vio, and a
reference supply on either the REF pin (ref-supply, external buffer)
or the REFIN pin (refin-supply, uses the on-chip reference buffer;
REFBUF_EN is set accordingly). Hardware reset is performed via
the reset controller framework; a software reset through SPI_CONFIG_A
is used as fallback when no hardware reset is available.

Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
16-bit transfer.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  11 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4691.c | 686 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 699 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 438ca850fa1c..24e4502b8292 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1490,6 +1490,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
+F:	drivers/iio/adc/ad4691.c
 
 ANALOG DEVICES INC AD4695 DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 60038ae8dfc4..3685a03aa8dc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -139,6 +139,17 @@ config AD4170_4
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4170-4.
 
+config AD4691
+	tristate "Analog Devices AD4691 Family ADC Driver"
+	depends on SPI
+	select REGMAP
+	help
+	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
+	  SPI analog to digital converters (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4691.
+
 config AD4695
 	tristate "Analog Device AD4695 ADC Driver"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index c76550415ff1..4ac1ea09d773 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD4080) += ad4080.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD4134) += ad4134.o
 obj-$(CONFIG_AD4170_4) += ad4170-4.o
+obj-$(CONFIG_AD4691) += ad4691.o
 obj-$(CONFIG_AD4695) += ad4695.o
 obj-$(CONFIG_AD4851) += ad4851.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
new file mode 100644
index 000000000000..5e02eb44ca44
--- /dev/null
+++ b/drivers/iio/adc/ad4691.c
@@ -0,0 +1,686 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024-2026 Analog Devices, Inc.
+ * Author: Radu Sabau <radu.sabau@analog.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/iio.h>
+
+#define AD4691_VREF_uV_MIN			2400000
+#define AD4691_VREF_uV_MAX			5250000
+#define AD4691_VREF_2P5_uV_MAX			2750000
+#define AD4691_VREF_3P0_uV_MAX			3250000
+#define AD4691_VREF_3P3_uV_MAX			3750000
+#define AD4691_VREF_4P096_uV_MAX		4500000
+
+#define AD4691_SPI_CONFIG_A_REG			0x000
+#define AD4691_SW_RESET				(BIT(7) | BIT(0))
+
+#define AD4691_STATUS_REG			0x014
+#define AD4691_CLAMP_STATUS1_REG		0x01A
+#define AD4691_CLAMP_STATUS2_REG		0x01B
+#define AD4691_DEVICE_SETUP			0x020
+#define AD4691_LDO_EN				BIT(4)
+#define AD4691_REF_CTRL				0x021
+#define AD4691_REF_CTRL_MASK			GENMASK(4, 2)
+#define AD4691_REFBUF_EN			BIT(0)
+#define AD4691_OSC_FREQ_REG			0x023
+#define AD4691_OSC_FREQ_MASK			GENMASK(3, 0)
+#define AD4691_STD_SEQ_CONFIG			0x025
+#define AD4691_SPARE_CONTROL			0x02A
+
+#define AD4691_OSC_EN_REG			0x180
+#define AD4691_STATE_RESET_REG			0x181
+#define AD4691_STATE_RESET_ALL			0x01
+#define AD4691_ADC_SETUP			0x182
+#define AD4691_AUTONOMOUS_MODE_VAL		0x02
+/*
+ * ACC_MASK_REG covers both mask bytes via ADDR_DESCENDING SPI: writing a
+ * 16-bit BE value to 0x185 auto-decrements to 0x184 for the second byte.
+ */
+#define AD4691_ACC_MASK_REG			0x185
+#define AD4691_ACC_COUNT_LIMIT(n)		(0x186 + (n))
+#define AD4691_GPIO_MODE1_REG			0x196
+#define AD4691_GPIO_MODE2_REG			0x197
+#define AD4691_GPIO_READ			0x1A0
+#define AD4691_ACC_STATUS_FULL1_REG		0x1B0
+#define AD4691_ACC_STATUS_FULL2_REG		0x1B1
+#define AD4691_ACC_STATUS_OVERRUN1_REG		0x1B2
+#define AD4691_ACC_STATUS_OVERRUN2_REG		0x1B3
+#define AD4691_ACC_STATUS_SAT1_REG		0x1B4
+#define AD4691_ACC_STATUS_SAT2_REG		0x1BE
+#define AD4691_ACC_SAT_OVR_REG(n)		(0x1C0 + (n))
+#define AD4691_AVG_IN(n)			(0x201 + (2 * (n)))
+#define AD4691_AVG_STS_IN(n)			(0x222 + (3 * (n)))
+#define AD4691_ACC_IN(n)			(0x252 + (3 * (n)))
+#define AD4691_ACC_STS_DATA(n)			(0x283 + (4 * (n)))
+
+enum ad4691_ref_ctrl {
+	AD4691_VREF_2P5   = 0,
+	AD4691_VREF_3P0   = 1,
+	AD4691_VREF_3P3   = 2,
+	AD4691_VREF_4P096 = 3,
+	AD4691_VREF_5P0   = 4,
+};
+
+struct ad4691_chip_info {
+	const struct iio_chan_spec *channels;
+	const char *name;
+	unsigned int num_channels;
+	unsigned int max_rate;
+};
+
+#define AD4691_CHANNEL(ch)						\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
+				    | BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+		.info_mask_separate_available =				\
+				      BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE),	\
+		.channel = ch,						\
+		.scan_index = ch,					\
+		.scan_type = {						\
+			.sign = 'u',					\
+			.realbits = 16,					\
+			.storagebits = 16,				\
+			.shift = 0,					\
+		},							\
+	}
+
+static const struct iio_chan_spec ad4691_channels[] = {
+	AD4691_CHANNEL(0),
+	AD4691_CHANNEL(1),
+	AD4691_CHANNEL(2),
+	AD4691_CHANNEL(3),
+	AD4691_CHANNEL(4),
+	AD4691_CHANNEL(5),
+	AD4691_CHANNEL(6),
+	AD4691_CHANNEL(7),
+	AD4691_CHANNEL(8),
+	AD4691_CHANNEL(9),
+	AD4691_CHANNEL(10),
+	AD4691_CHANNEL(11),
+	AD4691_CHANNEL(12),
+	AD4691_CHANNEL(13),
+	AD4691_CHANNEL(14),
+	AD4691_CHANNEL(15),
+};
+
+static const struct iio_chan_spec ad4693_channels[] = {
+	AD4691_CHANNEL(0),
+	AD4691_CHANNEL(1),
+	AD4691_CHANNEL(2),
+	AD4691_CHANNEL(3),
+	AD4691_CHANNEL(4),
+	AD4691_CHANNEL(5),
+	AD4691_CHANNEL(6),
+	AD4691_CHANNEL(7),
+};
+
+/*
+ * Internal oscillator frequency table. Index is the OSC_FREQ_REG[3:0] value.
+ * Index 0 (1 MHz) is only valid for AD4692/AD4694; AD4691/AD4693 support
+ * up to 500 kHz and use index 1 as their highest valid rate.
+ */
+static const unsigned int ad4691_osc_freqs[] = {
+	1000000,	/* 0x0: 1 MHz */
+	500000,		/* 0x1: 500 kHz */
+	400000,		/* 0x2: 400 kHz */
+	250000,		/* 0x3: 250 kHz */
+	200000,		/* 0x4: 200 kHz */
+	167000,		/* 0x5: 167 kHz */
+	133000,		/* 0x6: 133 kHz */
+	125000,		/* 0x7: 125 kHz */
+	100000,		/* 0x8: 100 kHz */
+	50000,		/* 0x9: 50 kHz */
+	25000,		/* 0xA: 25 kHz */
+	12500,		/* 0xB: 12.5 kHz */
+	10000,		/* 0xC: 10 kHz */
+	5000,		/* 0xD: 5 kHz */
+	2500,		/* 0xE: 2.5 kHz */
+	1250,		/* 0xF: 1.25 kHz */
+};
+
+static const struct ad4691_chip_info ad4691_chip_info = {
+	.channels = ad4691_channels,
+	.name = "ad4691",
+	.num_channels = ARRAY_SIZE(ad4691_channels),
+	.max_rate = 500 * HZ_PER_KHZ,
+};
+
+static const struct ad4691_chip_info ad4692_chip_info = {
+	.channels = ad4691_channels,
+	.name = "ad4692",
+	.num_channels = ARRAY_SIZE(ad4691_channels),
+	.max_rate = 1 * HZ_PER_MHZ,
+};
+
+static const struct ad4691_chip_info ad4693_chip_info = {
+	.channels = ad4693_channels,
+	.name = "ad4693",
+	.num_channels = ARRAY_SIZE(ad4693_channels),
+	.max_rate = 500 * HZ_PER_KHZ,
+};
+
+static const struct ad4691_chip_info ad4694_chip_info = {
+	.channels = ad4693_channels,
+	.name = "ad4694",
+	.num_channels = ARRAY_SIZE(ad4693_channels),
+	.max_rate = 1 * HZ_PER_MHZ,
+};
+
+struct ad4691_state {
+	const struct ad4691_chip_info	*info;
+	struct regmap			*regmap;
+	int				vref_uV;
+	bool				refbuf_en;
+	bool				ldo_en;
+	/*
+	 * Synchronize access to members of the driver state, and ensure
+	 * atomicity of consecutive SPI operations.
+	 */
+	struct mutex			lock;
+};
+
+static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct spi_device *spi = context;
+	u8 tx[2], rx[4];
+	int ret;
+
+	put_unaligned_be16(0x8000 | reg, tx);
+
+	switch (reg) {
+	case 0 ... AD4691_OSC_FREQ_REG:
+	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
+		ret = spi_write_then_read(spi, tx, 2, rx, 1);
+		if (ret)
+			return ret;
+		*val = rx[0];
+		return 0;
+	case AD4691_STD_SEQ_CONFIG:
+	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
+		ret = spi_write_then_read(spi, tx, 2, rx, 2);
+		if (ret)
+			return ret;
+		*val = get_unaligned_be16(rx);
+		return 0;
+	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
+	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
+		ret = spi_write_then_read(spi, tx, 2, rx, 3);
+		if (ret)
+			return ret;
+		*val = get_unaligned_be24(rx);
+		return 0;
+	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
+		ret = spi_write_then_read(spi, tx, 2, rx, 4);
+		if (ret)
+			return ret;
+		*val = get_unaligned_be32(rx);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4691_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct spi_device *spi = context;
+	u8 tx[4];
+
+	put_unaligned_be16(reg, tx);
+
+	switch (reg) {
+	case 0 ... AD4691_OSC_FREQ_REG:
+	case AD4691_SPARE_CONTROL ... AD4691_ACC_MASK_REG - 1:
+	case AD4691_ACC_MASK_REG + 1 ... AD4691_GPIO_MODE2_REG:
+		if (val > 0xFF)
+			return -EINVAL;
+		tx[2] = val;
+		return spi_write_then_read(spi, tx, 3, NULL, 0);
+	case AD4691_ACC_MASK_REG:
+	case AD4691_STD_SEQ_CONFIG:
+		if (val > 0xFFFF)
+			return -EINVAL;
+		put_unaligned_be16(val, &tx[2]);
+		return spi_write_then_read(spi, tx, 4, NULL, 0);
+	default:
+		return -EINVAL;
+	}
+}
+
+static bool ad4691_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case AD4691_STATUS_REG:
+	case AD4691_CLAMP_STATUS1_REG:
+	case AD4691_CLAMP_STATUS2_REG:
+	case AD4691_GPIO_READ:
+	case AD4691_ACC_STATUS_FULL1_REG ... AD4691_ACC_STATUS_SAT2_REG:
+	case AD4691_ACC_SAT_OVR_REG(0) ... AD4691_ACC_SAT_OVR_REG(15):
+	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
+	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
+	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
+	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool ad4691_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case 0 ... AD4691_OSC_FREQ_REG:
+	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
+	case AD4691_STD_SEQ_CONFIG:
+	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
+	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
+	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
+	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool ad4691_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case 0 ... AD4691_OSC_FREQ_REG:
+	case AD4691_STD_SEQ_CONFIG:
+	case AD4691_SPARE_CONTROL ... AD4691_GPIO_MODE2_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config ad4691_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 32,
+	.reg_read = ad4691_reg_read,
+	.reg_write = ad4691_reg_write,
+	.volatile_reg = ad4691_volatile_reg,
+	.readable_reg = ad4691_readable_reg,
+	.writeable_reg = ad4691_writeable_reg,
+	.max_register = AD4691_ACC_STS_DATA(15),
+	.cache_type = REGCACHE_MAPLE,
+};
+
+static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
+	if (ret)
+		return ret;
+
+	*val = ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
+	return IIO_VAL_INT;
+}
+
+static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
+	unsigned int i;
+
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
+		if ((int)ad4691_osc_freqs[i] == freq)
+			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
+						  AD4691_OSC_FREQ_MASK, i);
+	}
+
+	return -EINVAL;
+}
+
+static int ad4691_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type,
+			     int *length, long mask)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (const int *)&ad4691_osc_freqs[start];
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(ad4691_osc_freqs) - start;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4691_single_shot_read(struct iio_dev *indio_dev,
+				   struct iio_chan_spec const *chan, int *val)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	/*
+	 * Use AUTONOMOUS mode for single-shot reads. The chip always
+	 * operates in AUTONOMOUS mode in this driver revision.
+	 */
+	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
+			   AD4691_STATE_RESET_ALL);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+			   BIT(chan->channel));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
+			   (u16)~BIT(chan->channel));
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 1);
+	if (ret)
+		return ret;
+
+	/*
+	 * Wait for at least 2 internal oscillator periods for the
+	 * conversion to complete.
+	 */
+	fsleep(DIV_ROUND_UP(2 * USEC_PER_SEC,
+			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK,
+						       reg_val)]));
+
+	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, AD4691_AVG_IN(chan->channel), &reg_val);
+	if (ret)
+		return ret;
+
+	*val = reg_val;
+
+	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG, AD4691_STATE_RESET_ALL);
+	if (ret)
+		return ret;
+
+	return IIO_VAL_INT;
+}
+
+static int ad4691_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long info)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW: {
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
+		return ad4691_single_shot_read(indio_dev, chan, val);
+	}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad4691_get_sampling_freq(st, val);
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_uV / (MICRO / MILLI);
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4691_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad4691_set_sampling_freq(indio_dev, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4691_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static const struct iio_info ad4691_info = {
+	.read_raw = &ad4691_read_raw,
+	.write_raw = &ad4691_write_raw,
+	.read_avail = &ad4691_read_avail,
+	.debugfs_reg_access = &ad4691_reg_access,
+};
+
+static int ad4691_regulator_setup(struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	int ret;
+
+	ret = devm_regulator_get_enable(dev, "avdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get and enable AVDD\n");
+
+	ret = devm_regulator_get_enable(dev, "ldo-in");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "Failed to get and enable LDO-IN\n");
+	st->ldo_en = (ret == -ENODEV);
+
+	ret = devm_regulator_get_enable(dev, "vio");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get and enable VIO\n");
+
+	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
+	if (st->vref_uV >= 0) {
+		st->refbuf_en = false;
+	} else if (st->vref_uV == -ENODEV) {
+		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "refin");
+		st->refbuf_en = true;
+	}
+	if (st->vref_uV < 0)
+		return dev_err_probe(dev, st->vref_uV,
+				     "Failed to get reference supply\n");
+
+	if (st->vref_uV < AD4691_VREF_uV_MIN || st->vref_uV > AD4691_VREF_uV_MAX)
+		return dev_err_probe(dev, -EINVAL,
+				     "vref(%d) must be in the range [%u...%u]\n",
+				     st->vref_uV, AD4691_VREF_uV_MIN,
+				     AD4691_VREF_uV_MAX);
+
+	return 0;
+}
+
+static int ad4691_reset(struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	struct reset_control *rst;
+
+	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset\n");
+
+	if (!rst)
+		/* No hardware reset available, fall back to software reset. */
+		return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
+				    AD4691_SW_RESET);
+
+	reset_control_assert(rst);
+	/* Reset delay required. See datasheet Table 5. */
+	fsleep(300);
+	reset_control_deassert(rst);
+
+	return 0;
+}
+
+static int ad4691_config(struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	enum ad4691_ref_ctrl ref_val;
+	int ret;
+
+	switch (st->vref_uV) {
+	case AD4691_VREF_uV_MIN ... AD4691_VREF_2P5_uV_MAX:
+		ref_val = AD4691_VREF_2P5;
+		break;
+	case AD4691_VREF_2P5_uV_MAX + 1 ... AD4691_VREF_3P0_uV_MAX:
+		ref_val = AD4691_VREF_3P0;
+		break;
+	case AD4691_VREF_3P0_uV_MAX + 1 ... AD4691_VREF_3P3_uV_MAX:
+		ref_val = AD4691_VREF_3P3;
+		break;
+	case AD4691_VREF_3P3_uV_MAX + 1 ... AD4691_VREF_4P096_uV_MAX:
+		ref_val = AD4691_VREF_4P096;
+		break;
+	case AD4691_VREF_4P096_uV_MAX + 1 ... AD4691_VREF_uV_MAX:
+		ref_val = AD4691_VREF_5P0;
+		break;
+	default:
+		return dev_err_probe(dev, -EINVAL,
+				     "Unsupported vref voltage: %d uV\n",
+				     st->vref_uV);
+	}
+
+	ret = regmap_write(st->regmap, AD4691_REF_CTRL,
+			   FIELD_PREP(AD4691_REF_CTRL_MASK, ref_val) |
+			   (st->refbuf_en ? AD4691_REFBUF_EN : 0));
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to write REF_CTRL\n");
+
+	ret = regmap_write(st->regmap, AD4691_DEVICE_SETUP,
+			   st->ldo_en ? AD4691_LDO_EN : 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
+
+	/*
+	 * Set the internal oscillator to the highest valid rate for this chip.
+	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
+	 * at index 1 (500 kHz).
+	 */
+	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
+			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
+
+	/* Device always operates in AUTONOMOUS mode. */
+	return regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE_VAL);
+}
+
+static int ad4691_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ad4691_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->info = spi_get_device_match_data(spi);
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	st->regmap = devm_regmap_init(dev, NULL, spi, &ad4691_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to initialize regmap\n");
+
+	ret = ad4691_regulator_setup(st);
+	if (ret)
+		return ret;
+
+	ret = ad4691_reset(st);
+	if (ret)
+		return ret;
+
+	ret = ad4691_config(st);
+	if (ret)
+		return ret;
+
+	indio_dev->name = st->info->name;
+	indio_dev->info = &ad4691_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->channels = st->info->channels;
+	indio_dev->num_channels = st->info->num_channels;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad4691_of_match[] = {
+	{ .compatible = "adi,ad4691", .data = &ad4691_chip_info },
+	{ .compatible = "adi,ad4692", .data = &ad4692_chip_info },
+	{ .compatible = "adi,ad4693", .data = &ad4693_chip_info },
+	{ .compatible = "adi,ad4694", .data = &ad4694_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad4691_of_match);
+
+static const struct spi_device_id ad4691_id[] = {
+	{ "ad4691", (kernel_ulong_t)&ad4691_chip_info },
+	{ "ad4692", (kernel_ulong_t)&ad4692_chip_info },
+	{ "ad4693", (kernel_ulong_t)&ad4693_chip_info },
+	{ "ad4694", (kernel_ulong_t)&ad4694_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad4691_id);
+
+static struct spi_driver ad4691_driver = {
+	.driver = {
+		.name = "ad4691",
+		.of_match_table = ad4691_of_match,
+	},
+	.probe = ad4691_probe,
+	.id_table = ad4691_id,
+};
+module_spi_driver(ad4691_driver);
+
+MODULE_AUTHOR("Radu Sabau <radu.sabau@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4691 Family ADC Driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-20 11:03 [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
  2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
@ 2026-03-20 11:03 ` Radu Sabau via B4 Relay
  2026-03-20 16:14   ` Uwe Kleine-König
                     ` (2 more replies)
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
  3 siblings, 3 replies; 22+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-03-20 11:03 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	Radu Sabau

From: Radu Sabau <radu.sabau@analog.com>

Add buffered capture support using the IIO triggered buffer framework.

CNV Burst Mode: the GP pin identified by interrupt-names in the device
tree is configured as DATA_READY output. The IRQ handler stops
conversions and fires the IIO trigger; the trigger handler executes a
pre-built SPI message that reads all active channels from the AVG_IN
accumulator registers and then resets accumulator state and restarts
conversions for the next cycle.

Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
reads the previous result and starts the next conversion (pipelined
N+1 scheme). At preenable time a pre-built, optimised SPI message of
N+1 transfers is constructed (N channel reads plus one NOOP to drain
the pipeline). The trigger handler executes the message in a single
spi_sync() call and collects the results. An external trigger (e.g.
iio-trig-hrtimer) is required to drive the trigger at the desired
sample rate.

Both modes share the same trigger handler and push a complete scan —
one u16 slot per channel at its scan_index position, followed by a
timestamp — to the IIO buffer via iio_push_to_buffers_with_ts().

The CNV Burst Mode sampling frequency (PWM period) is exposed as a
buffer-level attribute via IIO_DEVICE_ATTR.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 drivers/iio/adc/Kconfig  |   2 +
 drivers/iio/adc/ad4691.c | 584 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 571 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 3685a03aa8dc..d498f16c0816 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -142,6 +142,8 @@ config AD4170_4
 config AD4691
 	tristate "Analog Devices AD4691 Family ADC Driver"
 	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	select REGMAP
 	help
 	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
index 5e02eb44ca44..db776de32846 100644
--- a/drivers/iio/adc/ad4691.c
+++ b/drivers/iio/adc/ad4691.c
@@ -9,9 +9,12 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/interrupt.h>
 #include <linux/math.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
@@ -19,7 +22,12 @@
 #include <linux/units.h>
 #include <linux/unaligned.h>
 
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 
 #define AD4691_VREF_uV_MIN			2400000
 #define AD4691_VREF_uV_MAX			5250000
@@ -28,6 +36,8 @@
 #define AD4691_VREF_3P3_uV_MAX			3750000
 #define AD4691_VREF_4P096_uV_MAX		4500000
 
+#define AD4691_CNV_DUTY_CYCLE_NS		380
+
 #define AD4691_SPI_CONFIG_A_REG			0x000
 #define AD4691_SW_RESET				(BIT(7) | BIT(0))
 
@@ -35,6 +45,7 @@
 #define AD4691_CLAMP_STATUS1_REG		0x01A
 #define AD4691_CLAMP_STATUS2_REG		0x01B
 #define AD4691_DEVICE_SETUP			0x020
+#define AD4691_MANUAL_MODE			BIT(2)
 #define AD4691_LDO_EN				BIT(4)
 #define AD4691_REF_CTRL				0x021
 #define AD4691_REF_CTRL_MASK			GENMASK(4, 2)
@@ -42,21 +53,29 @@
 #define AD4691_OSC_FREQ_REG			0x023
 #define AD4691_OSC_FREQ_MASK			GENMASK(3, 0)
 #define AD4691_STD_SEQ_CONFIG			0x025
+#define AD4691_SEQ_ALL_CHANNELS_OFF		0x00
 #define AD4691_SPARE_CONTROL			0x02A
 
+#define AD4691_NOOP				0x00
+#define AD4691_ADC_CHAN(ch)			((0x10 + (ch)) << 3)
+
 #define AD4691_OSC_EN_REG			0x180
 #define AD4691_STATE_RESET_REG			0x181
 #define AD4691_STATE_RESET_ALL			0x01
 #define AD4691_ADC_SETUP			0x182
-#define AD4691_AUTONOMOUS_MODE_VAL		0x02
+#define AD4691_CNV_BURST_MODE			0x01
+#define AD4691_AUTONOMOUS_MODE			0x02
 /*
  * ACC_MASK_REG covers both mask bytes via ADDR_DESCENDING SPI: writing a
  * 16-bit BE value to 0x185 auto-decrements to 0x184 for the second byte.
  */
 #define AD4691_ACC_MASK_REG			0x185
 #define AD4691_ACC_COUNT_LIMIT(n)		(0x186 + (n))
+#define AD4691_ACC_COUNT_VAL			0x1
 #define AD4691_GPIO_MODE1_REG			0x196
 #define AD4691_GPIO_MODE2_REG			0x197
+#define AD4691_GP_MODE_MASK			GENMASK(3, 0)
+#define AD4691_GP_MODE_DATA_READY		0x06
 #define AD4691_GPIO_READ			0x1A0
 #define AD4691_ACC_STATUS_FULL1_REG		0x1B0
 #define AD4691_ACC_STATUS_FULL2_REG		0x1B1
@@ -121,6 +140,7 @@ static const struct iio_chan_spec ad4691_channels[] = {
 	AD4691_CHANNEL(13),
 	AD4691_CHANNEL(14),
 	AD4691_CHANNEL(15),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
 };
 
 static const struct iio_chan_spec ad4693_channels[] = {
@@ -132,6 +152,7 @@ static const struct iio_chan_spec ad4693_channels[] = {
 	AD4691_CHANNEL(5),
 	AD4691_CHANNEL(6),
 	AD4691_CHANNEL(7),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
 };
 
 /*
@@ -189,16 +210,63 @@ static const struct ad4691_chip_info ad4694_chip_info = {
 struct ad4691_state {
 	const struct ad4691_chip_info	*info;
 	struct regmap			*regmap;
+
+	struct pwm_device		*conv_trigger;
+	struct iio_trigger		*trig;
+	int				irq;
+
+	bool				manual_mode;
+
 	int				vref_uV;
 	bool				refbuf_en;
 	bool				ldo_en;
+	u32				cnv_period_ns;
 	/*
 	 * Synchronize access to members of the driver state, and ensure
 	 * atomicity of consecutive SPI operations.
 	 */
 	struct mutex			lock;
+	/*
+	 * Per-buffer-enabl ree lifetimesources:
+	 * Manual Mode - a pre-built SPI message that clocks out N+1
+	 *		 transfers in one go.
+	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
+	 *		    transfers in one go.
+	 */
+	void				*scan_devm_group;
+	struct spi_message		scan_msg;
+	struct spi_transfer		*scan_xfers;
+	__be16				*scan_tx;
+	__be16				*scan_rx;
+	/* Scan buffer: one slot per channel (u16) plus timestamp */
+	struct {
+		u16 vals[16];
+		s64 ts __aligned(8);
+	} scan __aligned(IIO_DMA_MINALIGN);
 };
 
+/*
+ * Configure the given GP pin (0-3) as DATA_READY output.
+ * GP0/GP1 → GPIO_MODE1_REG, GP2/GP3 → GPIO_MODE2_REG.
+ * Even pins occupy bits [3:0], odd pins bits [7:4].
+ */
+static int ad4691_gpio_setup(struct ad4691_state *st, unsigned int gp_num)
+{
+	unsigned int shift = 4 * (gp_num % 2);
+
+	return regmap_update_bits(st->regmap,
+				  AD4691_GPIO_MODE1_REG + gp_num / 2,
+				  AD4691_GP_MODE_MASK << shift,
+				  AD4691_GP_MODE_DATA_READY << shift);
+}
+
+static void ad4691_disable_pwm(void *data)
+{
+	struct pwm_state state = { .enabled = false };
+
+	pwm_apply_might_sleep(data, &state);
+}
+
 static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct spi_device *spi = context;
@@ -341,14 +409,16 @@ static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
 static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
 {
 	struct ad4691_state *st = iio_priv(indio_dev);
-	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
 	unsigned int i;
 
+	if (freq > st->info->max_rate)
+		return -EINVAL;
+
 	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
 	if (IIO_DEV_ACQUIRE_FAILED(claim))
 		return -EBUSY;
 
-	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
+	for (i = 0; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
 		if ((int)ad4691_osc_freqs[i] == freq)
 			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
 						  AD4691_OSC_FREQ_MASK, i);
@@ -363,7 +433,10 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
 			     int *length, long mask)
 {
 	struct ad4691_state *st = iio_priv(indio_dev);
-	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
+	unsigned int start;
+
+	/* Skip frequencies that exceed this chip's maximum rate. */
+	start = (ad4691_osc_freqs[0] > st->info->max_rate) ? 1 : 0;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
@@ -386,8 +459,7 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
 	guard(mutex)(&st->lock);
 
 	/*
-	 * Use AUTONOMOUS mode for single-shot reads. The chip always
-	 * operates in AUTONOMOUS mode in this driver revision.
+	 * Use AUTONOMOUS mode for single-shot reads.
 	 */
 	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
 			   AD4691_STATE_RESET_ALL);
@@ -417,8 +489,7 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
 	 * conversion to complete.
 	 */
 	fsleep(DIV_ROUND_UP(2 * USEC_PER_SEC,
-			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK,
-						       reg_val)]));
+		ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)]));
 
 	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
 	if (ret)
@@ -488,6 +559,374 @@ static int ad4691_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 	return regmap_write(st->regmap, reg, writeval);
 }
 
+static int ad4691_set_pwm_freq(struct ad4691_state *st, int freq)
+{
+	if (!freq)
+		return -EINVAL;
+
+	st->cnv_period_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq);
+	return 0;
+}
+
+static int ad4691_sampling_enable(struct ad4691_state *st, bool enable)
+{
+	struct pwm_state conv_state = { };
+
+	conv_state.period = st->cnv_period_ns;
+	conv_state.duty_cycle = AD4691_CNV_DUTY_CYCLE_NS;
+	conv_state.polarity = PWM_POLARITY_NORMAL;
+	conv_state.enabled = enable;
+
+	return pwm_apply_might_sleep(st->conv_trigger, &conv_state);
+}
+
+/*
+ * ad4691_enter_conversion_mode - Switch the chip to its buffer conversion mode.
+ *
+ * Configures the ADC hardware registers for the mode selected at probe
+ * (CNV_BURST or MANUAL). Called from buffer preenable before starting
+ * sampling. The chip is in AUTONOMOUS mode during idle (for read_raw).
+ */
+static int ad4691_enter_conversion_mode(struct ad4691_state *st)
+{
+	int ret;
+
+	if (st->manual_mode)
+		return regmap_update_bits(st->regmap, AD4691_DEVICE_SETUP,
+					  AD4691_MANUAL_MODE, AD4691_MANUAL_MODE);
+
+	ret = regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_CNV_BURST_MODE);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD4691_STATE_RESET_REG,
+			    AD4691_STATE_RESET_ALL);
+}
+
+/*
+ * ad4691_exit_conversion_mode - Return the chip to AUTONOMOUS mode.
+ *
+ * Called from buffer postdisable to restore the chip to the
+ * idle state used by read_raw. Clears the sequencer and resets state.
+ */
+static int ad4691_exit_conversion_mode(struct ad4691_state *st)
+{
+	if (st->manual_mode)
+		return regmap_update_bits(st->regmap, AD4691_DEVICE_SETUP,
+					  AD4691_MANUAL_MODE, 0);
+
+	return regmap_write(st->regmap, AD4691_ADC_SETUP,
+			    AD4691_AUTONOMOUS_MODE);
+}
+
+static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	struct spi_device *spi = to_spi_device(dev);
+	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
+	unsigned int n_xfers = n_active + 1;
+	unsigned int k, i;
+	int ret;
+
+	st->scan_devm_group = devres_open_group(dev, NULL, GFP_KERNEL);
+	if (!st->scan_devm_group)
+		return -ENOMEM;
+
+	st->scan_xfers = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_xfers),
+				      GFP_KERNEL);
+	st->scan_tx = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_tx),
+				   GFP_KERNEL);
+	st->scan_rx = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_rx),
+				   GFP_KERNEL);
+	if (!st->scan_xfers || !st->scan_tx || !st->scan_rx) {
+		devres_release_group(dev, st->scan_devm_group);
+		return -ENOMEM;
+	}
+
+	spi_message_init(&st->scan_msg);
+
+	k = 0;
+	iio_for_each_active_channel(indio_dev, i) {
+		st->scan_tx[k] = cpu_to_be16(AD4691_ADC_CHAN(i));
+		st->scan_xfers[k].tx_buf = &st->scan_tx[k];
+		st->scan_xfers[k].rx_buf = &st->scan_rx[k];
+		st->scan_xfers[k].len = sizeof(__be16);
+		st->scan_xfers[k].cs_change = 1;
+		spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
+		k++;
+	}
+
+	/* Final NOOP transfer to retrieve last channel's result. */
+	st->scan_tx[k] = cpu_to_be16(AD4691_NOOP);
+	st->scan_xfers[k].tx_buf = &st->scan_tx[k];
+	st->scan_xfers[k].rx_buf = &st->scan_rx[k];
+	st->scan_xfers[k].len = sizeof(__be16);
+	spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
+
+	devres_close_group(dev, st->scan_devm_group);
+
+	st->scan_msg.spi = spi;
+
+	ret = spi_optimize_message(spi, &st->scan_msg);
+	if (ret) {
+		devres_release_group(dev, st->scan_devm_group);
+		return ret;
+	}
+
+	ret = ad4691_enter_conversion_mode(st);
+	if (ret) {
+		spi_unoptimize_message(&st->scan_msg);
+		devres_release_group(dev, st->scan_devm_group);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ad4691_manual_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	int ret;
+
+	ret = ad4691_exit_conversion_mode(st);
+	spi_unoptimize_message(&st->scan_msg);
+	devres_release_group(dev, st->scan_devm_group);
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops ad4691_manual_buffer_setup_ops = {
+	.preenable = &ad4691_manual_buffer_preenable,
+	.postdisable = &ad4691_manual_buffer_postdisable,
+};
+
+static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	struct spi_device *spi = to_spi_device(dev);
+	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
+	unsigned int bit, k, i;
+	int ret;
+
+	st->scan_devm_group = devres_open_group(dev, NULL, GFP_KERNEL);
+	if (!st->scan_devm_group)
+		return -ENOMEM;
+
+	st->scan_xfers = devm_kcalloc(dev, 2 * n_active, sizeof(*st->scan_xfers),
+				      GFP_KERNEL);
+	st->scan_tx = devm_kcalloc(dev, n_active, sizeof(*st->scan_tx),
+				   GFP_KERNEL);
+	st->scan_rx = devm_kcalloc(dev, n_active, sizeof(*st->scan_rx),
+				   GFP_KERNEL);
+	if (!st->scan_xfers || !st->scan_tx || !st->scan_rx) {
+		devres_release_group(dev, st->scan_devm_group);
+		return -ENOMEM;
+	}
+
+	spi_message_init(&st->scan_msg);
+
+	/*
+	 * Each AVG_IN read needs two transfers: a 2-byte address write phase
+	 * followed by a 2-byte data read phase. CS toggles between channels
+	 * (cs_change=1 on the read phase of all but the last channel).
+	 */
+	k = 0;
+	iio_for_each_active_channel(indio_dev, i) {
+		st->scan_tx[k] = cpu_to_be16(0x8000 | AD4691_AVG_IN(i));
+		st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
+		st->scan_xfers[2 * k].len = sizeof(__be16);
+		spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
+		st->scan_xfers[2 * k + 1].rx_buf = &st->scan_rx[k];
+		st->scan_xfers[2 * k + 1].len = sizeof(__be16);
+		if (k < n_active - 1)
+			st->scan_xfers[2 * k + 1].cs_change = 1;
+		spi_message_add_tail(&st->scan_xfers[2 * k + 1], &st->scan_msg);
+		k++;
+	}
+
+	devres_close_group(dev, st->scan_devm_group);
+
+	st->scan_msg.spi = spi;
+
+	ret = spi_optimize_message(spi, &st->scan_msg);
+	if (ret) {
+		devres_release_group(dev, st->scan_devm_group);
+		return ret;
+	}
+
+	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
+			   (u16)~(*indio_dev->active_scan_mask));
+	if (ret)
+		goto err;
+
+	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+			   *indio_dev->active_scan_mask);
+	if (ret)
+		goto err;
+
+	iio_for_each_active_channel(indio_dev, bit) {
+		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
+				   AD4691_ACC_COUNT_VAL);
+		if (ret)
+			goto err;
+	}
+
+	ret = ad4691_enter_conversion_mode(st);
+	if (ret)
+		goto err;
+
+	ret = ad4691_sampling_enable(st, true);
+	if (ret)
+		goto err;
+
+	enable_irq(st->irq);
+	return 0;
+err:
+	spi_unoptimize_message(&st->scan_msg);
+	devres_release_group(dev, st->scan_devm_group);
+	return ret;
+}
+
+static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	int ret;
+
+	disable_irq(st->irq);
+
+	ret = ad4691_sampling_enable(st, false);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+			   AD4691_SEQ_ALL_CHANNELS_OFF);
+	if (ret)
+		return ret;
+
+	ret = ad4691_exit_conversion_mode(st);
+	spi_unoptimize_message(&st->scan_msg);
+	devres_release_group(dev, st->scan_devm_group);
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {
+	.preenable = &ad4691_cnv_burst_buffer_preenable,
+	.postdisable = &ad4691_cnv_burst_buffer_postdisable,
+};
+
+static ssize_t sampling_frequency_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ad4691_state *st = iio_priv(indio_dev);
+
+	if (st->manual_mode)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%u\n", (u32)(NSEC_PER_SEC / st->cnv_period_ns));
+}
+
+static ssize_t sampling_frequency_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ad4691_state *st = iio_priv(indio_dev);
+	int freq, ret;
+
+	if (st->manual_mode)
+		return -ENODEV;
+
+	ret = kstrtoint(buf, 10, &freq);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = ad4691_set_pwm_freq(st, freq);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(sampling_frequency, 0644,
+		       sampling_frequency_show,
+		       sampling_frequency_store, 0);
+
+static const struct iio_dev_attr *ad4691_buffer_attrs[] = {
+	&iio_dev_attr_sampling_frequency,
+	NULL,
+};
+
+static irqreturn_t ad4691_irq(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct ad4691_state *st = iio_priv(indio_dev);
+
+	/*
+	 * GPx has asserted: stop conversions before reading so the
+	 * accumulator does not continue sampling while the trigger handler
+	 * processes the data. Then fire the IIO trigger to push the sample
+	 * to the buffer.
+	 */
+	ad4691_sampling_enable(st, false);
+	iio_trigger_poll(st->trig);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_trigger_ops ad4691_trigger_ops = {
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static irqreturn_t ad4691_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad4691_state *st = iio_priv(indio_dev);
+	unsigned int i, k = 0;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = spi_sync(st->scan_msg.spi, &st->scan_msg);
+	if (ret)
+		goto done;
+
+	if (st->manual_mode) {
+		iio_for_each_active_channel(indio_dev, i) {
+			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k + 1]);
+			k++;
+		}
+	} else {
+		iio_for_each_active_channel(indio_dev, i) {
+			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k]);
+			k++;
+		}
+
+		ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
+				   AD4691_STATE_RESET_ALL);
+		if (ret)
+			goto done;
+
+		ret = ad4691_sampling_enable(st, true);
+		if (ret)
+			goto done;
+	}
+
+	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
+				    pf->timestamp);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
 static const struct iio_info ad4691_info = {
 	.read_raw = &ad4691_read_raw,
 	.write_raw = &ad4691_write_raw,
@@ -495,6 +934,25 @@ static const struct iio_info ad4691_info = {
 	.debugfs_reg_access = &ad4691_reg_access,
 };
 
+static int ad4691_pwm_setup(struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	int ret;
+
+	st->conv_trigger = devm_pwm_get(dev, "cnv");
+	if (IS_ERR(st->conv_trigger))
+		return dev_err_probe(dev, PTR_ERR(st->conv_trigger),
+				     "Failed to get cnv pwm\n");
+
+	ret = devm_add_action_or_reset(dev, ad4691_disable_pwm,
+				       st->conv_trigger);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to register PWM disable action\n");
+
+	return ad4691_set_pwm_freq(st, st->info->max_rate);
+}
+
 static int ad4691_regulator_setup(struct ad4691_state *st)
 {
 	struct device *dev = regmap_get_device(st->regmap);
@@ -555,12 +1013,30 @@ static int ad4691_reset(struct ad4691_state *st)
 	return 0;
 }
 
-static int ad4691_config(struct ad4691_state *st)
+static int ad4691_config(struct ad4691_state *st, u32 max_speed_hz)
 {
 	struct device *dev = regmap_get_device(st->regmap);
 	enum ad4691_ref_ctrl ref_val;
+	const char *irq_name;
+	unsigned int gp_num;
 	int ret;
 
+	/*
+	 * Determine buffer conversion mode from DT: if a PWM is provided it
+	 * drives the CNV pin (CNV_BURST_MODE); otherwise CNV is tied to CS
+	 * and each SPI transfer triggers a conversion (MANUAL_MODE).
+	 * Both modes idle in AUTONOMOUS mode so that read_raw can use the
+	 * internal oscillator without disturbing the hardware configuration.
+	 */
+	if (device_property_present(dev, "pwms")) {
+		st->manual_mode = false;
+		ret = ad4691_pwm_setup(st);
+		if (ret)
+			return ret;
+	} else {
+		st->manual_mode = true;
+	}
+
 	switch (st->vref_uV) {
 	case AD4691_VREF_uV_MIN ... AD4691_VREF_2P5_uV_MAX:
 		ref_val = AD4691_VREF_2P5;
@@ -595,17 +1071,91 @@ static int ad4691_config(struct ad4691_state *st)
 		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
 
 	/*
-	 * Set the internal oscillator to the highest valid rate for this chip.
-	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
-	 * at index 1 (500 kHz).
+	 * Set the internal oscillator to the highest rate this chip supports.
+	 * Index 0 (1 MHz) exceeds the 500 kHz max of AD4691/AD4693, so those
+	 * chips start at index 1 (500 kHz).
 	 */
 	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
-			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);
+			   (ad4691_osc_freqs[0] > st->info->max_rate) ? 1 : 0);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
 
 	/* Device always operates in AUTONOMOUS mode. */
-	return regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE_VAL);
+	ret = regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to write ADC_SETUp\n");
+
+	if (st->manual_mode)
+		return 0;
+
+	ret = device_property_read_string_array(dev, "interrupt-names",
+						&irq_name, 1);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Failed to read interrupt-names\n");
+
+	if (strncmp(irq_name, "gp", 2) != 0 ||
+	    kstrtouint(irq_name + 2, 10, &gp_num) || gp_num > 3)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid interrupt name '%s'\n", irq_name);
+
+	return ad4691_gpio_setup(st, gp_num);
+}
+
+static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
+					 struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	int irq, ret;
+
+	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+					  indio_dev->name,
+					  iio_device_id(indio_dev));
+	if (!st->trig)
+		return -ENOMEM;
+
+	st->trig->ops = &ad4691_trigger_ops;
+	iio_trigger_set_drvdata(st->trig, st);
+
+	ret = devm_iio_trigger_register(dev, st->trig);
+	if (ret)
+		return dev_err_probe(dev, ret, "IIO trigger register failed\n");
+
+	indio_dev->trig = iio_trigger_get(st->trig);
+
+	if (!st->manual_mode) {
+		/*
+		 * GP0 asserts at end-of-conversion. The IRQ handler stops
+		 * conversions and fires the IIO trigger so the trigger handler
+		 * can read and push the sample to the buffer. The IRQ is kept
+		 * disabled until the buffer is enabled.
+		 */
+		irq = fwnode_irq_get(dev_fwnode(dev), 0);
+		if (irq < 0)
+			return dev_err_probe(dev, irq,
+					     "failed to get GP interrupt\n");
+
+		st->irq = irq;
+
+		ret = devm_request_threaded_irq(dev, irq, NULL,
+						&ad4691_irq,
+						IRQF_ONESHOT | IRQF_NO_AUTOEN,
+						indio_dev->name, indio_dev);
+		if (ret)
+			return ret;
+
+		return devm_iio_triggered_buffer_setup_ext(dev, indio_dev,
+							   &iio_pollfunc_store_time,
+							   &ad4691_trigger_handler,
+							   IIO_BUFFER_DIRECTION_IN,
+							   &ad4691_cnv_burst_buffer_setup_ops,
+							   ad4691_buffer_attrs);
+	}
+
+	return devm_iio_triggered_buffer_setup(dev, indio_dev,
+					       &iio_pollfunc_store_time,
+					       &ad4691_trigger_handler,
+					       &ad4691_manual_buffer_setup_ops);
 }
 
 static int ad4691_probe(struct spi_device *spi)
@@ -639,7 +1189,7 @@ static int ad4691_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = ad4691_config(st);
+	ret = ad4691_config(st, spi->max_speed_hz);
 	if (ret)
 		return ret;
 
@@ -650,6 +1200,10 @@ static int ad4691_probe(struct spi_device *spi)
 	indio_dev->channels = st->info->channels;
 	indio_dev->num_channels = st->info->num_channels;
 
+	ret = ad4691_setup_triggered_buffer(indio_dev, st);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
                   ` (2 preceding siblings ...)
  2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
@ 2026-03-20 11:03 ` Radu Sabau via B4 Relay
  2026-03-20 17:07   ` Andy Shevchenko
                     ` (4 more replies)
  3 siblings, 5 replies; 22+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-03-20 11:03 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	Radu Sabau

From: Radu Sabau <radu.sabau@analog.com>

Add SPI offload support to enable DMA-based, CPU-independent data
acquisition using the SPI Engine offload framework.

When an SPI offload is available (devm_spi_offload_get() succeeds),
the driver registers a DMA engine IIO buffer and uses dedicated buffer
setup operations. If no offload is available the existing software
triggered buffer path is used unchanged.

Both CNV Burst Mode and Manual Mode support offload, but use different
trigger mechanisms:

CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
signal on the GP pin specified by the trigger-source consumer reference
in the device tree (one cell = GP pin number 0-3). For this mode the
driver acts as both an SPI offload consumer (DMA RX stream, message
optimization) and a trigger source provider: it registers the
GP/DATA_READY output via devm_spi_offload_trigger_register() so the
offload framework can match the '#trigger-source-cells' phandle and
automatically fire the SPI Engine DMA transfer at end-of-conversion.

Manual Mode: the SPI Engine is triggered by a periodic trigger at
the configured sampling frequency. The pre-built SPI message uses
the pipelined CNV-on-CS protocol: N+1 4-byte transfers are issued
for N active channels (the first result is discarded as garbage from
the pipeline flush) and the remaining N results are captured by DMA.

All offload transfers use 32-bit frames (bits_per_word=32, len=4) for
DMA word alignment. This patch promotes the channel scan_type from
storagebits=16 (triggered-buffer path) to storagebits=32 to match the
DMA word size; the triggered-buffer paths are updated to the same layout
for consistency. CNV Burst Mode channel data arrives in the lower 16
bits of the 32-bit word (shift=0); Manual Mode data arrives in the upper
16 bits (shift=16), matching the 4-byte SPI transfer layout
[data_hi, data_lo, 0, 0]. A separate ad4691_manual_channels[] array
encodes the shift=16 scan type for manual mode.

Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 drivers/iio/adc/Kconfig  |   1 +
 drivers/iio/adc/ad4691.c | 470 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 435 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index d498f16c0816..93f090e9a562 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -144,6 +144,7 @@ config AD4691
 	depends on SPI
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select IIO_BUFFER_DMAENGINE
 	select REGMAP
 	help
 	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
index db776de32846..5e0fe993c17d 100644
--- a/drivers/iio/adc/ad4691.c
+++ b/drivers/iio/adc/ad4691.c
@@ -8,6 +8,7 @@
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/math.h>
@@ -19,10 +20,14 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/offload/consumer.h>
+#include <linux/spi/offload/provider.h>
 #include <linux/units.h>
 #include <linux/unaligned.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/buffer-dma.h>
+#include <linux/iio/buffer-dmaengine.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/trigger.h>
@@ -37,6 +42,7 @@
 #define AD4691_VREF_4P096_uV_MAX		4500000
 
 #define AD4691_CNV_DUTY_CYCLE_NS		380
+#define AD4691_CNV_HIGH_TIME_NS			430
 
 #define AD4691_SPI_CONFIG_A_REG			0x000
 #define AD4691_SW_RESET				(BIT(7) | BIT(0))
@@ -89,6 +95,12 @@
 #define AD4691_ACC_IN(n)			(0x252 + (3 * (n)))
 #define AD4691_ACC_STS_DATA(n)			(0x283 + (4 * (n)))
 
+/* SPI offload 32-bit word field masks (transmitted MSB first) */
+#define AD4691_OFFLOAD_BITS_PER_WORD		32
+#define AD4691_MSG_ADDR_HI			GENMASK(31, 24)
+#define AD4691_MSG_ADDR_LO			GENMASK(23, 16)
+#define AD4691_MSG_DATA				GENMASK(15, 8)
+
 enum ad4691_ref_ctrl {
 	AD4691_VREF_2P5   = 0,
 	AD4691_VREF_3P0   = 1,
@@ -99,12 +111,22 @@ enum ad4691_ref_ctrl {
 
 struct ad4691_chip_info {
 	const struct iio_chan_spec *channels;
+	const struct iio_chan_spec *manual_channels;
 	const char *name;
 	unsigned int num_channels;
 	unsigned int max_rate;
 };
 
-#define AD4691_CHANNEL(ch)						\
+/*
+ * 16-bit ADC data is stored in 32-bit slots to match the SPI offload DMA
+ * word size (32 bits per transfer). The shift reflects the data position
+ * within the 32-bit word:
+ *   CNV_BURST: RX = [dummy, dummy, data_hi, data_lo] -> shift = 0
+ *   MANUAL:    RX = [data_hi, data_lo, dummy, dummy] -> shift = 16
+ * The triggered-buffer paths store data in the same position for consistency.
+ * Do not "fix" storagebits to 16.
+ */
+#define AD4691_CHANNEL(ch, _shift)					\
 	{								\
 		.type = IIO_VOLTAGE,					\
 		.indexed = 1,						\
@@ -118,40 +140,72 @@ struct ad4691_chip_info {
 		.scan_type = {						\
 			.sign = 'u',					\
 			.realbits = 16,					\
-			.storagebits = 16,				\
-			.shift = 0,					\
+			.storagebits = 32,				\
+			.shift = _shift,				\
 		},							\
 	}
 
 static const struct iio_chan_spec ad4691_channels[] = {
-	AD4691_CHANNEL(0),
-	AD4691_CHANNEL(1),
-	AD4691_CHANNEL(2),
-	AD4691_CHANNEL(3),
-	AD4691_CHANNEL(4),
-	AD4691_CHANNEL(5),
-	AD4691_CHANNEL(6),
-	AD4691_CHANNEL(7),
-	AD4691_CHANNEL(8),
-	AD4691_CHANNEL(9),
-	AD4691_CHANNEL(10),
-	AD4691_CHANNEL(11),
-	AD4691_CHANNEL(12),
-	AD4691_CHANNEL(13),
-	AD4691_CHANNEL(14),
-	AD4691_CHANNEL(15),
+	AD4691_CHANNEL(0, 0),
+	AD4691_CHANNEL(1, 0),
+	AD4691_CHANNEL(2, 0),
+	AD4691_CHANNEL(3, 0),
+	AD4691_CHANNEL(4, 0),
+	AD4691_CHANNEL(5, 0),
+	AD4691_CHANNEL(6, 0),
+	AD4691_CHANNEL(7, 0),
+	AD4691_CHANNEL(8, 0),
+	AD4691_CHANNEL(9, 0),
+	AD4691_CHANNEL(10, 0),
+	AD4691_CHANNEL(11, 0),
+	AD4691_CHANNEL(12, 0),
+	AD4691_CHANNEL(13, 0),
+	AD4691_CHANNEL(14, 0),
+	AD4691_CHANNEL(15, 0),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
+};
+
+static const struct iio_chan_spec ad4691_manual_channels[] = {
+	AD4691_CHANNEL(0, 16),
+	AD4691_CHANNEL(1, 16),
+	AD4691_CHANNEL(2, 16),
+	AD4691_CHANNEL(3, 16),
+	AD4691_CHANNEL(4, 16),
+	AD4691_CHANNEL(5, 16),
+	AD4691_CHANNEL(6, 16),
+	AD4691_CHANNEL(7, 16),
+	AD4691_CHANNEL(8, 16),
+	AD4691_CHANNEL(9, 16),
+	AD4691_CHANNEL(10, 16),
+	AD4691_CHANNEL(11, 16),
+	AD4691_CHANNEL(12, 16),
+	AD4691_CHANNEL(13, 16),
+	AD4691_CHANNEL(14, 16),
+	AD4691_CHANNEL(15, 16),
 	IIO_CHAN_SOFT_TIMESTAMP(16),
 };
 
 static const struct iio_chan_spec ad4693_channels[] = {
-	AD4691_CHANNEL(0),
-	AD4691_CHANNEL(1),
-	AD4691_CHANNEL(2),
-	AD4691_CHANNEL(3),
-	AD4691_CHANNEL(4),
-	AD4691_CHANNEL(5),
-	AD4691_CHANNEL(6),
-	AD4691_CHANNEL(7),
+	AD4691_CHANNEL(0, 0),
+	AD4691_CHANNEL(1, 0),
+	AD4691_CHANNEL(2, 0),
+	AD4691_CHANNEL(3, 0),
+	AD4691_CHANNEL(4, 0),
+	AD4691_CHANNEL(5, 0),
+	AD4691_CHANNEL(6, 0),
+	AD4691_CHANNEL(7, 0),
+	IIO_CHAN_SOFT_TIMESTAMP(16),
+};
+
+static const struct iio_chan_spec ad4693_manual_channels[] = {
+	AD4691_CHANNEL(0, 16),
+	AD4691_CHANNEL(1, 16),
+	AD4691_CHANNEL(2, 16),
+	AD4691_CHANNEL(3, 16),
+	AD4691_CHANNEL(4, 16),
+	AD4691_CHANNEL(5, 16),
+	AD4691_CHANNEL(6, 16),
+	AD4691_CHANNEL(7, 16),
 	IIO_CHAN_SOFT_TIMESTAMP(16),
 };
 
@@ -181,6 +235,7 @@ static const unsigned int ad4691_osc_freqs[] = {
 
 static const struct ad4691_chip_info ad4691_chip_info = {
 	.channels = ad4691_channels,
+	.manual_channels = ad4691_manual_channels,
 	.name = "ad4691",
 	.num_channels = ARRAY_SIZE(ad4691_channels),
 	.max_rate = 500 * HZ_PER_KHZ,
@@ -188,6 +243,7 @@ static const struct ad4691_chip_info ad4691_chip_info = {
 
 static const struct ad4691_chip_info ad4692_chip_info = {
 	.channels = ad4691_channels,
+	.manual_channels = ad4691_manual_channels,
 	.name = "ad4692",
 	.num_channels = ARRAY_SIZE(ad4691_channels),
 	.max_rate = 1 * HZ_PER_MHZ,
@@ -195,6 +251,7 @@ static const struct ad4691_chip_info ad4692_chip_info = {
 
 static const struct ad4691_chip_info ad4693_chip_info = {
 	.channels = ad4693_channels,
+	.manual_channels = ad4693_manual_channels,
 	.name = "ad4693",
 	.num_channels = ARRAY_SIZE(ad4693_channels),
 	.max_rate = 500 * HZ_PER_KHZ,
@@ -202,6 +259,7 @@ static const struct ad4691_chip_info ad4693_chip_info = {
 
 static const struct ad4691_chip_info ad4694_chip_info = {
 	.channels = ad4693_channels,
+	.manual_channels = ad4693_manual_channels,
 	.name = "ad4694",
 	.num_channels = ARRAY_SIZE(ad4693_channels),
 	.max_rate = 1 * HZ_PER_MHZ,
@@ -227,9 +285,9 @@ struct ad4691_state {
 	 */
 	struct mutex			lock;
 	/*
-	 * Per-buffer-enabl ree lifetimesources:
-	 * Manual Mode - a pre-built SPI message that clocks out N+1
-	 *		 transfers in one go.
+	 * Per-buffer-enable lifetime resources (triggered-buffer paths):
+	 * Manual Mode    - a pre-built SPI message that clocks out N+1
+	 *		    transfers in one go.
 	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
 	 *		    transfers in one go.
 	 */
@@ -238,9 +296,19 @@ struct ad4691_state {
 	struct spi_transfer		*scan_xfers;
 	__be16				*scan_tx;
 	__be16				*scan_rx;
-	/* Scan buffer: one slot per channel (u16) plus timestamp */
+	/* SPI offload DMA path resources */
+	struct spi_offload		*offload;
+	/* SPI offload trigger - periodic (MANUAL) or DATA_READY (CNV_BURST) */
+	struct spi_offload_trigger	*offload_trigger;
+	u64				offload_trigger_hz;
+	struct spi_message		offload_msg;
+	/* Max 16 channel xfers + 1 state-reset or NOOP */
+	struct spi_transfer		offload_xfer[17];
+	u32				offload_tx_cmd[17];
+	u32				offload_tx_reset;
+	/* Scan buffer: one slot per channel (u32) plus timestamp */
 	struct {
-		u16 vals[16];
+		u32 vals[16];
 		s64 ts __aligned(8);
 	} scan __aligned(IIO_DMA_MINALIGN);
 };
@@ -260,6 +328,46 @@ static int ad4691_gpio_setup(struct ad4691_state *st, unsigned int gp_num)
 				  AD4691_GP_MODE_DATA_READY << shift);
 }
 
+static const struct spi_offload_config ad4691_offload_config = {
+	.capability_flags = SPI_OFFLOAD_CAP_TRIGGER |
+			    SPI_OFFLOAD_CAP_RX_STREAM_DMA,
+};
+
+static bool ad4691_offload_trigger_match(struct spi_offload_trigger *trigger,
+					 enum spi_offload_trigger_type type,
+					 u64 *args, u32 nargs)
+{
+	return type == SPI_OFFLOAD_TRIGGER_DATA_READY &&
+	       nargs == 1 && args[0] <= 3;
+}
+
+static int ad4691_offload_trigger_request(struct spi_offload_trigger *trigger,
+					  enum spi_offload_trigger_type type,
+					  u64 *args, u32 nargs)
+{
+	struct ad4691_state *st = spi_offload_trigger_get_priv(trigger);
+
+	if (nargs != 1)
+		return -EINVAL;
+
+	return ad4691_gpio_setup(st, (unsigned int)args[0]);
+}
+
+static int ad4691_offload_trigger_validate(struct spi_offload_trigger *trigger,
+					   struct spi_offload_trigger_config *config)
+{
+	if (config->type != SPI_OFFLOAD_TRIGGER_DATA_READY)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct spi_offload_trigger_ops ad4691_offload_trigger_ops = {
+	.match    = ad4691_offload_trigger_match,
+	.request  = ad4691_offload_trigger_request,
+	.validate = ad4691_offload_trigger_validate,
+};
+
 static void ad4691_disable_pwm(void *data)
 {
 	struct pwm_state state = { .enabled = false };
@@ -817,6 +925,206 @@ static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {
 	.postdisable = &ad4691_cnv_burst_buffer_postdisable,
 };
 
+static int ad4691_manual_offload_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	struct spi_device *spi = to_spi_device(dev);
+	struct spi_offload_trigger_config config = {
+		.type = SPI_OFFLOAD_TRIGGER_PERIODIC,
+	};
+	unsigned int bit, k;
+	int ret;
+
+	ret = ad4691_enter_conversion_mode(st);
+	if (ret)
+		return ret;
+
+	memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
+
+	/*
+	 * N+1 transfers for N channels. Each CS-low period triggers
+	 * a conversion AND returns the previous result (pipelined).
+	 *   TX: [AD4691_ADC_CHAN(n), 0x00, 0x00, 0x00]
+	 *   RX: [data_hi, data_lo, 0x00, 0x00]   (shift=16)
+	 * Transfer 0 RX is garbage; transfers 1..N carry real data.
+	 */
+	k = 0;
+	iio_for_each_active_channel(indio_dev, bit) {
+		st->offload_tx_cmd[k] =
+			cpu_to_be32(FIELD_PREP(AD4691_MSG_ADDR_HI,
+					       AD4691_ADC_CHAN(bit)));
+		st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
+		st->offload_xfer[k].len = sizeof(u32);
+		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
+		st->offload_xfer[k].cs_change = 1;
+		st->offload_xfer[k].cs_change_delay.value = AD4691_CNV_HIGH_TIME_NS;
+		st->offload_xfer[k].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+		/* First transfer RX is garbage — skip it. */
+		if (k > 0)
+			st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+		k++;
+	}
+
+	/* Final NOOP to flush pipeline and capture last channel. */
+	st->offload_tx_cmd[k] =
+		cpu_to_be32(FIELD_PREP(AD4691_MSG_ADDR_HI, AD4691_NOOP));
+	st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
+	st->offload_xfer[k].len = sizeof(u32);
+	st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
+	st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+	k++;
+
+	spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
+	st->offload_msg.offload = st->offload;
+
+	ret = spi_optimize_message(spi, &st->offload_msg);
+	if (ret)
+		goto err_exit_conversion;
+
+	config.periodic.frequency_hz = st->offload_trigger_hz;
+	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
+	if (ret)
+		goto err_unoptimize;
+
+	return 0;
+
+err_unoptimize:
+	spi_unoptimize_message(&st->offload_msg);
+err_exit_conversion:
+	ad4691_exit_conversion_mode(st);
+	return ret;
+}
+
+static int ad4691_manual_offload_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+
+	spi_offload_trigger_disable(st->offload, st->offload_trigger);
+	spi_unoptimize_message(&st->offload_msg);
+
+	return ad4691_exit_conversion_mode(st);
+}
+
+static const struct iio_buffer_setup_ops ad4691_manual_offload_buffer_setup_ops = {
+	.postenable = &ad4691_manual_offload_buffer_postenable,
+	.predisable = &ad4691_manual_offload_buffer_predisable,
+};
+
+static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	struct spi_device *spi = to_spi_device(dev);
+	struct spi_offload_trigger_config config = {
+		.type = SPI_OFFLOAD_TRIGGER_DATA_READY,
+	};
+	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
+	unsigned int bit, k;
+	int ret;
+
+	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
+			   (u16)~(*indio_dev->active_scan_mask));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+			   *indio_dev->active_scan_mask);
+	if (ret)
+		return ret;
+
+	iio_for_each_active_channel(indio_dev, bit) {
+		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
+				   AD4691_ACC_COUNT_VAL);
+		if (ret)
+			return ret;
+	}
+
+	ret = ad4691_enter_conversion_mode(st);
+	if (ret)
+		return ret;
+
+	memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
+
+	/*
+	 * N transfers to read N AVG_IN registers plus one state-reset
+	 * transfer (no RX) to re-arm DATA_READY.
+	 *   TX: [reg_hi | 0x80, reg_lo, 0x00, 0x00]
+	 *   RX: [0x00, 0x00, data_hi, data_lo]   (shift=0)
+	 */
+	k = 0;
+	iio_for_each_active_channel(indio_dev, bit) {
+		unsigned int reg = AD4691_AVG_IN(bit);
+
+		st->offload_tx_cmd[k] =
+			cpu_to_be32(((reg >> 8 | 0x80) << 24) |
+				    ((reg & 0xFF) << 16));
+		st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
+		st->offload_xfer[k].len = sizeof(u32);
+		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
+		st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+		if (k < n_active - 1)
+			st->offload_xfer[k].cs_change = 1;
+		k++;
+	}
+
+	/* State reset to re-arm DATA_READY for the next scan. */
+	st->offload_tx_reset =
+		cpu_to_be32(((AD4691_STATE_RESET_REG >> 8) << 24) |
+			    ((AD4691_STATE_RESET_REG & 0xFF) << 16) |
+			    (AD4691_STATE_RESET_ALL << 8));
+	st->offload_xfer[k].tx_buf = &st->offload_tx_reset;
+	st->offload_xfer[k].len = sizeof(u32);
+	st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
+	k++;
+
+	spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
+	st->offload_msg.offload = st->offload;
+
+	ret = spi_optimize_message(spi, &st->offload_msg);
+	if (ret)
+		goto err_exit_conversion;
+
+	ret = ad4691_sampling_enable(st, true);
+	if (ret)
+		goto err_unoptimize;
+
+	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
+	if (ret)
+		goto err_sampling_disable;
+
+	return 0;
+
+err_sampling_disable:
+	ad4691_sampling_enable(st, false);
+err_unoptimize:
+	spi_unoptimize_message(&st->offload_msg);
+err_exit_conversion:
+	ad4691_exit_conversion_mode(st);
+	return ret;
+}
+
+static int ad4691_cnv_burst_offload_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ad4691_state *st = iio_priv(indio_dev);
+	int ret;
+
+	spi_offload_trigger_disable(st->offload, st->offload_trigger);
+
+	ret = ad4691_sampling_enable(st, false);
+	if (ret)
+		return ret;
+
+	spi_unoptimize_message(&st->offload_msg);
+
+	return ad4691_exit_conversion_mode(st);
+}
+
+static const struct iio_buffer_setup_ops ad4691_cnv_burst_offload_buffer_setup_ops = {
+	.postenable = &ad4691_cnv_burst_offload_buffer_postenable,
+	.predisable = &ad4691_cnv_burst_offload_buffer_predisable,
+};
+
 static ssize_t sampling_frequency_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -824,6 +1132,9 @@ static ssize_t sampling_frequency_show(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad4691_state *st = iio_priv(indio_dev);
 
+	if (st->manual_mode && st->offload)
+		return sysfs_emit(buf, "%llu\n", st->offload_trigger_hz);
+
 	if (st->manual_mode)
 		return -ENODEV;
 
@@ -838,7 +1149,7 @@ static ssize_t sampling_frequency_store(struct device *dev,
 	struct ad4691_state *st = iio_priv(indio_dev);
 	int freq, ret;
 
-	if (st->manual_mode)
+	if (st->manual_mode && !st->offload)
 		return -ENODEV;
 
 	ret = kstrtoint(buf, 10, &freq);
@@ -847,6 +1158,20 @@ static ssize_t sampling_frequency_store(struct device *dev,
 
 	guard(mutex)(&st->lock);
 
+	if (st->manual_mode) {
+		struct spi_offload_trigger_config config = {
+			.type = SPI_OFFLOAD_TRIGGER_PERIODIC,
+			.periodic = { .frequency_hz = freq },
+		};
+
+		ret = spi_offload_trigger_validate(st->offload_trigger, &config);
+		if (ret)
+			return ret;
+
+		st->offload_trigger_hz = config.periodic.frequency_hz;
+		return len;
+	}
+
 	ret = ad4691_set_pwm_freq(st, freq);
 	if (ret)
 		return ret;
@@ -900,7 +1225,7 @@ static irqreturn_t ad4691_trigger_handler(int irq, void *p)
 
 	if (st->manual_mode) {
 		iio_for_each_active_channel(indio_dev, i) {
-			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k + 1]);
+			st->scan.vals[i] = (u32)be16_to_cpu(st->scan_rx[k + 1]) << 16;
 			k++;
 		}
 	} else {
@@ -1088,6 +1413,15 @@ static int ad4691_config(struct ad4691_state *st, u32 max_speed_hz)
 	if (st->manual_mode)
 		return 0;
 
+	/*
+	 * In the offload CNV Burst path the GP pin is supplied by the trigger
+	 * consumer via #trigger-source-cells; gpio_setup is called from
+	 * ad4691_offload_trigger_request() instead. For the non-offload path
+	 * derive the pin from the first interrupt-names entry (e.g. "gp0").
+	 */
+	if (device_property_present(dev, "#trigger-source-cells"))
+		return 0;
+
 	ret = device_property_read_string_array(dev, "interrupt-names",
 						&irq_name, 1);
 	if (ret < 0)
@@ -1158,6 +1492,56 @@ static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
 					       &ad4691_manual_buffer_setup_ops);
 }
 
+static int ad4691_setup_offload(struct iio_dev *indio_dev,
+				struct ad4691_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	struct dma_chan *rx_dma;
+	int ret;
+
+	if (st->manual_mode) {
+		st->offload_trigger =
+			devm_spi_offload_trigger_get(dev, st->offload,
+						     SPI_OFFLOAD_TRIGGER_PERIODIC);
+		if (IS_ERR(st->offload_trigger))
+			return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
+					     "Failed to get periodic offload trigger\n");
+
+		st->offload_trigger_hz = st->info->max_rate;
+	} else {
+		struct spi_offload_trigger_info trigger_info = {
+			.fwnode = dev_fwnode(dev),
+			.ops    = &ad4691_offload_trigger_ops,
+			.priv   = st,
+		};
+
+		ret = devm_spi_offload_trigger_register(dev, &trigger_info);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to register offload trigger\n");
+
+		st->offload_trigger =
+			devm_spi_offload_trigger_get(dev, st->offload,
+						     SPI_OFFLOAD_TRIGGER_DATA_READY);
+		if (IS_ERR(st->offload_trigger))
+			return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
+					     "Failed to get DATA_READY offload trigger\n");
+	}
+
+	rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, st->offload);
+	if (IS_ERR(rx_dma))
+		return dev_err_probe(dev, PTR_ERR(rx_dma),
+				     "Failed to get offload RX DMA channel\n");
+
+	if (st->manual_mode)
+		indio_dev->setup_ops = &ad4691_manual_offload_buffer_setup_ops;
+	else
+		indio_dev->setup_ops = &ad4691_cnv_burst_offload_buffer_setup_ops;
+
+	return devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev, rx_dma,
+							   IIO_BUFFER_DIRECTION_IN);
+}
+
 static int ad4691_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -1193,14 +1577,27 @@ static int ad4691_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	st->offload = devm_spi_offload_get(dev, spi, &ad4691_offload_config);
+	ret = PTR_ERR_OR_ZERO(st->offload);
+	if (ret == -ENODEV)
+		st->offload = NULL;
+	else if (ret)
+		return dev_err_probe(dev, ret, "Failed to get SPI offload\n");
+
 	indio_dev->name = st->info->name;
 	indio_dev->info = &ad4691_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	indio_dev->channels = st->info->channels;
+	if (st->manual_mode)
+		indio_dev->channels = st->info->manual_channels;
+	else
+		indio_dev->channels = st->info->channels;
 	indio_dev->num_channels = st->info->num_channels;
 
-	ret = ad4691_setup_triggered_buffer(indio_dev, st);
+	if (st->offload)
+		ret = ad4691_setup_offload(indio_dev, st);
+	else
+		ret = ad4691_setup_triggered_buffer(indio_dev, st);
 	if (ret)
 		return ret;
 
@@ -1238,3 +1635,4 @@ module_spi_driver(ad4691_driver);
 MODULE_AUTHOR("Radu Sabau <radu.sabau@analog.com>");
 MODULE_DESCRIPTION("Analog Devices AD4691 Family ADC Driver");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_DMA_BUFFER");

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
@ 2026-03-20 12:26   ` Rob Herring (Arm)
  2026-03-21 14:35   ` Jonathan Cameron
  2026-03-21 16:11   ` David Lechner
  2 siblings, 0 replies; 22+ messages in thread
From: Rob Herring (Arm) @ 2026-03-20 12:26 UTC (permalink / raw)
  To: Radu Sabau
  Cc: Andy Shevchenko, Lars-Peter Clausen, Uwe Kleine-König,
	linux-iio, linux-gpio, Mark Brown, devicetree, linux-pwm,
	David Lechner, Krzysztof Kozlowski, linux-kernel,
	Jonathan Cameron, Linus Walleij, Michael Hennerich,
	Bartosz Golaszewski, Philipp Zabel, Conor Dooley, Nuno Sá,
	Liam Girdwood


On Fri, 20 Mar 2026 13:03:55 +0200, Radu Sabau wrote:
> Add DT bindings for the Analog Devices AD4691 family of multichannel
> SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> 
> The binding describes the hardware connections:
> 
> - Power domains: avdd-supply (required), vio-supply, ref-supply or
>   refin-supply (external reference; the REFIN path enables the
>   internal reference buffer), and an optional ldo-in-supply, that if
>   absent, means the on-chip internal LDO will be used.
> 
> - Optional PWM on the CNV pin selects CNV Burst Mode; when absent,
>   Manual Mode is assumed with CNV tied to SPI CS.
> 
> - An optional reset GPIO (reset-gpios) for hardware reset.
> 
> - Up to four GP pins (gp0..gp3) usable as interrupt sources,
>   identified in firmware via interrupt-names "gp0".."gp3".
> 
> - gpio-controller with #gpio-cells = <2> for GP pin GPIO usage.
> 
> - #trigger-source-cells = <1>: one cell selecting the GP pin number
>   (0-3) used as the SPI offload trigger source.
> 
> Two binding examples are provided: CNV Burst Mode with SPI offload
> (DMA data acquisition driven by DATA_READY on a GP pin), and Manual
> Mode for CPU-driven triggered-buffer or single-shot capture.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad4691.yaml    | 173 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  2 files changed, 180 insertions(+)
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml: properties:interrupt-names: {'description': 'Names of the interrupt lines, matching the GP pin names.', 'minItems': 1, 'maxItems': 4, 'items': [{'const': 'gp0'}, {'const': 'gp1'}, {'const': 'gp2'}, {'const': 'gp3'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260320-ad4692-multichannel-sar-adc-driver-v4-1-052c1050507a@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.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/4] iio: adc: ad4691: add initial driver for AD4691 family
  2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
@ 2026-03-20 15:16   ` Andy Shevchenko
  2026-03-21 14:48   ` Jonathan Cameron
  2026-03-23 11:44   ` Nuno Sá
  2 siblings, 0 replies; 22+ messages in thread
From: Andy Shevchenko @ 2026-03-20 15:16 UTC (permalink / raw)
  To: radu.sabau
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, linux-iio, devicetree, linux-kernel, linux-pwm,
	linux-gpio

On Fri, Mar 20, 2026 at 01:03:56PM +0200, Radu Sabau via B4 Relay wrote:

> Add support for the Analog Devices AD4691 family of high-speed,
> low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> AD4694 (8-ch, 1 MSPS).
> 
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
> 
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
> 
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed via
> the reset controller framework; a software reset through SPI_CONFIG_A
> is used as fallback when no hardware reset is available.
> 
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.

...

> +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct spi_device *spi = context;
> +	u8 tx[2], rx[4];
> +	int ret;

> +	put_unaligned_be16(0x8000 | reg, tx);

I would expect that the config will have read_flag_mask set and here you just
use it. But it's fine like now, just add a comment

	/* Set bit 15 to mark the operation READ */

or something similar.

> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 1);
> +		if (ret)
> +			return ret;
> +		*val = rx[0];
> +		return 0;
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 2);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be16(rx);
> +		return 0;
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 3);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be24(rx);
> +		return 0;
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 4);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be32(rx);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static int ad4691_regulator_setup(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(dev, "avdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable AVDD\n");

> +	ret = devm_regulator_get_enable(dev, "ldo-in");
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "Failed to get and enable LDO-IN\n");
> +	st->ldo_en = (ret == -ENODEV);

You can use the approach from below

	ret = devm_regulator_get_enable(dev, "ldo-in");
	if (ret == -ENODEV)
		st->ldo_en = true;
	else if (ret)
		return dev_err_probe(dev, ret, "Failed to get and enable LDO-IN\n");
	// no other branches assuming ldo_en = false due to kzalloc():ed memory.


> +	ret = devm_regulator_get_enable(dev, "vio");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable VIO\n");
> +
> +	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");

> +	if (st->vref_uV >= 0) {
> +		st->refbuf_en = false;

Do you need this? Isn't 'st' allocated with kzalloc() or alike?

> +	} else if (st->vref_uV == -ENODEV) {
> +		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "refin");
> +		st->refbuf_en = true;

> +	}
> +	if (st->vref_uV < 0)
> +		return dev_err_probe(dev, st->vref_uV,
> +				     "Failed to get reference supply\n");

> +	if (st->vref_uV < AD4691_VREF_uV_MIN || st->vref_uV > AD4691_VREF_uV_MAX)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "vref(%d) must be in the range [%u...%u]\n",
> +				     st->vref_uV, AD4691_VREF_uV_MIN,
> +				     AD4691_VREF_uV_MAX);
> +
> +	return 0;
> +}

...

> +	if (!rst)

It's not an error check, so I would invert the condition.

> +		/* No hardware reset available, fall back to software reset. */
> +		return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
> +				    AD4691_SW_RESET);
> +
> +	reset_control_assert(rst);
> +	/* Reset delay required. See datasheet Table 5. */
> +	fsleep(300);
> +	reset_control_deassert(rst);

	if (rst) {
		reset_control_assert(rst);
		/* Reset delay required. See datasheet Table 5. */
		fsleep(300);
		reset_control_deassert(rst);

		return 0;
	}

	/* No hardware reset available, fall back to software reset. */
	return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG, AD4691_SW_RESET);


...

> +	ret = regmap_write(st->regmap, AD4691_REF_CTRL,
> +			   FIELD_PREP(AD4691_REF_CTRL_MASK, ref_val) |
> +			   (st->refbuf_en ? AD4691_REFBUF_EN : 0));

regmap_update_bits()?
regmap_assign_bits()?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write REF_CTRL\n");
> +
> +	ret = regmap_write(st->regmap, AD4691_DEVICE_SETUP,
> +			   st->ldo_en ? AD4691_LDO_EN : 0);

Ditto.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
> +
> +	/*
> +	 * Set the internal oscillator to the highest valid rate for this chip.
> +	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
> +	 * at index 1 (500 kHz).
> +	 */
> +	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
> +			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);

Ditto.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
@ 2026-03-20 16:14   ` Uwe Kleine-König
  2026-03-21 15:16   ` Jonathan Cameron
  2026-03-24 12:22   ` Nuno Sá
  2 siblings, 0 replies; 22+ messages in thread
From: Uwe Kleine-König @ 2026-03-20 16:14 UTC (permalink / raw)
  To: radu.sabau
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
	Linus Walleij, Bartosz Golaszewski, Philipp Zabel, linux-iio,
	devicetree, linux-kernel, linux-pwm, linux-gpio

[-- Attachment #1: Type: text/plain, Size: 12093 bytes --]

On Fri, Mar 20, 2026 at 01:03:57PM +0200, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add buffered capture support using the IIO triggered buffer framework.
> 
> CNV Burst Mode: the GP pin identified by interrupt-names in the device
> tree is configured as DATA_READY output. The IRQ handler stops
> conversions and fires the IIO trigger; the trigger handler executes a
> pre-built SPI message that reads all active channels from the AVG_IN
> accumulator registers and then resets accumulator state and restarts
> conversions for the next cycle.
> 
> Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> reads the previous result and starts the next conversion (pipelined
> N+1 scheme). At preenable time a pre-built, optimised SPI message of
> N+1 transfers is constructed (N channel reads plus one NOOP to drain
> the pipeline). The trigger handler executes the message in a single
> spi_sync() call and collects the results. An external trigger (e.g.
> iio-trig-hrtimer) is required to drive the trigger at the desired
> sample rate.
> 
> Both modes share the same trigger handler and push a complete scan —
> one u16 slot per channel at its scan_index position, followed by a
> timestamp — to the IIO buffer via iio_push_to_buffers_with_ts().
> 
> The CNV Burst Mode sampling frequency (PWM period) is exposed as a
> buffer-level attribute via IIO_DEVICE_ATTR.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  drivers/iio/adc/Kconfig  |   2 +
>  drivers/iio/adc/ad4691.c | 584 +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 571 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 3685a03aa8dc..d498f16c0816 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -142,6 +142,8 @@ config AD4170_4
>  config AD4691
>  	tristate "Analog Devices AD4691 Family ADC Driver"
>  	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	select REGMAP
>  	help
>  	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 5e02eb44ca44..db776de32846 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -9,9 +9,12 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> +#include <linux/interrupt.h>
>  #include <linux/math.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> @@ -19,7 +22,12 @@
>  #include <linux/units.h>
>  #include <linux/unaligned.h>
>  
> +#include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>  
>  #define AD4691_VREF_uV_MIN			2400000
>  #define AD4691_VREF_uV_MAX			5250000
> @@ -28,6 +36,8 @@
>  #define AD4691_VREF_3P3_uV_MAX			3750000
>  #define AD4691_VREF_4P096_uV_MAX		4500000
>  
> +#define AD4691_CNV_DUTY_CYCLE_NS		380
> +
>  #define AD4691_SPI_CONFIG_A_REG			0x000
>  #define AD4691_SW_RESET				(BIT(7) | BIT(0))
>  
> @@ -35,6 +45,7 @@
>  #define AD4691_CLAMP_STATUS1_REG		0x01A
>  #define AD4691_CLAMP_STATUS2_REG		0x01B
>  #define AD4691_DEVICE_SETUP			0x020
> +#define AD4691_MANUAL_MODE			BIT(2)
>  #define AD4691_LDO_EN				BIT(4)
>  #define AD4691_REF_CTRL				0x021
>  #define AD4691_REF_CTRL_MASK			GENMASK(4, 2)
> @@ -42,21 +53,29 @@
>  #define AD4691_OSC_FREQ_REG			0x023
>  #define AD4691_OSC_FREQ_MASK			GENMASK(3, 0)
>  #define AD4691_STD_SEQ_CONFIG			0x025
> +#define AD4691_SEQ_ALL_CHANNELS_OFF		0x00
>  #define AD4691_SPARE_CONTROL			0x02A
>  
> +#define AD4691_NOOP				0x00
> +#define AD4691_ADC_CHAN(ch)			((0x10 + (ch)) << 3)
> +
>  #define AD4691_OSC_EN_REG			0x180
>  #define AD4691_STATE_RESET_REG			0x181
>  #define AD4691_STATE_RESET_ALL			0x01
>  #define AD4691_ADC_SETUP			0x182
> -#define AD4691_AUTONOMOUS_MODE_VAL		0x02
> +#define AD4691_CNV_BURST_MODE			0x01
> +#define AD4691_AUTONOMOUS_MODE			0x02
>  /*
>   * ACC_MASK_REG covers both mask bytes via ADDR_DESCENDING SPI: writing a
>   * 16-bit BE value to 0x185 auto-decrements to 0x184 for the second byte.
>   */
>  #define AD4691_ACC_MASK_REG			0x185
>  #define AD4691_ACC_COUNT_LIMIT(n)		(0x186 + (n))
> +#define AD4691_ACC_COUNT_VAL			0x1
>  #define AD4691_GPIO_MODE1_REG			0x196
>  #define AD4691_GPIO_MODE2_REG			0x197
> +#define AD4691_GP_MODE_MASK			GENMASK(3, 0)
> +#define AD4691_GP_MODE_DATA_READY		0x06
>  #define AD4691_GPIO_READ			0x1A0
>  #define AD4691_ACC_STATUS_FULL1_REG		0x1B0
>  #define AD4691_ACC_STATUS_FULL2_REG		0x1B1
> @@ -121,6 +140,7 @@ static const struct iio_chan_spec ad4691_channels[] = {
>  	AD4691_CHANNEL(13),
>  	AD4691_CHANNEL(14),
>  	AD4691_CHANNEL(15),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),
>  };
>  
>  static const struct iio_chan_spec ad4693_channels[] = {
> @@ -132,6 +152,7 @@ static const struct iio_chan_spec ad4693_channels[] = {
>  	AD4691_CHANNEL(5),
>  	AD4691_CHANNEL(6),
>  	AD4691_CHANNEL(7),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),
>  };
>  
>  /*
> @@ -189,16 +210,63 @@ static const struct ad4691_chip_info ad4694_chip_info = {
>  struct ad4691_state {
>  	const struct ad4691_chip_info	*info;
>  	struct regmap			*regmap;
> +
> +	struct pwm_device		*conv_trigger;
> +	struct iio_trigger		*trig;
> +	int				irq;
> +
> +	bool				manual_mode;
> +
>  	int				vref_uV;
>  	bool				refbuf_en;
>  	bool				ldo_en;
> +	u32				cnv_period_ns;
>  	/*
>  	 * Synchronize access to members of the driver state, and ensure
>  	 * atomicity of consecutive SPI operations.
>  	 */
>  	struct mutex			lock;
> +	/*
> +	 * Per-buffer-enabl ree lifetimesources:
> +	 * Manual Mode - a pre-built SPI message that clocks out N+1
> +	 *		 transfers in one go.
> +	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
> +	 *		    transfers in one go.
> +	 */
> +	void				*scan_devm_group;
> +	struct spi_message		scan_msg;
> +	struct spi_transfer		*scan_xfers;
> +	__be16				*scan_tx;
> +	__be16				*scan_rx;
> +	/* Scan buffer: one slot per channel (u16) plus timestamp */
> +	struct {
> +		u16 vals[16];
> +		s64 ts __aligned(8);
> +	} scan __aligned(IIO_DMA_MINALIGN);
>  };
>  
> +/*
> + * Configure the given GP pin (0-3) as DATA_READY output.
> + * GP0/GP1 → GPIO_MODE1_REG, GP2/GP3 → GPIO_MODE2_REG.
> + * Even pins occupy bits [3:0], odd pins bits [7:4].
> + */
> +static int ad4691_gpio_setup(struct ad4691_state *st, unsigned int gp_num)
> +{
> +	unsigned int shift = 4 * (gp_num % 2);
> +
> +	return regmap_update_bits(st->regmap,
> +				  AD4691_GPIO_MODE1_REG + gp_num / 2,
> +				  AD4691_GP_MODE_MASK << shift,
> +				  AD4691_GP_MODE_DATA_READY << shift);
> +}
> +
> +static void ad4691_disable_pwm(void *data)
> +{
> +	struct pwm_state state = { .enabled = false };
> +
> +	pwm_apply_might_sleep(data, &state);
> +}
> +
>  static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
>  {
>  	struct spi_device *spi = context;
> @@ -341,14 +409,16 @@ static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
>  static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
>  {
>  	struct ad4691_state *st = iio_priv(indio_dev);
> -	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
>  	unsigned int i;
>  
> +	if (freq > st->info->max_rate)
> +		return -EINVAL;
> +
>  	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
>  	if (IIO_DEV_ACQUIRE_FAILED(claim))
>  		return -EBUSY;
>  
> -	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
> +	for (i = 0; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
>  		if ((int)ad4691_osc_freqs[i] == freq)
>  			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
>  						  AD4691_OSC_FREQ_MASK, i);
> @@ -363,7 +433,10 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
>  			     int *length, long mask)
>  {
>  	struct ad4691_state *st = iio_priv(indio_dev);
> -	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
> +	unsigned int start;
> +
> +	/* Skip frequencies that exceed this chip's maximum rate. */
> +	start = (ad4691_osc_freqs[0] > st->info->max_rate) ? 1 : 0;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> @@ -386,8 +459,7 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
>  	guard(mutex)(&st->lock);
>  
>  	/*
> -	 * Use AUTONOMOUS mode for single-shot reads. The chip always
> -	 * operates in AUTONOMOUS mode in this driver revision.
> +	 * Use AUTONOMOUS mode for single-shot reads.
>  	 */
>  	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
>  			   AD4691_STATE_RESET_ALL);
> @@ -417,8 +489,7 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
>  	 * conversion to complete.
>  	 */
>  	fsleep(DIV_ROUND_UP(2 * USEC_PER_SEC,
> -			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK,
> -						       reg_val)]));
> +		ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)]));
>  
>  	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
>  	if (ret)
> @@ -488,6 +559,374 @@ static int ad4691_reg_access(struct iio_dev *indio_dev, unsigned int reg,
>  	return regmap_write(st->regmap, reg, writeval);
>  }
>  
> +static int ad4691_set_pwm_freq(struct ad4691_state *st, int freq)
> +{
> +	if (!freq)
> +		return -EINVAL;
> +
> +	st->cnv_period_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq);

I wrote something about divisions in an earlier revision already.
Ideally there was a iio-specific policy how to convert a frequency to a
period.

Until such a policy exists (maybe even with a helper function), let me
point out that if you want to pick the period that is nearest to 1/freq,
most of the time

	st->cnv_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, freq);

is the better value to use. And in the remaining cases this is still a
very good value.

The analysis goes as follows:

Let Π be the function mapping an integer period request to the actually
implemented period (a rational number in general).

Let P := DIV_ROUND_DOWN(NSEC_PER_SEC, freq) and ε := P - Π(P).
With Π(x) ≤ x, we have ε ≥ 0.

The analysis is moot if DIV_ROUND_CLOSEST() and DIV_ROUND_UP() yield the
same value, so we can assume

	(P + ẟ) * freq = NSEC_PER_SEC

with 0 < ẟ < 0.5. The values to consider are Π(P) and Π(P+1).

The former is the better value if:

	  abs(P + ẟ - Π(P)) < abs(P + ẟ - Π(P+1))
	⟺ ẟ + ε < abs(P + ẟ - Π(P+1))

With Π(x + 1) ≥ Π(x) this can only hold if

	  Π(P+1) > P + 2ẟ + ε
	⟹ 1 > 2ẟ + ε

So we'd need that ε = P - Π(P) < 1, that is a possible period quite near
to P and another possible period value in the interval ]P + 2ẟ + ε; P + 1].

In that case Π(P+1) is a worse value than Π(P), but still

	abs(P + ẟ - Π(P+1)) < 1

, so even in this unlikely situation using P+1 is quite good.

Another advantage of DIV_ROUND_UP(NSEC_PER_SEC, freq) over
DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq) is that it yields a sensible
period for freq > 2*NSEC_PER_SEC.

> +	return 0;
> +}
> +
> +static int ad4691_sampling_enable(struct ad4691_state *st, bool enable)
> +{
> +	struct pwm_state conv_state = { };
> +
> +	conv_state.period = st->cnv_period_ns;
> +	conv_state.duty_cycle = AD4691_CNV_DUTY_CYCLE_NS;
> +	conv_state.polarity = PWM_POLARITY_NORMAL;
> +	conv_state.enabled = enable;

this can be written as:

	static int ad4691_sampling_enable(struct ad4691_state *st, bool enable)
	{
		struct pwm_state conv_state = {
			.period = st->cnv_period_ns;
			.duty_cycle = AD4691_CNV_DUTY_CYCLE_NS;
			.polarity = PWM_POLARITY_NORMAL;
			.enabled = enable;
		};
		...

	}

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
@ 2026-03-20 17:07   ` Andy Shevchenko
  2026-03-21 15:28   ` Jonathan Cameron
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Andy Shevchenko @ 2026-03-20 17:07 UTC (permalink / raw)
  To: radu.sabau, Wolfram Sang
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, linux-iio, devicetree, linux-kernel, linux-pwm,
	linux-gpio

On Fri, Mar 20, 2026 at 01:03:58PM +0200, Radu Sabau via B4 Relay wrote:

> Add SPI offload support to enable DMA-based, CPU-independent data
> acquisition using the SPI Engine offload framework.
> 
> When an SPI offload is available (devm_spi_offload_get() succeeds),
> the driver registers a DMA engine IIO buffer and uses dedicated buffer
> setup operations. If no offload is available the existing software
> triggered buffer path is used unchanged.
> 
> Both CNV Burst Mode and Manual Mode support offload, but use different
> trigger mechanisms:
> 
> CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on the GP pin specified by the trigger-source consumer reference
> in the device tree (one cell = GP pin number 0-3). For this mode the
> driver acts as both an SPI offload consumer (DMA RX stream, message
> optimization) and a trigger source provider: it registers the
> GP/DATA_READY output via devm_spi_offload_trigger_register() so the
> offload framework can match the '#trigger-source-cells' phandle and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
> 
> Manual Mode: the SPI Engine is triggered by a periodic trigger at
> the configured sampling frequency. The pre-built SPI message uses
> the pipelined CNV-on-CS protocol: N+1 4-byte transfers are issued
> for N active channels (the first result is discarded as garbage from
> the pipeline flush) and the remaining N results are captured by DMA.
> 
> All offload transfers use 32-bit frames (bits_per_word=32, len=4) for
> DMA word alignment. This patch promotes the channel scan_type from
> storagebits=16 (triggered-buffer path) to storagebits=32 to match the
> DMA word size; the triggered-buffer paths are updated to the same layout
> for consistency. CNV Burst Mode channel data arrives in the lower 16
> bits of the 32-bit word (shift=0); Manual Mode data arrives in the upper
> 16 bits (shift=16), matching the 4-byte SPI transfer layout
> [data_hi, data_lo, 0, 0]. A separate ad4691_manual_channels[] array
> encodes the shift=16 scan type for manual mode.
> 
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.

...

> +	struct spi_offload		*offload;
> +	/* SPI offload trigger - periodic (MANUAL) or DATA_READY (CNV_BURST) */
> +	struct spi_offload_trigger	*offload_trigger;
> +	u64				offload_trigger_hz;
> +	struct spi_message		offload_msg;
> +	/* Max 16 channel xfers + 1 state-reset or NOOP */
> +	struct spi_transfer		offload_xfer[17];
> +	u32				offload_tx_cmd[17];
> +	u32				offload_tx_reset;

Ouch! Can you guarantee this kilobytes (isn't it?) of memory will be used in
majority of the cases? When I got comment on replacing a single u8 by unsigned
long in one well used data structure in the kernel I was laughing, but this
single driver may beat the recode of memory waste on the embedded platforms.
Perhaps having a separate structure and allocate it separately when we sure
the offload is supported?

Cc'ed to Wolfram.

...

> +	/* Scan buffer: one slot per channel (u32) plus timestamp */
>  	struct {
> -		u16 vals[16];
> +		u32 vals[16];
>  		s64 ts __aligned(8);

This might break the existing cases or make code ugly and not actually
compatible with u32 layout.

>  	} scan __aligned(IIO_DMA_MINALIGN);

...

> +static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct spi_offload_trigger_config config = {
> +		.type = SPI_OFFLOAD_TRIGGER_DATA_READY,
> +	};

> +	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);

Should be bitmap_weight() with properly given amount of bits.


> +	unsigned int bit, k;
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~(*indio_dev->active_scan_mask));

This is not how we work with bitmaps. Use bitmap_read().

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   *indio_dev->active_scan_mask);

Ditto.

> +	if (ret)
> +		return ret;
> +
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
> +				   AD4691_ACC_COUNT_VAL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret)
> +		return ret;
> +
> +	memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
> +
> +	/*
> +	 * N transfers to read N AVG_IN registers plus one state-reset
> +	 * transfer (no RX) to re-arm DATA_READY.
> +	 *   TX: [reg_hi | 0x80, reg_lo, 0x00, 0x00]
> +	 *   RX: [0x00, 0x00, data_hi, data_lo]   (shift=0)
> +	 */
> +	k = 0;
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		unsigned int reg = AD4691_AVG_IN(bit);
> +
> +		st->offload_tx_cmd[k] =

> +			cpu_to_be32(((reg >> 8 | 0x80) << 24) |
> +				    ((reg & 0xFF) << 16));

Isn't this is just a cpu_to_be16(0x8000 | reg) ?

> +		st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
> +		st->offload_xfer[k].len = sizeof(u32);
> +		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
> +		st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +		if (k < n_active - 1)
> +			st->offload_xfer[k].cs_change = 1;
> +		k++;
> +	}
> +
> +	/* State reset to re-arm DATA_READY for the next scan. */
> +	st->offload_tx_reset =
> +		cpu_to_be32(((AD4691_STATE_RESET_REG >> 8) << 24) |
> +			    ((AD4691_STATE_RESET_REG & 0xFF) << 16) |
> +			    (AD4691_STATE_RESET_ALL << 8));

In similar way

		cpu_to_be32((AD4691_STATE_RESET_REG << 16) |
			    (AD4691_STATE_RESET_ALL << 8));

> +	st->offload_xfer[k].tx_buf = &st->offload_tx_reset;
> +	st->offload_xfer[k].len = sizeof(u32);
> +	st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
> +	k++;
> +
> +	spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
> +	st->offload_msg.offload = st->offload;
> +
> +	ret = spi_optimize_message(spi, &st->offload_msg);
> +	if (ret)
> +		goto err_exit_conversion;
> +
> +	ret = ad4691_sampling_enable(st, true);
> +	if (ret)
> +		goto err_unoptimize;
> +
> +	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
> +	if (ret)
> +		goto err_sampling_disable;
> +
> +	return 0;
> +
> +err_sampling_disable:
> +	ad4691_sampling_enable(st, false);
> +err_unoptimize:
> +	spi_unoptimize_message(&st->offload_msg);
> +err_exit_conversion:
> +	ad4691_exit_conversion_mode(st);
> +	return ret;
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
  2026-03-20 12:26   ` Rob Herring (Arm)
@ 2026-03-21 14:35   ` Jonathan Cameron
  2026-03-21 16:11   ` David Lechner
  2 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2026-03-21 14:35 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
	Linus Walleij, Bartosz Golaszewski, Philipp Zabel, linux-iio,
	devicetree, linux-kernel, linux-pwm, linux-gpio

On Fri, 20 Mar 2026 13:03:55 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add DT bindings for the Analog Devices AD4691 family of multichannel
> SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> 
> The binding describes the hardware connections:
> 
> - Power domains: avdd-supply (required), vio-supply,

Bit odd to call out avdd-supply as required but not vio-supply


> ref-supply or
>   refin-supply (external reference; the REFIN path enables the
>   internal reference buffer), and an optional ldo-in-supply, that if
>   absent, means the on-chip internal LDO will be used.
> 
> - Optional PWM on the CNV pin selects CNV Burst Mode; when absent,
>   Manual Mode is assumed with CNV tied to SPI CS.
> 
> - An optional reset GPIO (reset-gpios) for hardware reset.
> 
> - Up to four GP pins (gp0..gp3) usable as interrupt sources,
>   identified in firmware via interrupt-names "gp0".."gp3".
> 
> - gpio-controller with #gpio-cells = <2> for GP pin GPIO usage.
> 
> - #trigger-source-cells = <1>: one cell selecting the GP pin number
>   (0-3) used as the SPI offload trigger source.
> 
> Two binding examples are provided: CNV Burst Mode with SPI offload
> (DMA data acquisition driven by DATA_READY on a GP pin), and Manual
> Mode for CPU-driven triggered-buffer or single-shot capture.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad4691.yaml    | 173 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  2 files changed, 180 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> new file mode 100644
> index 000000000000..def9f32c78af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> @@ -0,0 +1,173 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4691.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4691 Family Multichannel SAR ADCs
> +
> +maintainers:
> +  - Radu Sabau <radu.sabau@analog.com>
> +
> +description: |
> +  The AD4691 family are high-speed, low-power, multichannel successive
> +  approximation register (SAR) analog-to-digital converters (ADCs) with
> +  an SPI-compatible serial interface. The ADC supports CNV Burst Mode,
> +  where an external PWM drives the CNV pin, and Manual Mode, where CNV
> +  is directly tied to the SPI chip-select.
> +
> +  Datasheets:
> +    * https://www.analog.com/en/products/ad4692.html

Odd ordering.  Put them in numeric order.

> +    * https://www.analog.com/en/products/ad4691.html
> +    * https://www.analog.com/en/products/ad4694.html
> +    * https://www.analog.com/en/products/ad4693.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
...

> +
> +  interrupt-names:
> +    description: Names of the interrupt lines, matching the GP pin names.
> +    minItems: 1
> +    maxItems: 4
> +    items:

I think this wants to be an enum.  Generally look for a similar example
and copy it + test the resulting binding with latest version of the
dts-schema.

> +      - const: gp0
> +      - const: gp1
> +      - const: gp2
> +      - const: gp3
> +



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/4] iio: adc: ad4691: add initial driver for AD4691 family
  2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
  2026-03-20 15:16   ` Andy Shevchenko
@ 2026-03-21 14:48   ` Jonathan Cameron
  2026-03-23 11:44   ` Nuno Sá
  2 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2026-03-21 14:48 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
	Linus Walleij, Bartosz Golaszewski, Philipp Zabel, linux-iio,
	devicetree, linux-kernel, linux-pwm, linux-gpio

On Fri, 20 Mar 2026 13:03:56 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add support for the Analog Devices AD4691 family of high-speed,
> low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> AD4694 (8-ch, 1 MSPS).
> 
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
> 
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
> 
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed via
> the reset controller framework; a software reset through SPI_CONFIG_A
> is used as fallback when no hardware reset is available.
> 
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Hi Radu,

Just a few minor comments from me.
I'd have tweak them whilst applying but looks like you are doing a v5 anyway
for other comments.

Jonathan


> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 000000000000..5e02eb44ca44
> --- /dev/null
> +++ b/drivers/iio/adc/ad4691.c

> +#define AD4691_CHANNEL(ch)						\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
> +				    | BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +		.info_mask_separate_available =				\
> +				      BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE),	\
> +		.channel = ch,						\
> +		.scan_index = ch,					\
> +		.scan_type = {						\
> +			.sign = 'u',					\
> +			.realbits = 16,					\
> +			.storagebits = 16,				\
> +			.shift = 0,					\

Unless there is a reason to need the 0 to act as documentation we normally
assume it's the natural default and don't set it explicitly (let the C
struct initialization rules deal with setting it to 0 for us).

> +		},							\
> +	}


> +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;

Use 1 * HZ_PER_MHZ

May seem pointless but that conversion scale macro on it's own looks
strange with respect to what is it converting.

> +	unsigned int i;
> +
> +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> +		return -EBUSY;
> +
> +	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
> +		if ((int)ad4691_osc_freqs[i] == freq)

I would flip this around to reduce the indent of the path for when
we have a match.

		if ((int)ad4691_osc_freqs[i] != freq)
			continue;

		return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
					  AD4691_OSC_FREQ_MASK, i);


> +			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
> +						  AD4691_OSC_FREQ_MASK, i);
> +	}
> +
> +	return -EINVAL;
> +}

> +static int ad4691_single_shot_read(struct iio_dev *indio_dev,
> +				   struct iio_chan_spec const *chan, int *val)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	/*
> +	 * Use AUTONOMOUS mode for single-shot reads. The chip always
> +	 * operates in AUTONOMOUS mode in this driver revision.
> +	 */
> +	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +			   AD4691_STATE_RESET_ALL);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   BIT(chan->channel));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~BIT(chan->channel));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 1);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Wait for at least 2 internal oscillator periods for the
> +	 * conversion to complete.
> +	 */
> +	fsleep(DIV_ROUND_UP(2 * USEC_PER_SEC,
> +			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK,
> +						       reg_val)]));

Go long on this line for readability.
			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)]));
Is fine.  The whole 80-100 char range is fine to use if it improves redability and
here I think it does.

> +
> +	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4691_AVG_IN(chan->channel), &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	*val = reg_val;
> +
> +	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG, AD4691_STATE_RESET_ALL);
> +	if (ret)
> +		return ret;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int ad4691_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +

I think we should have convention of no blank line here.
The bit above is setting a variable then we are checking it for error.

> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		return ad4691_single_shot_read(indio_dev, chan, val);
> +	}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad4691_get_sampling_freq(st, val);
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref_uV / (MICRO / MILLI);
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static int ad4691_regulator_setup(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(dev, "avdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable AVDD\n");
> +
> +	ret = devm_regulator_get_enable(dev, "ldo-in");
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "Failed to get and enable LDO-IN\n");
> +	st->ldo_en = (ret == -ENODEV);
> +
> +	ret = devm_regulator_get_enable(dev, "vio");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable VIO\n");

Unless there is an ordering constraint for power supplies, I'd move this up to
just after avdd so the two non optional ones are done together.

> +
> +	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (st->vref_uV >= 0) {
> +		st->refbuf_en = false;
> +	} else if (st->vref_uV == -ENODEV) {
> +		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "refin");
> +		st->refbuf_en = true;
> +	}


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
  2026-03-20 16:14   ` Uwe Kleine-König
@ 2026-03-21 15:16   ` Jonathan Cameron
  2026-03-23  9:03     ` Sabau, Radu bogdan
  2026-03-24 12:22   ` Nuno Sá
  2 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2026-03-21 15:16 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
	Linus Walleij, Bartosz Golaszewski, Philipp Zabel, linux-iio,
	devicetree, linux-kernel, linux-pwm, linux-gpio

On Fri, 20 Mar 2026 13:03:57 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add buffered capture support using the IIO triggered buffer framework.
> 
> CNV Burst Mode: the GP pin identified by interrupt-names in the device

The first I assume?  Might be up to 4 of them.

> tree is configured as DATA_READY output. The IRQ handler stops
> conversions and fires the IIO trigger; the trigger handler executes a
> pre-built SPI message that reads all active channels from the AVG_IN
> accumulator registers and then resets accumulator state and restarts
> conversions for the next cycle.

No oversampling configuration?  If it's a fixed length burst I'd still
expect to see an indication of what it is and if we can flip back to
no oversampling by changing mode, that should be oversampling == 1.
Seems there is a depth setting for the averaging filters, that superficially
at least appears to be the right control for this.

Seems like there is an SPI burst mode as well?  That feels like very
standard oversampling.

> 
> Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> reads the previous result and starts the next conversion (pipelined
> N+1 scheme). At preenable time a pre-built, optimised SPI message of
> N+1 transfers is constructed (N channel reads plus one NOOP to drain
> the pipeline). The trigger handler executes the message in a single
> spi_sync() call and collects the results. An external trigger (e.g.
> iio-trig-hrtimer) is required to drive the trigger at the desired
> sample rate.
> 
> Both modes share the same trigger handler and push a complete scan —
> one u16 slot per channel at its scan_index position, followed by a
> timestamp — to the IIO buffer via iio_push_to_buffers_with_ts().
> 
> The CNV Burst Mode sampling frequency (PWM period) is exposed as a
> buffer-level attribute via IIO_DEVICE_ATTR.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
A few comments inline.  All the modes etc in this driver are complex
enough I think this one needs a driver specific document in Documentation/iio

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 5e02eb44ca44..db776de32846 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c

> @@ -121,6 +140,7 @@ static const struct iio_chan_spec ad4691_channels[] = {
>  	AD4691_CHANNEL(13),
>  	AD4691_CHANNEL(14),
>  	AD4691_CHANNEL(15),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),
>  };
>  
>  static const struct iio_chan_spec ad4693_channels[] = {
> @@ -132,6 +152,7 @@ static const struct iio_chan_spec ad4693_channels[] = {
>  	AD4691_CHANNEL(5),
>  	AD4691_CHANNEL(6),
>  	AD4691_CHANNEL(7),
> +	IIO_CHAN_SOFT_TIMESTAMP(16),

Any big advantage in pushing it all the way down to 16?  It's ABI compliant
as these only have to be monotonic, but I'd feel it was more natural as 8 unless
there is something I'm missing.

>  };
>  
>  /*
> @@ -189,16 +210,63 @@ static const struct ad4691_chip_info ad4694_chip_info = {
>  struct ad4691_state {
>  	const struct ad4691_chip_info	*info;
>  	struct regmap			*regmap;
> +
> +	struct pwm_device		*conv_trigger;
> +	struct iio_trigger		*trig;
> +	int				irq;
> +
> +	bool				manual_mode;
> +
>  	int				vref_uV;
>  	bool				refbuf_en;
>  	bool				ldo_en;
> +	u32				cnv_period_ns;
>  	/*
>  	 * Synchronize access to members of the driver state, and ensure
>  	 * atomicity of consecutive SPI operations.
>  	 */
>  	struct mutex			lock;
> +	/*
> +	 * Per-buffer-enabl ree lifetimesources:
> +	 * Manual Mode - a pre-built SPI message that clocks out N+1
> +	 *		 transfers in one go.
> +	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
> +	 *		    transfers in one go.
> +	 */
> +	void				*scan_devm_group;
> +	struct spi_message		scan_msg;
> +	struct spi_transfer		*scan_xfers;
> +	__be16				*scan_tx;
> +	__be16				*scan_rx;
> +	/* Scan buffer: one slot per channel (u16) plus timestamp */
> +	struct {
> +		u16 vals[16];
> +		s64 ts __aligned(8);

aligned_s64

That was introduced last year IIRC.


> +	} scan __aligned(IIO_DMA_MINALIGN);
>  };

>  static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
>  {
>  	struct spi_device *spi = context;
> @@ -341,14 +409,16 @@ static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
>  static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
>  {
>  	struct ad4691_state *st = iio_priv(indio_dev);
> -	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
>  	unsigned int i;
>  
> +	if (freq > st->info->max_rate)
> +		return -EINVAL;
> +
>  	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
>  	if (IIO_DEV_ACQUIRE_FAILED(claim))
>  		return -EBUSY;
>  
> -	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
> +	for (i = 0; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {

Why is the start offset no longer relevant?

>  		if ((int)ad4691_osc_freqs[i] == freq)
>  			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
>  						  AD4691_OSC_FREQ_MASK, i);

> @@ -386,8 +459,7 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
>  	guard(mutex)(&st->lock);
>  
>  	/*
> -	 * Use AUTONOMOUS mode for single-shot reads. The chip always
> -	 * operates in AUTONOMOUS mode in this driver revision.
> +	 * Use AUTONOMOUS mode for single-shot reads.
This edit doesn't seem to belong in this patch. I'd just put the new
text in place in the earlier patch.

> +static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct spi_device *spi = to_spi_device(dev);
> +	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
> +	unsigned int n_xfers = n_active + 1;
> +	unsigned int k, i;
> +	int ret;
> +
> +	st->scan_devm_group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!st->scan_devm_group)
> +		return -ENOMEM;
> +
> +	st->scan_xfers = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_xfers),
> +				      GFP_KERNEL);
> +	st->scan_tx = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_tx),
> +				   GFP_KERNEL);
> +	st->scan_rx = devm_kcalloc(dev, n_xfers, sizeof(*st->scan_rx),
> +				   GFP_KERNEL);
> +	if (!st->scan_xfers || !st->scan_tx || !st->scan_rx) {
> +		devres_release_group(dev, st->scan_devm_group);
> +		return -ENOMEM;
> +	}
> +
> +	spi_message_init(&st->scan_msg);
> +
> +	k = 0;
> +	iio_for_each_active_channel(indio_dev, i) {
> +		st->scan_tx[k] = cpu_to_be16(AD4691_ADC_CHAN(i));
> +		st->scan_xfers[k].tx_buf = &st->scan_tx[k];
> +		st->scan_xfers[k].rx_buf = &st->scan_rx[k];
> +		st->scan_xfers[k].len = sizeof(__be16);
> +		st->scan_xfers[k].cs_change = 1;
> +		spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> +		k++;
> +	}
> +
> +	/* Final NOOP transfer to retrieve last channel's result. */
> +	st->scan_tx[k] = cpu_to_be16(AD4691_NOOP);
> +	st->scan_xfers[k].tx_buf = &st->scan_tx[k];
> +	st->scan_xfers[k].rx_buf = &st->scan_rx[k];
> +	st->scan_xfers[k].len = sizeof(__be16);
> +	spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> +
> +	devres_close_group(dev, st->scan_devm_group);

Similar comment to below on devres groups not really being appropriate
here.

> +
> +	st->scan_msg.spi = spi;
> +
> +	ret = spi_optimize_message(spi, &st->scan_msg);
> +	if (ret) {
> +		devres_release_group(dev, st->scan_devm_group);
If you were keeping this, better to do an error handling block via
gotos like you do for the other similar code.

> +		return ret;
> +	}
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret) {
> +		spi_unoptimize_message(&st->scan_msg);
> +		devres_release_group(dev, st->scan_devm_group);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad4691_manual_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	ret = ad4691_exit_conversion_mode(st);
> +	spi_unoptimize_message(&st->scan_msg);
> +	devres_release_group(dev, st->scan_devm_group);
> +	return ret;
> +}
;
> +
> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct spi_device *spi = to_spi_device(dev);
> +	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
> +	unsigned int bit, k, i;
> +	int ret;
> +
> +	st->scan_devm_group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!st->scan_devm_group)
> +		return -ENOMEM;
> +
> +	st->scan_xfers = devm_kcalloc(dev, 2 * n_active, sizeof(*st->scan_xfers),
> +				      GFP_KERNEL);

See later.  I don't see a benefit in using a devres group here. It's nothing
to do with device lifetime.

> +	st->scan_tx = devm_kcalloc(dev, n_active, sizeof(*st->scan_tx),
> +				   GFP_KERNEL);
> +	st->scan_rx = devm_kcalloc(dev, n_active, sizeof(*st->scan_rx),
> +				   GFP_KERNEL);
> +	if (!st->scan_xfers || !st->scan_tx || !st->scan_rx) {
> +		devres_release_group(dev, st->scan_devm_group);
> +		return -ENOMEM;
> +	}
> +
> +	spi_message_init(&st->scan_msg);
> +
> +	/*
> +	 * Each AVG_IN read needs two transfers: a 2-byte address write phase
> +	 * followed by a 2-byte data read phase. CS toggles between channels
> +	 * (cs_change=1 on the read phase of all but the last channel).
> +	 */
> +	k = 0;
> +	iio_for_each_active_channel(indio_dev, i) {
> +		st->scan_tx[k] = cpu_to_be16(0x8000 | AD4691_AVG_IN(i));
> +		st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
> +		st->scan_xfers[2 * k].len = sizeof(__be16);
> +		spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
> +		st->scan_xfers[2 * k + 1].rx_buf = &st->scan_rx[k];
> +		st->scan_xfers[2 * k + 1].len = sizeof(__be16);
> +		if (k < n_active - 1)
> +			st->scan_xfers[2 * k + 1].cs_change = 1;
> +		spi_message_add_tail(&st->scan_xfers[2 * k + 1], &st->scan_msg);
> +		k++;
> +	}
> +
> +	devres_close_group(dev, st->scan_devm_group);

If you were keeping this (against comment below) why close it here?  Close it once
devm calls are done above.

> +
> +	st->scan_msg.spi = spi;
> +
> +	ret = spi_optimize_message(spi, &st->scan_msg);
> +	if (ret) {
> +		devres_release_group(dev, st->scan_devm_group);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~(*indio_dev->active_scan_mask));
> +	if (ret)
> +		goto err;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   *indio_dev->active_scan_mask);
> +	if (ret)
> +		goto err;
> +
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
> +				   AD4691_ACC_COUNT_VAL);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret)
> +		goto err;
> +
> +	ret = ad4691_sampling_enable(st, true);
> +	if (ret)
> +		goto err;
> +
> +	enable_irq(st->irq);
> +	return 0;
> +err:
> +	spi_unoptimize_message(&st->scan_msg);
> +	devres_release_group(dev, st->scan_devm_group);
> +	return ret;
> +}
> +
> +static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	disable_irq(st->irq);
> +
> +	ret = ad4691_sampling_enable(st, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   AD4691_SEQ_ALL_CHANNELS_OFF);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad4691_exit_conversion_mode(st);
> +	spi_unoptimize_message(&st->scan_msg);
> +	devres_release_group(dev, st->scan_devm_group);

I'm not seeing an obvious reason for devres group handling here.
Normally we do that if there is a path to remove() in which the cleanup
still needs to be done.  Here there isn't as we always turn the buffer
off earlier in the remove flow.  So I'd do things the old fashioned way
and free them by hand.


> +	return ret;
> +}

> +static IIO_DEVICE_ATTR(sampling_frequency, 0644,
> +		       sampling_frequency_show,
> +		       sampling_frequency_store, 0);
> +
> +static const struct iio_dev_attr *ad4691_buffer_attrs[] = {
> +	&iio_dev_attr_sampling_frequency,
> +	NULL,

No comma.

> +};

> +
> +static irqreturn_t ad4691_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int i, k = 0;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);

General rule (see cleanup.h comments) is don't use guard() or __free()
stuff in a function that also does goto.  Just handle the mutex()
the old fashioned way.  Alternative is to use a helper function and
just leave the iio_trigger_notify_done() in this outer call.

> +
> +	ret = spi_sync(st->scan_msg.spi, &st->scan_msg);
> +	if (ret)
> +		goto done;
> +
> +	if (st->manual_mode) {
> +		iio_for_each_active_channel(indio_dev, i) {
> +			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k + 1]);
> +			k++;
> +		}
> +	} else {
> +		iio_for_each_active_channel(indio_dev, i) {
> +			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k]);
> +			k++;
> +		}
> +
> +		ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +				   AD4691_STATE_RESET_ALL);
> +		if (ret)
> +			goto done;
> +
> +		ret = ad4691_sampling_enable(st, true);
> +		if (ret)
> +			goto done;
> +	}
> +
> +	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
> +				    pf->timestamp);
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +


> @@ -595,17 +1071,91 @@ static int ad4691_config(struct ad4691_state *st)
>  		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
>  
>  	/*
> -	 * Set the internal oscillator to the highest valid rate for this chip.
> -	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
> -	 * at index 1 (500 kHz).
> +	 * Set the internal oscillator to the highest rate this chip supports.

This reword doesn't obviously fit in this patch. Move it to earlier one.

> +	 * Index 0 (1 MHz) exceeds the 500 kHz max of AD4691/AD4693, so those
> +	 * chips start at index 1 (500 kHz).
>  	 */
>  	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
> -			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);
> +			   (ad4691_osc_freqs[0] > st->info->max_rate) ? 1 : 0);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
>  
>  	/* Device always operates in AUTONOMOUS mode. */
> -	return regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE_VAL);
> +	ret = regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write ADC_SETUp\n");
> +
> +	if (st->manual_mode)
> +		return 0;
> +
> +	ret = device_property_read_string_array(dev, "interrupt-names",
> +						&irq_name, 1);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to read interrupt-names\n");
> +
> +	if (strncmp(irq_name, "gp", 2) != 0 ||
> +	    kstrtouint(irq_name + 2, 10, &gp_num) || gp_num > 3)

No need to check this. Just request the individual GPIOs when needed by name.

> +		return dev_err_probe(dev, -EINVAL,
> +				     "Invalid interrupt name '%s'\n", irq_name);
> +
> +	return ad4691_gpio_setup(st, gp_num);
> +}
> +
> +static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
> +					 struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int irq, ret;
> +
> +	st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +					  indio_dev->name,
> +					  iio_device_id(indio_dev));
> +	if (!st->trig)
> +		return -ENOMEM;
> +
> +	st->trig->ops = &ad4691_trigger_ops;
> +	iio_trigger_set_drvdata(st->trig, st);
> +
> +	ret = devm_iio_trigger_register(dev, st->trig);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "IIO trigger register failed\n");
> +
> +	indio_dev->trig = iio_trigger_get(st->trig);
> +
> +	if (!st->manual_mode) {
> +		/*
> +		 * GP0 asserts at end-of-conversion. The IRQ handler stops
> +		 * conversions and fires the IIO trigger so the trigger handler
> +		 * can read and push the sample to the buffer. The IRQ is kept
> +		 * disabled until the buffer is enabled.
> +		 */
> +		irq = fwnode_irq_get(dev_fwnode(dev), 0);

Given the presence of interrupt names, there is no guarantee that this is GP0.
You need to get it by name to know that.  They can come in any order, or indeed
any given one can be not wired.

> +		if (irq < 0)
> +			return dev_err_probe(dev, irq,
> +					     "failed to get GP interrupt\n");
> +
> +		st->irq = irq;
> +
> +		ret = devm_request_threaded_irq(dev, irq, NULL,
> +						&ad4691_irq,
> +						IRQF_ONESHOT | IRQF_NO_AUTOEN,

When we are not enabling at boot it is usually helpful to add a comment for why
not.

> +						indio_dev->name, indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		return devm_iio_triggered_buffer_setup_ext(dev, indio_dev,
> +							   &iio_pollfunc_store_time,
> +							   &ad4691_trigger_handler,
> +							   IIO_BUFFER_DIRECTION_IN,
> +							   &ad4691_cnv_burst_buffer_setup_ops,
> +							   ad4691_buffer_attrs);
> +	}
> +
> +	return devm_iio_triggered_buffer_setup(dev, indio_dev,
> +					       &iio_pollfunc_store_time,
> +					       &ad4691_trigger_handler,
> +					       &ad4691_manual_buffer_setup_ops);
>  }


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
  2026-03-20 17:07   ` Andy Shevchenko
@ 2026-03-21 15:28   ` Jonathan Cameron
  2026-03-22  3:05   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2026-03-21 15:28 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
	Linus Walleij, Bartosz Golaszewski, Philipp Zabel, linux-iio,
	devicetree, linux-kernel, linux-pwm, linux-gpio

On Fri, 20 Mar 2026 13:03:58 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add SPI offload support to enable DMA-based, CPU-independent data
> acquisition using the SPI Engine offload framework.
> 
> When an SPI offload is available (devm_spi_offload_get() succeeds),
> the driver registers a DMA engine IIO buffer and uses dedicated buffer
> setup operations. If no offload is available the existing software
> triggered buffer path is used unchanged.
> 
> Both CNV Burst Mode and Manual Mode support offload, but use different
> trigger mechanisms:
> 
> CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on the GP pin specified by the trigger-source consumer reference
> in the device tree (one cell = GP pin number 0-3). For this mode the
> driver acts as both an SPI offload consumer (DMA RX stream, message
> optimization) and a trigger source provider: it registers the
> GP/DATA_READY output via devm_spi_offload_trigger_register() so the
> offload framework can match the '#trigger-source-cells' phandle and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
> 
> Manual Mode: the SPI Engine is triggered by a periodic trigger at
> the configured sampling frequency. The pre-built SPI message uses
> the pipelined CNV-on-CS protocol: N+1 4-byte transfers are issued
> for N active channels (the first result is discarded as garbage from
> the pipeline flush) and the remaining N results are captured by DMA.
> 
> All offload transfers use 32-bit frames (bits_per_word=32, len=4) for
> DMA word alignment. This patch promotes the channel scan_type from
> storagebits=16 (triggered-buffer path) to storagebits=32 to match the
> DMA word size; the triggered-buffer paths are updated to the same layout
> for consistency. 

That's quite a large cost for the kfifo sizing, particularly if timestamps
are enabled. I think I'd prefer we kept the exiting paths using 16 bit data
storage for each channel.

> CNV Burst Mode channel data arrives in the lower 16
> bits of the 32-bit word (shift=0); Manual Mode data arrives in the upper
> 16 bits (shift=16), matching the 4-byte SPI transfer layout
> [data_hi, data_lo, 0, 0]. A separate ad4691_manual_channels[] array
> encodes the shift=16 scan type for manual mode.

That's odd - but fair enough if that's what the IP ends up doing.

> 
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>

Various other minor comments inline.

> -#define AD4691_CHANNEL(ch)						\
> +/*
> + * 16-bit ADC data is stored in 32-bit slots to match the SPI offload DMA
> + * word size (32 bits per transfer). The shift reflects the data position

As mentioned elsewhere, I don't think we care about matching the offload
layout. Lots of existing drivers don't and if we can we want to minimize
wasted space whilst still keep the data naturally aligned to make accesses easy.

> + * within the 32-bit word:
> + *   CNV_BURST: RX = [dummy, dummy, data_hi, data_lo] -> shift = 0
> + *   MANUAL:    RX = [data_hi, data_lo, dummy, dummy] -> shift = 16
> + * The triggered-buffer paths store data in the same position for consistency.
> + * Do not "fix" storagebits to 16.
> + */
> +#define AD4691_CHANNEL(ch, _shift)					\
>  	{								\
>  		.type = IIO_VOLTAGE,					\
>  		.indexed = 1,						\
> @@ -118,40 +140,72 @@ struct ad4691_chip_info {
>  		.scan_type = {						\
>  			.sign = 'u',					\
>  			.realbits = 16,					\
> -			.storagebits = 16,				\
> -			.shift = 0,					\
> +			.storagebits = 32,				\
> +			.shift = _shift,				\
>  		},							\
>  	}

> @@ -227,9 +285,9 @@ struct ad4691_state {
>  	 */
>  	struct mutex			lock;
>  	/*
> -	 * Per-buffer-enabl ree lifetimesources:
> -	 * Manual Mode - a pre-built SPI message that clocks out N+1
> -	 *		 transfers in one go.
> +	 * Per-buffer-enable lifetime resources (triggered-buffer paths):
> +	 * Manual Mode    - a pre-built SPI message that clocks out N+1
> +	 *		    transfers in one go.
>  	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
>  	 *		    transfers in one go.
>  	 */
> @@ -238,9 +296,19 @@ struct ad4691_state {
>  	struct spi_transfer		*scan_xfers;
>  	__be16				*scan_tx;
>  	__be16				*scan_rx;
> -	/* Scan buffer: one slot per channel (u16) plus timestamp */
> +	/* SPI offload DMA path resources */
> +	struct spi_offload		*offload;
> +	/* SPI offload trigger - periodic (MANUAL) or DATA_READY (CNV_BURST) */
> +	struct spi_offload_trigger	*offload_trigger;
> +	u64				offload_trigger_hz;
> +	struct spi_message		offload_msg;
> +	/* Max 16 channel xfers + 1 state-reset or NOOP */
> +	struct spi_transfer		offload_xfer[17];
> +	u32				offload_tx_cmd[17];

Andy already commented on this being large.  Allocating separately
probably makes sense.

> +	u32				offload_tx_reset;
> +	/* Scan buffer: one slot per channel (u32) plus timestamp */
>  	struct {
> -		u16 vals[16];
> +		u32 vals[16];
>  		s64 ts __aligned(8);
>  	} scan __aligned(IIO_DMA_MINALIGN);
>  };


> +static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct spi_offload_trigger_config config = {
> +		.type = SPI_OFFLOAD_TRIGGER_DATA_READY,
> +	};
> +	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
> +	unsigned int bit, k;
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~(*indio_dev->active_scan_mask));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   *indio_dev->active_scan_mask);
> +	if (ret)
> +		return ret;
> +
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
> +				   AD4691_ACC_COUNT_VAL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret)
> +		return ret;
> +
> +	memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
> +
> +	/*
> +	 * N transfers to read N AVG_IN registers plus one state-reset
> +	 * transfer (no RX) to re-arm DATA_READY.
> +	 *   TX: [reg_hi | 0x80, reg_lo, 0x00, 0x00]
> +	 *   RX: [0x00, 0x00, data_hi, data_lo]   (shift=0)
> +	 */
> +	k = 0;
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		unsigned int reg = AD4691_AVG_IN(bit);
> +
> +		st->offload_tx_cmd[k] =
> +			cpu_to_be32(((reg >> 8 | 0x80) << 24) |
> +				    ((reg & 0xFF) << 16));

This is an odd looking construct. Maybe it's worth casting offload_tx_cmd[k] to
a u8 * and just filling the two bytes in directly.

> +		st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
> +		st->offload_xfer[k].len = sizeof(u32);
> +		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
> +		st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +		if (k < n_active - 1)
> +			st->offload_xfer[k].cs_change = 1;
> +		k++;
> +	}
> +
> +	/* State reset to re-arm DATA_READY for the next scan. */
> +	st->offload_tx_reset =
> +		cpu_to_be32(((AD4691_STATE_RESET_REG >> 8) << 24) |
> +			    ((AD4691_STATE_RESET_REG & 0xFF) << 16) |
> +			    (AD4691_STATE_RESET_ALL << 8));
Similar to above. Feels like we should be manipulating this as a u8[4]

> +	st->offload_xfer[k].tx_buf = &st->offload_tx_reset;
> +	st->offload_xfer[k].len = sizeof(u32);
> +	st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
> +	k++;
> +
> +	spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
> +	st->offload_msg.offload = st->offload;
> +
> +	ret = spi_optimize_message(spi, &st->offload_msg);
> +	if (ret)
> +		goto err_exit_conversion;
> +
> +	ret = ad4691_sampling_enable(st, true);
> +	if (ret)
> +		goto err_unoptimize;
> +
> +	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
> +	if (ret)
> +		goto err_sampling_disable;
> +
> +	return 0;
> +
> +err_sampling_disable:
> +	ad4691_sampling_enable(st, false);
> +err_unoptimize:
> +	spi_unoptimize_message(&st->offload_msg);
> +err_exit_conversion:
> +	ad4691_exit_conversion_mode(st);
> +	return ret;
> +}


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family
  2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
  2026-03-20 12:26   ` Rob Herring (Arm)
  2026-03-21 14:35   ` Jonathan Cameron
@ 2026-03-21 16:11   ` David Lechner
  2 siblings, 0 replies; 22+ messages in thread
From: David Lechner @ 2026-03-21 16:11 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio

On 3/20/26 6:03 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add DT bindings for the Analog Devices AD4691 family of multichannel
> SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> 

...

> +allOf:
> +  # ref-supply and refin-supply are mutually exclusive, one is required
> +  - oneOf:
> +      - required:
> +          - ref-supply
> +      - required:
> +          - refin-supply
> +
> +  # CNV Burst Mode (pwms present) without SPI offload requires a DRDY interrupt.
> +  # Offload configurations expose '#trigger-source-cells' instead.

This sounds like a current driver limitation, not a wiring limitation.
So doesn't belong in the devicetree.

A driver could use e.g. timer wait for the conversion instead of an interrupt.

> +  - if:
> +      required:
> +        - pwms
> +      not:
> +        required:
> +          - '#trigger-source-cells'
> +    then:
> +      required:
> +        - interrupts
> +

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
  2026-03-20 17:07   ` Andy Shevchenko
  2026-03-21 15:28   ` Jonathan Cameron
@ 2026-03-22  3:05   ` kernel test robot
  2026-03-23 13:06   ` kernel test robot
  2026-03-25 11:32   ` kernel test robot
  4 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2026-03-22  3:05 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: oe-kbuild-all, linux-iio, devicetree, linux-kernel, linux-pwm,
	linux-gpio, Radu Sabau

Hi Radu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 11439c4635edd669ae435eec308f4ab8a0804808]

url:    https://github.com/intel-lab-lkp/linux/commits/Radu-Sabau-via-B4-Relay/dt-bindings-iio-adc-add-AD4691-family/20260321-120718
base:   11439c4635edd669ae435eec308f4ab8a0804808
patch link:    https://lore.kernel.org/r/20260320-ad4692-multichannel-sar-adc-driver-v4-4-052c1050507a%40analog.com
patch subject: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
config: nios2-randconfig-r122-20260322 (https://download.01.org/0day-ci/archive/20260322/202603221011.rzczUvUN-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260322/202603221011.rzczUvUN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603221011.rzczUvUN-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/ad4691.c:954:39: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int @@     got restricted __be32 [usertype] @@
   drivers/iio/adc/ad4691.c:954:39: sparse:     expected unsigned int
   drivers/iio/adc/ad4691.c:954:39: sparse:     got restricted __be32 [usertype]
   drivers/iio/adc/ad4691.c:970:31: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int @@     got restricted __be32 [usertype] @@
   drivers/iio/adc/ad4691.c:970:31: sparse:     expected unsigned int
   drivers/iio/adc/ad4691.c:970:31: sparse:     got restricted __be32 [usertype]
   drivers/iio/adc/ad4691.c:1059:39: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int @@     got restricted __be32 [usertype] @@
   drivers/iio/adc/ad4691.c:1059:39: sparse:     expected unsigned int
   drivers/iio/adc/ad4691.c:1059:39: sparse:     got restricted __be32 [usertype]
>> drivers/iio/adc/ad4691.c:1072:30: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] offload_tx_reset @@     got restricted __be32 [usertype] @@
   drivers/iio/adc/ad4691.c:1072:30: sparse:     expected unsigned int [usertype] offload_tx_reset
   drivers/iio/adc/ad4691.c:1072:30: sparse:     got restricted __be32 [usertype]

vim +954 drivers/iio/adc/ad4691.c

   927	
   928	static int ad4691_manual_offload_buffer_postenable(struct iio_dev *indio_dev)
   929	{
   930		struct ad4691_state *st = iio_priv(indio_dev);
   931		struct device *dev = regmap_get_device(st->regmap);
   932		struct spi_device *spi = to_spi_device(dev);
   933		struct spi_offload_trigger_config config = {
   934			.type = SPI_OFFLOAD_TRIGGER_PERIODIC,
   935		};
   936		unsigned int bit, k;
   937		int ret;
   938	
   939		ret = ad4691_enter_conversion_mode(st);
   940		if (ret)
   941			return ret;
   942	
   943		memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
   944	
   945		/*
   946		 * N+1 transfers for N channels. Each CS-low period triggers
   947		 * a conversion AND returns the previous result (pipelined).
   948		 *   TX: [AD4691_ADC_CHAN(n), 0x00, 0x00, 0x00]
   949		 *   RX: [data_hi, data_lo, 0x00, 0x00]   (shift=16)
   950		 * Transfer 0 RX is garbage; transfers 1..N carry real data.
   951		 */
   952		k = 0;
   953		iio_for_each_active_channel(indio_dev, bit) {
 > 954			st->offload_tx_cmd[k] =
   955				cpu_to_be32(FIELD_PREP(AD4691_MSG_ADDR_HI,
   956						       AD4691_ADC_CHAN(bit)));
   957			st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
   958			st->offload_xfer[k].len = sizeof(u32);
   959			st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
   960			st->offload_xfer[k].cs_change = 1;
   961			st->offload_xfer[k].cs_change_delay.value = AD4691_CNV_HIGH_TIME_NS;
   962			st->offload_xfer[k].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
   963			/* First transfer RX is garbage — skip it. */
   964			if (k > 0)
   965				st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
   966			k++;
   967		}
   968	
   969		/* Final NOOP to flush pipeline and capture last channel. */
   970		st->offload_tx_cmd[k] =
   971			cpu_to_be32(FIELD_PREP(AD4691_MSG_ADDR_HI, AD4691_NOOP));
   972		st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
   973		st->offload_xfer[k].len = sizeof(u32);
   974		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
   975		st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
   976		k++;
   977	
   978		spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
   979		st->offload_msg.offload = st->offload;
   980	
   981		ret = spi_optimize_message(spi, &st->offload_msg);
   982		if (ret)
   983			goto err_exit_conversion;
   984	
   985		config.periodic.frequency_hz = st->offload_trigger_hz;
   986		ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
   987		if (ret)
   988			goto err_unoptimize;
   989	
   990		return 0;
   991	
   992	err_unoptimize:
   993		spi_unoptimize_message(&st->offload_msg);
   994	err_exit_conversion:
   995		ad4691_exit_conversion_mode(st);
   996		return ret;
   997	}
   998	
   999	static int ad4691_manual_offload_buffer_predisable(struct iio_dev *indio_dev)
  1000	{
  1001		struct ad4691_state *st = iio_priv(indio_dev);
  1002	
  1003		spi_offload_trigger_disable(st->offload, st->offload_trigger);
  1004		spi_unoptimize_message(&st->offload_msg);
  1005	
  1006		return ad4691_exit_conversion_mode(st);
  1007	}
  1008	
  1009	static const struct iio_buffer_setup_ops ad4691_manual_offload_buffer_setup_ops = {
  1010		.postenable = &ad4691_manual_offload_buffer_postenable,
  1011		.predisable = &ad4691_manual_offload_buffer_predisable,
  1012	};
  1013	
  1014	static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
  1015	{
  1016		struct ad4691_state *st = iio_priv(indio_dev);
  1017		struct device *dev = regmap_get_device(st->regmap);
  1018		struct spi_device *spi = to_spi_device(dev);
  1019		struct spi_offload_trigger_config config = {
  1020			.type = SPI_OFFLOAD_TRIGGER_DATA_READY,
  1021		};
  1022		unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
  1023		unsigned int bit, k;
  1024		int ret;
  1025	
  1026		ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
  1027				   (u16)~(*indio_dev->active_scan_mask));
  1028		if (ret)
  1029			return ret;
  1030	
  1031		ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
  1032				   *indio_dev->active_scan_mask);
  1033		if (ret)
  1034			return ret;
  1035	
  1036		iio_for_each_active_channel(indio_dev, bit) {
  1037			ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
  1038					   AD4691_ACC_COUNT_VAL);
  1039			if (ret)
  1040				return ret;
  1041		}
  1042	
  1043		ret = ad4691_enter_conversion_mode(st);
  1044		if (ret)
  1045			return ret;
  1046	
  1047		memset(st->offload_xfer, 0, sizeof(st->offload_xfer));
  1048	
  1049		/*
  1050		 * N transfers to read N AVG_IN registers plus one state-reset
  1051		 * transfer (no RX) to re-arm DATA_READY.
  1052		 *   TX: [reg_hi | 0x80, reg_lo, 0x00, 0x00]
  1053		 *   RX: [0x00, 0x00, data_hi, data_lo]   (shift=0)
  1054		 */
  1055		k = 0;
  1056		iio_for_each_active_channel(indio_dev, bit) {
  1057			unsigned int reg = AD4691_AVG_IN(bit);
  1058	
  1059			st->offload_tx_cmd[k] =
  1060				cpu_to_be32(((reg >> 8 | 0x80) << 24) |
  1061					    ((reg & 0xFF) << 16));
  1062			st->offload_xfer[k].tx_buf = &st->offload_tx_cmd[k];
  1063			st->offload_xfer[k].len = sizeof(u32);
  1064			st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
  1065			st->offload_xfer[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
  1066			if (k < n_active - 1)
  1067				st->offload_xfer[k].cs_change = 1;
  1068			k++;
  1069		}
  1070	
  1071		/* State reset to re-arm DATA_READY for the next scan. */
> 1072		st->offload_tx_reset =
  1073			cpu_to_be32(((AD4691_STATE_RESET_REG >> 8) << 24) |
  1074				    ((AD4691_STATE_RESET_REG & 0xFF) << 16) |
  1075				    (AD4691_STATE_RESET_ALL << 8));
  1076		st->offload_xfer[k].tx_buf = &st->offload_tx_reset;
  1077		st->offload_xfer[k].len = sizeof(u32);
  1078		st->offload_xfer[k].bits_per_word = AD4691_OFFLOAD_BITS_PER_WORD;
  1079		k++;
  1080	
  1081		spi_message_init_with_transfers(&st->offload_msg, st->offload_xfer, k);
  1082		st->offload_msg.offload = st->offload;
  1083	
  1084		ret = spi_optimize_message(spi, &st->offload_msg);
  1085		if (ret)
  1086			goto err_exit_conversion;
  1087	
  1088		ret = ad4691_sampling_enable(st, true);
  1089		if (ret)
  1090			goto err_unoptimize;
  1091	
  1092		ret = spi_offload_trigger_enable(st->offload, st->offload_trigger, &config);
  1093		if (ret)
  1094			goto err_sampling_disable;
  1095	
  1096		return 0;
  1097	
  1098	err_sampling_disable:
  1099		ad4691_sampling_enable(st, false);
  1100	err_unoptimize:
  1101		spi_unoptimize_message(&st->offload_msg);
  1102	err_exit_conversion:
  1103		ad4691_exit_conversion_mode(st);
  1104		return ret;
  1105	}
  1106	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-21 15:16   ` Jonathan Cameron
@ 2026-03-23  9:03     ` Sabau, Radu bogdan
  0 siblings, 0 replies; 22+ messages in thread
From: Sabau, Radu bogdan @ 2026-03-23  9:03 UTC (permalink / raw)
  To: Jonathan Cameron, Radu Sabau via B4 Relay
  Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pwm@vger.kernel.org, linux-gpio@vger.kernel.org



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, March 21, 2026 5:17 PM
> To: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> 
> [External]
> 
> On Fri, 20 Mar 2026 13:03:57 +0200
> Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> wrote:
> 
> > From: Radu Sabau <radu.sabau@analog.com>
> >

...

> > tree is configured as DATA_READY output. The IRQ handler stops
> > conversions and fires the IIO trigger; the trigger handler executes a
> > pre-built SPI message that reads all active channels from the AVG_IN
> > accumulator registers and then resets accumulator state and restarts
> > conversions for the next cycle.
> 
> No oversampling configuration?  If it's a fixed length burst I'd still
> expect to see an indication of what it is and if we can flip back to
> no oversampling by changing mode, that should be oversampling == 1.
> Seems there is a depth setting for the averaging filters, that superficially
> at least appears to be the right control for this.
> 
> Seems like there is an SPI burst mode as well?  That feels like very
> standard oversampling.
> 

Hi Jonathan,

You are right! Those depth registers which I wrongly name ACC_COUNT_LIMIT
still (name changed upon chip's release,  I will fix in the next version of the driver)
indeed work as expected.

I verified this using a Logic Analyzer on the SPI signals and GP interrupt to verify
the timing -> GP falls after a longer time if depth is increased -> oversampling is used.

Since this would be a nice addition, I will implement this then in the
next version.

Thank you for this!
Radu

> >
> > Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> > reads the previous result and starts the next conversion (pipelined
> > N+1 scheme). At preenable time a pre-built, optimised SPI message of
> > N+1 transfers is constructed (N channel reads plus one NOOP to drain
> > the pipeline). The trigger handler executes the message in a single
> > spi_sync() call and collects the results. An external trigger (e.g.
> > iio-trig-hrtimer) is required to drive the trigger at the desired
> > sample rate.
> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/4] iio: adc: ad4691: add initial driver for AD4691 family
  2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
  2026-03-20 15:16   ` Andy Shevchenko
  2026-03-21 14:48   ` Jonathan Cameron
@ 2026-03-23 11:44   ` Nuno Sá
  2 siblings, 0 replies; 22+ messages in thread
From: Nuno Sá @ 2026-03-23 11:44 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio

Hi Radu,

minor comments on top of what was said already,

On Fri, 2026-03-20 at 13:03 +0200, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add support for the Analog Devices AD4691 family of high-speed,
> low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> AD4694 (8-ch, 1 MSPS).
> 
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
> 
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
> 
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed via
> the reset controller framework; a software reset through SPI_CONFIG_A
> is used as fallback when no hardware reset is available.
> 
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  MAINTAINERS              |   1 +
>  drivers/iio/adc/Kconfig  |  11 +
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/ad4691.c | 686 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 699 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 438ca850fa1c..24e4502b8292 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1490,6 +1490,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> +F:	drivers/iio/adc/ad4691.c
>  

...

> 
> +
> +struct ad4691_state {
> +	const struct ad4691_chip_info	*info;
> +	struct regmap			*regmap;
> +	int				vref_uV;
> +	bool				refbuf_en;
> +	bool				ldo_en;
> +	/*
> +	 * Synchronize access to members of the driver state, and ensure
> +	 * atomicity of consecutive SPI operations.
> +	 */
> +	struct mutex			lock;
> +};

I would not use tabs and align all the members. Makes thinks harder in the future if
an update is needed. I would just use a simple space.

> +
> +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct spi_device *spi = context;
> +	u8 tx[2], rx[4];
> +	int ret;
> +
> +	put_unaligned_be16(0x8000 | reg, tx);
> +
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 1);
> +		if (ret)
> +			return ret;
> +		*val = rx[0];
> +		return 0;
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 2);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be16(rx);
> +		return 0;
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 3);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be24(rx);
> +		return 0;
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		ret = spi_write_then_read(spi, tx, 2, rx, 4);
> +		if (ret)
> +			return ret;
> +		*val = get_unaligned_be32(rx);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4691_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct spi_device *spi = context;
> +	u8 tx[4];
> +
> +	put_unaligned_be16(reg, tx);
> +
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_MASK_REG - 1:
> +	case AD4691_ACC_MASK_REG + 1 ... AD4691_GPIO_MODE2_REG:
> +		if (val > 0xFF)
> +			return -EINVAL;
> +		tx[2] = val;
> +		return spi_write_then_read(spi, tx, 3, NULL, 0);
> +	case AD4691_ACC_MASK_REG:
> +	case AD4691_STD_SEQ_CONFIG:
> +		if (val > 0xFFFF)
> +			return -EINVAL;
> +		put_unaligned_be16(val, &tx[2]);
> +		return spi_write_then_read(spi, tx, 4, NULL, 0);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static bool ad4691_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case AD4691_STATUS_REG:
> +	case AD4691_CLAMP_STATUS1_REG:
> +	case AD4691_CLAMP_STATUS2_REG:
> +	case AD4691_GPIO_READ:
> +	case AD4691_ACC_STATUS_FULL1_REG ... AD4691_ACC_STATUS_SAT2_REG:
> +	case AD4691_ACC_SAT_OVR_REG(0) ... AD4691_ACC_SAT_OVR_REG(15):
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool ad4691_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_SPARE_CONTROL ... AD4691_ACC_SAT_OVR_REG(15):
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> +	case AD4691_AVG_STS_IN(0) ... AD4691_AVG_STS_IN(15):
> +	case AD4691_ACC_IN(0) ... AD4691_ACC_IN(15):
> +	case AD4691_ACC_STS_DATA(0) ... AD4691_ACC_STS_DATA(15):
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool ad4691_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case 0 ... AD4691_OSC_FREQ_REG:
> +	case AD4691_STD_SEQ_CONFIG:
> +	case AD4691_SPARE_CONTROL ... AD4691_GPIO_MODE2_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config ad4691_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 32,
> +	.reg_read = ad4691_reg_read,
> +	.reg_write = ad4691_reg_write,
> +	.volatile_reg = ad4691_volatile_reg,
> +	.readable_reg = ad4691_readable_reg,
> +	.writeable_reg = ad4691_writeable_reg,
> +	.max_register = AD4691_ACC_STS_DATA(15),
> +	.cache_type = REGCACHE_MAPLE,
> +};
> +
> +static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
> +{
> +	unsigned int reg_val;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	*val = ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
> +	return IIO_VAL_INT;
> +}
> +
> +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
> +	unsigned int i;
> +
> +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> +		return -EBUSY;
> +
> +	for (i = start; i < ARRAY_SIZE(ad4691_osc_freqs); i++) {
> +		if ((int)ad4691_osc_freqs[i] == freq)

maybe just make the array signed.

> +			return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
> +						  AD4691_OSC_FREQ_MASK, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad4691_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type,
> +			     int *length, long mask)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int start = (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = (const int *)&ad4691_osc_freqs[start];
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(ad4691_osc_freqs) - start;
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4691_single_shot_read(struct iio_dev *indio_dev,
> +				   struct iio_chan_spec const *chan, int *val)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	/*
> +	 * Use AUTONOMOUS mode for single-shot reads. The chip always
> +	 * operates in AUTONOMOUS mode in this driver revision.
> +	 */
> +	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +			   AD4691_STATE_RESET_ALL);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   BIT(chan->channel));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~BIT(chan->channel));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 1);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Wait for at least 2 internal oscillator periods for the
> +	 * conversion to complete.
> +	 */
> +	fsleep(DIV_ROUND_UP(2 * USEC_PER_SEC,
> +			    ad4691_osc_freqs[FIELD_GET(AD4691_OSC_FREQ_MASK,
> +						       reg_val)]));
> +
> +	ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD4691_AVG_IN(chan->channel), &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	*val = reg_val;
> +
> +	ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG, AD4691_STATE_RESET_ALL);
> +	if (ret)
> +		return ret;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int ad4691_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +
nit: I would drop the extra new line. The check below is related to the
above.

> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		return ad4691_single_shot_read(indio_dev, chan, val);
> +	}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad4691_get_sampling_freq(st, val);
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->vref_uV / (MICRO / MILLI);
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4691_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad4691_set_sampling_freq(indio_dev, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad4691_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> +			     unsigned int writeval, unsigned int *readval)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +
> +	if (readval)
> +		return regmap_read(st->regmap, reg, readval);
> +
> +	return regmap_write(st->regmap, reg, writeval);
> +}
> +
> +static const struct iio_info ad4691_info = {
> +	.read_raw = &ad4691_read_raw,
> +	.write_raw = &ad4691_write_raw,
> +	.read_avail = &ad4691_read_avail,
> +	.debugfs_reg_access = &ad4691_reg_access,
> +};
> +
> +static int ad4691_regulator_setup(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(dev, "avdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable AVDD\n");
> +
> +	ret = devm_regulator_get_enable(dev, "ldo-in");
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "Failed to get and enable LDO-IN\n");
> +	st->ldo_en = (ret == -ENODEV);
> +
> +	ret = devm_regulator_get_enable(dev, "vio");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get and enable VIO\n");
> +
> +	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (st->vref_uV >= 0) {
> +		st->refbuf_en = false;
> +	} else if (st->vref_uV == -ENODEV) {
> +		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "refin");
> +		st->refbuf_en = true;
> +	}
> +	if (st->vref_uV < 0)
> +		return dev_err_probe(dev, st->vref_uV,
> +				     "Failed to get reference supply\n");
> +
> +	if (st->vref_uV < AD4691_VREF_uV_MIN || st->vref_uV > AD4691_VREF_uV_MAX)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "vref(%d) must be in the range [%u...%u]\n",
> +				     st->vref_uV, AD4691_VREF_uV_MIN,
> +				     AD4691_VREF_uV_MAX);
> +
> +	return 0;
> +}
> +
> +static int ad4691_reset(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct reset_control *rst;
> +
> +	rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(rst))
> +		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset\n");
> +
> +	if (!rst)
> +		/* No hardware reset available, fall back to software reset. */
> +		return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
> +				    AD4691_SW_RESET);
> +
> +	reset_control_assert(rst);

Can't we ask for the reset in the asserted state already?

> +	/* Reset delay required. See datasheet Table 5. */
> +	fsleep(300);
> +	reset_control_deassert(rst);
> +
> +	return 0;
> +}
> +
> +static int ad4691_config(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	enum ad4691_ref_ctrl ref_val;
> +	int ret;
> +
> +	switch (st->vref_uV) {
> +	case AD4691_VREF_uV_MIN ... AD4691_VREF_2P5_uV_MAX:
> +		ref_val = AD4691_VREF_2P5;
> +		break;
> +	case AD4691_VREF_2P5_uV_MAX + 1 ... AD4691_VREF_3P0_uV_MAX:
> +		ref_val = AD4691_VREF_3P0;
> +		break;
> +	case AD4691_VREF_3P0_uV_MAX + 1 ... AD4691_VREF_3P3_uV_MAX:
> +		ref_val = AD4691_VREF_3P3;
> +		break;
> +	case AD4691_VREF_3P3_uV_MAX + 1 ... AD4691_VREF_4P096_uV_MAX:
> +		ref_val = AD4691_VREF_4P096;
> +		break;
> +	case AD4691_VREF_4P096_uV_MAX + 1 ... AD4691_VREF_uV_MAX:
> +		ref_val = AD4691_VREF_5P0;
> +		break;
> +	default:
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Unsupported vref voltage: %d uV\n",
> +				     st->vref_uV);
> +	}
> +
> +	ret = regmap_write(st->regmap, AD4691_REF_CTRL,
> +			   FIELD_PREP(AD4691_REF_CTRL_MASK, ref_val) |
> +			   (st->refbuf_en ? AD4691_REFBUF_EN : 0));
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write REF_CTRL\n");
> +
> +	ret = regmap_write(st->regmap, AD4691_DEVICE_SETUP,
> +			   st->ldo_en ? AD4691_LDO_EN : 0);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
> +
> +	/*
> +	 * Set the internal oscillator to the highest valid rate for this chip.
> +	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
> +	 * at index 1 (500 kHz).
> +	 */
> +	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
> +			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
> +
> +	/* Device always operates in AUTONOMOUS mode. */

Kind of obvious from the code. Drop the comment please.


- Nuno Sá

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
                     ` (2 preceding siblings ...)
  2026-03-22  3:05   ` kernel test robot
@ 2026-03-23 13:06   ` kernel test robot
  2026-03-25 11:32   ` kernel test robot
  4 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2026-03-23 13:06 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: llvm, oe-kbuild-all, linux-iio, devicetree, linux-kernel,
	linux-pwm, linux-gpio, Radu Sabau

Hi Radu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 11439c4635edd669ae435eec308f4ab8a0804808]

url:    https://github.com/intel-lab-lkp/linux/commits/Radu-Sabau-via-B4-Relay/dt-bindings-iio-adc-add-AD4691-family/20260321-120718
base:   11439c4635edd669ae435eec308f4ab8a0804808
patch link:    https://lore.kernel.org/r/20260320-ad4692-multichannel-sar-adc-driver-v4-4-052c1050507a%40analog.com
patch subject: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260323/202603232017.8IO2whBG-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260323/202603232017.8IO2whBG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603232017.8IO2whBG-lkp@intel.com/

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> WARNING: modpost: module ad4691 uses symbol devm_iio_dmaengine_buffer_setup_with_handle from namespace IIO_DMAENGINE_BUFFER, but does not import it.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
  2026-03-20 16:14   ` Uwe Kleine-König
  2026-03-21 15:16   ` Jonathan Cameron
@ 2026-03-24 12:22   ` Nuno Sá
  2026-03-25 12:47     ` Sabau, Radu bogdan
  2 siblings, 1 reply; 22+ messages in thread
From: Nuno Sá @ 2026-03-24 12:22 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio

Hi Radu,

Some comments from me.

On Fri, 2026-03-20 at 13:03 +0200, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add buffered capture support using the IIO triggered buffer framework.
> 
> CNV Burst Mode: the GP pin identified by interrupt-names in the device
> tree is configured as DATA_READY output. The IRQ handler stops
> conversions and fires the IIO trigger; the trigger handler executes a
> pre-built SPI message that reads all active channels from the AVG_IN
> accumulator registers and then resets accumulator state and restarts
> conversions for the next cycle.
> 
> Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> reads the previous result and starts the next conversion (pipelined
> N+1 scheme). At preenable time a pre-built, optimised SPI message of
> N+1 transfers is constructed (N channel reads plus one NOOP to drain
> the pipeline). The trigger handler executes the message in a single
> spi_sync() call and collects the results. An external trigger (e.g.
> iio-trig-hrtimer) is required to drive the trigger at the desired
> sample rate.
> 
> Both modes share the same trigger handler and push a complete scan —
> one u16 slot per channel at its scan_index position, followed by a
> timestamp — to the IIO buffer via iio_push_to_buffers_with_ts().
> 
> The CNV Burst Mode sampling frequency (PWM period) is exposed as a
> buffer-level attribute via IIO_DEVICE_ATTR.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  drivers/iio/adc/Kconfig  |   2 +
>  drivers/iio/adc/ad4691.c | 584 +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 571 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 3685a03aa8dc..d498f16c0816 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -142,6 +142,8 @@ config AD4170_4
>  config AD4691
>  	tristate "Analog Devices AD4691 Family ADC Driver"
>  	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	select REGMAP
>  	help
>  	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 5e02eb44ca44..db776de32846 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -9,9 +9,12 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> +#include <linux/interrupt.h>
>  #include <linux/math.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> @@ -19,7 +22,12 @@
>  #include <linux/units.h>
>  #include <linux/unaligned.h>
>  
> +#include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>  
>  #define AD4691_VREF_uV_MIN			2400000
>  #define AD4691_VREF_uV_MAX			5250000
> @@ -28,6 +36,8 @@
>  #define AD4691_VREF_3P3_uV_MAX			3750000
>  #define AD4691_VREF_4P096_uV_MAX		4500000
>  
> +#define AD4691_CNV_DUTY_CYCLE_NS		380
> +
>  #define AD4691_SPI_CONFIG_A_REG			0x000
>  #define AD4691_SW_RESET				(BIT(7) | BIT(0))
>  

...

> 
> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct spi_device *spi = to_spi_device(dev);
> +	unsigned int n_active = hweight_long(*indio_dev->active_scan_mask);
> +	unsigned int bit, k, i;
> +	int ret;
> +
> +	st->scan_devm_group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!st->scan_devm_group)
> +		return -ENOMEM;

Agree with Jonathan. Not seeing a valid reason for the above.

> +
> +	st->scan_xfers = devm_kcalloc(dev, 2 * n_active, sizeof(*st->scan_xfers),
> +				      GFP_KERNEL);
> +	st->scan_tx = devm_kcalloc(dev, n_active, sizeof(*st->scan_tx),
> +				   GFP_KERNEL);
> +	st->scan_rx = devm_kcalloc(dev, n_active, sizeof(*st->scan_rx),
> +				   GFP_KERNEL);
> +	if (!st->scan_xfers || !st->scan_tx || !st->scan_rx) {
> +		devres_release_group(dev, st->scan_devm_group);
> +		return -ENOMEM;
> +	}
> +
> +	spi_message_init(&st->scan_msg);
> +
> +	/*
> +	 * Each AVG_IN read needs two transfers: a 2-byte address write phase
> +	 * followed by a 2-byte data read phase. CS toggles between channels
> +	 * (cs_change=1 on the read phase of all but the last channel).
> +	 */
> +	k = 0;
> +	iio_for_each_active_channel(indio_dev, i) {
> +		st->scan_tx[k] = cpu_to_be16(0x8000 | AD4691_AVG_IN(i));
> +		st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
> +		st->scan_xfers[2 * k].len = sizeof(__be16);
> +		spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
> +		st->scan_xfers[2 * k + 1].rx_buf = &st->scan_rx[k];
> +		st->scan_xfers[2 * k + 1].len = sizeof(__be16);
> +		if (k < n_active - 1)
> +			st->scan_xfers[2 * k + 1].cs_change = 1;
> +		spi_message_add_tail(&st->scan_xfers[2 * k + 1], &st->scan_msg);
> +		k++;
> +	}
> +
> +	devres_close_group(dev, st->scan_devm_group);
> +
> +	st->scan_msg.spi = spi;
> +
> +	ret = spi_optimize_message(spi, &st->scan_msg);
> +	if (ret) {
> +		devres_release_group(dev, st->scan_devm_group);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> +			   (u16)~(*indio_dev->active_scan_mask));
> +	if (ret)
> +		goto err;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   *indio_dev->active_scan_mask);
> +	if (ret)
> +		goto err;
> +
> +	iio_for_each_active_channel(indio_dev, bit) {
> +		ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
> +				   AD4691_ACC_COUNT_VAL);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret)
> +		goto err;
> +
> +	ret = ad4691_sampling_enable(st, true);
> +	if (ret)
> +		goto err;
> +
> +	enable_irq(st->irq);
> +	return 0;
> +err:
> +	spi_unoptimize_message(&st->scan_msg);
> +	devres_release_group(dev, st->scan_devm_group);
> +	return ret;
> +}
> +
> +static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	disable_irq(st->irq);

Should we use disable_irq_sync()?
> +
> +	ret = ad4691_sampling_enable(st, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   AD4691_SEQ_ALL_CHANNELS_OFF);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad4691_exit_conversion_mode(st);
> +	spi_unoptimize_message(&st->scan_msg);
> +	devres_release_group(dev, st->scan_devm_group);
> +	return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {
> +	.preenable = &ad4691_cnv_burst_buffer_preenable,
> +	.postdisable = &ad4691_cnv_burst_buffer_postdisable,
> +};
> +
> +static ssize_t sampling_frequency_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +
> +	if (st->manual_mode)
> +		return -ENODEV;

Can the above happen at all? I think you're making sure (at probe) this interface
never get's exposed in manual mode.

> +
> +	return sysfs_emit(buf, "%u\n", (u32)(NSEC_PER_SEC / st->cnv_period_ns));
> +}
> +
> +static ssize_t sampling_frequency_store(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	int freq, ret;
> +
> +	if (st->manual_mode)
> +		return -ENODEV;
> +
> +	ret = kstrtoint(buf, 10, &freq);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = ad4691_set_pwm_freq(st, freq);
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(sampling_frequency, 0644,
> +		       sampling_frequency_show,
> +		       sampling_frequency_store, 0);
> +
> +static const struct iio_dev_attr *ad4691_buffer_attrs[] = {
> +	&iio_dev_attr_sampling_frequency,
> +	NULL,
> +};
> +
> +static irqreturn_t ad4691_irq(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +
> +	/*
> +	 * GPx has asserted: stop conversions before reading so the
> +	 * accumulator does not continue sampling while the trigger handler
> +	 * processes the data. Then fire the IIO trigger to push the sample
> +	 * to the buffer.
> +	 */
> +	ad4691_sampling_enable(st, false);
> +	iio_trigger_poll(st->trig);

Not sure you need to save trig in your struct. We already have it in indio_dev->trig. Sure,
it is a private member but still fairly common to see (this patch included):

indio_dev->trig = iio_trigger_get(trig);

So I would say we either assume it's public or start to not allow the above
pattern.

Alternatively, I don't think you're using indio_dev driverdata right? Could save it
in there.

> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_trigger_ops ad4691_trigger_ops = {
> +	.validate_device = iio_trigger_validate_own_device,
> +};
> +
> +static irqreturn_t ad4691_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int i, k = 0;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = spi_sync(st->scan_msg.spi, &st->scan_msg);
> +	if (ret)
> +		goto done;
> +
> +	if (st->manual_mode) {
> +		iio_for_each_active_channel(indio_dev, i) {
> +			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k + 1]);
> +			k++;
> +		}
> +	} else {
> +		iio_for_each_active_channel(indio_dev, i) {
> +			st->scan.vals[i] = be16_to_cpu(st->scan_rx[k]);
> +			k++;
> +		}
> +
> +		ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +				   AD4691_STATE_RESET_ALL);
> +		if (ret)
> +			goto done;
> +
> +		ret = ad4691_sampling_enable(st, true);
> +		if (ret)
> +			goto done;
> +	}
> +
> +	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
> +				    pf->timestamp);
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +
>  static const struct iio_info ad4691_info = {
>  	.read_raw = &ad4691_read_raw,
>  	.write_raw = &ad4691_write_raw,
> @@ -495,6 +934,25 @@ static const struct iio_info ad4691_info = {
>  	.debugfs_reg_access = &ad4691_reg_access,
>  };
>  
> +static int ad4691_pwm_setup(struct ad4691_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	int ret;
> +
> +	st->conv_trigger = devm_pwm_get(dev, "cnv");
> +	if (IS_ERR(st->conv_trigger))
> +		return dev_err_probe(dev, PTR_ERR(st->conv_trigger),
> +				     "Failed to get cnv pwm\n");
> +
> +	ret = devm_add_action_or_reset(dev, ad4691_disable_pwm,
> +				       st->conv_trigger);

This is a suspicious pattern. But I do see it's used like this in more places and it's a no-op
if PWM is already disabled. Still, not sure if agree with this kind "unbalanced" handling.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to register PWM disable action\n");
> +
> +	return ad4691_set_pwm_freq(st, st->info->max_rate);
> +}
> +
>  static int ad4691_regulator_setup(struct ad4691_state *st)
>  {
>  	struct device *dev = regmap_get_device(st->regmap);
> @@ -555,12 +1013,30 @@ static int ad4691_reset(struct ad4691_state *st)
>  	return 0;
>  }
>  
> -static int ad4691_config(struct ad4691_state *st)
> +static int ad4691_config(struct ad4691_state *st, u32 max_speed_hz)

My eyes might be failing me but where is 'max_speed_hz' used?


>  {
>  	struct device *dev = regmap_get_device(st->regmap);
>  	enum ad4691_ref_ctrl ref_val;
> +	const char *irq_name;
> +	unsigned int gp_num;
>  	int ret;
>  
> +	/*
> +	 * Determine buffer conversion mode from DT: if a PWM is provided it
> +	 * drives the CNV pin (CNV_BURST_MODE); otherwise CNV is tied to CS
> +	 * and each SPI transfer triggers a conversion (MANUAL_MODE).
> +	 * Both modes idle in AUTONOMOUS mode so that read_raw can use the
> +	 * internal oscillator without disturbing the hardware configuration.
> +	 */
> +	if (device_property_present(dev, "pwms")) {
> +		st->manual_mode = false;
> +		ret = ad4691_pwm_setup(st);
> +		if (ret)
> +			return ret;
> +	} else {
> +		st->manual_mode = true;
> +	}
> +
>  	switch (st->vref_uV) {
>  	case AD4691_VREF_uV_MIN ... AD4691_VREF_2P5_uV_MAX:
>  		ref_val = AD4691_VREF_2P5;
> @@ -595,17 +1071,91 @@ static int ad4691_config(struct ad4691_state *st)
>  		return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
>  
>  	/*
> -	 * Set the internal oscillator to the highest valid rate for this chip.
> -	 * Index 0 (1 MHz) is valid only for AD4692/AD4694; AD4691/AD4693 start
> -	 * at index 1 (500 kHz).
> +	 * Set the internal oscillator to the highest rate this chip supports.
> +	 * Index 0 (1 MHz) exceeds the 500 kHz max of AD4691/AD4693, so those
> +	 * chips start at index 1 (500 kHz).
>  	 */
>  	ret = regmap_write(st->regmap, AD4691_OSC_FREQ_REG,
> -			   (st->info->max_rate == HZ_PER_MHZ) ? 0 : 1);
> +			   (ad4691_osc_freqs[0] > st->info->max_rate) ? 1 : 0);

Does this belong to this commit?

>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
>  
>  	/* Device always operates in AUTONOMOUS mode. */
> -	return regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE_VAL);
> +	ret = regmap_write(st->regmap, AD4691_ADC_SETUP, AD4691_AUTONOMOUS_MODE);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to write ADC_SETUp\n");
> +
> +	if (st->manual_mode)
> +		return 0;
> +
> +	ret = device_property_read_string_array(dev, "interrupt-names",
> +						&irq_name, 1);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to read interrupt-names\n");
> +
> +	if (strncmp(irq_name, "gp", 2) != 0 ||
> +	    kstrtouint(irq_name + 2, 10, &gp_num) || gp_num > 3)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Invalid interrupt name '%s'\n", irq_name);
> +

I would likely prefer something like [1] rather than the string parsing.

[1]: https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/iio/imu/adis16480.c#L1582

- Nuno Sá


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
  2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
                     ` (3 preceding siblings ...)
  2026-03-23 13:06   ` kernel test robot
@ 2026-03-25 11:32   ` kernel test robot
  4 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2026-03-25 11:32 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: oe-kbuild-all, linux-iio, devicetree, linux-kernel, linux-pwm,
	linux-gpio, Radu Sabau

Hi Radu,

kernel test robot noticed the following build errors:

[auto build test ERROR on 11439c4635edd669ae435eec308f4ab8a0804808]

url:    https://github.com/intel-lab-lkp/linux/commits/Radu-Sabau-via-B4-Relay/dt-bindings-iio-adc-add-AD4691-family/20260321-120718
base:   11439c4635edd669ae435eec308f4ab8a0804808
patch link:    https://lore.kernel.org/r/20260320-ad4692-multichannel-sar-adc-driver-v4-4-052c1050507a%40analog.com
patch subject: [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support
config: openrisc-randconfig-r063-20260325 (https://download.01.org/0day-ci/archive/20260325/202603251904.qcAGC4cf-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251904.qcAGC4cf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603251904.qcAGC4cf-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: module ad4691 uses symbol devm_iio_dmaengine_buffer_setup_with_handle from namespace IIO_DMAENGINE_BUFFER, but does not import it.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-24 12:22   ` Nuno Sá
@ 2026-03-25 12:47     ` Sabau, Radu bogdan
  2026-03-25 13:12       ` Nuno Sá
  0 siblings, 1 reply; 22+ messages in thread
From: Sabau, Radu bogdan @ 2026-03-25 12:47 UTC (permalink / raw)
  To: Nuno Sá, Lars-Peter Clausen, Hennerich, Michael,
	Jonathan Cameron, David Lechner, Sa, Nuno, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
	linux-gpio@vger.kernel.org



> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Tuesday, March 24, 2026 2:23 PM

...

> > +static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > +	struct ad4691_state *st = iio_priv(indio_dev);
> > +	struct device *dev = regmap_get_device(st->regmap);
> > +	int ret;
> > +
> > +	disable_irq(st->irq);
> 
> Should we use disable_irq_sync()?

Isn't disable_irq() already calling synchronize_irq() inside it? I can't see
disable_irq_sync() in the current kernel, only disable_irq_nosync().

> > +
> > +	ret = ad4691_sampling_enable(st, false);
> > +	if (ret)
> > +		return ret;
> > +


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support
  2026-03-25 12:47     ` Sabau, Radu bogdan
@ 2026-03-25 13:12       ` Nuno Sá
  0 siblings, 0 replies; 22+ messages in thread
From: Nuno Sá @ 2026-03-25 13:12 UTC (permalink / raw)
  To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
	Jonathan Cameron, David Lechner, Sa, Nuno, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
	Bartosz Golaszewski, Philipp Zabel
  Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
	linux-gpio@vger.kernel.org

On Wed, 2026-03-25 at 12:47 +0000, Sabau, Radu bogdan wrote:
> 
> 
> > -----Original Message-----
> > From: Nuno Sá <noname.nuno@gmail.com>
> > Sent: Tuesday, March 24, 2026 2:23 PM
> 
> ...
> 
> > > +static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
> > > +{
> > > +	struct ad4691_state *st = iio_priv(indio_dev);
> > > +	struct device *dev = regmap_get_device(st->regmap);
> > > +	int ret;
> > > +
> > > +	disable_irq(st->irq);
> > 
> > Should we use disable_irq_sync()?
> 
> Isn't disable_irq() already calling synchronize_irq() inside it? I can't see
> disable_irq_sync() in the current kernel, only disable_irq_nosync().

You're right! Sorry for the noise

- Nuno Sá

> 
> > > +
> > > +	ret = ad4691_sampling_enable(st, false);
> > > +	if (ret)
> > > +		return ret;
> > > +

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-03-25 13:11 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 11:03 [PATCH v4 0/4] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
2026-03-20 11:03 ` [PATCH v4 1/4] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
2026-03-20 12:26   ` Rob Herring (Arm)
2026-03-21 14:35   ` Jonathan Cameron
2026-03-21 16:11   ` David Lechner
2026-03-20 11:03 ` [PATCH v4 2/4] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
2026-03-20 15:16   ` Andy Shevchenko
2026-03-21 14:48   ` Jonathan Cameron
2026-03-23 11:44   ` Nuno Sá
2026-03-20 11:03 ` [PATCH v4 3/4] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
2026-03-20 16:14   ` Uwe Kleine-König
2026-03-21 15:16   ` Jonathan Cameron
2026-03-23  9:03     ` Sabau, Radu bogdan
2026-03-24 12:22   ` Nuno Sá
2026-03-25 12:47     ` Sabau, Radu bogdan
2026-03-25 13:12       ` Nuno Sá
2026-03-20 11:03 ` [PATCH v4 4/4] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
2026-03-20 17:07   ` Andy Shevchenko
2026-03-21 15:28   ` Jonathan Cameron
2026-03-22  3:05   ` kernel test robot
2026-03-23 13:06   ` kernel test robot
2026-03-25 11:32   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox