* [PATCH v10 1/6] dt-bindings: iio: adc: add AD4691 family
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 2:32 ` sashiko-bot
2026-05-11 11:54 ` [PATCH v10 2/6] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, Radu Sabau, Conor Dooley
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). Digital core VDD is supplied either
externally via vdd-supply, or generated by the on-chip LDO fed
from ldo-in-supply; the two are mutually exclusive and one must
be present.
- 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.
The four variants are not compatible with each other: AD4691/AD4692 have
16 analog input channels while AD4693/AD4694 have 8, and AD4691/AD4693
top out at 500 kSPS while AD4692/AD4694 reach 1 MSPS. These differences
in channel count and maximum sample rate require distinct compatible
strings so the driver can select the correct channel configuration and
rate limits.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
.../devicetree/bindings/iio/adc/adi,ad4691.yaml | 180 +++++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 187 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..af28a0c1cfa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
@@ -0,0 +1,180 @@
+# 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/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
+
+$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).
+
+ vdd-supply:
+ description:
+ External 1.8V digital core supply. When present, the internal LDO is
+ disabled (LDO_EN = 0). Mutually exclusive with ldo-in-supply.
+
+ ldo-in-supply:
+ description:
+ LDO input supply (2.4V to 5.5V). When present and vdd-supply is absent,
+ the internal LDO generates 1.8V VDD from this input (LDO_EN = 1).
+ Mutually exclusive with vdd-supply.
+
+ 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:
+ enum:
+ - gp0
+ - gp1
+ - gp2
+ - 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:
+ # vdd-supply and ldo-in-supply are mutually exclusive, one is required:
+ # either an external 1.8V VDD is provided or the internal LDO is fed from
+ # ldo-in-supply to generate VDD.
+ - oneOf:
+ - required:
+ - vdd-supply
+ - required:
+ - ldo-in-supply
+ # ref-supply and refin-supply are mutually exclusive, one is required
+ - oneOf:
+ - required:
+ - ref-supply
+ - required:
+ - refin-supply
+
+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>;
+ ldo-in-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>;
+ ldo-in-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] 16+ messages in thread* Re: [PATCH v10 1/6] dt-bindings: iio: adc: add AD4691 family
2026-05-11 11:54 ` [PATCH v10 1/6] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
@ 2026-05-12 2:32 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 2:32 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: conor+dt, robh, devicetree, krzk+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The AD4691 binding introduces GPIO provider capabilities (`gpio-controller`, `#gpio-cells`) but fails to list them as required properties and omits them from the examples.
--
commit 3d6e918750431cd1fe37b615bc0436f12070ffa3
Author: Radu Sabau <radu.sabau@analog.com>
dt-bindings: iio: adc: add AD4691 family
This commit adds Device Tree bindings for the Analog Devices AD4691 family
of multichannel SAR ADCs. It describes the hardware connections, including
power domains, PWM, reset GPIO, and interrupt configurations.
> 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 0000000000000..af28a0c1cfa9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
[ ... ]
> +required:
> + - compatible
> + - reg
> + - avdd-supply
> + - vio-supply
Should gpio-controller and #gpio-cells be added to this required list?
The binding introduces GPIO provider capabilities, but these properties
are not marked as required. Subsystem guidelines indicate that when
hardware acts as a provider, the corresponding standard properties must
be included in the required list to ensure complete device tree nodes.
[ ... ]
> +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>;
> + ldo-in-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>;
Are the gpio-controller and #gpio-cells properties missing from this example?
The subsystem guidelines state that the examples section must include all
required properties to pass dt_binding_check.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v10 2/6] iio: adc: ad4691: add initial driver for AD4691 family
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
2026-05-11 11:54 ` [PATCH v10 1/6] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 4:09 ` sashiko-bot
2026-05-12 15:25 ` Jonathan Cameron
2026-05-11 11:54 ` [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
` (3 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, 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.
IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_shared_by_all because
the AD4691 family has a single internal oscillator whose frequency
register is shared across all channels. Writing sampling_frequency for
any one channel necessarily changes the conversion rate for every other
channel, so the shared annotation correctly reflects the hardware
behaviour.
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4691.c | 731 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 745 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..5e601a87e5f3 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -139,6 +139,18 @@ 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
+ depends on REGULATOR || COMPILE_TEST
+ 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..5b72216bca80
--- /dev/null
+++ b/drivers/iio/adc/ad4691.c
@@ -0,0 +1,731 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024-2026 Analog Devices, Inc.
+ * Author: Radu Sabau <radu.sabau@analog.com>
+ */
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/limits.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_ADC_MODE_MASK GENMASK(1, 0)
+#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_DEPTH_IN(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)))
+
+static const char * const ad4691_supplies[] = { "avdd", "vio" };
+
+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_channel_info {
+ const struct iio_chan_spec *channels __counted_by_ptr(num_channels);
+ unsigned int num_channels;
+};
+
+struct ad4691_chip_info {
+ const char *name;
+ unsigned int max_rate;
+ const struct ad4691_channel_info *sw_info;
+};
+
+#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, \
+ }, \
+ }
+
+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 int ad4691_osc_freqs_Hz[] = {
+ [0x0] = 1000000,
+ [0x1] = 500000,
+ [0x2] = 400000,
+ [0x3] = 250000,
+ [0x4] = 200000,
+ [0x5] = 167000,
+ [0x6] = 133000,
+ [0x7] = 125000,
+ [0x8] = 100000,
+ [0x9] = 50000,
+ [0xA] = 25000,
+ [0xB] = 12500,
+ [0xC] = 10000,
+ [0xD] = 5000,
+ [0xE] = 2500,
+ [0xF] = 1250,
+};
+
+static const struct ad4691_channel_info ad4691_sw_info = {
+ .channels = ad4691_channels,
+ .num_channels = ARRAY_SIZE(ad4691_channels),
+};
+
+static const struct ad4691_channel_info ad4693_sw_info = {
+ .channels = ad4693_channels,
+ .num_channels = ARRAY_SIZE(ad4693_channels),
+};
+
+static const struct ad4691_chip_info ad4691_chip_info = {
+ .name = "ad4691",
+ .max_rate = 500 * HZ_PER_KHZ,
+ .sw_info = &ad4691_sw_info,
+};
+
+static const struct ad4691_chip_info ad4692_chip_info = {
+ .name = "ad4692",
+ .max_rate = 1 * HZ_PER_MHZ,
+ .sw_info = &ad4691_sw_info,
+};
+
+static const struct ad4691_chip_info ad4693_chip_info = {
+ .name = "ad4693",
+ .max_rate = 500 * HZ_PER_KHZ,
+ .sw_info = &ad4693_sw_info,
+};
+
+static const struct ad4691_chip_info ad4694_chip_info = {
+ .name = "ad4694",
+ .max_rate = 1 * HZ_PER_MHZ,
+ .sw_info = &ad4693_sw_info,
+};
+
+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;
+
+ /* Set bit 15 to mark the operation as READ. */
+ put_unaligned_be16(0x8000 | 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_ACC_SAT_OVR_REG(15):
+ ret = spi_write_then_read(spi, tx, sizeof(tx), rx, 1);
+ if (ret)
+ return ret;
+ *val = rx[0];
+ return 0;
+ case AD4691_ACC_MASK_REG:
+ case AD4691_STD_SEQ_CONFIG:
+ case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
+ ret = spi_write_then_read(spi, tx, sizeof(tx), 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, sizeof(tx), 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, sizeof(tx), 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 > U8_MAX)
+ 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 > U16_MAX)
+ 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,
+};
+
+/*
+ * Index 0 in ad4691_osc_freqs_Hz is 1 MHz — valid only for AD4692/AD4694
+ * (max_rate == 1 MHz). AD4691/AD4693 cap at 500 kHz so their valid range
+ * starts at index 1.
+ */
+static unsigned int ad4691_samp_freq_start(const struct ad4691_chip_info *info)
+{
+ return (info->max_rate == 1 * HZ_PER_MHZ) ? 0 : 1;
+}
+
+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, ®_val);
+ if (ret)
+ return ret;
+
+ *val = ad4691_osc_freqs_Hz[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 = ad4691_samp_freq_start(st->info);
+
+ IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ if (IIO_DEV_ACQUIRE_FAILED(claim))
+ return -EBUSY;
+
+ for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
+ if (ad4691_osc_freqs_Hz[i] != freq)
+ continue;
+ 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 = ad4691_samp_freq_start(st->info);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *vals = &ad4691_osc_freqs_Hz[start];
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(ad4691_osc_freqs_Hz) - 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, osc_idx, period_us;
+ int ret;
+
+ guard(mutex)(&st->lock);
+
+ /* Use AUTONOMOUS mode for single-shot reads. */
+ 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,
+ ~BIT(chan->channel) & GENMASK(15, 0));
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 1);
+ if (ret)
+ return ret;
+
+ osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
+ /* Wait 2 oscillator periods for the conversion to complete. */
+ period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
+ fsleep(period_us);
+
+ ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(st->regmap, AD4691_AVG_IN(chan->channel), ®_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_bulk_get_enable(dev, ARRAY_SIZE(ad4691_supplies),
+ ad4691_supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get and enable supplies\n");
+
+ /*
+ * vdd-supply and ldo-in-supply are mutually exclusive:
+ * vdd-supply present → external 1.8V VDD; disable internal LDO.
+ * vdd-supply absent → enable internal LDO fed from ldo-in-supply.
+ * Having both simultaneously is strongly inadvisable per the datasheet.
+ */
+ ret = devm_regulator_get_enable_optional(dev, "vdd");
+ if (ret == -ENODEV) {
+ ret = devm_regulator_get_enable(dev, "ldo-in");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to get and enable LDO-IN\n");
+ st->ldo_en = true;
+ } else if (ret) {
+ return dev_err_probe(dev, ret, "Failed to get and enable VDD\n");
+ }
+
+ st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
+ 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) {
+ /*
+ * Assert the reset line before sleeping to guarantee a proper
+ * reset pulse on every probe, including driver reloads where
+ * the line may already be deasserted (reset_control_put() does
+ * not re-assert on release).
+ * devm_reset_control_get_optional_exclusive_deasserted() cannot
+ * be used because it deasserts immediately without delay; the
+ * datasheet (Table 5) requires a ≥300 µs reset pulse width
+ * before deassertion.
+ */
+ reset_control_assert(rst);
+ fsleep(300);
+ return reset_control_deassert(rst);
+ }
+
+ /* No hardware reset available, fall back to software reset. */
+ return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
+ AD4691_SW_RESET);
+}
+
+static int ad4691_config(struct ad4691_state *st)
+{
+ struct device *dev = regmap_get_device(st->regmap);
+ enum ad4691_ref_ctrl ref_val;
+ unsigned int 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);
+ }
+
+ val = FIELD_PREP(AD4691_REF_CTRL_MASK, ref_val);
+ if (st->refbuf_en)
+ val |= AD4691_REFBUF_EN;
+
+ ret = regmap_write(st->regmap, AD4691_REF_CTRL, val);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to write REF_CTRL\n");
+
+ ret = regmap_assign_bits(st->regmap, AD4691_DEVICE_SETUP,
+ AD4691_LDO_EN, st->ldo_en);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
+
+ /*
+ * 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,
+ ad4691_samp_freq_start(st->info));
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
+
+ ret = regmap_update_bits(st->regmap, AD4691_ADC_SETUP,
+ AD4691_ADC_MODE_MASK, AD4691_AUTONOMOUS_MODE);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to write ADC_SETUP\n");
+
+ return 0;
+}
+
+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(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->info = spi_get_device_match_data(spi);
+ if (!st->info)
+ return -ENODEV;
+
+ 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->sw_info->channels;
+ indio_dev->num_channels = st->info->sw_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] 16+ messages in thread* Re: [PATCH v10 2/6] iio: adc: ad4691: add initial driver for AD4691 family
2026-05-11 11:54 ` [PATCH v10 2/6] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
@ 2026-05-12 4:09 ` sashiko-bot
2026-05-12 15:25 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 4:09 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: devicetree, krzk+dt, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Sparse register arrays matched using broad GCC ranges cause unaligned multi-byte SPI reads.
- [High] `ad4691_get_sampling_freq()` performs an SPI read without acquiring the required synchronization lock.
- [Medium] SAMP_FREQ is incorrectly placed in info_mask_separate instead of info_mask_shared_by_all.
--
commit c07ff01f28d7750e0cbd4ed3b20b3bab0c7401f0
Author: Radu Sabau <radu.sabau@analog.com>
Subject: iio: adc: ad4691: add initial driver for AD4691 family
This commit adds support for the Analog Devices AD4691 family of high-speed,
low-power multichannel SAR ADCs. It 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.
> IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_shared_by_all because
> the AD4691 family has a single internal oscillator whose frequency
> register is shared across all channels. Writing sampling_frequency for
> any one channel necessarily changes the conversion rate for every other
> channel, so the shared annotation correctly reflects the hardware
> behaviour.
The commit message explicitly states that IIO_CHAN_INFO_SAMP_FREQ is exposed
as info_mask_shared_by_all, but the code appears to add it to
info_mask_separate instead:
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 0000000000000..5b72216bca80e
> --- /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, \
Does this incorrectly generate independent sysfs attributes for each channel?
If so, writing to one channel's attribute might alter the shared internal
oscillator, silently changing the conversion rate for all other channels.
[ ... ]
> +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
[ ... ]
> + switch (reg) {
> + case 0 ... AD4691_OSC_FREQ_REG:
> + case AD4691_SPARE_CONTROL ... AD4691_ACC_MASK_REG - 1:
> + case AD4691_ACC_MASK_REG + 1 ... AD4691_ACC_SAT_OVR_REG(15):
> + ret = spi_write_then_read(spi, tx, sizeof(tx), rx, 1);
> + if (ret)
> + return ret;
> + *val = rx[0];
> + return 0;
> + case AD4691_ACC_MASK_REG:
> + case AD4691_STD_SEQ_CONFIG:
> + case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> + ret = spi_write_then_read(spi, tx, sizeof(tx), rx, 2);
Since AD4691_AVG_IN(n) evaluates to 0x201 + 2*n, the valid base addresses
are spaced by 2. Will the range match inadvertently cover the intermediate
unaligned addresses?
If so, iterating over the regmap (e.g., via the debugfs registers dump) could
cause unaligned multi-byte SPI reads that cross register boundaries.
[ ... ]
> +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):
A similar question applies here for the sparse register arrays like
AD4691_AVG_STS_IN, AD4691_ACC_IN, and AD4691_ACC_STS_DATA. Could this cause
readable_reg() to return true for invalid unaligned addresses?
[ ... ]
> +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, ®_val);
Is it necessary to acquire st->lock before performing this regmap_read()?
Looking at ad4691_single_shot_read(), it holds the lock during the conversion
sequence, including the fsleep() wait for the conversion to complete. If a
user reads the sampling frequency concurrently, could this SPI transaction
interleave with the conversion sequence?
(Note: I see this is fixed in a later commit that adds oversampling support by
removing the SPI read entirely, but mentioning it here for the current patch).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v10 2/6] iio: adc: ad4691: add initial driver for AD4691 family
2026-05-11 11:54 ` [PATCH v10 2/6] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
2026-05-12 4:09 ` sashiko-bot
@ 2026-05-12 15:25 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2026-05-12 15:25 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,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-pwm, linux-gpio, linux-doc
On Mon, 11 May 2026 14:54:14 +0300
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.
>
> IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_shared_by_all because
> the AD4691 family has a single internal oscillator whose frequency
> register is shared across all channels. Writing sampling_frequency for
> any one channel necessarily changes the conversion rate for every other
> channel, so the shared annotation correctly reflects the hardware
> behaviour.
Sashiko correctly points out that this last paragraph doesn't correspond to
the driver. Needs an update.
https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38%40analog.com
Otherwise just a few more sashiko things inline and my
comments on whether they are true - I think they are in this case...
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 000000000000..5b72216bca80
> --- /dev/null
> +++ b/drivers/iio/adc/ad4691.c
> +
> +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;
> +
> + /* Set bit 15 to mark the operation as READ. */
> + put_unaligned_be16(0x8000 | 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_ACC_SAT_OVR_REG(15):
> + ret = spi_write_then_read(spi, tx, sizeof(tx), rx, 1);
> + if (ret)
> + return ret;
> + *val = rx[0];
> + return 0;
> + case AD4691_ACC_MASK_REG:
> + case AD4691_STD_SEQ_CONFIG:
> + case AD4691_AVG_IN(0) ... AD4691_AVG_IN(15):
> + ret = spi_write_then_read(spi, tx, sizeof(tx), rx, 2);
Just to check - (another sashiko one) is it a problem if via debugfs we
read addresses that aren't the base ones of these bigger reads?
> + 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, sizeof(tx), 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, sizeof(tx), rx, 4);
> + if (ret)
> + return ret;
> + *val = get_unaligned_be32(rx);
> + return 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)
I think this will all end up easier to follow if you add some checks in
this and the next one to exclude the latter parts of multi address registers.
Neater to do it here than in the read and write calls themselves.
> +{
> + 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 int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
> +{
> + unsigned int reg_val;
> + int ret;
> +
I think sashiko's comment on locking here is a false positive because you'll
always get the value from regcache. Maybe worth a comment on that though.
Sashiko suggests the direct read goes away later anyway.
> + ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
> + if (ret)
> + return ret;
> +
> + *val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
> + return IIO_VAL_INT;
> +}
> +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);
This is where we perhaps have too much freedom given the effective gaps
in register addresses due for the larger 'registers'.
Using those wrong addresses might not be a problem belong filling the
regcache with garbage. If we can just screen them out in the readable
writeable checks that would be better.
> +}
...
> +static int ad4691_config(struct ad4691_state *st)
> +{
> + struct device *dev = regmap_get_device(st->regmap);
> + enum ad4691_ref_ctrl ref_val;
> + unsigned int 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);
> + }
> +
> + val = FIELD_PREP(AD4691_REF_CTRL_MASK, ref_val);
> + if (st->refbuf_en)
> + val |= AD4691_REFBUF_EN;
> +
> + ret = regmap_write(st->regmap, AD4691_REF_CTRL, val);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to write REF_CTRL\n");
> +
> + ret = regmap_assign_bits(st->regmap, AD4691_DEVICE_SETUP,
> + AD4691_LDO_EN, st->ldo_en);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to write DEVICE_SETUP\n");
> +
> + /*
> + * 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,
As per the comment above - I think this is why we don't have a bug reading
back the frequency during an ADC sampling sequence. This ensures we have
the value cached.
> + ad4691_samp_freq_start(st->info));
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
> +
> + ret = regmap_update_bits(st->regmap, AD4691_ADC_SETUP,
> + AD4691_ADC_MODE_MASK, AD4691_AUTONOMOUS_MODE);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to write ADC_SETUP\n");
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
2026-05-11 11:54 ` [PATCH v10 1/6] dt-bindings: iio: adc: add AD4691 family Radu Sabau via B4 Relay
2026-05-11 11:54 ` [PATCH v10 2/6] iio: adc: ad4691: add initial driver for " Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 6:03 ` sashiko-bot
2026-05-12 15:45 ` Jonathan Cameron
2026-05-11 11:54 ` [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
` (2 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, 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 active channel, densely paccked in scan_index order,
followed by a timestamp.
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 | 563 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 553 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 5e601a87e5f3..484363458658 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -143,6 +143,8 @@ config AD4691
tristate "Analog Devices AD4691 Family ADC Driver"
depends on SPI
depends on REGULATOR || COMPILE_TEST
+ 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 5b72216bca80..b295a26d1309 100644
--- a/drivers/iio/adc/ad4691.c
+++ b/drivers/iio/adc/ad4691.c
@@ -5,24 +5,35 @@
*/
#include <linux/array_size.h>
#include <linux/bitfield.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/dev_printk.h>
#include <linux/device/devres.h>
+#include <linux/dmaengine.h>
#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kstrtox.h>
#include <linux/limits.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>
+#include <linux/string.h>
#include <linux/spi/spi.h>
#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
@@ -31,6 +42,9 @@
#define AD4691_VREF_3P3_uV_MAX 3750000
#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))
@@ -38,6 +52,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)
@@ -45,13 +60,18 @@
#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_ADC_MODE_MASK GENMASK(1, 0)
+#define AD4691_CNV_BURST_MODE 0x01
#define AD4691_AUTONOMOUS_MODE 0x02
/*
* ACC_MASK_REG covers both mask bytes via ADDR_DESCENDING SPI: writing a
@@ -61,6 +81,8 @@
#define AD4691_ACC_DEPTH_IN(n) (0x186 + (n))
#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
@@ -130,6 +152,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[] = {
@@ -141,6 +164,17 @@ static const struct iio_chan_spec ad4693_channels[] = {
AD4691_CHANNEL(5),
AD4691_CHANNEL(6),
AD4691_CHANNEL(7),
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const struct ad4691_channel_info ad4691_sw_info = {
+ .channels = ad4691_channels,
+ .num_channels = ARRAY_SIZE(ad4691_channels),
+};
+
+static const struct ad4691_channel_info ad4693_sw_info = {
+ .channels = ad4693_channels,
+ .num_channels = ARRAY_SIZE(ad4693_channels),
};
/*
@@ -167,15 +201,7 @@ static const int ad4691_osc_freqs_Hz[] = {
[0xF] = 1250,
};
-static const struct ad4691_channel_info ad4691_sw_info = {
- .channels = ad4691_channels,
- .num_channels = ARRAY_SIZE(ad4691_channels),
-};
-
-static const struct ad4691_channel_info ad4693_sw_info = {
- .channels = ad4693_channels,
- .num_channels = ARRAY_SIZE(ad4693_channels),
-};
+static const char * const ad4691_gp_names[] = { "gp0", "gp1", "gp2", "gp3" };
static const struct ad4691_chip_info ad4691_chip_info = {
.name = "ad4691",
@@ -204,7 +230,14 @@ static const struct ad4691_chip_info ad4694_chip_info = {
struct ad4691_state {
const struct ad4691_chip_info *info;
struct regmap *regmap;
+ struct spi_device *spi;
+
+ struct pwm_device *conv_trigger;
+ int irq;
int vref_uV;
+ u32 cnv_period_ns;
+
+ bool manual_mode;
bool refbuf_en;
bool ldo_en;
/*
@@ -212,8 +245,56 @@ struct ad4691_state {
* atomicity of consecutive SPI operations.
*/
struct mutex lock;
+ /*
+ * Per-buffer-enable lifetime resources:
+ * 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.
+ */
+ struct spi_message scan_msg;
+ /*
+ * max 16 + 1 NOOP (manual) or 2*16 + 1 state-reset (CNV burst).
+ */
+ struct spi_transfer scan_xfers[34];
+ /*
+ * CNV burst: 16 AVG_IN addresses = 16. Manual: 16 channel cmds +
+ * 1 NOOP = 17. Stored as native u16; put_unaligned_be16() fills each
+ * slot so the SPI controller (bits_per_word=8) sends bytes MSB-first.
+ */
+ u16 scan_tx[17] __aligned(IIO_DMA_MINALIGN);
+ /*
+ * CNV burst state-reset: 4-byte write [addr_hi, addr_lo,
+ * STATE_RESET_ALL, OSC_EN=1]. CS is asserted throughout, so
+ * ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as a
+ * deliberate side-write, keeping the oscillator enabled. Shared
+ * with the offload path (mutually exclusive at probe).
+ */
+ u8 scan_tx_reset[4] __aligned(IIO_DMA_MINALIGN);
+ /*
+ * Scan buffer: one BE16 slot per active channel, plus timestamp.
+ * DMA-aligned because scan_xfers point rx_buf directly into vals[].
+ */
+ IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, vals, 16);
};
+/*
+ * 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 bit_off = gp_num % 2;
+ unsigned int reg_off = gp_num / 2;
+ unsigned int shift = 4 * bit_off;
+
+ return regmap_update_bits(st->regmap,
+ AD4691_GPIO_MODE1_REG + reg_off,
+ AD4691_GP_MODE_MASK << shift,
+ AD4691_GP_MODE_DATA_READY << shift);
+}
+
static int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
{
struct spi_device *spi = context;
@@ -509,13 +590,384 @@ 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, unsigned int freq)
+{
+ if (!freq)
+ return -EINVAL;
+
+ st->cnv_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, freq);
+ return 0;
+}
+
+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,
+ };
+
+ 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_update_bits(st->regmap, AD4691_ADC_SETUP,
+ AD4691_ADC_MODE_MASK, 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_update_bits(st->regmap, AD4691_ADC_SETUP,
+ AD4691_ADC_MODE_MASK, AD4691_AUTONOMOUS_MODE);
+}
+
+static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+ unsigned int k, i;
+ bool first;
+ int ret;
+
+ memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
+ memset(st->scan_tx, 0, sizeof(st->scan_tx));
+
+ spi_message_init(&st->scan_msg);
+
+ first = true;
+ k = 0;
+ iio_for_each_active_channel(indio_dev, i) {
+ /*
+ * Channel-select command occupies the first (high) byte of the
+ * 16-bit DIN frame; the second byte is a don't-care zero pad.
+ * put_unaligned_be16() writes [cmd, 0x00] in memory so the
+ * SPI controller sends the command byte first on the wire.
+ */
+ put_unaligned_be16((u16)(AD4691_ADC_CHAN(i) << 8), &st->scan_tx[k]);
+ st->scan_xfers[k].tx_buf = &st->scan_tx[k];
+ /*
+ * The pipeline means xfer[0] receives the residual from the
+ * previous sequence, not a valid sample for channel i. Point
+ * it at vals[0]; xfer[1] (or the NOOP when only one channel
+ * is active) will overwrite that slot with the real result.
+ * Subsequent transfers write into vals[k-1] so each result
+ * lands at the next dense slot, giving a contiguous buffer
+ * for iio_push_to_buffers_with_ts().
+ */
+ if (first) {
+ st->scan_xfers[k].rx_buf = &st->vals[0];
+ first = false;
+ } else {
+ st->scan_xfers[k].rx_buf = &st->vals[k - 1];
+ }
+ st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[k].cs_change = 1;
+ spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
+ k++;
+ }
+
+ /* Final NOOP transfer retrieves the last channel's result. */
+ st->scan_xfers[k].tx_buf = &st->scan_tx[k]; /* scan_tx[k] == 0 == NOOP */
+ st->scan_xfers[k].rx_buf = &st->vals[k - 1];
+ st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
+ spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
+
+ ret = spi_optimize_message(st->spi, &st->scan_msg);
+ if (ret)
+ return ret;
+
+ ret = ad4691_enter_conversion_mode(st);
+ if (ret) {
+ spi_unoptimize_message(&st->scan_msg);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad4691_manual_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad4691_exit_conversion_mode(st);
+ spi_unoptimize_message(&st->scan_msg);
+ 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);
+ unsigned int acc_mask;
+ unsigned int k, i;
+ int ret;
+
+ memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
+ memset(st->scan_tx, 0, sizeof(st->scan_tx));
+
+ 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) {
+ put_unaligned_be16(0x8000 | AD4691_AVG_IN(i), &st->scan_tx[k]);
+ st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
+ st->scan_xfers[2 * k].len = sizeof(st->scan_tx[k]);
+ spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
+ st->scan_xfers[2 * k + 1].rx_buf = &st->vals[k];
+ st->scan_xfers[2 * k + 1].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[2 * k + 1].cs_change = 1;
+ spi_message_add_tail(&st->scan_xfers[2 * k + 1], &st->scan_msg);
+ k++;
+ }
+
+ /*
+ * Append a 4-byte state-reset transfer [addr_hi, addr_lo,
+ * STATE_RESET_ALL, OSC_EN=1]. CS is asserted throughout, so
+ * ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as a
+ * deliberate side-write, keeping the oscillator enabled.
+ */
+ put_unaligned_be16(AD4691_STATE_RESET_REG, st->scan_tx_reset);
+ st->scan_tx_reset[2] = AD4691_STATE_RESET_ALL;
+ st->scan_tx_reset[3] = 1;
+ st->scan_xfers[2 * k].tx_buf = st->scan_tx_reset;
+ st->scan_xfers[2 * k].len = sizeof(st->scan_tx_reset);
+ st->scan_xfers[2 * k].cs_change = 1;
+ spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
+
+ ret = spi_optimize_message(st->spi, &st->scan_msg);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+ bitmap_read(indio_dev->active_scan_mask, 0,
+ iio_get_masklength(indio_dev)));
+ if (ret)
+ goto err_unoptimize;
+
+ acc_mask = ~bitmap_read(indio_dev->active_scan_mask, 0,
+ iio_get_masklength(indio_dev)) & GENMASK(15, 0);
+ ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG, acc_mask);
+ if (ret)
+ goto err_unoptimize;
+
+ ret = ad4691_enter_conversion_mode(st);
+ if (ret)
+ goto err_unoptimize;
+
+ return 0;
+
+err_unoptimize:
+ spi_unoptimize_message(&st->scan_msg);
+ return ret;
+}
+
+static int ad4691_cnv_burst_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+ int ret;
+
+ /*
+ * Start the PWM and unmask the IRQ here in postenable, not in
+ * preenable. The IIO core attaches the trigger poll function between
+ * preenable and postenable; enabling sampling or unmasking the IRQ
+ * before that point risks a DATA_READY assertion landing before the
+ * poll function is registered. iio_trigger_poll() would drop the
+ * event, disable_irq_nosync() would fire, and enable_irq() would
+ * never be called, leaving the IRQ permanently masked.
+ */
+ ret = ad4691_sampling_enable(st, true);
+ if (ret)
+ return ret;
+
+ enable_irq(st->irq);
+ return 0;
+}
+
+static int ad4691_cnv_burst_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+
+ disable_irq(st->irq);
+ return ad4691_sampling_enable(st, false);
+}
+
+static int ad4691_cnv_burst_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad4691_exit_conversion_mode(st);
+ spi_unoptimize_message(&st->scan_msg);
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {
+ .preenable = &ad4691_cnv_burst_buffer_preenable,
+ .postenable = &ad4691_cnv_burst_buffer_postenable,
+ .predisable = &ad4691_cnv_burst_buffer_predisable,
+ .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);
+
+ return sysfs_emit(buf, "%lu\n", 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);
+ unsigned int freq;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &freq);
+ if (ret)
+ return ret;
+
+ IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ if (IIO_DEV_ACQUIRE_FAILED(claim))
+ return -EBUSY;
+
+ ret = ad4691_set_pwm_freq(st, freq);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR_RW(sampling_frequency, 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);
+
+ /*
+ * Disable before polling so the IRQ cannot be re-enabled by the
+ * trigger handler before we disable it here. Keep the DATA_READY IRQ
+ * disabled until the trigger handler has finished reading the scan,
+ * to prevent a new assertion mid-transfer. The PWM continues running
+ * uninterrupted; the IRQ is re-enabled in ad4691_trigger_handler once
+ * spi_sync completes.
+ *
+ * IRQF_ONESHOT already masks the hardware line during this threaded
+ * handler, so disable_irq_nosync here ensures the IRQ stays disabled
+ * even after IRQF_ONESHOT unmasks on return.
+ */
+ disable_irq_nosync(st->irq);
+ iio_trigger_poll(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static const struct iio_trigger_ops ad4691_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static int ad4691_read_scan(struct iio_dev *indio_dev, s64 ts)
+{
+ struct ad4691_state *st = iio_priv(indio_dev);
+ int ret;
+
+ guard(mutex)(&st->lock);
+
+ ret = spi_sync(st->spi, &st->scan_msg);
+ if (ret)
+ return ret;
+
+ /*
+ * rx_buf pointers in scan_xfers point directly into scan.vals, so no
+ * copy is needed. The scan_msg already includes a STATE_RESET at the
+ * end (appended in preenable), so no explicit reset is needed here.
+ */
+ iio_push_to_buffers_with_ts(indio_dev, st->vals, sizeof(st->vals), ts);
+ return 0;
+}
+
+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);
+
+ ad4691_read_scan(indio_dev, pf->timestamp);
+ if (!st->manual_mode)
+ enable_irq(st->irq);
+ 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,
.read_avail = &ad4691_read_avail,
.debugfs_reg_access = &ad4691_reg_access,
+ .validate_trigger = iio_validate_own_trigger,
};
+static int ad4691_pwm_setup(struct ad4691_state *st)
+{
+ struct device *dev = regmap_get_device(st->regmap);
+
+ 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");
+
+ 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);
@@ -598,6 +1050,22 @@ static int ad4691_config(struct ad4691_state *st)
unsigned int val;
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;
@@ -651,6 +1119,75 @@ static int ad4691_config(struct ad4691_state *st)
return 0;
}
+static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
+ struct ad4691_state *st)
+{
+ struct device *dev = regmap_get_device(st->regmap);
+ struct iio_trigger *trig;
+ unsigned int i;
+ int irq, ret;
+
+ trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+ iio_device_id(indio_dev));
+ if (!trig)
+ return -ENOMEM;
+
+ trig->ops = &ad4691_trigger_ops;
+ iio_trigger_set_drvdata(trig, st);
+
+ ret = devm_iio_trigger_register(dev, trig);
+ if (ret)
+ return dev_err_probe(dev, ret, "IIO trigger register failed\n");
+
+ indio_dev->trig = iio_trigger_get(trig);
+
+ if (st->manual_mode)
+ return devm_iio_triggered_buffer_setup(dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad4691_trigger_handler,
+ &ad4691_manual_buffer_setup_ops);
+
+ /*
+ * The GP pin named in interrupt-names 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 = -ENXIO;
+ for (i = 0; i < ARRAY_SIZE(ad4691_gp_names); i++) {
+ irq = fwnode_irq_get_byname(dev_fwnode(dev),
+ ad4691_gp_names[i]);
+ if (irq > 0 || irq == -EPROBE_DEFER)
+ break;
+ }
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "failed to get GP interrupt\n");
+
+ st->irq = irq;
+
+ ret = ad4691_gpio_setup(st, i);
+ if (ret)
+ return ret;
+
+ /*
+ * IRQ is kept disabled until the buffer is enabled to prevent
+ * spurious DATA_READY events before the SPI message is set up.
+ */
+ 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);
+}
+
static int ad4691_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
@@ -663,6 +1200,7 @@ static int ad4691_probe(struct spi_device *spi)
return -ENOMEM;
st = iio_priv(indio_dev);
+ st->spi = spi;
st->info = spi_get_device_match_data(spi);
if (!st->info)
return -ENODEV;
@@ -692,8 +1230,9 @@ static int ad4691_probe(struct spi_device *spi)
indio_dev->info = &ad4691_info;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = st->info->sw_info->channels;
- indio_dev->num_channels = st->info->sw_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] 16+ messages in thread* Re: [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support
2026-05-11 11:54 ` [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
@ 2026-05-12 6:03 ` sashiko-bot
2026-05-12 15:45 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 6:03 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: krzk+dt, robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [Critical] Heap buffer overflow in SPI transfer array when the IIO timestamp channel is enabled.
- [High] Permanent IRQ masking deadlock due to top-half/bottom-half trigger re-entrancy race.
- [High] Manual Mode is non-functional because external triggers are strictly rejected.
- [High] Data corruption on Little-Endian architectures due to missing IIO_BE endianness.
- [High] Overlapping DMA mappings of the same memory address in Manual Mode SPI message.
- [High] Asynchronous `STATE_RESET_ALL` command destroys accumulated samples.
- [High] Incorrect SPI chip select state left asserted due to `cs_change = 1` on final transfer.
- [Medium] Memory leak of IIO trigger object on probe error paths.
- [Medium] Timestamp jitter and incorrect context due to threaded IRQ for IIO trigger polling.
- [Medium] Missing CS change delay in Manual Mode violates CNV high time requirements.
--
commit b68e9561666e82fd6a3730608ccc29e5b0af1991
Author: Radu Sabau <radu.sabau@analog.com>
iio: adc: ad4691: add triggered buffer support
Add buffered capture support using the IIO triggered buffer framework.
CNV Burst Mode uses a PWM to drive CNV and an IRQ for DATA_READY. Manual
Mode ties CNV to CS and requires an external trigger.
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 5b72216bca80e..b295a26d13093 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
[ ... ]
> @@ -212,8 +245,56 @@ struct ad4691_state {
[ ... ]
> + /*
> + * Scan buffer: one BE16 slot per active channel, plus timestamp.
> + * DMA-aligned because scan_xfers point rx_buf directly into vals[].
> + */
> + IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, vals, 16);
> };
Does the IIO core know these values are big-endian?
Because the IIO_BE endianness flag is not set in the channel specification's
scan_type, the IIO core will default to native endianness. On little-endian
architectures, will this result in corrupted ADC values being passed to
userspace?
[ ... ]
> +static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
> +{
[ ... ]
> + iio_for_each_active_channel(indio_dev, i) {
If the soft timestamp channel is enabled, it is included in the
active_scan_mask. Will this loop execute 17 times when all 16 hardware
channels and the timestamp are active?
If k reaches 16, appending the final NOOP transfer below writes to
st->scan_xfers[17] and st->scan_tx[17]. For CNV burst mode, appending the
final state-reset transfer writes to st->scan_xfers[34].
Does this overflow the strictly bounded arrays scan_tx[17] and scan_xfers[34],
corrupting adjacent memory on the heap?
[ ... ]
> + if (first) {
> + st->scan_xfers[k].rx_buf = &st->vals[0];
> + first = false;
> + } else {
> + st->scan_xfers[k].rx_buf = &st->vals[k - 1];
> + }
When k=1, does xfer[1].rx_buf point to the exact same memory address
as xfer[0].rx_buf (&st->vals[0])?
Mapping the same physical memory region multiple times for DMA_FROM_DEVICE in a
single transaction violates the DMA API. On systems using swiotlb or
non-coherent caches, could the unmapping of xfer[0] race with and overwrite
the valid sample received in xfer[1]?
> + st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
> + st->scan_xfers[k].cs_change = 1;
Does this CS toggle meet the ADC's minimum CNV high time requirements?
The AD4691 requires a minimum CNV high time of 430ns, but no CS inactive delay
is specified here. Will this rely entirely on the SPI controller's default
deassertion time, potentially violating hardware timing and corrupting
conversions?
[ ... ]
> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
[ ... ]
> + /*
> + * Append a 4-byte state-reset transfer [addr_hi, addr_lo,
> + * STATE_RESET_ALL, OSC_EN=1]. CS is asserted throughout, so
> + * ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as a
> + * deliberate side-write, keeping the oscillator enabled.
> + */
> + put_unaligned_be16(AD4691_STATE_RESET_REG, st->scan_tx_reset);
> + st->scan_tx_reset[2] = AD4691_STATE_RESET_ALL;
The commit message notes that conversions are stopped during the SPI read, but
does the PWM continue to pulse and run conversions during the readout window?
If conversions for the next burst begin accumulating in the hardware
immediately, does this final STATE_RESET_ALL command wipe out the new
conversions that occurred during the readout window, causing dropped samples?
> + st->scan_tx_reset[3] = 1;
> + st->scan_xfers[2 * k].tx_buf = st->scan_tx_reset;
> + st->scan_xfers[2 * k].len = sizeof(st->scan_tx_reset);
> + st->scan_xfers[2 * k].cs_change = 1;
> + spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
Setting cs_change = 1 on the final transfer of a SPI message causes the chip
select to remain asserted indefinitely after the transaction completes.
Will this leave the SPI bus locked and break subsequent communications since
the ADC requires CS to transition high to frame the next command?
[ ... ]
> +static irqreturn_t ad4691_irq(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct ad4691_state *st = iio_priv(indio_dev);
> +
> + /*
> + * Disable before polling so the IRQ cannot be re-enabled by the
> + * trigger handler before we disable it here. Keep the DATA_READY IRQ
> + * disabled until the trigger handler has finished reading the scan,
> + * to prevent a new assertion mid-transfer. The PWM continues running
> + * uninterrupted; the IRQ is re-enabled in ad4691_trigger_handler once
> + * spi_sync completes.
> + *
> + * IRQF_ONESHOT already masks the hardware line during this threaded
> + * handler, so disable_irq_nosync here ensures the IRQ stays disabled
> + * even after IRQF_ONESHOT unmasks on return.
> + */
> + disable_irq_nosync(st->irq);
> + iio_trigger_poll(indio_dev->trig);
Can this manual IRQ masking cause a deadlock?
If an interrupt fires immediately after ad4691_trigger_handler() calls
enable_irq() but before it calls iio_trigger_notify_done(), ad4691_irq()
could execute again, disable the IRQ, and call iio_trigger_poll().
Because the IIO core's use_count is still > 0, the event would be dropped.
Since the trigger handler won't execute again, enable_irq() is never called,
leaving the interrupt permanently masked.
Could this be avoided by using the iio_trigger_ops->reenable callback instead?
[ ... ]
> 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,
> + .validate_trigger = iio_validate_own_trigger,
The commit message states that Manual Mode relies on an external trigger
like iio-trig-hrtimer. However, doesn't iio_validate_own_trigger strictly
ensure the device can only bind to its internally allocated trigger?
Since Manual Mode does not configure any IRQ to fire this internal trigger,
will users be completely blocked from driving the buffer in Manual Mode?
[ ... ]
> +static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
> + struct ad4691_state *st)
> +{
[ ... ]
> + indio_dev->trig = iio_trigger_get(trig);
> +
> + if (st->manual_mode)
> + return devm_iio_triggered_buffer_setup(dev, indio_dev,
> + &iio_pollfunc_store_time,
> + &ad4691_trigger_handler,
> + &ad4691_manual_buffer_setup_ops);
> +
> + /*
> + * The GP pin named in interrupt-names 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 = -ENXIO;
> + for (i = 0; i < ARRAY_SIZE(ad4691_gp_names); i++) {
> + irq = fwnode_irq_get_byname(dev_fwnode(dev),
> + ad4691_gp_names[i]);
If this function fails (e.g., fwnode_irq_get_byname returns -EPROBE_DEFER),
does it leak the trigger object?
Because INDIO_BUFFER_TRIGGERED is not yet set in indio_dev->modes, the IIO
core's iio_dev_release() will skip calling
iio_device_unregister_trigger_consumer(), meaning the reference acquired by
iio_trigger_get() above is never dropped.
[ ... ]
> + /*
> + * IRQ is kept disabled until the buffer is enabled to prevent
> + * spurious DATA_READY events before the SPI message is set up.
> + */
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + &ad4691_irq,
> + IRQF_ONESHOT | IRQF_NO_AUTOEN,
> + indio_dev->name, indio_dev);
Does capturing the hardware timestamp in a threaded IRQ inject unpredictable
scheduling latency?
The driver requests the DATA_READY interrupt with a NULL hardirq handler,
forcing ad4691_irq to run in a kernel thread. This calls iio_trigger_poll(),
which executes iio_pollfunc_store_time to capture the timestamp. This violates
the IIO core expectation that iio_trigger_poll runs in hardirq context, adding
jitter to the timestamp.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support
2026-05-11 11:54 ` [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
2026-05-12 6:03 ` sashiko-bot
@ 2026-05-12 15:45 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2026-05-12 15:45 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,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-pwm, linux-gpio, linux-doc
On Mon, 11 May 2026 14:54:15 +0300
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
> 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 active channel, densely paccked in scan_index order,
> followed by a timestamp.
>
> 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>
Sashiko pointed out you have a buffer that is big endian but
chan_spec doesn't reflect that. That should have generated obvious
garbage output (unless you are actually testing on a be machine!)
Various other things came up, some of which I thought were in previous
reviews - but maybe I'm confusing drivers.
Thanks
Jonathan
> @@ -204,7 +230,14 @@ static const struct ad4691_chip_info ad4694_chip_info = {
> struct ad4691_state {
> const struct ad4691_chip_info *info;
> struct regmap *regmap;
> + struct spi_device *spi;
> +
> + struct pwm_device *conv_trigger;
> + int irq;
> int vref_uV;
> + u32 cnv_period_ns;
> +
> + bool manual_mode;
> bool refbuf_en;
> bool ldo_en;
> /*
> @@ -212,8 +245,56 @@ struct ad4691_state {
> * atomicity of consecutive SPI operations.
> */
> struct mutex lock;
> + /*
> + * Per-buffer-enable lifetime resources:
> + * 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.
> + */
> + struct spi_message scan_msg;
> + /*
> + * max 16 + 1 NOOP (manual) or 2*16 + 1 state-reset (CNV burst).
> + */
> + struct spi_transfer scan_xfers[34];
> + /*
> + * CNV burst: 16 AVG_IN addresses = 16. Manual: 16 channel cmds +
> + * 1 NOOP = 17. Stored as native u16; put_unaligned_be16() fills each
> + * slot so the SPI controller (bits_per_word=8) sends bytes MSB-first.
> + */
> + u16 scan_tx[17] __aligned(IIO_DMA_MINALIGN);
> + /*
> + * CNV burst state-reset: 4-byte write [addr_hi, addr_lo,
> + * STATE_RESET_ALL, OSC_EN=1]. CS is asserted throughout, so
> + * ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as a
> + * deliberate side-write, keeping the oscillator enabled. Shared
> + * with the offload path (mutually exclusive at probe).
> + */
> + u8 scan_tx_reset[4] __aligned(IIO_DMA_MINALIGN);
> + /*
> + * Scan buffer: one BE16 slot per active channel, plus timestamp.
> + * DMA-aligned because scan_xfers point rx_buf directly into vals[].
> + */
> + IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, vals, 16);
As per comments elsewhere: If it is big endian why is the channel not marked
as such in the iio_chan_spec?
> };
> +
> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct ad4691_state *st = iio_priv(indio_dev);
> + unsigned int acc_mask;
> + unsigned int k, i;
> + int ret;
> +
> + memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
> + memset(st->scan_tx, 0, sizeof(st->scan_tx));
> +
> + 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) {
> + put_unaligned_be16(0x8000 | AD4691_AVG_IN(i), &st->scan_tx[k]);
> + st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
> + st->scan_xfers[2 * k].len = sizeof(st->scan_tx[k]);
> + spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
> + st->scan_xfers[2 * k + 1].rx_buf = &st->vals[k];
> + st->scan_xfers[2 * k + 1].len = sizeof(st->scan_tx[k]);
> + st->scan_xfers[2 * k + 1].cs_change = 1;
> + spi_message_add_tail(&st->scan_xfers[2 * k + 1], &st->scan_msg);
> + k++;
> + }
> +
> + /*
> + * Append a 4-byte state-reset transfer [addr_hi, addr_lo,
> + * STATE_RESET_ALL, OSC_EN=1]. CS is asserted throughout, so
> + * ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as a
> + * deliberate side-write, keeping the oscillator enabled.
> + */
> + put_unaligned_be16(AD4691_STATE_RESET_REG, st->scan_tx_reset);
> + st->scan_tx_reset[2] = AD4691_STATE_RESET_ALL;
> + st->scan_tx_reset[3] = 1;
> + st->scan_xfers[2 * k].tx_buf = st->scan_tx_reset;
> + st->scan_xfers[2 * k].len = sizeof(st->scan_tx_reset);
> + st->scan_xfers[2 * k].cs_change = 1;
Our old friend - cs_change = 1 is very rarely the right thing to do on a
final message. I thought this came up in an earlier version.
> + spi_message_add_tail(&st->scan_xfers[2 * k], &st->scan_msg);
> +
> + ret = spi_optimize_message(st->spi, &st->scan_msg);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> + bitmap_read(indio_dev->active_scan_mask, 0,
> + iio_get_masklength(indio_dev)));
> + if (ret)
> + goto err_unoptimize;
> +
> + acc_mask = ~bitmap_read(indio_dev->active_scan_mask, 0,
> + iio_get_masklength(indio_dev)) & GENMASK(15, 0);
> + ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG, acc_mask);
> + if (ret)
> + goto err_unoptimize;
> +
> + ret = ad4691_enter_conversion_mode(st);
> + if (ret)
> + goto err_unoptimize;
> +
> + return 0;
> +
> +err_unoptimize:
> + spi_unoptimize_message(&st->scan_msg);
> + return ret;
> +}
> +
> +static const struct iio_trigger_ops ad4691_trigger_ops = {
> + .validate_device = iio_trigger_validate_own_device,
Same thing about manual mode as mentioned below.
> +};
> +
> +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);
> +
> + ad4691_read_scan(indio_dev, pf->timestamp);
> + if (!st->manual_mode)
> + enable_irq(st->irq);
Maybe it was a different driver but I thought I commented on this before.
There are a bunch of races if you reenable this here - needs to be
in the trigger reenable callback.
(Sashiko is pointing this out as well with more detail on what those
races are) The short story is that you can race and have a trigger between
the enable and the notify_done which will be dropped on the floor meaning
we never get in here again - IIRC there is (rather convoluted) code to handle that
corner case in via the reenable callback and a work item.
> + 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,
> .read_avail = &ad4691_read_avail,
> .debugfs_reg_access = &ad4691_reg_access,
> + .validate_trigger = iio_validate_own_trigger,
Sashiko noted that this seems odd. If we only support our own
trigger how does manual mode work given it isn't registering one?
You will need to do something more clever to handle that.
Either pick between info structures or use custom validation callbacks.
> };
...
> @@ -651,6 +1119,75 @@ static int ad4691_config(struct ad4691_state *st)
> return 0;
> }
>
> +static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
> + struct ad4691_state *st)
> +{
> + struct device *dev = regmap_get_device(st->regmap);
> + struct iio_trigger *trig;
> + unsigned int i;
> + int irq, ret;
> +
> + trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!trig)
> + return -ENOMEM;
> +
> + trig->ops = &ad4691_trigger_ops;
> + iio_trigger_set_drvdata(trig, st);
> +
> + ret = devm_iio_trigger_register(dev, trig);
> + if (ret)
> + return dev_err_probe(dev, ret, "IIO trigger register failed\n");
> +
> + indio_dev->trig = iio_trigger_get(trig);
Sashiko correctly points out the general bug we have right now about leaking
trigger context on any error.
Need to solve that for all drivers though so don't worry about it here.
It did give more complex reasoning than normal however!
> +
> + if (st->manual_mode)
> + return devm_iio_triggered_buffer_setup(dev, indio_dev,
> + &iio_pollfunc_store_time,
> + &ad4691_trigger_handler,
> + &ad4691_manual_buffer_setup_ops);
> +
> + /*
> + * The GP pin named in interrupt-names 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 = -ENXIO;
> + for (i = 0; i < ARRAY_SIZE(ad4691_gp_names); i++) {
> + irq = fwnode_irq_get_byname(dev_fwnode(dev),
> + ad4691_gp_names[i]);
> + if (irq > 0 || irq == -EPROBE_DEFER)
> + break;
> + }
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "failed to get GP interrupt\n");
> +
> + st->irq = irq;
> +
> + ret = ad4691_gpio_setup(st, i);
> + if (ret)
> + return ret;
> +
> + /*
> + * IRQ is kept disabled until the buffer is enabled to prevent
> + * spurious DATA_READY events before the SPI message is set up.
> + */
> + 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);
> +}
> +
> static int ad4691_probe(struct spi_device *spi)
> {
> struct device *dev = &spi->dev;
> @@ -663,6 +1200,7 @@ static int ad4691_probe(struct spi_device *spi)
> return -ENOMEM;
>
> st = iio_priv(indio_dev);
> + st->spi = spi;
> st->info = spi_get_device_match_data(spi);
> if (!st->info)
> return -ENODEV;
> @@ -692,8 +1230,9 @@ static int ad4691_probe(struct spi_device *spi)
> indio_dev->info = &ad4691_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> - indio_dev->channels = st->info->sw_info->channels;
> - indio_dev->num_channels = st->info->sw_info->num_channels;
You've lost me here. Where are these now set?
> + ret = ad4691_setup_triggered_buffer(indio_dev, st);
> + if (ret)
> + return ret;
>
> return devm_iio_device_register(dev, indio_dev);
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
` (2 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v10 3/6] iio: adc: ad4691: add triggered buffer support Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 8:12 ` sashiko-bot
2026-05-12 15:49 ` Jonathan Cameron
2026-05-11 11:54 ` [PATCH v10 5/6] iio: adc: ad4691: add oversampling support Radu Sabau via B4 Relay
2026-05-11 11:54 ` [PATCH v10 6/6] docs: iio: adc: ad4691: add driver documentation Radu Sabau via B4 Relay
5 siblings, 2 replies; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, 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 16-bit 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 16-bit frames (bits_per_word=16, len=2).
The channel scan_type (storagebits=16, shift=0) is shared between the
software triggered-buffer and offload paths; no separate scan_type or
channel array is needed for the offload case at this stage. Oversampling
support and mode-specific channel array distinctions are introduced in
the following commit.
IIO_BUFFER_DMAENGINE is selected because the offload path uses
devm_iio_dmaengine_buffer_setup_with_handle() to allocate and
attach the DMA RX buffer to the IIO device.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/adc/Kconfig | 2 +
drivers/iio/adc/ad4691.c | 400 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 399 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 484363458658..44c8dbe3ff0d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -144,8 +144,10 @@ config AD4691
depends on SPI
depends on REGULATOR || COMPILE_TEST
select IIO_BUFFER
+ select IIO_BUFFER_DMAENGINE
select IIO_TRIGGERED_BUFFER
select REGMAP
+ select SPI_OFFLOAD
help
Say yes here to build support for Analog Devices AD4691 Family MuxSAR
SPI analog to digital converters (ADC).
diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
index b295a26d1309..011a3cd46da0 100644
--- a/drivers/iio/adc/ad4691.c
+++ b/drivers/iio/adc/ad4691.c
@@ -25,10 +25,14 @@
#include <linux/reset.h>
#include <linux/string.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>
@@ -44,6 +48,11 @@
#define AD4691_CNV_DUTY_CYCLE_NS 380
#define AD4691_CNV_HIGH_TIME_NS 430
+/*
+ * Conservative default for the manual offload periodic trigger. Low enough
+ * to work safely out of the box across all OSR and channel count combinations.
+ */
+#define AD4691_OFFLOAD_INITIAL_TRIGGER_HZ (100 * HZ_PER_KHZ)
#define AD4691_SPI_CONFIG_A_REG 0x000
#define AD4691_SW_RESET (BIT(7) | BIT(0))
@@ -115,6 +124,7 @@ struct ad4691_chip_info {
const char *name;
unsigned int max_rate;
const struct ad4691_channel_info *sw_info;
+ const struct ad4691_channel_info *offload_info;
};
#define AD4691_CHANNEL(ch) \
@@ -177,6 +187,18 @@ static const struct ad4691_channel_info ad4693_sw_info = {
.num_channels = ARRAY_SIZE(ad4693_channels),
};
+static const struct ad4691_channel_info ad4691_offload_info = {
+ .channels = ad4691_channels,
+ /* Exclude the soft timestamp entry; num_channels caps access. */
+ .num_channels = ARRAY_SIZE(ad4691_channels) - 1,
+};
+
+static const struct ad4691_channel_info ad4693_offload_info = {
+ .channels = ad4693_channels,
+ /* Exclude the soft timestamp entry; num_channels caps access. */
+ .num_channels = ARRAY_SIZE(ad4693_channels) - 1,
+};
+
/*
* 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
@@ -207,24 +229,34 @@ static const struct ad4691_chip_info ad4691_chip_info = {
.name = "ad4691",
.max_rate = 500 * HZ_PER_KHZ,
.sw_info = &ad4691_sw_info,
+ .offload_info = &ad4691_offload_info,
};
static const struct ad4691_chip_info ad4692_chip_info = {
.name = "ad4692",
.max_rate = 1 * HZ_PER_MHZ,
.sw_info = &ad4691_sw_info,
+ .offload_info = &ad4691_offload_info,
};
static const struct ad4691_chip_info ad4693_chip_info = {
.name = "ad4693",
.max_rate = 500 * HZ_PER_KHZ,
.sw_info = &ad4693_sw_info,
+ .offload_info = &ad4693_offload_info,
};
static const struct ad4691_chip_info ad4694_chip_info = {
.name = "ad4694",
.max_rate = 1 * HZ_PER_MHZ,
.sw_info = &ad4693_sw_info,
+ .offload_info = &ad4693_offload_info,
+};
+
+struct ad4691_offload_state {
+ struct spi_offload *offload;
+ struct spi_offload_trigger *trigger;
+ u64 trigger_hz;
};
struct ad4691_state {
@@ -259,8 +291,11 @@ struct ad4691_state {
struct spi_transfer scan_xfers[34];
/*
* CNV burst: 16 AVG_IN addresses = 16. Manual: 16 channel cmds +
- * 1 NOOP = 17. Stored as native u16; put_unaligned_be16() fills each
- * slot so the SPI controller (bits_per_word=8) sends bytes MSB-first.
+ * 1 NOOP = 17. Stored as native u16. The non-offload path fills slots
+ * with put_unaligned_be16() (bits_per_word=8, bytes go out in memory
+ * order). The offload path assigns native values directly
+ * (bits_per_word=bpw, SPI reads each slot as a native 16-bit word and
+ * shifts it out MSB-first).
*/
u16 scan_tx[17] __aligned(IIO_DMA_MINALIGN);
/*
@@ -276,6 +311,8 @@ struct ad4691_state {
* DMA-aligned because scan_xfers point rx_buf directly into vals[].
*/
IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, vals, 16);
+ /* NULL when no SPI offload hardware is present */
+ struct ad4691_offload_state *offload;
};
/*
@@ -295,6 +332,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, 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 int ad4691_reg_read(void *context, unsigned int reg, unsigned int *val)
{
struct spi_device *spi = context;
@@ -847,6 +924,223 @@ 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 ad4691_offload_state *offload = st->offload;
+ 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 bpw = indio_dev->channels[0].scan_type.realbits;
+ unsigned int bit, k;
+ int ret;
+
+ ret = ad4691_enter_conversion_mode(st);
+ if (ret)
+ return ret;
+
+ memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
+ memset(st->scan_tx, 0, sizeof(st->scan_tx));
+
+ /*
+ * 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]
+ * RX: [data_hi, data_lo] (storagebits=16, shift=0)
+ * Transfer 0 RX is garbage; transfers 1..N carry real data.
+ * scan_tx is reused for TX commands (mutually exclusive with the
+ * non-offload triggered-buffer path).
+ *
+ * bits_per_word=bpw: the SPI controller reads tx_buf as a native
+ * 16-bit word and shifts it out MSB-first. Store the exact 16-bit
+ * value we want on the wire as a plain native u16 — no endianness
+ * macro — so the wire bytes are correct on both LE and BE hosts.
+ * The channel-select command is a single byte; shift it to the MSB
+ * position so SPI sends it first, with a zero pad in the LSB.
+ */
+ k = 0;
+ iio_for_each_active_channel(indio_dev, bit) {
+ st->scan_tx[k] = (u16)(AD4691_ADC_CHAN(bit) << 8);
+ st->scan_xfers[k].tx_buf = &st->scan_tx[k];
+ st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[k].bits_per_word = bpw;
+ st->scan_xfers[k].cs_change = 1;
+ st->scan_xfers[k].cs_change_delay.value = AD4691_CNV_HIGH_TIME_NS;
+ st->scan_xfers[k].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+ /* First transfer RX is garbage — skip it. */
+ if (k > 0)
+ st->scan_xfers[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+ k++;
+ }
+
+ /* Final NOOP transfer retrieves the last channel's result. */
+ st->scan_xfers[k].tx_buf = &st->scan_tx[k]; /* scan_tx[k] == 0 == NOOP */
+ st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[k].bits_per_word = bpw;
+ st->scan_xfers[k].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+ k++;
+
+ spi_message_init_with_transfers(&st->scan_msg, st->scan_xfers, k);
+ st->scan_msg.offload = offload->offload;
+
+ ret = spi_optimize_message(spi, &st->scan_msg);
+ if (ret)
+ goto err_exit_conversion;
+
+ config.periodic.frequency_hz = offload->trigger_hz;
+ ret = spi_offload_trigger_enable(offload->offload, offload->trigger, &config);
+ if (ret)
+ goto err_unoptimize;
+
+ return 0;
+
+err_unoptimize:
+ spi_unoptimize_message(&st->scan_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);
+ struct ad4691_offload_state *offload = st->offload;
+
+ spi_offload_trigger_disable(offload->offload, offload->trigger);
+ spi_unoptimize_message(&st->scan_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 ad4691_offload_state *offload = st->offload;
+ 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 bpw = indio_dev->channels[0].scan_type.realbits;
+ unsigned int acc_mask;
+ unsigned int bit, k;
+ int ret;
+
+ ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
+ bitmap_read(indio_dev->active_scan_mask, 0,
+ iio_get_masklength(indio_dev)));
+ if (ret)
+ return ret;
+
+ acc_mask = ~bitmap_read(indio_dev->active_scan_mask, 0,
+ iio_get_masklength(indio_dev)) & GENMASK(15, 0);
+ ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG, acc_mask);
+ if (ret)
+ return ret;
+
+ ret = ad4691_enter_conversion_mode(st);
+ if (ret)
+ return ret;
+
+ memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
+ memset(st->scan_tx, 0, sizeof(st->scan_tx));
+
+ /*
+ * Each AVG_IN register read uses two transfers:
+ * TX: [reg_hi | 0x80, reg_lo] (address phase, CS stays asserted)
+ * RX: [data_hi, data_lo] (bpw-wide data phase, storagebits=16)
+ * Both TX and RX use bits_per_word=bpw: the SPI controller reads tx_buf
+ * as a native 16-bit word and shifts it out MSB-first. Store the exact
+ * 16-bit wire value as a plain native u16 — no endianness macro — so the
+ * wire bytes are correct on both LE and BE hosts. The read-address
+ * (0x8000 | reg) is already the 16-bit value we want on the wire.
+ * scan_tx is reused for TX addresses (mutually exclusive with the
+ * non-offload triggered-buffer path).
+ */
+ k = 0;
+ iio_for_each_active_channel(indio_dev, bit) {
+ st->scan_tx[k] = 0x8000 | AD4691_AVG_IN(bit);
+
+ /* TX: address phase, CS stays asserted into data phase */
+ st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
+ st->scan_xfers[2 * k].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[2 * k].bits_per_word = bpw;
+
+ /* RX: data phase, CS toggles after to delimit the next register op */
+ st->scan_xfers[2 * k + 1].len = sizeof(st->scan_tx[k]);
+ st->scan_xfers[2 * k + 1].bits_per_word = bpw;
+ st->scan_xfers[2 * k + 1].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
+ st->scan_xfers[2 * k + 1].cs_change = 1;
+ k++;
+ }
+
+ /*
+ * State reset: single 4-byte write [addr_hi, addr_lo, STATE_RESET_ALL,
+ * OSC_EN=1]. ADDR_DESCENDING writes byte[3]=1 to OSC_EN_REG (0x180) as
+ * a deliberate side-write, keeping the oscillator enabled.
+ * scan_tx_reset is shared with the non-offload path (len=4 here vs
+ * len=3 there) since the two paths are mutually exclusive at probe.
+ */
+ put_unaligned_be16(AD4691_STATE_RESET_REG, st->scan_tx_reset);
+ st->scan_tx_reset[2] = AD4691_STATE_RESET_ALL;
+ st->scan_tx_reset[3] = 1;
+ st->scan_xfers[2 * k].tx_buf = st->scan_tx_reset;
+ st->scan_xfers[2 * k].len = sizeof(st->scan_tx_reset);
+ /*
+ * 4-byte u8 buffer assembled with put_unaligned_be16(); leave
+ * bits_per_word at the default (8) so bytes go out in memory order.
+ */
+
+ spi_message_init_with_transfers(&st->scan_msg, st->scan_xfers, 2 * k + 1);
+ st->scan_msg.offload = offload->offload;
+
+ ret = spi_optimize_message(spi, &st->scan_msg);
+ if (ret)
+ goto err_exit_conversion;
+
+ ret = spi_offload_trigger_enable(offload->offload, offload->trigger, &config);
+ if (ret)
+ goto err_unoptimize;
+
+ ret = ad4691_sampling_enable(st, true);
+ if (ret)
+ goto err_disable_trigger;
+
+ return 0;
+
+err_disable_trigger:
+ spi_offload_trigger_disable(offload->offload, offload->trigger);
+err_unoptimize:
+ spi_unoptimize_message(&st->scan_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);
+ struct ad4691_offload_state *offload = st->offload;
+
+ ad4691_sampling_enable(st, false);
+ spi_offload_trigger_disable(offload->offload, offload->trigger);
+ spi_unoptimize_message(&st->scan_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)
@@ -854,6 +1148,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", READ_ONCE(st->offload->trigger_hz));
+
return sysfs_emit(buf, "%lu\n", NSEC_PER_SEC / st->cnv_period_ns);
}
@@ -874,6 +1171,20 @@ static ssize_t sampling_frequency_store(struct device *dev,
if (IIO_DEV_ACQUIRE_FAILED(claim))
return -EBUSY;
+ if (st->manual_mode && st->offload) {
+ 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;
+
+ WRITE_ONCE(st->offload->trigger_hz, config.periodic.frequency_hz);
+ return len;
+ }
+
ret = ad4691_set_pwm_freq(st, freq);
if (ret)
return ret;
@@ -1188,9 +1499,75 @@ static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
ad4691_buffer_attrs);
}
+static int ad4691_setup_offload(struct iio_dev *indio_dev,
+ struct ad4691_state *st,
+ struct spi_offload *spi_offload)
+{
+ struct device *dev = regmap_get_device(st->regmap);
+ struct ad4691_offload_state *offload;
+ struct dma_chan *rx_dma;
+ int ret;
+
+ offload = devm_kzalloc(dev, sizeof(*offload), GFP_KERNEL);
+ if (!offload)
+ return -ENOMEM;
+
+ offload->offload = spi_offload;
+ st->offload = offload;
+
+ if (st->manual_mode) {
+ offload->trigger =
+ devm_spi_offload_trigger_get(dev, offload->offload,
+ SPI_OFFLOAD_TRIGGER_PERIODIC);
+ if (IS_ERR(offload->trigger))
+ return dev_err_probe(dev, PTR_ERR(offload->trigger),
+ "Failed to get periodic offload trigger\n");
+
+ offload->trigger_hz = AD4691_OFFLOAD_INITIAL_TRIGGER_HZ;
+ } 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");
+
+ offload->trigger =
+ devm_spi_offload_trigger_get(dev, offload->offload,
+ SPI_OFFLOAD_TRIGGER_DATA_READY);
+ if (IS_ERR(offload->trigger))
+ return dev_err_probe(dev, PTR_ERR(offload->trigger),
+ "Failed to get DATA_READY offload trigger\n");
+ }
+
+ rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, offload->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;
+
+ ret = devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev, rx_dma,
+ IIO_BUFFER_DIRECTION_IN);
+ if (ret)
+ return ret;
+
+ indio_dev->buffer->attrs = ad4691_buffer_attrs;
+
+ return 0;
+}
+
static int ad4691_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct spi_offload *spi_offload;
struct iio_dev *indio_dev;
struct ad4691_state *st;
int ret;
@@ -1226,11 +1603,26 @@ static int ad4691_probe(struct spi_device *spi)
if (ret)
return ret;
+ spi_offload = devm_spi_offload_get(dev, spi, &ad4691_offload_config);
+ ret = PTR_ERR_OR_ZERO(spi_offload);
+ if (ret == -ENODEV)
+ spi_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;
- ret = ad4691_setup_triggered_buffer(indio_dev, st);
+ if (spi_offload) {
+ indio_dev->channels = st->info->offload_info->channels;
+ indio_dev->num_channels = st->info->offload_info->num_channels;
+ ret = ad4691_setup_offload(indio_dev, st, spi_offload);
+ } else {
+ indio_dev->channels = st->info->sw_info->channels;
+ indio_dev->num_channels = st->info->sw_info->num_channels;
+ ret = ad4691_setup_triggered_buffer(indio_dev, st);
+ }
if (ret)
return ret;
@@ -1268,3 +1660,5 @@ 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");
+MODULE_IMPORT_NS("IIO_DMAENGINE_BUFFER");
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support
2026-05-11 11:54 ` [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
@ 2026-05-12 8:12 ` sashiko-bot
2026-05-12 15:49 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 8:12 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: krzk+dt, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The software-triggered buffer and SPI offload paths produce memory buffers with differing endianness, but they share the exact same IIO channel `scan_type`, corrupting data on Little-Endian hosts for one of the paths.
--
commit 7a6cc358e39a6b61c1d3018b7172b6303e1f28e5
Author: Radu Sabau <radu.sabau@analog.com>
iio: adc: ad4691: add SPI offload support
This commit adds SPI offload support to enable DMA-based, CPU-independent
data acquisition using the SPI Engine offload framework. It supports both
CNV Burst Mode and Manual Mode, sharing the channel scan_type between the
software-triggered and offload paths.
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
[ ... ]
> @@ -847,6 +924,223 @@ static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {
[ ... ]
> +static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
[ ... ]
> + iio_for_each_active_channel(indio_dev, bit) {
> + st->scan_tx[k] = 0x8000 | AD4691_AVG_IN(bit);
> +
> + /* TX: address phase, CS stays asserted into data phase */
> + st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
> + st->scan_xfers[2 * k].len = sizeof(st->scan_tx[k]);
> + st->scan_xfers[2 * k].bits_per_word = bpw;
> +
> + /* RX: data phase, CS toggles after to delimit the next register op */
> + st->scan_xfers[2 * k + 1].len = sizeof(st->scan_tx[k]);
> + st->scan_xfers[2 * k + 1].bits_per_word = bpw;
> + st->scan_xfers[2 * k + 1].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> + st->scan_xfers[2 * k + 1].cs_change = 1;
> + k++;
> + }
Will this offload RX stream configuration result in different endianness
compared to the software-triggered path?
In the software-triggered path, the SPI RX uses the default bits_per_word
of 8, meaning the ADC's MSB-first 16-bit samples are stored byte-by-byte in
memory as Big-Endian.
However, here in the offload path, bits_per_word is set to 16. The SPI
engine will reconstruct the 16-bit frames, and the DMA controller will
write them to memory as native 16-bit words, which will be Little-Endian
on LE hosts.
Since both paths share the same IIO channel scan_type definition (which
lacks an explicit endianness and defaults to IIO_CPU), won't one of the
paths produce incorrectly decoded data for userspace on Little-Endian
systems?
It is understood that a subsequent commit ("iio: adc: ad4691: add
oversampling support") adds an endianness of IIO_BE to the channel
definitions to address the software path. Since the offload path still
shares these definitions, this would cause IIO to advertise the offload
data as Big-Endian even though it provides CPU-endian data.
It appears the driver might need separate channel arrays for the offload
path with endianness set to IIO_CPU, or it needs to byte-swap the software
buffer to CPU endianness before pushing it to IIO.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=4
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support
2026-05-11 11:54 ` [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
2026-05-12 8:12 ` sashiko-bot
@ 2026-05-12 15:49 ` Jonathan Cameron
1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2026-05-12 15:49 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,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-pwm, linux-gpio, linux-doc
On Mon, 11 May 2026 14:54:16 +0300
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 16-bit 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 16-bit frames (bits_per_word=16, len=2).
> The channel scan_type (storagebits=16, shift=0) is shared between the
> software triggered-buffer and offload paths; no separate scan_type or
> channel array is needed for the offload case at this stage. Oversampling
> support and mode-specific channel array distinctions are introduced in
> the following commit.
>
> IIO_BUFFER_DMAENGINE is selected because the offload path uses
> devm_iio_dmaengine_buffer_setup_with_handle() to allocate and
> attach the DMA RX buffer to the IIO device.
More stuff from sashiko - it also not noticed the BE marking is
in a later patch - but given you are doing 16bit words here likely
the chan spec needs to be different.
Hohum. It then seems to have timed out. Maybe that should be a new
rule - if your patch set is too big for sashiko it's too big and
complex for reviewers too ;)
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
I didn't really spot anything myself on this pass so just a question
of analysing the bot output and groaning at what I missed :(
Jonathan
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v10 5/6] iio: adc: ad4691: add oversampling support
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
` (3 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 21:26 ` sashiko-bot
2026-05-11 11:54 ` [PATCH v10 6/6] docs: iio: adc: ad4691: add driver documentation Radu Sabau via B4 Relay
5 siblings, 1 reply; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, Radu Sabau
From: Radu Sabau <radu.sabau@analog.com>
Add per-channel oversampling ratio (OSR) support for CNV burst mode.
The accumulator depth register (ACC_DEPTH_IN) is programmed with the
selected OSR at buffer enable time and before each single-shot read.
Supported OSR values: 1, 2, 4, 8, 16, 32.
Introduce AD4691_MANUAL_CHANNEL() for manual mode channels, which do
not expose the oversampling ratio attribute since OSR is not applicable
in that mode. A separate manual_channels array is added to
struct ad4691_channel_info and selected at probe time;
in_voltageN_sampling_frequency represents the effective output rate for
channel N, defined as osc_freq / osr[N]. The chip has one internal
oscillator shared by all channels; each channel independently
accumulates osr[N] oscillator cycles before producing a result.
Writing sampling_frequency computes needed_osc = freq * osr[N] and
snaps down to the largest oscillator table entry that satisfies both
osc <= needed_osc and osc % osr[N] == 0, guaranteeing an exact integer
read-back. The result is stored in target_osc_freq_Hz and written to
OSC_FREQ_REG at buffer enable and single-shot time, so sampling_frequency
and oversampling_ratio can be set in any order.
in_voltageN_sampling_frequency_available is computed dynamically from
the channel's current OSR, listing only oscillator table entries that
divide evenly by osr[N], expressed as effective rates. The list becomes
sparser as OSR increases, capping at max_rate / osr[N].
Writing oversampling_ratio stores the new OSR for that channel;
target_osc_freq_Hz is left unchanged. The effective rate read back via
in_voltageN_sampling_frequency becomes target_osc_freq_Hz / new_osr
automatically. The two attributes are orthogonal: sampling_frequency
controls the oscillator, oversampling_ratio controls the averaging depth.
OSR defaults to 1 (no accumulation) for all channels.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/adc/ad4691.c | 268 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 234 insertions(+), 34 deletions(-)
diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
index 011a3cd46da0..842c4012801c 100644
--- a/drivers/iio/adc/ad4691.c
+++ b/drivers/iio/adc/ad4691.c
@@ -25,6 +25,7 @@
#include <linux/reset.h>
#include <linux/string.h>
#include <linux/spi/spi.h>
+#include <linux/types.h>
#include <linux/spi/offload/consumer.h>
#include <linux/spi/offload/provider.h>
#include <linux/units.h>
@@ -117,6 +118,7 @@ enum ad4691_ref_ctrl {
struct ad4691_channel_info {
const struct iio_chan_spec *channels __counted_by_ptr(num_channels);
+ const struct iio_chan_spec *manual_channels __counted_by_ptr(num_channels);
unsigned int num_channels;
};
@@ -127,12 +129,39 @@ struct ad4691_chip_info {
const struct ad4691_channel_info *offload_info;
};
+/* CNV burst mode channel — exposes oversampling ratio. */
#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 = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .info_mask_separate_available = \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
+ 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, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+/*
+ * Manual mode channel — no oversampling ratio attribute. OSR is not
+ * supported in manual mode; ACC_DEPTH_IN is not configured during manual
+ * buffer enable.
+ */
+#define AD4691_MANUAL_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), \
@@ -177,25 +206,65 @@ static const struct iio_chan_spec ad4693_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(8),
};
+static const struct iio_chan_spec ad4691_manual_channels[] = {
+ AD4691_MANUAL_CHANNEL(0),
+ AD4691_MANUAL_CHANNEL(1),
+ AD4691_MANUAL_CHANNEL(2),
+ AD4691_MANUAL_CHANNEL(3),
+ AD4691_MANUAL_CHANNEL(4),
+ AD4691_MANUAL_CHANNEL(5),
+ AD4691_MANUAL_CHANNEL(6),
+ AD4691_MANUAL_CHANNEL(7),
+ AD4691_MANUAL_CHANNEL(8),
+ AD4691_MANUAL_CHANNEL(9),
+ AD4691_MANUAL_CHANNEL(10),
+ AD4691_MANUAL_CHANNEL(11),
+ AD4691_MANUAL_CHANNEL(12),
+ AD4691_MANUAL_CHANNEL(13),
+ AD4691_MANUAL_CHANNEL(14),
+ AD4691_MANUAL_CHANNEL(15),
+ IIO_CHAN_SOFT_TIMESTAMP(16),
+};
+
+static const struct iio_chan_spec ad4693_manual_channels[] = {
+ AD4691_MANUAL_CHANNEL(0),
+ AD4691_MANUAL_CHANNEL(1),
+ AD4691_MANUAL_CHANNEL(2),
+ AD4691_MANUAL_CHANNEL(3),
+ AD4691_MANUAL_CHANNEL(4),
+ AD4691_MANUAL_CHANNEL(5),
+ AD4691_MANUAL_CHANNEL(6),
+ AD4691_MANUAL_CHANNEL(7),
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+};
+
+static const int ad4691_oversampling_ratios[] = { 1, 2, 4, 8, 16, 32 };
+
static const struct ad4691_channel_info ad4691_sw_info = {
.channels = ad4691_channels,
+ .manual_channels = ad4691_manual_channels,
.num_channels = ARRAY_SIZE(ad4691_channels),
};
static const struct ad4691_channel_info ad4693_sw_info = {
.channels = ad4693_channels,
+ .manual_channels = ad4693_manual_channels,
.num_channels = ARRAY_SIZE(ad4693_channels),
};
static const struct ad4691_channel_info ad4691_offload_info = {
.channels = ad4691_channels,
- /* Exclude the soft timestamp entry; num_channels caps access. */
+ /*
+ * Offload paths share the SW channel arrays. num_channels caps access
+ * before the soft timestamp entry, so no separate array is needed.
+ */
+ .manual_channels = ad4691_manual_channels,
.num_channels = ARRAY_SIZE(ad4691_channels) - 1,
};
static const struct ad4691_channel_info ad4693_offload_info = {
.channels = ad4693_channels,
- /* Exclude the soft timestamp entry; num_channels caps access. */
+ .manual_channels = ad4693_manual_channels,
.num_channels = ARRAY_SIZE(ad4693_channels) - 1,
};
@@ -268,6 +337,19 @@ struct ad4691_state {
int irq;
int vref_uV;
u32 cnv_period_ns;
+ /*
+ * Snapped oscillator frequency (Hz) shared by all channels. Set when
+ * sampling_frequency or oversampling_ratio is written; written to
+ * OSC_FREQ_REG at buffer enable and single-shot time so both attributes
+ * can be set in any order. Reading in_voltageN_sampling_frequency
+ * returns target_osc_freq_Hz / osr[N] — the effective rate for that
+ * channel given its oversampling ratio.
+ */
+ u32 target_osc_freq_Hz;
+ /* Per-channel oversampling ratio; always 1 in manual mode. */
+ u8 osr[16];
+ /* Scratch buffer for read_avail SAMP_FREQ; content is OSR-dependent. */
+ int samp_freq_avail[16][ARRAY_SIZE(ad4691_osc_freqs_Hz)];
bool manual_mode;
bool refbuf_en;
@@ -341,8 +423,7 @@ 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;
+ return type == SPI_OFFLOAD_TRIGGER_DATA_READY && nargs == 1 && args[0] <= 3;
}
static int ad4691_offload_trigger_request(struct spi_offload_trigger *trigger,
@@ -501,6 +582,16 @@ static const struct regmap_config ad4691_regmap_config = {
.cache_type = REGCACHE_MAPLE,
};
+/* Write target_osc_freq_Hz to OSC_FREQ_REG. Called at use time. */
+static int ad4691_write_osc_freq(struct ad4691_state *st)
+{
+ for (unsigned int i = 0; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
+ if (ad4691_osc_freqs_Hz[i] == st->target_osc_freq_Hz)
+ return regmap_write(st->regmap, AD4691_OSC_FREQ_REG, i);
+ }
+ return -EINVAL;
+}
+
/*
* Index 0 in ad4691_osc_freqs_Hz is 1 MHz — valid only for AD4692/AD4694
* (max_rate == 1 MHz). AD4691/AD4693 cap at 500 kHz so their valid range
@@ -511,36 +602,58 @@ static unsigned int ad4691_samp_freq_start(const struct ad4691_chip_info *info)
return (info->max_rate == 1 * HZ_PER_MHZ) ? 0 : 1;
}
-static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
+/*
+ * Find the largest oscillator table entry that is both <= needed_osc and
+ * evenly divisible by osr (guaranteeing an integer effective rate on
+ * read-back). Returns 0 if no such entry exists in the chip's valid range.
+ */
+static unsigned int ad4691_find_osc_freq(struct ad4691_state *st,
+ unsigned int needed_osc,
+ unsigned int osr)
{
- unsigned int reg_val;
- int ret;
+ unsigned int start = ad4691_samp_freq_start(st->info);
- ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
- if (ret)
- return ret;
+ for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
+ if ((unsigned int)ad4691_osc_freqs_Hz[i] > needed_osc)
+ continue;
+ if (ad4691_osc_freqs_Hz[i] % osr)
+ continue;
+ return ad4691_osc_freqs_Hz[i];
+ }
+ return 0;
+}
- *val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
+static int ad4691_get_sampling_freq(struct ad4691_state *st, u8 osr, int *val)
+{
+ *val = st->target_osc_freq_Hz / osr;
return IIO_VAL_INT;
}
-static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
+static int ad4691_set_sampling_freq(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int freq)
{
struct ad4691_state *st = iio_priv(indio_dev);
- unsigned int start = ad4691_samp_freq_start(st->info);
+ unsigned int osr = st->osr[chan->channel];
+ unsigned int found;
IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
if (IIO_DEV_ACQUIRE_FAILED(claim))
return -EBUSY;
- for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
- if (ad4691_osc_freqs_Hz[i] != freq)
- continue;
- return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
- AD4691_OSC_FREQ_MASK, i);
- }
+ if (freq <= 0 || freq > st->info->max_rate / osr)
+ return -EINVAL;
- return -EINVAL;
+ found = ad4691_find_osc_freq(st, freq * osr, osr);
+ if (!found)
+ return -EINVAL;
+
+ /*
+ * Store the snapped oscillator frequency; OSC_FREQ_REG is written at
+ * buffer enable and single-shot time so that sampling_frequency and
+ * oversampling_ratio can be set in any order.
+ */
+ st->target_osc_freq_Hz = found;
+ return 0;
}
static int ad4691_read_avail(struct iio_dev *indio_dev,
@@ -552,10 +665,30 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
unsigned int start = ad4691_samp_freq_start(st->info);
switch (mask) {
- case IIO_CHAN_INFO_SAMP_FREQ:
- *vals = &ad4691_osc_freqs_Hz[start];
+ case IIO_CHAN_INFO_SAMP_FREQ: {
+ unsigned int osr = st->osr[chan->channel];
+ int n = 0;
+
+ /*
+ * Only oscillator frequencies evenly divisible by the channel's
+ * OSR yield an integer effective rate; expose those as effective
+ * rates (osc / osr) so the user works entirely in output-sample
+ * space.
+ */
+ for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
+ if (ad4691_osc_freqs_Hz[i] % osr)
+ continue;
+ st->samp_freq_avail[chan->channel][n++] = ad4691_osc_freqs_Hz[i] / osr;
+ }
+ *vals = st->samp_freq_avail[chan->channel];
*type = IIO_VAL_INT;
- *length = ARRAY_SIZE(ad4691_osc_freqs_Hz) - start;
+ *length = n;
+ return IIO_AVAIL_LIST;
+ }
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *vals = ad4691_oversampling_ratios;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(ad4691_oversampling_ratios);
return IIO_AVAIL_LIST;
default:
return -EINVAL;
@@ -566,7 +699,7 @@ 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, osc_idx, period_us;
+ unsigned int reg_val, period_us;
int ret;
guard(mutex)(&st->lock);
@@ -587,7 +720,12 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
if (ret)
return ret;
- ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
+ ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(chan->channel),
+ st->osr[chan->channel]);
+ if (ret)
+ return ret;
+
+ ret = ad4691_write_osc_freq(st);
if (ret)
return ret;
@@ -595,9 +733,12 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
if (ret)
return ret;
- osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
- /* Wait 2 oscillator periods for the conversion to complete. */
- period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
+ /*
+ * Wait osr + 1 oscillator periods: osr for accumulation, +1 for the
+ * pipeline margin (one extra period ensures the final result is ready).
+ */
+ period_us = DIV_ROUND_UP((st->osr[chan->channel] + 1) * USEC_PER_SEC,
+ st->target_osc_freq_Hz);
fsleep(period_us);
ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
@@ -632,7 +773,10 @@ static int ad4691_read_raw(struct iio_dev *indio_dev,
return ad4691_single_shot_read(indio_dev, chan, val);
}
case IIO_CHAN_INFO_SAMP_FREQ:
- return ad4691_get_sampling_freq(st, val);
+ return ad4691_get_sampling_freq(st, st->osr[chan->channel], val);
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *val = st->osr[chan->channel];
+ return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
*val = st->vref_uV / (MICRO / MILLI);
*val2 = chan->scan_type.realbits;
@@ -646,9 +790,40 @@ static int ad4691_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long mask)
{
+ struct ad4691_state *st = iio_priv(indio_dev);
+
switch (mask) {
case IIO_CHAN_INFO_SAMP_FREQ:
- return ad4691_set_sampling_freq(indio_dev, val);
+ return ad4691_set_sampling_freq(indio_dev, chan, val);
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
+ IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+ if (IIO_DEV_ACQUIRE_FAILED(claim))
+ return -EBUSY;
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(ad4691_oversampling_ratios); i++) {
+ unsigned int old_effective, found;
+
+ if (ad4691_oversampling_ratios[i] != val)
+ continue;
+ /*
+ * Snap target_osc_freq_Hz to the largest table entry
+ * that is both <= old_effective * new_osr and evenly
+ * divisible by new_osr, preserving an integer read-back
+ * of in_voltageN_sampling_frequency after the OSR change.
+ */
+ old_effective = st->target_osc_freq_Hz /
+ st->osr[chan->channel];
+ found = ad4691_find_osc_freq(st,
+ old_effective * (unsigned int)val,
+ val);
+ if (!found)
+ return -EINVAL;
+ st->target_osc_freq_Hz = found;
+ st->osr[chan->channel] = val;
+ return 0;
+ }
+ return -EINVAL;
+ }
default:
return -EINVAL;
}
@@ -703,6 +878,10 @@ static int ad4691_enter_conversion_mode(struct ad4691_state *st)
return regmap_update_bits(st->regmap, AD4691_DEVICE_SETUP,
AD4691_MANUAL_MODE, AD4691_MANUAL_MODE);
+ ret = ad4691_write_osc_freq(st);
+ if (ret)
+ return ret;
+
ret = regmap_update_bits(st->regmap, AD4691_ADC_SETUP,
AD4691_ADC_MODE_MASK, AD4691_CNV_BURST_MODE);
if (ret)
@@ -866,6 +1045,12 @@ static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
if (ret)
goto err_unoptimize;
+ iio_for_each_active_channel(indio_dev, i) {
+ ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(i), st->osr[i]);
+ if (ret)
+ goto err_unoptimize;
+ }
+
ret = ad4691_enter_conversion_mode(st);
if (ret)
goto err_unoptimize;
@@ -1045,6 +1230,12 @@ static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
if (ret)
return ret;
+ iio_for_each_active_channel(indio_dev, bit) {
+ ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(bit), st->osr[bit]);
+ if (ret)
+ return ret;
+ }
+
ret = ad4691_enter_conversion_mode(st);
if (ret)
return ret;
@@ -1422,6 +1613,8 @@ static int ad4691_config(struct ad4691_state *st)
if (ret)
return dev_err_probe(dev, ret, "Failed to write OSC_FREQ\n");
+ st->target_osc_freq_Hz = ad4691_osc_freqs_Hz[ad4691_samp_freq_start(st->info)];
+
ret = regmap_update_bits(st->regmap, AD4691_ADC_SETUP,
AD4691_ADC_MODE_MASK, AD4691_AUTONOMOUS_MODE);
if (ret)
@@ -1581,6 +1774,7 @@ static int ad4691_probe(struct spi_device *spi)
st->info = spi_get_device_match_data(spi);
if (!st->info)
return -ENODEV;
+ memset(st->osr, 1, sizeof(st->osr));
ret = devm_mutex_init(dev, &st->lock);
if (ret)
@@ -1615,11 +1809,17 @@ static int ad4691_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE;
if (spi_offload) {
- indio_dev->channels = st->info->offload_info->channels;
+ if (st->manual_mode)
+ indio_dev->channels = st->info->offload_info->manual_channels;
+ else
+ indio_dev->channels = st->info->offload_info->channels;
indio_dev->num_channels = st->info->offload_info->num_channels;
ret = ad4691_setup_offload(indio_dev, st, spi_offload);
} else {
- indio_dev->channels = st->info->sw_info->channels;
+ if (st->manual_mode)
+ indio_dev->channels = st->info->sw_info->manual_channels;
+ else
+ indio_dev->channels = st->info->sw_info->channels;
indio_dev->num_channels = st->info->sw_info->num_channels;
ret = ad4691_setup_triggered_buffer(indio_dev, st);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v10 5/6] iio: adc: ad4691: add oversampling support
2026-05-11 11:54 ` [PATCH v10 5/6] iio: adc: ad4691: add oversampling support Radu Sabau via B4 Relay
@ 2026-05-12 21:26 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 21:26 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: devicetree, robh, krzk+dt, conor+dt
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Buffer pre-enable loops iterate over the soft timestamp channel, causing an out-of-bounds array read and overwriting the GPIO configuration register.
- [High] Missing `.endianness = IIO_BE` in `AD4691_MANUAL_CHANNEL` macro.
- [Medium] Time-of-Check to Time-of-Use (TOCTOU) race in `ad4691_set_sampling_freq`.
- [Medium] Data race and unprotected shared state in `ad4691_read_avail`.
- [Medium] Unlocked reads in `ad4691_read_raw` lead to torn state reads.
- [Medium] Writing `oversampling_ratio` silently modifies the global oscillator frequency, contradicting the commit message.
--
commit 52e8aead1e278f9a7f54f1b813184c36fb2a964a
Author: Radu Sabau <radu.sabau@analog.com>
iio: adc: ad4691: add oversampling support
This commit adds per-channel oversampling ratio (OSR) support for CNV
burst mode on the AD4691. It introduces attributes for sampling frequency
and OSR, and splits channel specifications to handle manual mode where OSR
is not applicable.
> Writing oversampling_ratio stores the new OSR for that channel;
> target_osc_freq_Hz is left unchanged. The effective rate read back via
> in_voltageN_sampling_frequency becomes target_osc_freq_Hz / new_osr
> automatically. The two attributes are orthogonal: sampling_frequency
> controls the oscillator, oversampling_ratio controls the averaging depth.
Is this description accurate?
Looking at ad4691_write_raw() for IIO_CHAN_INFO_OVERSAMPLING_RATIO, the code
actively snaps target_osc_freq_Hz to a new value to preserve the previous
effective rate. Because target_osc_freq_Hz corresponds to a single global
hardware oscillator, modifying it here silently changes the effective sampling
rate of all other active channels.
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 011a3cd46da0d..842c4012801cc 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
[ ... ]
> @@ -127,12 +129,39 @@ struct ad4691_chip_info {
> const struct ad4691_channel_info *offload_info;
> };
>
[ ... ]
> +/*
> + * Manual mode channel — no oversampling ratio attribute. OSR is not
> + * supported in manual mode; ACC_DEPTH_IN is not configured during manual
> + * buffer enable.
> + */
> +#define AD4691_MANUAL_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, \
> }, \
> }
Are we missing the endianness assignment in this macro?
The regular AD4691_CHANNEL macro specifies an endianness of IIO_BE. Without
it here, the IIO core defaults to CPU endianness. Since the hardware
transmits MSB-first over SPI, userspace applications running on little-endian
hosts might interpret the manual mode buffer data incorrectly.
[ ... ]
> @@ -511,36 +602,58 @@ static unsigned int ad4691_samp_freq_start(const struct ad4691_chip_info *info)
> return (info->max_rate == 1 * HZ_PER_MHZ) ? 0 : 1;
> }
>
[ ... ]
> -static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
> +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int freq)
> {
> struct ad4691_state *st = iio_priv(indio_dev);
> - unsigned int start = ad4691_samp_freq_start(st->info);
> + unsigned int osr = st->osr[chan->channel];
> + unsigned int found;
>
> IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> if (IIO_DEV_ACQUIRE_FAILED(claim))
> return -EBUSY;
Could reading the oversampling ratio before acquiring the IIO direct mode
lock lead to a race condition?
If a concurrent write to the oversampling ratio attribute occurs, this
function will block on the lock. Once it unblocks, it proceeds with the
stale osr value to calculate and set the new target oscillator frequency,
which might not be evenly divisible by the newly set oversampling ratio.
[ ... ]
> @@ -552,10 +665,30 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
> unsigned int start = ad4691_samp_freq_start(st->info);
>
> switch (mask) {
> - case IIO_CHAN_INFO_SAMP_FREQ:
> - *vals = &ad4691_osc_freqs_Hz[start];
> + case IIO_CHAN_INFO_SAMP_FREQ: {
> + unsigned int osr = st->osr[chan->channel];
> + int n = 0;
> +
> + /*
> + * Only oscillator frequencies evenly divisible by the channel's
> + * OSR yield an integer effective rate; expose those as effective
> + * rates (osc / osr) so the user works entirely in output-sample
> + * space.
> + */
> + for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
> + if (ad4691_osc_freqs_Hz[i] % osr)
> + continue;
> + st->samp_freq_avail[chan->channel][n++] = ad4691_osc_freqs_Hz[i] / osr;
> + }
> + *vals = st->samp_freq_avail[chan->channel];
> + *type = IIO_VAL_INT;
> + *length = n;
> + return IIO_AVAIL_LIST;
Does populating and returning the shared scratch array without locking create
a data race?
Concurrent sysfs reads of the sampling frequency available attribute for the
same channel could result in multiple threads writing to the
st->samp_freq_avail array simultaneously.
[ ... ]
> @@ -632,7 +773,10 @@ static int ad4691_read_raw(struct iio_dev *indio_dev,
> return ad4691_single_shot_read(indio_dev, chan, val);
> }
> case IIO_CHAN_INFO_SAMP_FREQ:
> - return ad4691_get_sampling_freq(st, val);
> + return ad4691_get_sampling_freq(st, st->osr[chan->channel], val);
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *val = st->osr[chan->channel];
> + return IIO_VAL_INT;
Should these attribute reads be protected by a lock?
Since ad4691_write_raw() modifies both target_osc_freq_Hz and the channel
OSR sequentially, a concurrent unlocked read could observe the newly updated
oscillator frequency but the old oversampling ratio, returning an incorrect
effective sampling rate.
[ ... ]
> @@ -866,6 +1045,12 @@ static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> if (ret)
> goto err_unoptimize;
>
> + iio_for_each_active_channel(indio_dev, i) {
> + ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(i), st->osr[i]);
> + if (ret)
> + goto err_unoptimize;
> + }
> +
Can this loop iterate past the bounds of the osr array?
If the IIO soft timestamp channel is enabled, its bit is set with scan_index
16. The macro iterates over all enabled channels, causing this loop to access
st->osr[16], which exceeds the 16-element array.
Writing this out-of-bounds value using AD4691_ACC_DEPTH_IN(16) evaluates to
register 0x196, which is AD4691_GPIO_MODE1_REG. This could inadvertently
overwrite the device's GPIO configuration.
[ ... ]
> @@ -1045,6 +1230,12 @@ static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
> if (ret)
> return ret;
>
> + iio_for_each_active_channel(indio_dev, bit) {
> + ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(bit), st->osr[bit]);
> + if (ret)
> + return ret;
> + }
> +
Is it possible for the soft timestamp channel to be active here as well,
leading to the same out-of-bounds access on st->osr[bit]?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=5
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v10 6/6] docs: iio: adc: ad4691: add driver documentation
2026-05-11 11:54 [PATCH v10 0/6] iio: adc: ad4691: add driver for AD4691 multichannel SAR ADC family Radu Sabau via B4 Relay
` (4 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v10 5/6] iio: adc: ad4691: add oversampling support Radu Sabau via B4 Relay
@ 2026-05-11 11:54 ` Radu Sabau via B4 Relay
2026-05-12 21:29 ` sashiko-bot
5 siblings, 1 reply; 16+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-11 11:54 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, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc, Radu Sabau
From: Radu Sabau <radu.sabau@analog.com>
Add RST documentation for the AD4691 family ADC driver covering
supported devices, IIO channels, operating modes, oversampling,
reference voltage, LDO supply, reset, GP pins, SPI offload support,
and buffer data format.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
Documentation/iio/ad4691.rst | 206 +++++++++++++++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 1 +
3 files changed, 208 insertions(+)
diff --git a/Documentation/iio/ad4691.rst b/Documentation/iio/ad4691.rst
new file mode 100644
index 000000000000..6f719c5b21df
--- /dev/null
+++ b/Documentation/iio/ad4691.rst
@@ -0,0 +1,206 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+AD4691 driver
+=============
+
+ADC driver for Analog Devices Inc. AD4691 family of multichannel SAR ADCs.
+The module name is ``ad4691``.
+
+
+Supported devices
+=================
+
+The following chips are supported by this driver:
+
+* `AD4691 <https://www.analog.com/en/products/ad4691.html>`_ — 16-channel, 500 kSPS
+* `AD4692 <https://www.analog.com/en/products/ad4692.html>`_ — 16-channel, 1 MSPS
+* `AD4693 <https://www.analog.com/en/products/ad4693.html>`_ — 8-channel, 500 kSPS
+* `AD4694 <https://www.analog.com/en/products/ad4694.html>`_ — 8-channel, 1 MSPS
+
+
+IIO channels
+============
+
+Each physical ADC input maps to one IIO voltage channel. The AD4691 and AD4692
+expose 16 channels (``voltage0`` through ``voltage15``); the AD4693 and AD4694
+expose 8 channels (``voltage0`` through ``voltage7``).
+
+All channels share a common scale (``in_voltage_scale``), derived from the
+reference voltage. Each channel independently exposes:
+
+* ``in_voltageN_raw`` — single-shot ADC result
+* ``in_voltageN_sampling_frequency`` — per-channel effective output rate,
+ defined as the internal oscillator frequency divided by the channel's
+ oversampling ratio. Writing this attribute selects the nearest achievable
+ rate for the current OSR; the value read back reflects the actual rate after
+ snapping to the closest valid oscillator entry.
+* ``in_voltageN_sampling_frequency_available`` — list of achievable effective
+ rates for the channel's current oversampling ratio. The list updates
+ dynamically when the oversampling ratio changes.
+
+The following attributes are only available in CNV Burst Mode:
+
+* ``in_voltageN_oversampling_ratio`` — per-channel hardware oversampling depth;
+ see `Oversampling`_ below.
+* ``in_voltageN_oversampling_ratio_available`` — valid ratios: 1, 2, 4, 8, 16,
+ 32.
+
+
+Operating modes
+===============
+
+The driver supports two operating modes, selected automatically from the
+device tree at probe time.
+
+Manual Mode
+-----------
+
+Selected when no ``pwms`` property is present in the device tree. The CNV pin
+is tied to the SPI chip-select: every CS assertion triggers a conversion and
+returns the previous result. A user-defined IIO trigger (e.g. hrtimer trigger)
+drives the buffer.
+
+Oversampling is not supported in Manual Mode.
+
+CNV Burst Mode
+--------------
+
+Selected when a ``pwms`` property is present in the device tree. A PWM drives
+the CNV pin at the configured conversion rate. A GP pin wired to the SoC and
+declared in the device tree signals DATA_READY at the end of each burst,
+triggering a readout of all active channel results into the IIO buffer.
+
+The buffer output rate is controlled by the ``sampling_frequency`` attribute
+on the IIO buffer. In practice the PWM rate should be set low enough to allow
+the SPI readout to complete before the next conversion burst begins.
+
+Autonomous Mode (idle / single-shot)
+-------------------------------------
+
+When the IIO buffer is disabled, ``in_voltageN_raw`` reads perform a single
+conversion on the requested channel using the internal oscillator. The
+oscillator is started and stopped around each read to save power.
+
+
+Oversampling
+============
+
+In CNV Burst Mode each channel has an independent hardware accumulator that
+averages a configurable number of successive conversions. The result is always
+returned as a 16-bit mean, so ``realbits`` and ``storagebits`` are unaffected
+by the oversampling ratio. Valid ratios are 1, 2, 4, 8, 16 and 32; the default
+is 1 (no averaging). Oversampling is not supported in Manual Mode.
+
+.. code-block:: bash
+
+ # Set oversampling ratio to 16 on channel 0
+ echo 16 > /sys/bus/iio/devices/iio:device0/in_voltage0_oversampling_ratio
+
+ # Read the resulting effective sampling frequency
+ cat /sys/bus/iio/devices/iio:device0/in_voltage0_sampling_frequency
+
+Writing ``oversampling_ratio`` stores the new depth for that channel and
+snaps the internal oscillator to the largest valid table entry that is both
+less than or equal to ``old_effective_rate × new_osr`` and evenly divisible
+by ``new_osr``. This preserves an integer read-back of
+``in_voltageN_sampling_frequency`` after the change and keeps the oscillator
+as close as possible to the previous effective rate.
+
+All channels share one internal oscillator. Writing ``sampling_frequency`` for
+any channel updates the oscillator and therefore affects the effective rate
+read back from all other channels.
+
+
+Reference voltage
+=================
+
+The driver supports two reference configurations, mutually exclusive:
+
+* **External reference** (``ref-supply``): a voltage between 2.4 V and 5.25 V
+ supplied externally.
+* **Buffered internal reference** (``refin-supply``): an internal reference
+ buffer is enabled by the driver.
+
+Exactly one of ``ref-supply`` or ``refin-supply`` must be present in the
+device tree. The reference voltage determines the full-scale range reported
+via ``in_voltage_scale``.
+
+
+LDO supply
+==========
+
+The chip contains an internal LDO that powers part of the analog front-end.
+The supply configuration is mutually exclusive:
+
+* **External VDD** (``vdd-supply``): an external 1.8 V supply is used directly;
+ the internal LDO is disabled.
+* **Internal LDO** (``ldo-in-supply``): the internal LDO is enabled and fed
+ from the ``ldo-in`` regulator. Use this when no external 1.8 V VDD is present.
+
+Exactly one of ``vdd-supply`` or ``ldo-in-supply`` must be provided.
+
+
+Reset
+=====
+
+The driver supports two reset mechanisms:
+
+* **Hardware reset** (``reset-gpios`` in device tree): asserted at probe by
+ the reset controller framework.
+* **Software reset** (fallback when ``reset-gpios`` is absent): written
+ automatically at probe.
+
+
+GP pins and interrupts
+======================
+
+The chip exposes up to four general-purpose (GP) pins. In CNV Burst Mode
+(non-offload), one GP pin must be wired to an interrupt-capable SoC input and
+declared in the device tree using the ``interrupts`` and ``interrupt-names``
+properties. The ``interrupt-names`` value identifies which GP pin is used
+(``"gp0"`` through ``"gp3"``).
+
+Example device tree fragment::
+
+ adc@0 {
+ compatible = "adi,ad4692";
+ ...
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio0>;
+ interrupt-names = "gp0";
+ };
+
+
+SPI offload support
+===================
+
+When a SPI offload engine (e.g. the AXI SPI Engine) is present, the driver
+uses DMA-backed transfers for CPU-independent, high-throughput data capture.
+SPI offload is detected automatically at probe; if no offload hardware is
+available the driver falls back to the software triggered-buffer path.
+
+Two SPI offload sub-modes exist:
+
+CNV Burst offload
+-----------------
+
+Used when a ``pwms`` property is present and SPI offload is available. The PWM
+drives CNV at the configured rate; on DATA_READY the offload engine reads all
+active channel results and streams them directly to the IIO DMA buffer with no
+CPU involvement. The GP pin used as DATA_READY trigger is supplied by the
+trigger-source consumer at buffer enable time; no ``interrupt-names`` entry is
+required.
+
+Manual offload
+--------------
+
+Used when no ``pwms`` property is present and SPI offload is available. A
+periodic SPI offload trigger controls the conversion rate and the offload engine
+streams results directly to the IIO DMA buffer.
+
+The ``sampling_frequency`` attribute on the IIO buffer controls the trigger
+rate (in Hz). The initial rate is 100 kHz.
+
+Oversampling is not supported in Manual Mode.
+
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index ba3e609c6a13..007e0a1fcc5a 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -23,6 +23,7 @@ Industrial I/O Kernel Drivers
ad4000
ad4030
ad4062
+ ad4691
ad4695
ad7191
ad7380
diff --git a/MAINTAINERS b/MAINTAINERS
index 24e4502b8292..875ea2455d91 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: Documentation/iio/ad4691.rst
F: drivers/iio/adc/ad4691.c
ANALOG DEVICES INC AD4695 DRIVER
--
2.43.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v10 6/6] docs: iio: adc: ad4691: add driver documentation
2026-05-11 11:54 ` [PATCH v10 6/6] docs: iio: adc: ad4691: add driver documentation Radu Sabau via B4 Relay
@ 2026-05-12 21:29 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-05-12 21:29 UTC (permalink / raw)
To: Radu Sabau via B4 Relay; +Cc: robh, devicetree, krzk+dt, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to include documentation for the 'buffer data format', but this information is completely missing from the added RST file.
--
commit bf768cbe74081c6f8c0f23dda3ab5e9fc4930fd2
Author: Radu Sabau <radu.sabau@analog.com>
docs: iio: adc: ad4691: add driver documentation
This commit adds reStructuredText documentation for the Analog Devices
AD4691 family of multichannel SAR ADCs. It covers supported devices, IIO
channels, operating modes, and other hardware features.
> Add RST documentation for the AD4691 family ADC driver covering
> supported devices, IIO channels, operating modes, oversampling,
> reference voltage, LDO supply, reset, GP pins, SPI offload support,
> and buffer data format.
The commit message lists "buffer data format" as one of the topics covered
by the new documentation. Was this section accidentally left out of the
patch?
Looking through the added Documentation/iio/ad4691.rst file, there does
not appear to be any section or explanation detailing the buffer data format.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=6
^ permalink raw reply [flat|nested] 16+ messages in thread