* [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer
@ 2026-04-17 8:17 Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
` (8 more replies)
0 siblings, 9 replies; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
This patch series adds support for the Analog Devices AD9910 DDS.
This is a RFC so that we can agree/discuss on the design that follows:
This is a follow-up of the V2 discussion. For V1, we reached into
this channel composition agreement where physical channels may have
sub-channels. That adds the flexibility necessary for this design.
During V2, some feedback indicated that the ABI is too device-specific,
so DRG/RAM destination and operating modes are configured through
alternate paths and profile channels are created.
The AD9910 DDS core can be driven through several independent mechanisms:
single tone profiles, a digital ramp generator, an internal RAM playback
engine, a parallel data port, and output shift keying. Each of these
represents a distinct signal path into the DDS accumulator, so the driver
models them as separate IIO output channels (all IIO_ALTVOLTAGE type).
This per-channel separation allows userspace to configure each mode
independently through its own set of sysfs attributes, and to
enable/disable modes individually via IIO_CHAN_INFO_ENABLE, relying on
the hardware's own mode selection architecture.
The AD9910 register map is not suited for the regmap framework: register
widths vary across the map (16, 32, and 64 bits). The driver instead
implements direct SPI access helpers with a software register cache, using
type-specific read/write/update functions (ad9910_reg{16,32,64}_{read,
write,update}) that handle endianness conversion and cache coherency.
Registers are cached for several reasons. The control/function registers
(CFR1, CFR2) are frequently queried to determine the current operating
mode (e.g., checking RAM_ENABLE before every profile register access),
and caching avoids repeated SPI read transactions for what are
essentially state checks. The cache also enables efficient
read-modify-write updates on multi-byte registers: the update functions
merge new field values with the cached register content without issuing
a SPI read, and skip the write entirely when the value is unchanged.
Finally, the profile registers serve dual purposes depending on whether
RAM mode is active -- they hold single tone parameters (FTW, POW, ASF)
in normal operation but are repurposed for RAM playback configuration
(start/end address, step rate, operating mode) when RAM is enabled. A
shadow register array (reg_profile[]) preserves the inactive mode's
settings across transitions, so no state is lost when switching between
single tone and RAM operation.
RAM data is loaded through firmware upload infrastructure. Userspace
writes the waveform data as a raw binary buffer (up to 4096 bytes for
the full 1024x32-bit RAM), and the driver reverses the byte array and
transfers it to the device in a single SPI transaction. Per-profile
start/end addresses and playback parameters (operating mode, step rate,
no-dwell control) are also configured through firmware update, using
metadata in the header.
Streaming data to the DDS core through the parallel data port at the
PD_CLK rate is not covered by this series. That functionality would
be added in a separate patch series, building on top of the IIO backend
infrastructure to provide a proper buffered data path.
As I am pushing implementation, as lot has been done already without much
supervision or agreement, still I would be interested on hearing about
the design choices discussed above.
Kind regards,
Rodrigo Alencar
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v3:
- RAM custom configs (address range, destination, modes) loaded during firmware write.
- DRG destination defined when attrs are written.
- DRG modes broken down into enable attrs for ramp up/down channels.
- Add separate profile channels, switching done through enable attr
- Link to v2: https://lore.kernel.org/r/20260318-ad9910-iio-driver-v2-0-e79f93becf11@analog.com
Changes in v2:
- Device-tree bindings changes.
- RAM loading to use firmware update interface.
- Rearrange of channels into a hierarchy.
- Link to v1: https://lore.kernel.org/r/20260220-ad9910-iio-driver-v1-0-3b264aa48a10@analog.com
---
Rodrigo Alencar (9):
dt-bindings: iio: frequency: add ad9910
iio: frequency: ad9910: initial driver implementation
iio: frequency: ad9910: add simple parallel port mode support
iio: frequency: ad9910: add digital ramp generator support
iio: frequency: ad9910: add RAM mode support
iio: frequency: ad9910: add output shift keying support
iio: frequency: ad9910: add channel labels
Documentation: ABI: testing: add docs for ad9910 sysfs entries
docs: iio: add documentation for ad9910 driver
.../ABI/testing/sysfs-bus-iio-frequency-ad9910 | 62 +
.../bindings/iio/frequency/adi,ad9910.yaml | 189 ++
Documentation/iio/ad9910.rst | 586 ++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 10 +
drivers/iio/frequency/Kconfig | 20 +
drivers/iio/frequency/Makefile | 1 +
drivers/iio/frequency/ad9910.c | 2118 ++++++++++++++++++++
8 files changed, 2987 insertions(+)
---
base-commit: ff0843ceb1fb11a6b73e0e77b932ef7967aecd4b
change-id: 20260218-ad9910-iio-driver-9b3d214c251f
Best regards,
--
Rodrigo Alencar <rodrigo.alencar@analog.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 11:01 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation Rodrigo Alencar via B4 Relay
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
configurations for clocks, DAC current, reset and basic GPIO control.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../bindings/iio/frequency/adi,ad9910.yaml | 189 +++++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 196 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
new file mode 100644
index 000000000000..61e879bca5c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
@@ -0,0 +1,189 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,ad9910.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD9910 Direct Digital Synthesizer
+
+maintainers:
+ - Rodrigo Alencar <rodrigo.alencar@analog.com>
+
+description:
+ The AD9910 is a 1 GSPS direct digital synthesizer (DDS) with an integrated
+ 14-bit DAC. It features single tone mode with 8 configurable profiles,
+ a digital ramp generator, RAM control, OSK, and a parallel data port for
+ high-speed streaming.
+
+ https://www.analog.com/en/products/ad9910.html
+
+properties:
+ compatible:
+ const: adi,ad9910
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 70000000
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+ description:
+ First clock is always the reference clock (REF_CLK), while the second
+ clock is an optional synchronization clock (SYNC_IN).
+
+ clock-names:
+ oneOf:
+ - items:
+ - const: ref_clk
+ - items:
+ - const: ref_clk
+ - const: sync_in
+
+ '#clock-cells':
+ const: 1
+
+ clock-output-names:
+ minItems: 1
+ maxItems: 3
+ items:
+ enum: [ sync_clk, pdclk, sync_out ]
+
+ interrupts:
+ minItems: 1
+ maxItems: 2
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 2
+ items:
+ enum: [ drover, ram_swp_ovr ]
+
+ dvdd-io33-supply:
+ description: 3.3V Digital I/O supply.
+
+ avdd33-supply:
+ description: 3.3V Analog DAC supply.
+
+ dvdd18-supply:
+ description: 1.8V Digital Core supply.
+
+ avdd18-supply:
+ description: 1.8V Analog Core supply.
+
+ reset-gpios:
+ description:
+ GPIOs controlling the Main Device reset.
+
+ io-reset-gpios:
+ maxItems: 1
+ description:
+ GPIO controlling the I/O_RESET pin.
+
+ powerdown-gpios:
+ maxItems: 1
+ description:
+ GPIO controlling the EXT_PWR_DWN pin.
+
+ update-gpios:
+ maxItems: 1
+ description:
+ GPIO controlling the I/O_UPDATE pin.
+
+ profile-gpios:
+ minItems: 3
+ maxItems: 3
+ description:
+ GPIOs controlling the PROFILE[2:0] pins for profile selection.
+
+ sync-err-gpios:
+ maxItems: 1
+ description:
+ GPIO used to read SYNC_SMP_ERR pin status.
+
+ adi,pll-enable:
+ type: boolean
+ description:
+ Indicates that a loop filter is connected and the internal PLL is enabled.
+ Often used when the reference clock is provided by a crystal or by a
+ single-ended on-board oscillator.
+
+ adi,charge-pump-current-microamp:
+ minimum: 212
+ maximum: 387
+ default: 212
+ description:
+ PLL charge pump current in microamps. Only applicable when the internal
+ PLL is enabled. The value is rounded to the nearest supported step. This
+ value depends mostly on the loop filter design.
+
+ adi,refclk-out-drive-strength:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ disabled, low, medium, high ]
+ default: disabled
+ description:
+ Reference clock output (DRV0) drive strength. Only applicable when
+ the internal PLL is enabled.
+
+ adi,dac-output-current-microamp:
+ minimum: 8640
+ maximum: 31590
+ default: 20070
+ description:
+ DAC full-scale output current in microamps.
+
+dependencies:
+ adi,charge-pump-current-microamp: [ 'adi,pll-enable' ]
+ adi,refclk-out-drive-strength: [ 'adi,pll-enable' ]
+ interrupts: [ interrupt-names ]
+ clocks: [ clock-names ]
+ '#clock-cells': [ clock-output-names ]
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - dvdd-io33-supply
+ - avdd33-supply
+ - dvdd18-supply
+ - avdd18-supply
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ dds@0 {
+ compatible = "adi,ad9910";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ clocks = <&ad9910_refclk>;
+ clock-names = "ref_clk";
+
+ dvdd-io33-supply = <&vdd_io33>;
+ avdd33-supply = <&vdd_a33>;
+ dvdd18-supply = <&vdd_d18>;
+ avdd18-supply = <&vdd_a18>;
+
+ reset-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+ io-reset-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+ powerdown-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ update-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
+ profile-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>,
+ <&gpio 5 GPIO_ACTIVE_HIGH>,
+ <&gpio 6 GPIO_ACTIVE_HIGH>;
+
+ adi,pll-enable;
+ adi,charge-pump-current-microamp = <387>;
+ adi,refclk-out-drive-strength = "disabled";
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 08d8ddf4ef68..2ca8b68e5daa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1630,6 +1630,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad9739a.yaml
F: drivers/iio/dac/ad9739a.c
+ANALOG DEVICES INC AD9910 DRIVER
+M: Rodrigo Alencar <rodrigo.alencar@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
+
ANALOG DEVICES INC MAX22007 DRIVER
M: Janani Sunil <janani.sunil@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 11:42 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support Rodrigo Alencar via B4 Relay
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add the core AD9910 DDS driver infrastructure with single tone mode
support. This includes SPI register access, profile management via GPIO
pins, PLL/DAC configuration from firmware properties, and single tone
frequency/phase/amplitude control through IIO attributes.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/frequency/Kconfig | 18 +
drivers/iio/frequency/Makefile | 1 +
drivers/iio/frequency/ad9910.c | 1052 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 1072 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2ca8b68e5daa..6403439b530d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1636,6 +1636,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
+F: drivers/iio/frequency/ad9910.c
ANALOG DEVICES INC MAX22007 DRIVER
M: Janani Sunil <janani.sunil@analog.com>
diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index 583cbdf4e8cd..180e74f62d11 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -23,6 +23,24 @@ config AD9523
endmenu
+menu "Direct Digital Synthesis"
+
+config AD9910
+ tristate "Analog Devices AD9910 Direct Digital Synthesizer"
+ depends on SPI
+ depends on GPIOLIB
+ help
+ Say yes here to build support for Analog Devices AD9910
+ 1 GSPS, 14-Bit DDS with integrated DAC.
+
+ Supports single tone mode with 8 configurable profiles
+ and digital ramp generation.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad9910.
+
+endmenu
+
#
# Phase-Locked Loop (PLL) frequency synthesizers
#
diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
index 70d0e0b70e80..39271dd209ca 100644
--- a/drivers/iio/frequency/Makefile
+++ b/drivers/iio/frequency/Makefile
@@ -5,6 +5,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AD9523) += ad9523.o
+obj-$(CONFIG_AD9910) += ad9910.o
obj-$(CONFIG_ADF4350) += adf4350.o
obj-$(CONFIG_ADF4371) += adf4371.o
obj-$(CONFIG_ADF4377) += adf4377.o
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
new file mode 100644
index 000000000000..e9005037db1a
--- /dev/null
+++ b/drivers/iio/frequency/ad9910.c
@@ -0,0 +1,1052 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD9910 SPI DDS (Direct Digital Synthesizer) driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/log2.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/units.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* Register addresses */
+#define AD9910_REG_CFR1 0x00
+#define AD9910_REG_CFR2 0x01
+#define AD9910_REG_CFR3 0x02
+#define AD9910_REG_AUX_DAC 0x03
+#define AD9910_REG_IO_UPDATE_RATE 0x04
+#define AD9910_REG_FTW 0x07
+#define AD9910_REG_POW 0x08
+#define AD9910_REG_ASF 0x09
+#define AD9910_REG_MULTICHIP_SYNC 0x0A
+#define AD9910_REG_DRG_LIMIT 0x0B
+#define AD9910_REG_DRG_STEP 0x0C
+#define AD9910_REG_DRG_RATE 0x0D
+#define AD9910_REG_PROFILE0 0x0E
+#define AD9910_REG_PROFILE1 0x0F
+#define AD9910_REG_PROFILE2 0x10
+#define AD9910_REG_PROFILE3 0x11
+#define AD9910_REG_PROFILE4 0x12
+#define AD9910_REG_PROFILE5 0x13
+#define AD9910_REG_PROFILE6 0x14
+#define AD9910_REG_PROFILE7 0x15
+#define AD9910_REG_RAM 0x16
+
+#define AD9910_REG_NUM_CACHED 0x16
+
+#define AD9910_REG_PROFILE(x) (AD9910_REG_PROFILE0 + (x))
+#define AD9910_REG_HIGH32_FLAG_MSK BIT(8)
+
+/* CFR1 bit definitions */
+#define AD9910_CFR1_RAM_ENABLE_MSK BIT(31)
+#define AD9910_CFR1_RAM_PLAYBACK_DEST_MSK GENMASK(30, 29)
+#define AD9910_CFR1_OSK_MANUAL_EXT_CTL_MSK BIT(23)
+#define AD9910_CFR1_INV_SINC_EN_MSK BIT(22)
+#define AD9910_CFR1_INT_PROFILE_CTL_MSK GENMASK(20, 17)
+#define AD9910_CFR1_SELECT_SINE_MSK BIT(16)
+#define AD9910_CFR1_LOAD_LRR_IO_UPDATE_MSK BIT(15)
+#define AD9910_CFR1_AUTOCLR_DIG_RAMP_ACCUM_MSK BIT(14)
+#define AD9910_CFR1_AUTOCLR_PHASE_ACCUM_MSK BIT(13)
+#define AD9910_CFR1_CLEAR_DIG_RAMP_ACCUM_MSK BIT(12)
+#define AD9910_CFR1_CLEAR_PHASE_ACCUM_MSK BIT(11)
+#define AD9910_CFR1_LOAD_ARR_IO_UPDATE_MSK BIT(10)
+#define AD9910_CFR1_OSK_ENABLE_MSK BIT(9)
+#define AD9910_CFR1_SELECT_AUTO_OSK_MSK BIT(8)
+#define AD9910_CFR1_DIGITAL_POWER_DOWN_MSK BIT(7)
+#define AD9910_CFR1_DAC_POWER_DOWN_MSK BIT(6)
+#define AD9910_CFR1_REFCLK_INPUT_POWER_DOWN_MSK BIT(5)
+#define AD9910_CFR1_AUX_DAC_POWER_DOWN_MSK BIT(4)
+#define AD9910_CFR1_SOFT_POWER_DOWN_MSK GENMASK(7, 4)
+#define AD9910_CFR1_EXT_POWER_DOWN_CTL_MSK BIT(3)
+#define AD9910_CFR1_SDIO_INPUT_ONLY_MSK BIT(1)
+#define AD9910_CFR1_LSB_FIRST_MSK BIT(0)
+
+/* CFR2 bit definitions */
+#define AD9910_CFR2_AMP_SCALE_SINGLE_TONE_MSK BIT(24)
+#define AD9910_CFR2_INTERNAL_IO_UPDATE_MSK BIT(23)
+#define AD9910_CFR2_SYNC_CLK_EN_MSK BIT(22)
+#define AD9910_CFR2_DRG_DEST_MSK GENMASK(21, 20)
+#define AD9910_CFR2_DRG_ENABLE_MSK BIT(19)
+#define AD9910_CFR2_DRG_NO_DWELL_HIGH_MSK BIT(18)
+#define AD9910_CFR2_DRG_NO_DWELL_LOW_MSK BIT(17)
+#define AD9910_CFR2_DRG_NO_DWELL_MSK GENMASK(18, 17)
+#define AD9910_CFR2_READ_EFFECTIVE_FTW_MSK BIT(16)
+#define AD9910_CFR2_IO_UPDATE_RATE_CTL_MSK GENMASK(15, 14)
+#define AD9910_CFR2_PDCLK_ENABLE_MSK BIT(11)
+#define AD9910_CFR2_PDCLK_INVERT_MSK BIT(10)
+#define AD9910_CFR2_TXENABLE_INVERT_MSK BIT(9)
+#define AD9910_CFR2_MATCHED_LATENCY_EN_MSK BIT(7)
+#define AD9910_CFR2_DATA_ASM_HOLD_LAST_MSK BIT(6)
+#define AD9910_CFR2_SYNC_TIMING_VAL_DISABLE_MSK BIT(5)
+#define AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK BIT(4)
+#define AD9910_CFR2_FM_GAIN_MSK GENMASK(3, 0)
+
+/* CFR3 bit definitions */
+#define AD9910_CFR3_OPEN_MSK 0x08070000
+#define AD9910_CFR3_DRV0_MSK GENMASK(29, 28)
+#define AD9910_CFR3_VCO_SEL_MSK GENMASK(26, 24)
+#define AD9910_CFR3_ICP_MSK GENMASK(21, 19)
+#define AD9910_CFR3_REFCLK_DIV_BYPASS_MSK BIT(15)
+#define AD9910_CFR3_REFCLK_DIV_RESETB_MSK BIT(14)
+#define AD9910_CFR3_PFD_RESET_MSK BIT(10)
+#define AD9910_CFR3_PLL_EN_MSK BIT(8)
+#define AD9910_CFR3_N_MSK GENMASK(7, 1)
+
+/* Auxiliary DAC Control Register Bits */
+#define AD9910_AUX_DAC_FSC_MSK GENMASK(7, 0)
+
+/* ASF Register Bits */
+#define AD9910_ASF_RAMP_RATE_MSK GENMASK(31, 16)
+#define AD9910_ASF_SCALE_FACTOR_MSK GENMASK(15, 2)
+#define AD9910_ASF_STEP_SIZE_MSK GENMASK(1, 0)
+
+/* Multichip Sync Register Bits */
+#define AD9910_MC_SYNC_VALIDATION_DELAY_MSK GENMASK(31, 28)
+#define AD9910_MC_SYNC_RECEIVER_ENABLE_MSK BIT(27)
+#define AD9910_MC_SYNC_GENERATOR_ENABLE_MSK BIT(26)
+#define AD9910_MC_SYNC_GENERATOR_POLARITY_MSK BIT(25)
+#define AD9910_MC_SYNC_STATE_PRESET_MSK GENMASK(23, 18)
+#define AD9910_MC_SYNC_OUTPUT_DELAY_MSK GENMASK(15, 11)
+#define AD9910_MC_SYNC_INPUT_DELAY_MSK GENMASK(7, 3)
+
+/* Profile Register Format (Single Tone Mode) */
+#define AD9910_PROFILE_ST_ASF_MSK GENMASK_ULL(61, 48)
+#define AD9910_PROFILE_ST_POW_MSK GENMASK_ULL(47, 32)
+#define AD9910_PROFILE_ST_FTW_MSK GENMASK_ULL(31, 0)
+
+/* Device constants */
+#define AD9910_PI_NANORAD 3141592653UL
+
+#define AD9910_MAX_SYSCLK_HZ (1000 * HZ_PER_MHZ)
+#define AD9910_MAX_PHASE_MICRORAD (AD9910_PI_NANORAD / 500)
+
+#define AD9910_ASF_MAX GENMASK(13, 0)
+#define AD9910_POW_MAX GENMASK(15, 0)
+#define AD9910_NUM_PROFILES 8
+
+/* PLL constants */
+#define AD9910_PLL_MIN_N 12
+#define AD9910_PLL_MAX_N 127
+
+#define AD9910_PLL_IN_MIN_FREQ_HZ (3200 * HZ_PER_KHZ)
+#define AD9910_PLL_IN_MAX_FREQ_HZ (60 * HZ_PER_MHZ)
+
+#define AD9910_PLL_OUT_MIN_FREQ_HZ (420 * HZ_PER_MHZ)
+#define AD9910_PLL_OUT_MAX_FREQ_HZ (1000 * HZ_PER_MHZ)
+
+#define AD9910_VCO0_RANGE_AUTO_MAX_HZ (457 * HZ_PER_MHZ)
+#define AD9910_VCO1_RANGE_AUTO_MAX_HZ (530 * HZ_PER_MHZ)
+#define AD9910_VCO2_RANGE_AUTO_MAX_HZ (632 * HZ_PER_MHZ)
+#define AD9910_VCO3_RANGE_AUTO_MAX_HZ (775 * HZ_PER_MHZ)
+#define AD9910_VCO4_RANGE_AUTO_MAX_HZ (897 * HZ_PER_MHZ)
+#define AD9910_VCO_RANGE_NUM 6
+
+#define AD9910_REFCLK_OUT_DRV_DISABLED 0
+
+#define AD9910_ICP_MIN_uA 212
+#define AD9910_ICP_MAX_uA 387
+#define AD9910_ICP_STEP_uA 25
+
+#define AD9910_DAC_IOUT_MAX_uA 31590
+#define AD9910_DAC_IOUT_DEFAULT_uA 20070
+#define AD9910_DAC_IOUT_MIN_uA 8640
+
+#define AD9910_REFDIV2_MIN_FREQ_HZ (120 * HZ_PER_MHZ)
+#define AD9910_REFDIV2_MAX_FREQ_HZ (1900 * HZ_PER_MHZ)
+
+#define AD9910_SPI_DATA_IDX 1
+#define AD9910_SPI_DATA_LEN_MAX sizeof(__be64)
+#define AD9910_SPI_MESSAGE_LEN_MAX (AD9910_SPI_DATA_IDX + AD9910_SPI_DATA_LEN_MAX)
+#define AD9910_SPI_READ_MSK BIT(7)
+#define AD9910_SPI_ADDR_MSK GENMASK(4, 0)
+
+/**
+ * enum ad9910_channel - AD9910 channel identifiers in priority order
+ *
+ * @AD9910_CHANNEL_PHY: Physical output channel
+ * @AD9910_CHANNEL_PROFILE_0: Profile 0 output channel
+ * @AD9910_CHANNEL_PROFILE_1: Profile 1 output channel
+ * @AD9910_CHANNEL_PROFILE_2: Profile 2 output channel
+ * @AD9910_CHANNEL_PROFILE_3: Profile 3 output channel
+ * @AD9910_CHANNEL_PROFILE_4: Profile 4 output channel
+ * @AD9910_CHANNEL_PROFILE_5: Profile 5 output channel
+ * @AD9910_CHANNEL_PROFILE_6: Profile 6 output channel
+ * @AD9910_CHANNEL_PROFILE_7: Profile 7 output channel
+ */
+enum ad9910_channel {
+ AD9910_CHANNEL_PHY = 100,
+ AD9910_CHANNEL_PROFILE_0 = 101,
+ AD9910_CHANNEL_PROFILE_1 = 102,
+ AD9910_CHANNEL_PROFILE_2 = 103,
+ AD9910_CHANNEL_PROFILE_3 = 104,
+ AD9910_CHANNEL_PROFILE_4 = 105,
+ AD9910_CHANNEL_PROFILE_5 = 106,
+ AD9910_CHANNEL_PROFILE_6 = 107,
+ AD9910_CHANNEL_PROFILE_7 = 108,
+};
+
+enum {
+ AD9910_CHAN_IDX_PHY,
+ AD9910_CHAN_IDX_PROFILE_0,
+ AD9910_CHAN_IDX_PROFILE_1,
+ AD9910_CHAN_IDX_PROFILE_2,
+ AD9910_CHAN_IDX_PROFILE_3,
+ AD9910_CHAN_IDX_PROFILE_4,
+ AD9910_CHAN_IDX_PROFILE_5,
+ AD9910_CHAN_IDX_PROFILE_6,
+ AD9910_CHAN_IDX_PROFILE_7,
+ AD9910_CHAN_IDX_PARALLEL_PORT,
+ AD9910_CHAN_IDX_DRG,
+ AD9910_CHAN_IDX_DRG_RAMP_UP,
+ AD9910_CHAN_IDX_DRG_RAMP_DOWN,
+ AD9910_CHAN_IDX_RAM,
+ AD9910_CHAN_IDX_OSK,
+};
+
+enum {
+ AD9910_POWERDOWN,
+};
+
+struct ad9910_data {
+ u32 sysclk_freq_hz;
+ u32 dac_output_current;
+
+ u16 pll_charge_pump_current;
+ u8 refclk_out_drv;
+ bool pll_enabled;
+};
+
+union ad9910_reg {
+ u64 val64;
+ u32 val32;
+ u16 val16;
+};
+
+struct ad9910_state {
+ struct spi_device *spi;
+ struct clk *refclk;
+
+ struct gpio_desc *gpio_pwdown;
+ struct gpio_desc *gpio_update;
+ struct gpio_descs *gpio_profile;
+
+ /* cached registers */
+ union ad9910_reg reg[AD9910_REG_NUM_CACHED];
+
+ /* Lock for accessing device registers and state variables */
+ struct mutex lock;
+
+ struct ad9910_data data;
+ u8 profile;
+
+ union {
+ __be64 be64;
+ __be32 be32;
+ __be16 be16;
+ } rx_buf;
+ /*
+ * RAM loading requires a reasonable amount of bytes, at the same time
+ * DMA capable SPI drivers requires the transfer buffers to live in
+ * their own cache lines.
+ */
+ u8 tx_buf[AD9910_SPI_MESSAGE_LEN_MAX] __aligned(IIO_DMA_MINALIGN);
+};
+
+/**
+ * ad9910_rational_scale() - Perform scaling of input given a reference.
+ * @input: The input value to be scaled.
+ * @scale: The numerator of the scaling factor.
+ * @reference: The denominator of the scaling factor.
+ *
+ * Closest rounding with mul_u64_add_u64_div_u64
+ *
+ * Return: The scaled value.
+ */
+#define ad9910_rational_scale(input, scale, reference) ({ \
+ u64 _tmp = (reference); \
+ mul_u64_add_u64_div_u64(input, scale, _tmp >> 1, _tmp); \
+})
+
+static int ad9910_io_update(struct ad9910_state *st)
+{
+ if (st->gpio_update) {
+ gpiod_set_value_cansleep(st->gpio_update, 1);
+ udelay(1);
+ gpiod_set_value_cansleep(st->gpio_update, 0);
+ }
+
+ return 0;
+}
+
+static inline int ad9910_spi_read(struct ad9910_state *st, u8 reg, size_t len)
+{
+ st->tx_buf[0] = AD9910_SPI_READ_MSK |
+ FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
+ return spi_write_then_read(st->spi, st->tx_buf, 1, &st->rx_buf, len);
+}
+
+static inline int ad9910_spi_write(struct ad9910_state *st, u8 reg, size_t len,
+ bool update)
+{
+ int ret;
+
+ st->tx_buf[0] = FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
+ ret = spi_write(st->spi, st->tx_buf, AD9910_SPI_DATA_IDX + len);
+ if (!ret && update)
+ return ad9910_io_update(st);
+
+ return ret;
+}
+
+#define AD9910_REG_READ_FN(nb) \
+static int ad9910_reg##nb##_read(struct ad9910_state *st, u8 reg, \
+ u##nb * data) \
+{ \
+ int ret; \
+ \
+ ret = ad9910_spi_read(st, reg, sizeof(*data)); \
+ if (ret) \
+ return ret; \
+ \
+ *data = be##nb##_to_cpu(st->rx_buf.be##nb); \
+ return ret; \
+}
+
+AD9910_REG_READ_FN(16)
+AD9910_REG_READ_FN(32)
+AD9910_REG_READ_FN(64)
+
+#define AD9910_REG_WRITE_FN(nb) \
+static int ad9910_reg##nb##_write(struct ad9910_state *st, u8 reg, \
+ u##nb data, bool update) \
+{ \
+ int ret; \
+ \
+ put_unaligned_be##nb(data, &st->tx_buf[AD9910_SPI_DATA_IDX]); \
+ ret = ad9910_spi_write(st, reg, sizeof(data), update); \
+ if (ret) \
+ return ret; \
+ \
+ st->reg[reg].val##nb = data; \
+ return ret; \
+}
+
+AD9910_REG_WRITE_FN(16)
+AD9910_REG_WRITE_FN(32)
+AD9910_REG_WRITE_FN(64)
+
+#define AD9910_REG_UPDATE_FN(nb) \
+static int ad9910_reg##nb##_update(struct ad9910_state *st, \
+ u8 reg, u##nb mask, \
+ u##nb data, bool update) \
+{ \
+ u##nb reg_val = (st->reg[reg].val##nb & ~mask) | (data & mask); \
+ \
+ if (reg_val == st->reg[reg].val##nb && !update) \
+ return 0; \
+ \
+ return ad9910_reg##nb##_write(st, reg, reg_val, update); \
+}
+
+AD9910_REG_UPDATE_FN(16)
+AD9910_REG_UPDATE_FN(32)
+AD9910_REG_UPDATE_FN(64)
+
+static int ad9910_set_dac_current(struct ad9910_state *st, bool update)
+{
+ u32 fsc_code;
+
+ /* FSC = (86.4 / Rset) * (1 + CODE/256) where Rset = 10k ohms */
+ fsc_code = DIV_ROUND_CLOSEST(st->data.dac_output_current, 90) - 96;
+ fsc_code &= 0xFFU;
+
+ return ad9910_reg32_write(st, AD9910_REG_AUX_DAC, fsc_code, update);
+}
+
+static int ad9910_set_sysclk_freq(struct ad9910_state *st, u32 freq_hz,
+ bool update)
+{
+ u32 sysclk_freq_hz, refclk_freq_hz = clk_get_rate(st->refclk);
+ u32 tmp32, vco_sel;
+ int ret;
+
+ if (st->data.pll_enabled) {
+ if (refclk_freq_hz < AD9910_PLL_IN_MIN_FREQ_HZ ||
+ refclk_freq_hz > AD9910_PLL_IN_MAX_FREQ_HZ) {
+ dev_err(&st->spi->dev,
+ "REF_CLK frequency %u Hz is out of PLL input range\n",
+ refclk_freq_hz);
+ return -ERANGE;
+ }
+
+ tmp32 = DIV_ROUND_CLOSEST(freq_hz, refclk_freq_hz);
+ tmp32 = clamp(tmp32, AD9910_PLL_MIN_N, AD9910_PLL_MAX_N);
+ sysclk_freq_hz = refclk_freq_hz * tmp32;
+
+ if (sysclk_freq_hz < AD9910_PLL_OUT_MIN_FREQ_HZ ||
+ sysclk_freq_hz > AD9910_PLL_OUT_MAX_FREQ_HZ) {
+ dev_err(&st->spi->dev,
+ "PLL output frequency %u Hz is out of range\n",
+ sysclk_freq_hz);
+ return -ERANGE;
+ }
+
+ if (sysclk_freq_hz <= AD9910_VCO0_RANGE_AUTO_MAX_HZ)
+ vco_sel = 0;
+ else if (sysclk_freq_hz <= AD9910_VCO1_RANGE_AUTO_MAX_HZ)
+ vco_sel = 1;
+ else if (sysclk_freq_hz <= AD9910_VCO2_RANGE_AUTO_MAX_HZ)
+ vco_sel = 2;
+ else if (sysclk_freq_hz <= AD9910_VCO3_RANGE_AUTO_MAX_HZ)
+ vco_sel = 3;
+ else if (sysclk_freq_hz <= AD9910_VCO4_RANGE_AUTO_MAX_HZ)
+ vco_sel = 4;
+ else
+ vco_sel = 5;
+
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
+ AD9910_CFR3_N_MSK | AD9910_CFR3_VCO_SEL_MSK,
+ FIELD_PREP(AD9910_CFR3_N_MSK, tmp32) |
+ FIELD_PREP(AD9910_CFR3_VCO_SEL_MSK, vco_sel),
+ update);
+ if (ret)
+ return ret;
+ } else {
+ tmp32 = DIV_ROUND_CLOSEST(refclk_freq_hz, freq_hz);
+ tmp32 = clamp(tmp32, 1, 2);
+ sysclk_freq_hz = refclk_freq_hz / tmp32;
+ tmp32 = FIELD_PREP(AD9910_CFR3_REFCLK_DIV_BYPASS_MSK, tmp32 % 2);
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
+ AD9910_CFR3_REFCLK_DIV_BYPASS_MSK,
+ tmp32, update);
+ if (ret)
+ return ret;
+ }
+
+ st->data.sysclk_freq_hz = sysclk_freq_hz;
+
+ return 0;
+}
+
+static int ad9910_profile_set(struct ad9910_state *st, u8 profile)
+{
+ DECLARE_BITMAP(values, BITS_PER_TYPE(profile));
+
+ st->profile = profile;
+ values[0] = profile;
+ gpiod_multi_set_value_cansleep(st->gpio_profile, values);
+
+ return 0;
+}
+
+static int ad9910_powerdown_set(struct ad9910_state *st, bool enable)
+{
+ return gpiod_set_value_cansleep(st->gpio_pwdown, enable);
+}
+
+static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ int val;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_POWERDOWN:
+ val = !!FIELD_GET(AD9910_CFR1_SOFT_POWER_DOWN_MSK,
+ st->reg[AD9910_REG_CFR1].val32);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return iio_format_value(buf, IIO_VAL_INT, 1, &val);
+}
+
+static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+
+ u32 val32;
+ int ret;
+
+ ret = kstrtou32(buf, 10, &val32);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_POWERDOWN:
+ val32 = val32 ? AD9910_CFR1_SOFT_POWER_DOWN_MSK : 0;
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_SOFT_POWER_DOWN_MSK,
+ val32, true);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret ?: len;
+}
+
+#define AD9910_EXT_INFO_TMPL(_name, _ident, _shared, _fn_desc) { \
+ .name = _name, \
+ .read = ad9910_ ## _fn_desc ## _read, \
+ .write = ad9910_ ## _fn_desc ## _write, \
+ .private = _ident, \
+ .shared = _shared, \
+}
+
+#define AD9910_EXT_INFO(_name, _ident, _shared) \
+ AD9910_EXT_INFO_TMPL(_name, _ident, _shared, ext_info)
+
+static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
+ AD9910_EXT_INFO("powerdown", AD9910_POWERDOWN, IIO_SEPARATE),
+ { }
+};
+
+#define AD9910_PROFILE_CHAN(idx) { \
+ .type = IIO_ALTVOLTAGE, \
+ .indexed = 1, \
+ .output = 1, \
+ .channel = AD9910_CHANNEL_PROFILE_ ## idx, \
+ .address = AD9910_CHAN_IDX_PROFILE_ ## idx, \
+ .scan_index = -1, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) | \
+ BIT(IIO_CHAN_INFO_FREQUENCY) | \
+ BIT(IIO_CHAN_INFO_PHASE) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+static const struct iio_chan_spec ad9910_channels[] = {
+ [AD9910_CHAN_IDX_PHY] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_PHY,
+ .address = AD9910_CHAN_IDX_PHY,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .ext_info = ad9910_phy_ext_info,
+ },
+ [AD9910_CHAN_IDX_PROFILE_0] = AD9910_PROFILE_CHAN(0),
+ [AD9910_CHAN_IDX_PROFILE_1] = AD9910_PROFILE_CHAN(1),
+ [AD9910_CHAN_IDX_PROFILE_2] = AD9910_PROFILE_CHAN(2),
+ [AD9910_CHAN_IDX_PROFILE_3] = AD9910_PROFILE_CHAN(3),
+ [AD9910_CHAN_IDX_PROFILE_4] = AD9910_PROFILE_CHAN(4),
+ [AD9910_CHAN_IDX_PROFILE_5] = AD9910_PROFILE_CHAN(5),
+ [AD9910_CHAN_IDX_PROFILE_6] = AD9910_PROFILE_CHAN(6),
+ [AD9910_CHAN_IDX_PROFILE_7] = AD9910_PROFILE_CHAN(7),
+};
+
+static int ad9910_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ u64 tmp64;
+ u32 tmp32;
+
+ guard(mutex)(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_ENABLE:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = (chan->channel - AD9910_CHANNEL_PROFILE_0);
+ *val = (tmp32 == st->profile);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_FREQUENCY:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp32 = FIELD_GET(AD9910_PROFILE_ST_FTW_MSK,
+ st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ break;
+ default:
+ return -EINVAL;
+ }
+ tmp64 = (u64)tmp32 * st->data.sysclk_freq_hz;
+ *val = tmp64 >> 32;
+ *val2 = ((tmp64 & GENMASK_ULL(31, 0)) * MICRO) >> 32;
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_PHASE:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp64 = FIELD_GET(AD9910_PROFILE_ST_POW_MSK,
+ st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ tmp32 = (tmp64 * AD9910_MAX_PHASE_MICRORAD) >> 16;
+ *val = tmp32 / MICRO;
+ *val2 = tmp32 % MICRO;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp64 = FIELD_GET(AD9910_PROFILE_ST_ASF_MSK,
+ st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ *val = 0;
+ *val2 = tmp64 * MICRO >> 14;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PHY:
+ *val = st->data.sysclk_freq_hz;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad9910_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ u64 tmp64;
+ u32 tmp32;
+
+ guard(mutex)(&st->lock);
+
+ switch (info) {
+ case IIO_CHAN_INFO_ENABLE:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ if (!val) {
+ if (tmp32 != st->profile)
+ return 0;
+
+ tmp32 = (tmp32 + 1) % AD9910_NUM_PROFILES;
+ }
+
+ return ad9910_profile_set(st, tmp32);
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_FREQUENCY:
+ if (!in_range(val, 0, st->data.sysclk_freq_hz / 2))
+ return -EINVAL;
+
+ tmp64 = ad9910_rational_scale((u64)val * MICRO + val2, BIT_ULL(32),
+ (u64)MICRO * st->data.sysclk_freq_hz);
+ tmp64 = min(tmp64, U32_MAX);
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp64 = FIELD_PREP(AD9910_PROFILE_ST_FTW_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
+ AD9910_PROFILE_ST_FTW_MSK,
+ tmp64, true);
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_PHASE:
+ if (val < 0 || val2 < 0)
+ return -EINVAL;
+
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp64 = (u64)val * MICRO + val2;
+ if (tmp64 >= AD9910_MAX_PHASE_MICRORAD)
+ return -EINVAL;
+
+ tmp64 <<= 16;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_MAX_PHASE_MICRORAD);
+ tmp64 = min(tmp64, AD9910_POW_MAX);
+ tmp64 = FIELD_PREP(AD9910_PROFILE_ST_POW_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
+ AD9910_PROFILE_ST_POW_MSK,
+ tmp64, true);
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ if (val < 0 || val > 1 || (val == 1 && val2 > 0))
+ return -EINVAL;
+
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ tmp64 = ((u64)val * MICRO + val2) << 14;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, MICRO);
+ tmp64 = min(tmp64, AD9910_ASF_MAX);
+ tmp64 = FIELD_PREP(AD9910_PROFILE_ST_ASF_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
+ AD9910_PROFILE_ST_ASF_MSK,
+ tmp64, true);
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return ad9910_set_sysclk_freq(st, val, true);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad9910_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_ENABLE:
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_FREQUENCY:
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_PHASE:
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->channel) {
+ case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad9910_debugfs_reg_read(struct ad9910_state *st, bool high32,
+ unsigned int reg, unsigned int *readval)
+{
+ union ad9910_reg tmp;
+ int ret;
+
+ switch (reg) {
+ case AD9910_REG_DRG_LIMIT:
+ case AD9910_REG_DRG_STEP:
+ case AD9910_REG_PROFILE0 ... AD9910_REG_PROFILE7:
+ ret = ad9910_reg64_read(st, reg, &tmp.val64);
+ if (ret)
+ return ret;
+ *readval = high32 ? upper_32_bits(tmp.val64) :
+ lower_32_bits(tmp.val64);
+ return 0;
+ case AD9910_REG_POW:
+ ret = ad9910_reg16_read(st, reg, &tmp.val16);
+ if (ret)
+ return ret;
+ *readval = tmp.val16;
+ return 0;
+ default:
+ ret = ad9910_reg32_read(st, reg, &tmp.val32);
+ if (ret)
+ return ret;
+ *readval = tmp.val32;
+ return 0;
+ }
+}
+
+static int ad9910_debugfs_reg_write(struct ad9910_state *st, bool high32,
+ unsigned int reg, unsigned int writeval)
+{
+ switch (reg) {
+ case AD9910_REG_DRG_LIMIT:
+ case AD9910_REG_DRG_STEP:
+ case AD9910_REG_PROFILE0 ... AD9910_REG_PROFILE7:
+ if (high32)
+ return ad9910_reg64_update(st, reg, GENMASK_ULL(63, 32),
+ FIELD_PREP(GENMASK_ULL(63, 32), writeval),
+ true);
+
+ return ad9910_reg64_update(st, reg, GENMASK_ULL(31, 0),
+ writeval, true);
+ case AD9910_REG_POW:
+ return ad9910_reg16_write(st, reg, writeval, true);
+ default:
+ return ad9910_reg32_write(st, reg, writeval, true);
+ }
+}
+
+static int ad9910_debugfs_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg, unsigned int writeval,
+ unsigned int *readval)
+{
+ bool high32 = FIELD_GET(AD9910_REG_HIGH32_FLAG_MSK, reg);
+ struct ad9910_state *st = iio_priv(indio_dev);
+
+ /*
+ * REG_HIGH32_FLAG is only used for regiter access to indicate upper 32
+ * bits of 64-bit registers. It is a workaround for debugfs_reg_access()
+ * limitation which only supports 32-bit values.
+ */
+ reg &= ~AD9910_REG_HIGH32_FLAG_MSK;
+ if (reg >= AD9910_REG_RAM)
+ return -EINVAL;
+
+ guard(mutex)(&st->lock);
+
+ if (readval)
+ return ad9910_debugfs_reg_read(st, high32, reg, readval);
+ else
+ return ad9910_debugfs_reg_write(st, high32, reg, writeval);
+}
+
+static const struct iio_info ad9910_info = {
+ .read_raw = ad9910_read_raw,
+ .write_raw = ad9910_write_raw,
+ .write_raw_get_fmt = ad9910_write_raw_get_fmt,
+ .debugfs_reg_access = &ad9910_debugfs_reg_access,
+};
+
+static int ad9910_cfg_sysclk(struct ad9910_state *st, bool update)
+{
+ u32 tmp32, cfr3 = AD9910_CFR3_OPEN_MSK;
+
+ cfr3 |= AD9910_CFR3_VCO_SEL_MSK |
+ FIELD_PREP(AD9910_CFR3_DRV0_MSK, st->data.refclk_out_drv);
+
+ if (st->data.pll_enabled) {
+ tmp32 = st->data.pll_charge_pump_current - AD9910_ICP_MIN_uA;
+ tmp32 = DIV_ROUND_CLOSEST(tmp32, AD9910_ICP_STEP_uA);
+ cfr3 |= FIELD_PREP(AD9910_CFR3_ICP_MSK, tmp32) |
+ AD9910_CFR3_PLL_EN_MSK;
+ } else {
+ cfr3 |= AD9910_CFR3_REFCLK_DIV_RESETB_MSK |
+ AD9910_CFR3_PFD_RESET_MSK;
+ }
+ st->reg[AD9910_REG_CFR3].val32 = cfr3;
+
+ return ad9910_set_sysclk_freq(st, AD9910_PLL_OUT_MAX_FREQ_HZ, update);
+}
+
+static int ad9910_parse_fw(struct ad9910_state *st)
+{
+ static const char * const refclk_out_drv0[] = {
+ "disabled", "low", "medium", "high",
+ };
+ struct device *dev = &st->spi->dev;
+ u32 tmp;
+ int ret;
+
+ st->data.pll_enabled = device_property_read_bool(dev, "adi,pll-enable");
+ if (st->data.pll_enabled) {
+ tmp = AD9910_ICP_MIN_uA;
+ device_property_read_u32(dev, "adi,charge-pump-current-microamp", &tmp);
+ if (tmp < AD9910_ICP_MIN_uA || tmp > AD9910_ICP_MAX_uA)
+ return dev_err_probe(dev, -ERANGE,
+ "invalid charge pump current %u\n", tmp);
+ st->data.pll_charge_pump_current = tmp;
+
+ st->data.refclk_out_drv = AD9910_REFCLK_OUT_DRV_DISABLED;
+ ret = device_property_match_property_string(dev,
+ "adi,refclk-out-drive-strength",
+ refclk_out_drv0,
+ ARRAY_SIZE(refclk_out_drv0));
+ if (ret >= 0)
+ st->data.refclk_out_drv = ret;
+ }
+
+ tmp = AD9910_DAC_IOUT_DEFAULT_uA;
+ device_property_read_u32(dev, "adi,dac-output-current-microamp", &tmp);
+ if (tmp < AD9910_DAC_IOUT_MIN_uA || tmp > AD9910_DAC_IOUT_MAX_uA)
+ return dev_err_probe(dev, -ERANGE,
+ "Invalid DAC output current %u uA\n", tmp);
+ st->data.dac_output_current = tmp;
+
+ return 0;
+}
+
+static int ad9910_setup(struct ad9910_state *st, struct reset_control *dev_rst)
+{
+ u32 reg32;
+ int ret;
+
+ ret = reset_control_deassert(dev_rst);
+ if (ret)
+ return ret;
+
+ ret = ad9910_reg32_write(st, AD9910_REG_CFR1,
+ AD9910_CFR1_SDIO_INPUT_ONLY_MSK, false);
+ if (ret)
+ return ret;
+
+ reg32 = AD9910_CFR2_AMP_SCALE_SINGLE_TONE_MSK |
+ AD9910_CFR2_SYNC_TIMING_VAL_DISABLE_MSK |
+ AD9910_CFR2_DRG_NO_DWELL_MSK |
+ AD9910_CFR2_DATA_ASM_HOLD_LAST_MSK |
+ AD9910_CFR2_SYNC_CLK_EN_MSK |
+ AD9910_CFR2_PDCLK_ENABLE_MSK;
+ ret = ad9910_reg32_write(st, AD9910_REG_CFR2, reg32, false);
+ if (ret)
+ return ret;
+
+ ret = ad9910_cfg_sysclk(st, false);
+ if (ret)
+ return ret;
+
+ ret = ad9910_set_dac_current(st, false);
+ if (ret)
+ return ret;
+
+ return ad9910_io_update(st);
+}
+
+static void ad9910_release(void *data)
+{
+ struct ad9910_state *st = data;
+
+ if (!ad9910_powerdown_set(st, true))
+ return;
+
+ ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_SOFT_POWER_DOWN_MSK,
+ AD9910_CFR1_SOFT_POWER_DOWN_MSK,
+ true);
+}
+
+static int ad9910_probe(struct spi_device *spi)
+{
+ static const char * const supplies[] = {
+ "dvdd-io33", "avdd33", "dvdd18", "avdd18",
+ };
+ struct reset_control *dev_rst;
+ struct gpio_desc *io_rst_gpio;
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct ad9910_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->spi = spi;
+
+ st->refclk = devm_clk_get_enabled(dev, "ref_clk");
+ if (IS_ERR(st->refclk))
+ return dev_err_probe(dev, PTR_ERR(st->refclk),
+ "Failed to get reference clock\n");
+
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+ ret = devm_mutex_init(dev, &st->lock);
+ if (ret)
+ return ret;
+
+ indio_dev->name = "ad9910";
+ indio_dev->info = &ad9910_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = ad9910_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ad9910_channels);
+
+ dev_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
+ if (IS_ERR(dev_rst))
+ return dev_err_probe(dev, PTR_ERR(dev_rst),
+ "failed to get device reset control\n");
+
+ /*
+ * The IO RESET pin is not used in this driver, as we assume that all
+ * SPI transfers are complete, but if it is wired up, we need to make
+ * sure it is not floating. We can use either a reset controller or a
+ * GPIO for this.
+ */
+ io_rst_gpio = devm_gpiod_get_optional(dev, "io-reset", GPIOD_OUT_LOW);
+ if (IS_ERR(io_rst_gpio))
+ return dev_err_probe(dev, PTR_ERR(io_rst_gpio),
+ "failed to get io reset gpio\n");
+
+ st->gpio_pwdown = devm_gpiod_get_optional(dev, "powerdown",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_pwdown))
+ return dev_err_probe(dev, PTR_ERR(st->gpio_pwdown),
+ "failed to get powerdown gpio\n");
+
+ st->gpio_update = devm_gpiod_get_optional(dev, "update", GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_update))
+ return dev_err_probe(dev, PTR_ERR(st->gpio_update),
+ "failed to get update gpio\n");
+
+ st->gpio_profile = devm_gpiod_get_array_optional(dev, "profile",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_profile))
+ return dev_err_probe(dev, PTR_ERR(st->gpio_profile),
+ "failed to get profile gpios\n");
+
+ ret = ad9910_parse_fw(st);
+ if (ret)
+ return ret;
+
+ ret = ad9910_setup(st, dev_rst);
+ if (ret)
+ return dev_err_probe(dev, ret, "device setup failed\n");
+
+ ret = devm_add_action_or_reset(dev, ad9910_release, st);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add release action\n");
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct spi_device_id ad9910_id[] = {
+ { "ad9910" },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad9910_id);
+
+static const struct of_device_id ad9910_of_match[] = {
+ { .compatible = "adi,ad9910" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad9910_of_match);
+
+static struct spi_driver ad9910_driver = {
+ .driver = {
+ .name = "ad9910",
+ .of_match_table = ad9910_of_match,
+ },
+ .probe = ad9910_probe,
+ .id_table = ad9910_id,
+};
+module_spi_driver(ad9910_driver);
+
+MODULE_AUTHOR("Rodrigo Alencar <rodrigo.alencar@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD9910 DDS driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 11:59 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support Rodrigo Alencar via B4 Relay
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add parallel port channel with frequency scale, frequency offset, phase
offset, and amplitude offset extended attributes for configuring the
parallel data path.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/frequency/ad9910.c | 152 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index e9005037db1a..5b4076028a29 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -114,9 +114,13 @@
/* Auxiliary DAC Control Register Bits */
#define AD9910_AUX_DAC_FSC_MSK GENMASK(7, 0)
+/* POW Register Bits */
+#define AD9910_POW_PP_LSB_MSK GENMASK(7, 0)
+
/* ASF Register Bits */
#define AD9910_ASF_RAMP_RATE_MSK GENMASK(31, 16)
#define AD9910_ASF_SCALE_FACTOR_MSK GENMASK(15, 2)
+#define AD9910_ASF_SCALE_FACTOR_PP_LSB_MSK GENMASK(7, 2)
#define AD9910_ASF_STEP_SIZE_MSK GENMASK(1, 0)
/* Multichip Sync Register Bits */
@@ -140,7 +144,9 @@
#define AD9910_MAX_PHASE_MICRORAD (AD9910_PI_NANORAD / 500)
#define AD9910_ASF_MAX GENMASK(13, 0)
+#define AD9910_ASF_PP_LSB_MAX GENMASK(5, 0)
#define AD9910_POW_MAX GENMASK(15, 0)
+#define AD9910_POW_PP_LSB_MAX GENMASK(7, 0)
#define AD9910_NUM_PROFILES 8
/* PLL constants */
@@ -191,6 +197,7 @@
* @AD9910_CHANNEL_PROFILE_5: Profile 5 output channel
* @AD9910_CHANNEL_PROFILE_6: Profile 6 output channel
* @AD9910_CHANNEL_PROFILE_7: Profile 7 output channel
+ * @AD9910_CHANNEL_PARALLEL_PORT: Parallel port output channel
*/
enum ad9910_channel {
AD9910_CHANNEL_PHY = 100,
@@ -202,6 +209,7 @@ enum ad9910_channel {
AD9910_CHANNEL_PROFILE_5 = 106,
AD9910_CHANNEL_PROFILE_6 = 107,
AD9910_CHANNEL_PROFILE_7 = 108,
+ AD9910_CHANNEL_PARALLEL_PORT = 110,
};
enum {
@@ -224,6 +232,10 @@ enum {
enum {
AD9910_POWERDOWN,
+ AD9910_PP_FREQ_SCALE,
+ AD9910_PP_FREQ_OFFSET,
+ AD9910_PP_PHASE_OFFSET,
+ AD9910_PP_AMP_OFFSET,
};
struct ad9910_data {
@@ -478,6 +490,10 @@ static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
val = !!FIELD_GET(AD9910_CFR1_SOFT_POWER_DOWN_MSK,
st->reg[AD9910_REG_CFR1].val32);
break;
+ case AD9910_PP_FREQ_SCALE:
+ val = BIT(FIELD_GET(AD9910_CFR2_FM_GAIN_MSK,
+ st->reg[AD9910_REG_CFR2].val32));
+ break;
default:
return -EINVAL;
}
@@ -508,6 +524,113 @@ static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
AD9910_CFR1_SOFT_POWER_DOWN_MSK,
val32, true);
break;
+ case AD9910_PP_FREQ_SCALE:
+ if (val32 > BIT(15) || !is_power_of_2(val32))
+ return -EINVAL;
+
+ val32 = FIELD_PREP(AD9910_CFR2_FM_GAIN_MSK, ilog2(val32));
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_FM_GAIN_MSK,
+ val32, true);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret ?: len;
+}
+
+static ssize_t ad9910_pp_attrs_read(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ int vals[2];
+ u32 tmp32;
+ u64 tmp64;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_PP_FREQ_OFFSET:
+ tmp64 = (u64)st->reg[AD9910_REG_FTW].val32 * st->data.sysclk_freq_hz;
+ vals[0] = tmp64 >> 32;
+ vals[1] = ((tmp64 & GENMASK_ULL(31, 0)) * MICRO) >> 32;
+ break;
+ case AD9910_PP_PHASE_OFFSET:
+ tmp32 = FIELD_GET(AD9910_POW_PP_LSB_MSK,
+ st->reg[AD9910_REG_POW].val16);
+ tmp32 = (tmp32 * AD9910_MAX_PHASE_MICRORAD) >> 16;
+ vals[0] = tmp32 / MICRO;
+ vals[1] = tmp32 % MICRO;
+ break;
+ case AD9910_PP_AMP_OFFSET:
+ tmp32 = FIELD_GET(AD9910_ASF_SCALE_FACTOR_PP_LSB_MSK,
+ st->reg[AD9910_REG_ASF].val32);
+ vals[0] = 0;
+ vals[1] = (u64)tmp32 * MICRO >> 14;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, ARRAY_SIZE(vals), vals);
+}
+
+static ssize_t ad9910_pp_attrs_write(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ int val, val2;
+ u32 tmp32;
+ int ret;
+
+ ret = iio_str_to_fixpoint(buf, MICRO / 10, &val, &val2);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_PP_FREQ_OFFSET:
+ if (!in_range(val, 0, st->data.sysclk_freq_hz / 2))
+ return -EINVAL;
+
+ tmp32 = ad9910_rational_scale((u64)val * MICRO + val2, BIT_ULL(32),
+ (u64)MICRO * st->data.sysclk_freq_hz);
+ ret = ad9910_reg32_write(st, AD9910_REG_FTW, tmp32, true);
+ break;
+ case AD9910_PP_PHASE_OFFSET:
+ if (val)
+ return -EINVAL;
+
+ if (!in_range(val2, 0, (AD9910_MAX_PHASE_MICRORAD >> 8)))
+ return -EINVAL;
+
+ tmp32 = DIV_ROUND_CLOSEST((u32)val2 << 16, AD9910_MAX_PHASE_MICRORAD);
+ tmp32 = min(tmp32, AD9910_POW_PP_LSB_MAX);
+ tmp32 = FIELD_PREP(AD9910_POW_PP_LSB_MSK, tmp32);
+ ret = ad9910_reg16_update(st, AD9910_REG_POW,
+ AD9910_POW_PP_LSB_MSK,
+ tmp32, true);
+ break;
+ case AD9910_PP_AMP_OFFSET:
+ if (val)
+ return -EINVAL;
+
+ if (!in_range(val2, 0, (MICRO >> 8)))
+ return -EINVAL;
+
+ tmp32 = DIV_ROUND_CLOSEST((u32)val2 << 14, MICRO);
+ tmp32 = min(tmp32, AD9910_ASF_PP_LSB_MAX);
+ tmp32 = FIELD_PREP(AD9910_ASF_SCALE_FACTOR_PP_LSB_MSK, tmp32);
+ ret = ad9910_reg32_update(st, AD9910_REG_ASF,
+ AD9910_ASF_SCALE_FACTOR_PP_LSB_MSK,
+ tmp32, true);
+ break;
default:
return -EINVAL;
}
@@ -526,11 +649,22 @@ static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
#define AD9910_EXT_INFO(_name, _ident, _shared) \
AD9910_EXT_INFO_TMPL(_name, _ident, _shared, ext_info)
+#define AD9910_PP_EXT_INFO(_name, _ident) \
+ AD9910_EXT_INFO_TMPL(_name, _ident, IIO_SEPARATE, pp_attrs)
+
static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
AD9910_EXT_INFO("powerdown", AD9910_POWERDOWN, IIO_SEPARATE),
{ }
};
+static const struct iio_chan_spec_ext_info ad9910_pp_ext_info[] = {
+ AD9910_EXT_INFO("frequency_scale", AD9910_PP_FREQ_SCALE, IIO_SEPARATE),
+ AD9910_PP_EXT_INFO("frequency_offset", AD9910_PP_FREQ_OFFSET),
+ AD9910_PP_EXT_INFO("phase_offset", AD9910_PP_PHASE_OFFSET),
+ AD9910_PP_EXT_INFO("scale_offset", AD9910_PP_AMP_OFFSET),
+ { }
+};
+
#define AD9910_PROFILE_CHAN(idx) { \
.type = IIO_ALTVOLTAGE, \
.indexed = 1, \
@@ -563,6 +697,15 @@ static const struct iio_chan_spec ad9910_channels[] = {
[AD9910_CHAN_IDX_PROFILE_5] = AD9910_PROFILE_CHAN(5),
[AD9910_CHAN_IDX_PROFILE_6] = AD9910_PROFILE_CHAN(6),
[AD9910_CHAN_IDX_PROFILE_7] = AD9910_PROFILE_CHAN(7),
+ [AD9910_CHAN_IDX_PARALLEL_PORT] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_PARALLEL_PORT,
+ .address = AD9910_CHAN_IDX_PARALLEL_PORT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE),
+ .ext_info = ad9910_pp_ext_info,
+ },
};
static int ad9910_read_raw(struct iio_dev *indio_dev,
@@ -582,6 +725,10 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp32 = (chan->channel - AD9910_CHANNEL_PROFILE_0);
*val = (tmp32 == st->profile);
break;
+ case AD9910_CHANNEL_PARALLEL_PORT:
+ *val = FIELD_GET(AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK,
+ st->reg[AD9910_REG_CFR2].val32);
+ break;
default:
return -EINVAL;
}
@@ -661,6 +808,11 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
}
return ad9910_profile_set(st, tmp32);
+ case AD9910_CHANNEL_PARALLEL_PORT:
+ tmp32 = FIELD_PREP(AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK, !!val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK,
+ tmp32, true);
default:
return -EINVAL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (2 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 12:05 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 5/9] iio: frequency: ad9910: add RAM mode support Rodrigo Alencar via B4 Relay
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add DRG channels with destination selection (frequency, phase, or
amplitude) based on attribute writes, dwell mode control,
configurable upper/lower limits, increment/decrement step sizes, and
step rate settings.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/frequency/ad9910.c | 425 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 423 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index 5b4076028a29..c9ec677cd63a 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -132,6 +132,18 @@
#define AD9910_MC_SYNC_OUTPUT_DELAY_MSK GENMASK(15, 11)
#define AD9910_MC_SYNC_INPUT_DELAY_MSK GENMASK(7, 3)
+/* Digital Ramp Limit Register */
+#define AD9910_DRG_LIMIT_UPPER_MSK GENMASK_ULL(63, 32)
+#define AD9910_DRG_LIMIT_LOWER_MSK GENMASK_ULL(31, 0)
+
+/* Digital Ramp Step Register */
+#define AD9910_DRG_STEP_DEC_MSK GENMASK_ULL(63, 32)
+#define AD9910_DRG_STEP_INC_MSK GENMASK_ULL(31, 0)
+
+/* Digital Ramp Rate Register */
+#define AD9910_DRG_RATE_DEC_MSK GENMASK(31, 16)
+#define AD9910_DRG_RATE_INC_MSK GENMASK(15, 0)
+
/* Profile Register Format (Single Tone Mode) */
#define AD9910_PROFILE_ST_ASF_MSK GENMASK_ULL(61, 48)
#define AD9910_PROFILE_ST_POW_MSK GENMASK_ULL(47, 32)
@@ -147,6 +159,7 @@
#define AD9910_ASF_PP_LSB_MAX GENMASK(5, 0)
#define AD9910_POW_MAX GENMASK(15, 0)
#define AD9910_POW_PP_LSB_MAX GENMASK(7, 0)
+#define AD9910_STEP_RATE_MAX GENMASK(15, 0)
#define AD9910_NUM_PROFILES 8
/* PLL constants */
@@ -198,6 +211,9 @@
* @AD9910_CHANNEL_PROFILE_6: Profile 6 output channel
* @AD9910_CHANNEL_PROFILE_7: Profile 7 output channel
* @AD9910_CHANNEL_PARALLEL_PORT: Parallel port output channel
+ * @AD9910_CHANNEL_DRG: Digital Ramp Generator output channel
+ * @AD9910_CHANNEL_DRG_RAMP_UP: DRG ramp up channel
+ * @AD9910_CHANNEL_DRG_RAMP_DOWN: DRG ramp down channel
*/
enum ad9910_channel {
AD9910_CHANNEL_PHY = 100,
@@ -210,6 +226,24 @@ enum ad9910_channel {
AD9910_CHANNEL_PROFILE_6 = 107,
AD9910_CHANNEL_PROFILE_7 = 108,
AD9910_CHANNEL_PARALLEL_PORT = 110,
+ AD9910_CHANNEL_DRG = 120,
+ AD9910_CHANNEL_DRG_RAMP_UP = 121,
+ AD9910_CHANNEL_DRG_RAMP_DOWN = 122,
+};
+
+/**
+ * enum ad9910_destination - AD9910 DDS core parameter destination
+ *
+ * @AD9910_DEST_FREQUENCY: Frequency destination
+ * @AD9910_DEST_PHASE: Phase destination
+ * @AD9910_DEST_AMPLITUDE: Amplitude destination
+ * @AD9910_DEST_POLAR: Polar destination
+ */
+enum ad9910_destination {
+ AD9910_DEST_FREQUENCY,
+ AD9910_DEST_PHASE,
+ AD9910_DEST_AMPLITUDE,
+ AD9910_DEST_POLAR,
};
enum {
@@ -236,6 +270,9 @@ enum {
AD9910_PP_FREQ_OFFSET,
AD9910_PP_PHASE_OFFSET,
AD9910_PP_AMP_OFFSET,
+ AD9910_DRG_FREQ_STEP,
+ AD9910_DRG_PHASE_STEP,
+ AD9910_DRG_AMP_STEP,
};
struct ad9910_data {
@@ -475,6 +512,16 @@ static int ad9910_powerdown_set(struct ad9910_state *st, bool enable)
return gpiod_set_value_cansleep(st->gpio_pwdown, enable);
}
+static inline int ad9910_drg_destination_set(struct ad9910_state *st,
+ enum ad9910_destination dest,
+ bool update)
+{
+ return ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_DRG_DEST_MSK,
+ FIELD_PREP(AD9910_CFR2_DRG_DEST_MSK, dest),
+ update);
+}
+
static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
uintptr_t private,
const struct iio_chan_spec *chan,
@@ -638,6 +685,140 @@ static ssize_t ad9910_pp_attrs_write(struct iio_dev *indio_dev,
return ret ?: len;
}
+static ssize_t ad9910_drg_attrs_read(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ unsigned int type;
+ int vals[2];
+ u64 tmp64;
+
+ guard(mutex)(&st->lock);
+
+ switch (chan->channel) {
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp64 = FIELD_GET(AD9910_DRG_STEP_INC_MSK,
+ st->reg[AD9910_REG_DRG_STEP].val64);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp64 = FIELD_GET(AD9910_DRG_STEP_DEC_MSK,
+ st->reg[AD9910_REG_DRG_STEP].val64);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (private) {
+ case AD9910_DRG_FREQ_STEP:
+ type = IIO_VAL_INT_PLUS_MICRO;
+ tmp64 *= st->data.sysclk_freq_hz;
+ vals[0] = tmp64 >> 32;
+ vals[1] = ((tmp64 & GENMASK_ULL(31, 0)) * MICRO) >> 32;
+ break;
+ case AD9910_DRG_PHASE_STEP:
+ type = IIO_VAL_INT_PLUS_NANO;
+ tmp64 *= AD9910_PI_NANORAD;
+ tmp64 >>= 31;
+ vals[0] = div_u64_rem(tmp64, NANO, &vals[1]);
+ break;
+ case AD9910_DRG_AMP_STEP:
+ type = IIO_VAL_INT_PLUS_NANO;
+ vals[0] = 0;
+ vals[1] = tmp64 * NANO >> 32;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return iio_format_value(buf, type, ARRAY_SIZE(vals), vals);
+}
+
+static ssize_t ad9910_drg_attrs_write(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ enum ad9910_destination dest;
+ int val, val2;
+ u64 tmp64;
+ int ret;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_DRG_FREQ_STEP:
+ ret = iio_str_to_fixpoint(buf, MICRO / 10, &val, &val2);
+ if (ret)
+ return ret;
+
+ if (!in_range(val, 0, st->data.sysclk_freq_hz / 2))
+ return -EINVAL;
+
+ tmp64 = (u64)val * MICRO + val2;
+ tmp64 = ad9910_rational_scale(tmp64, BIT_ULL(32),
+ (u64)MICRO * st->data.sysclk_freq_hz);
+ dest = AD9910_DEST_FREQUENCY;
+ break;
+ case AD9910_DRG_PHASE_STEP:
+ ret = iio_str_to_fixpoint(buf, NANO / 10, &val, &val2);
+ if (ret)
+ return ret;
+
+ if (val < 0 || val2 < 0)
+ return -EINVAL;
+
+ tmp64 = (u64)val * NANO + val2;
+ if (tmp64 > 2ULL * AD9910_PI_NANORAD)
+ return -EINVAL;
+
+ tmp64 <<= 31;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_PI_NANORAD);
+ dest = AD9910_DEST_PHASE;
+ break;
+ case AD9910_DRG_AMP_STEP:
+ ret = iio_str_to_fixpoint(buf, NANO / 10, &val, &val2);
+ if (ret)
+ return ret;
+
+ if (val < 0 || val > 1 || (val == 1 && val2 > 0))
+ return -EINVAL;
+
+ tmp64 = ((u64)val * NANO + val2) << 32;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, NANO);
+ dest = AD9910_DEST_AMPLITUDE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ tmp64 = min(tmp64, U32_MAX);
+ ret = ad9910_drg_destination_set(st, dest, false);
+ if (ret)
+ return ret;
+
+ switch (chan->channel) {
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ ret = ad9910_reg64_update(st, AD9910_REG_DRG_STEP,
+ AD9910_DRG_STEP_INC_MSK,
+ FIELD_PREP(AD9910_DRG_STEP_INC_MSK, tmp64),
+ true);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ ret = ad9910_reg64_update(st, AD9910_REG_DRG_STEP,
+ AD9910_DRG_STEP_DEC_MSK,
+ FIELD_PREP(AD9910_DRG_STEP_DEC_MSK, tmp64),
+ true);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret ?: len;
+}
+
#define AD9910_EXT_INFO_TMPL(_name, _ident, _shared, _fn_desc) { \
.name = _name, \
.read = ad9910_ ## _fn_desc ## _read, \
@@ -652,6 +833,9 @@ static ssize_t ad9910_pp_attrs_write(struct iio_dev *indio_dev,
#define AD9910_PP_EXT_INFO(_name, _ident) \
AD9910_EXT_INFO_TMPL(_name, _ident, IIO_SEPARATE, pp_attrs)
+#define AD9910_DRG_EXT_INFO(_name, _ident) \
+ AD9910_EXT_INFO_TMPL(_name, _ident, IIO_SEPARATE, drg_attrs)
+
static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
AD9910_EXT_INFO("powerdown", AD9910_POWERDOWN, IIO_SEPARATE),
{ }
@@ -665,6 +849,13 @@ static const struct iio_chan_spec_ext_info ad9910_pp_ext_info[] = {
{ }
};
+static const struct iio_chan_spec_ext_info ad9910_drg_ramp_ext_info[] = {
+ AD9910_DRG_EXT_INFO("frequency_step", AD9910_DRG_FREQ_STEP),
+ AD9910_DRG_EXT_INFO("phase_step", AD9910_DRG_PHASE_STEP),
+ AD9910_DRG_EXT_INFO("scale_step", AD9910_DRG_AMP_STEP),
+ { }
+};
+
#define AD9910_PROFILE_CHAN(idx) { \
.type = IIO_ALTVOLTAGE, \
.indexed = 1, \
@@ -706,6 +897,43 @@ static const struct iio_chan_spec ad9910_channels[] = {
.info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE),
.ext_info = ad9910_pp_ext_info,
},
+ [AD9910_CHAN_IDX_DRG] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_DRG,
+ .address = AD9910_CHAN_IDX_DRG,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE),
+ },
+ [AD9910_CHAN_IDX_DRG_RAMP_UP] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_DRG_RAMP_UP,
+ .address = AD9910_CHAN_IDX_DRG_RAMP_UP,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) |
+ BIT(IIO_CHAN_INFO_FREQUENCY) |
+ BIT(IIO_CHAN_INFO_PHASE) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .ext_info = ad9910_drg_ramp_ext_info,
+ },
+ [AD9910_CHAN_IDX_DRG_RAMP_DOWN] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_DRG_RAMP_DOWN,
+ .address = AD9910_CHAN_IDX_DRG_RAMP_DOWN,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) |
+ BIT(IIO_CHAN_INFO_FREQUENCY) |
+ BIT(IIO_CHAN_INFO_PHASE) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .ext_info = ad9910_drg_ramp_ext_info,
+ },
};
static int ad9910_read_raw(struct iio_dev *indio_dev,
@@ -729,6 +957,18 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = FIELD_GET(AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK,
st->reg[AD9910_REG_CFR2].val32);
break;
+ case AD9910_CHANNEL_DRG:
+ *val = FIELD_GET(AD9910_CFR2_DRG_ENABLE_MSK,
+ st->reg[AD9910_REG_CFR2].val32);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ *val = FIELD_GET(AD9910_CFR2_DRG_NO_DWELL_HIGH_MSK,
+ st->reg[AD9910_REG_CFR2].val32);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ *val = FIELD_GET(AD9910_CFR2_DRG_NO_DWELL_LOW_MSK,
+ st->reg[AD9910_REG_CFR2].val32);
+ break;
default:
return -EINVAL;
}
@@ -740,6 +980,14 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp32 = FIELD_GET(AD9910_PROFILE_ST_FTW_MSK,
st->reg[AD9910_REG_PROFILE(tmp32)].val64);
break;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp32 = FIELD_GET(AD9910_DRG_LIMIT_UPPER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp32 = FIELD_GET(AD9910_DRG_LIMIT_LOWER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ break;
default:
return -EINVAL;
}
@@ -757,6 +1005,18 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = tmp32 / MICRO;
*val2 = tmp32 % MICRO;
return IIO_VAL_INT_PLUS_MICRO;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp64 = FIELD_GET(AD9910_DRG_LIMIT_UPPER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ tmp64 = (tmp64 * AD9910_PI_NANORAD) >> 31;
+ *val = div_u64_rem(tmp64, NANO, val2);
+ return IIO_VAL_INT_PLUS_NANO;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp64 = FIELD_GET(AD9910_DRG_LIMIT_LOWER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ tmp64 = (tmp64 * AD9910_PI_NANORAD) >> 31;
+ *val = div_u64_rem(tmp64, NANO, val2);
+ return IIO_VAL_INT_PLUS_NANO;
default:
return -EINVAL;
}
@@ -769,6 +1029,18 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = 0;
*val2 = tmp64 * MICRO >> 14;
return IIO_VAL_INT_PLUS_MICRO;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp64 = FIELD_GET(AD9910_DRG_LIMIT_UPPER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ *val = 0;
+ *val2 = tmp64 * NANO >> 32;
+ return IIO_VAL_INT_PLUS_NANO;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp64 = FIELD_GET(AD9910_DRG_LIMIT_LOWER_MSK,
+ st->reg[AD9910_REG_DRG_LIMIT].val64);
+ *val = 0;
+ *val2 = tmp64 * NANO >> 32;
+ return IIO_VAL_INT_PLUS_NANO;
default:
return -EINVAL;
}
@@ -777,9 +1049,23 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
case AD9910_CHANNEL_PHY:
*val = st->data.sysclk_freq_hz;
return IIO_VAL_INT;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp32 = FIELD_GET(AD9910_DRG_RATE_INC_MSK,
+ st->reg[AD9910_REG_DRG_RATE].val32);
+ break;
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp32 = FIELD_GET(AD9910_DRG_RATE_DEC_MSK,
+ st->reg[AD9910_REG_DRG_RATE].val32);
+ break;
default:
return -EINVAL;
}
+ if (!tmp32)
+ return -ERANGE;
+ tmp32 *= 4;
+ *val = st->data.sysclk_freq_hz / tmp32;
+ *val2 = div_u64((u64)(st->data.sysclk_freq_hz % tmp32) * MICRO, tmp32);
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -792,6 +1078,7 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
struct ad9910_state *st = iio_priv(indio_dev);
u64 tmp64;
u32 tmp32;
+ int ret;
guard(mutex)(&st->lock);
@@ -813,6 +1100,21 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg32_update(st, AD9910_REG_CFR2,
AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK,
tmp32, true);
+ case AD9910_CHANNEL_DRG:
+ tmp32 = FIELD_PREP(AD9910_CFR2_DRG_ENABLE_MSK, val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_DRG_ENABLE_MSK,
+ tmp32, true);
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp32 = FIELD_PREP(AD9910_CFR2_DRG_NO_DWELL_HIGH_MSK, !!val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_DRG_NO_DWELL_HIGH_MSK,
+ tmp32, true);
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp32 = FIELD_PREP(AD9910_CFR2_DRG_NO_DWELL_LOW_MSK, !!val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR2,
+ AD9910_CFR2_DRG_NO_DWELL_LOW_MSK,
+ tmp32, true);
default:
return -EINVAL;
}
@@ -830,6 +1132,28 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_FTW_MSK,
tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ ret = ad9910_drg_destination_set(st,
+ AD9910_DEST_FREQUENCY,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_UPPER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_UPPER_MSK,
+ tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ ret = ad9910_drg_destination_set(st,
+ AD9910_DEST_FREQUENCY,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_LOWER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_LOWER_MSK,
+ tmp64, true);
default:
return -EINVAL;
}
@@ -851,6 +1175,40 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_POW_MSK,
tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp64 = (u64)val * NANO + val2;
+ if (tmp64 > 2ULL * AD9910_PI_NANORAD)
+ return -EINVAL;
+
+ ret = ad9910_drg_destination_set(st, AD9910_DEST_PHASE,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 <<= 31;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_PI_NANORAD);
+ tmp64 = min(tmp64, U32_MAX);
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_UPPER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_UPPER_MSK,
+ tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp64 = (u64)val * NANO + val2;
+ if (tmp64 > 2ULL * AD9910_PI_NANORAD)
+ return -EINVAL;
+
+ ret = ad9910_drg_destination_set(st, AD9910_DEST_PHASE,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 <<= 31;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_PI_NANORAD);
+ tmp64 = min(tmp64, U32_MAX);
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_LOWER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_LOWER_MSK,
+ tmp64, true);
default:
return -EINVAL;
}
@@ -868,11 +1226,62 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_ASF_MSK,
tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ ret = ad9910_drg_destination_set(st,
+ AD9910_DEST_AMPLITUDE,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 = ((u64)val * NANO + val2) << 32;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, NANO);
+ tmp64 = min(tmp64, U32_MAX);
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_UPPER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_UPPER_MSK,
+ tmp64, true);
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ ret = ad9910_drg_destination_set(st,
+ AD9910_DEST_AMPLITUDE,
+ false);
+ if (ret)
+ return ret;
+
+ tmp64 = ((u64)val * NANO + val2) << 32;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, NANO);
+ tmp64 = min(tmp64, U32_MAX);
+ tmp64 = FIELD_PREP(AD9910_DRG_LIMIT_LOWER_MSK, tmp64);
+ return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
+ AD9910_DRG_LIMIT_LOWER_MSK,
+ tmp64, true);
default:
return -EINVAL;
}
case IIO_CHAN_INFO_SAMP_FREQ:
- return ad9910_set_sysclk_freq(st, val, true);
+ if (chan->channel == AD9910_CHANNEL_PHY)
+ return ad9910_set_sysclk_freq(st, val, true);
+
+ tmp64 = ((u64)val * MICRO + val2) * 4;
+ if (!tmp64)
+ return -EINVAL;
+
+ tmp64 = DIV64_U64_ROUND_CLOSEST((u64)st->data.sysclk_freq_hz * MICRO, tmp64);
+ tmp32 = clamp(tmp64, 1U, AD9910_STEP_RATE_MAX);
+
+ switch (chan->channel) {
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ tmp32 = FIELD_PREP(AD9910_DRG_RATE_INC_MSK, tmp32);
+ return ad9910_reg32_update(st, AD9910_REG_DRG_RATE,
+ AD9910_DRG_RATE_INC_MSK,
+ tmp32, true);
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ tmp32 = FIELD_PREP(AD9910_DRG_RATE_DEC_MSK, tmp32);
+ return ad9910_reg32_update(st, AD9910_REG_DRG_RATE,
+ AD9910_DRG_RATE_DEC_MSK,
+ tmp32, true);
+ default:
+ return -EINVAL;
+ }
default:
return -EINVAL;
}
@@ -892,11 +1301,16 @@ static int ad9910_write_raw_get_fmt(struct iio_dev *indio_dev,
switch (chan->channel) {
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
return IIO_VAL_INT_PLUS_MICRO;
+ case AD9910_CHANNEL_DRG_RAMP_UP:
+ case AD9910_CHANNEL_DRG_RAMP_DOWN:
+ return IIO_VAL_INT_PLUS_NANO;
default:
return -EINVAL;
}
case IIO_CHAN_INFO_SAMP_FREQ:
- return IIO_VAL_INT;
+ if (chan->channel == AD9910_CHANNEL_PHY)
+ return IIO_VAL_INT;
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -1075,6 +1489,13 @@ static int ad9910_setup(struct ad9910_state *st, struct reset_control *dev_rst)
if (ret)
return ret;
+ /* configure step rate with default values */
+ reg32 = FIELD_PREP(AD9910_DRG_RATE_DEC_MSK, 1) |
+ FIELD_PREP(AD9910_DRG_RATE_INC_MSK, 1);
+ ret = ad9910_reg32_write(st, AD9910_REG_DRG_RATE, reg32, false);
+ if (ret)
+ return ret;
+
return ad9910_io_update(st);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 5/9] iio: frequency: ad9910: add RAM mode support
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (3 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 6/9] iio: frequency: ad9910: add output shift keying support Rodrigo Alencar via B4 Relay
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add RAM control channel, which includes:
- RAM data loading via firmware upload interface;
- Per-profile configuration and DDS core parameter destination as firmware
metadata;
- Profile switching relying on profile channels;
- Sampling frequency control of the active profile;
- ram-enable-aware read/write paths that redirect single tone
frequency/phase/amplitude access through reg_profile cache when RAM is
active;
When RAM is enabled, the DDS profile parameters (frequency, phase,
amplitude) for the single tone mode are sourced from a shadow register
cache (reg_profile[]) since the profile registers are repurposed for RAM
control.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/frequency/Kconfig | 2 +
drivers/iio/frequency/ad9910.c | 328 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 324 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index 180e74f62d11..a5b2e5cb5269 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -29,6 +29,8 @@ config AD9910
tristate "Analog Devices AD9910 Direct Digital Synthesizer"
depends on SPI
depends on GPIOLIB
+ select FW_LOADER
+ select FW_UPLOAD
help
Say yes here to build support for Analog Devices AD9910
1 GSPS, 14-Bit DDS with integrated DAC.
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index c9ec677cd63a..7880beaa0bc4 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -8,9 +8,11 @@
#include <linux/array_size.h>
#include <linux/bitfield.h>
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/device/devres.h>
#include <linux/err.h>
+#include <linux/firmware.h>
#include <linux/gpio/consumer.h>
#include <linux/log2.h>
#include <linux/math64.h>
@@ -149,6 +151,15 @@
#define AD9910_PROFILE_ST_POW_MSK GENMASK_ULL(47, 32)
#define AD9910_PROFILE_ST_FTW_MSK GENMASK_ULL(31, 0)
+/* Profile Register Format (RAM Mode) */
+#define AD9910_PROFILE_RAM_OPEN_MSK GENMASK_ULL(61, 57)
+#define AD9910_PROFILE_RAM_STEP_RATE_MSK GENMASK_ULL(55, 40)
+#define AD9910_PROFILE_RAM_END_ADDR_MSK GENMASK_ULL(39, 30)
+#define AD9910_PROFILE_RAM_START_ADDR_MSK GENMASK_ULL(23, 14)
+#define AD9910_PROFILE_RAM_NO_DWELL_HIGH_MSK BIT_ULL(5)
+#define AD9910_PROFILE_RAM_ZERO_CROSSING_MSK BIT_ULL(3)
+#define AD9910_PROFILE_RAM_MODE_CONTROL_MSK GENMASK_ULL(2, 0)
+
/* Device constants */
#define AD9910_PI_NANORAD 3141592653UL
@@ -162,6 +173,15 @@
#define AD9910_STEP_RATE_MAX GENMASK(15, 0)
#define AD9910_NUM_PROFILES 8
+#define AD9910_RAM_FW_MAGIC 0x00AD9910
+#define AD9910_RAM_SIZE_MAX_WORDS 1024
+#define AD9910_RAM_WORD_SIZE sizeof(u32)
+#define AD9910_RAM_SIZE_MAX_BYTES (AD9910_RAM_SIZE_MAX_WORDS * AD9910_RAM_WORD_SIZE)
+#define AD9910_RAM_ADDR_MAX (AD9910_RAM_SIZE_MAX_WORDS - 1)
+
+#define AD9910_RAM_ENABLED(st) \
+ FIELD_GET(AD9910_CFR1_RAM_ENABLE_MSK, (st)->reg[AD9910_REG_CFR1].val32)
+
/* PLL constants */
#define AD9910_PLL_MIN_N 12
#define AD9910_PLL_MAX_N 127
@@ -193,7 +213,7 @@
#define AD9910_REFDIV2_MAX_FREQ_HZ (1900 * HZ_PER_MHZ)
#define AD9910_SPI_DATA_IDX 1
-#define AD9910_SPI_DATA_LEN_MAX sizeof(__be64)
+#define AD9910_SPI_DATA_LEN_MAX AD9910_RAM_SIZE_MAX_BYTES
#define AD9910_SPI_MESSAGE_LEN_MAX (AD9910_SPI_DATA_IDX + AD9910_SPI_DATA_LEN_MAX)
#define AD9910_SPI_READ_MSK BIT(7)
#define AD9910_SPI_ADDR_MSK GENMASK(4, 0)
@@ -214,6 +234,7 @@
* @AD9910_CHANNEL_DRG: Digital Ramp Generator output channel
* @AD9910_CHANNEL_DRG_RAMP_UP: DRG ramp up channel
* @AD9910_CHANNEL_DRG_RAMP_DOWN: DRG ramp down channel
+ * @AD9910_CHANNEL_RAM: RAM control output channel
*/
enum ad9910_channel {
AD9910_CHANNEL_PHY = 100,
@@ -229,6 +250,7 @@ enum ad9910_channel {
AD9910_CHANNEL_DRG = 120,
AD9910_CHANNEL_DRG_RAMP_UP = 121,
AD9910_CHANNEL_DRG_RAMP_DOWN = 122,
+ AD9910_CHANNEL_RAM = 130,
};
/**
@@ -246,6 +268,27 @@ enum ad9910_destination {
AD9910_DEST_POLAR,
};
+/**
+ * struct ad9910_ram_fw - AD9910 RAM firmware format
+ * @magic: Magic number for RAM firmware validation
+ * @cfr1: Value of CFR1 register to be configured (not all fields are
+ * used, but this is included here for convenience)
+ * @profiles: Array of RAM profile configurations
+ * @reserved: Reserved field for future use, should be set to 0
+ * @wcount: Number of RAM words to be written
+ * @words: Array of RAM words to be written. Data pattern should be set in
+ * reverse order and wcount specifies the number of words in this
+ * array
+ */
+struct ad9910_ram_fw {
+ __be32 magic;
+ __be32 cfr1;
+ __be64 profiles[AD9910_NUM_PROFILES];
+ __be32 reserved;
+ __be32 wcount;
+ __be32 words[] __counted_by_be(wcount);
+} __packed;
+
enum {
AD9910_CHAN_IDX_PHY,
AD9910_CHAN_IDX_PROFILE_0,
@@ -293,6 +336,7 @@ union ad9910_reg {
struct ad9910_state {
struct spi_device *spi;
struct clk *refclk;
+ struct fw_upload *ram_fwu;
struct gpio_desc *gpio_pwdown;
struct gpio_desc *gpio_update;
@@ -301,12 +345,22 @@ struct ad9910_state {
/* cached registers */
union ad9910_reg reg[AD9910_REG_NUM_CACHED];
+ /*
+ * alternate profile registers used to store RAM profile settings when
+ * RAM mode is disabled and Single Tone profile settings when RAM mode
+ * is enabled.
+ */
+ u64 reg_profile[AD9910_NUM_PROFILES];
+
/* Lock for accessing device registers and state variables */
struct mutex lock;
struct ad9910_data data;
u8 profile;
+ bool ram_fwu_cancel;
+ char ram_fwu_name[20];
+
union {
__be64 be64;
__be32 be32;
@@ -335,6 +389,22 @@ struct ad9910_state {
mul_u64_add_u64_div_u64(input, scale, _tmp >> 1, _tmp); \
})
+static inline u64 ad9910_ram_profile_val(struct ad9910_state *st)
+{
+ if (AD9910_RAM_ENABLED(st))
+ return st->reg[AD9910_REG_PROFILE(st->profile)].val64;
+ else
+ return st->reg_profile[st->profile];
+}
+
+static inline u64 ad9910_st_profile_val(struct ad9910_state *st, u8 profile)
+{
+ if (AD9910_RAM_ENABLED(st))
+ return st->reg_profile[profile];
+ else
+ return st->reg[AD9910_REG_PROFILE(profile)].val64;
+}
+
static int ad9910_io_update(struct ad9910_state *st)
{
if (st->gpio_update) {
@@ -934,6 +1004,18 @@ static const struct iio_chan_spec ad9910_channels[] = {
BIT(IIO_CHAN_INFO_SAMP_FREQ),
.ext_info = ad9910_drg_ramp_ext_info,
},
+ [AD9910_CHAN_IDX_RAM] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_RAM,
+ .address = AD9910_CHAN_IDX_RAM,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) |
+ BIT(IIO_CHAN_INFO_FREQUENCY) |
+ BIT(IIO_CHAN_INFO_PHASE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ },
};
static int ad9910_read_raw(struct iio_dev *indio_dev,
@@ -969,6 +1051,10 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = FIELD_GET(AD9910_CFR2_DRG_NO_DWELL_LOW_MSK,
st->reg[AD9910_REG_CFR2].val32);
break;
+ case AD9910_CHANNEL_RAM:
+ *val = FIELD_GET(AD9910_CFR1_RAM_ENABLE_MSK,
+ st->reg[AD9910_REG_CFR1].val32);
+ break;
default:
return -EINVAL;
}
@@ -978,7 +1064,7 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
tmp32 = FIELD_GET(AD9910_PROFILE_ST_FTW_MSK,
- st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ ad9910_st_profile_val(st, tmp32));
break;
case AD9910_CHANNEL_DRG_RAMP_UP:
tmp32 = FIELD_GET(AD9910_DRG_LIMIT_UPPER_MSK,
@@ -988,6 +1074,9 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp32 = FIELD_GET(AD9910_DRG_LIMIT_LOWER_MSK,
st->reg[AD9910_REG_DRG_LIMIT].val64);
break;
+ case AD9910_CHANNEL_RAM:
+ tmp32 = st->reg[AD9910_REG_FTW].val32;
+ break;
default:
return -EINVAL;
}
@@ -1000,7 +1089,7 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
tmp64 = FIELD_GET(AD9910_PROFILE_ST_POW_MSK,
- st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ ad9910_st_profile_val(st, tmp32));
tmp32 = (tmp64 * AD9910_MAX_PHASE_MICRORAD) >> 16;
*val = tmp32 / MICRO;
*val2 = tmp32 % MICRO;
@@ -1017,6 +1106,12 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp64 = (tmp64 * AD9910_PI_NANORAD) >> 31;
*val = div_u64_rem(tmp64, NANO, val2);
return IIO_VAL_INT_PLUS_NANO;
+ case AD9910_CHANNEL_RAM:
+ tmp64 = st->reg[AD9910_REG_POW].val16;
+ tmp32 = (tmp64 * AD9910_MAX_PHASE_MICRORAD) >> 16;
+ *val = tmp32 / MICRO;
+ *val2 = tmp32 % MICRO;
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -1025,7 +1120,7 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
tmp64 = FIELD_GET(AD9910_PROFILE_ST_ASF_MSK,
- st->reg[AD9910_REG_PROFILE(tmp32)].val64);
+ ad9910_st_profile_val(st, tmp32));
*val = 0;
*val2 = tmp64 * MICRO >> 14;
return IIO_VAL_INT_PLUS_MICRO;
@@ -1057,6 +1152,10 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp32 = FIELD_GET(AD9910_DRG_RATE_DEC_MSK,
st->reg[AD9910_REG_DRG_RATE].val32);
break;
+ case AD9910_CHANNEL_RAM:
+ tmp32 = FIELD_GET(AD9910_PROFILE_RAM_STEP_RATE_MSK,
+ ad9910_ram_profile_val(st));
+ break;
default:
return -EINVAL;
}
@@ -1078,7 +1177,7 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
struct ad9910_state *st = iio_priv(indio_dev);
u64 tmp64;
u32 tmp32;
- int ret;
+ int ret, i;
guard(mutex)(&st->lock);
@@ -1115,6 +1214,26 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg32_update(st, AD9910_REG_CFR2,
AD9910_CFR2_DRG_NO_DWELL_LOW_MSK,
tmp32, true);
+ case AD9910_CHANNEL_RAM:
+ if (AD9910_RAM_ENABLED(st) == !!val)
+ return 0;
+
+ /* switch profile configs */
+ for (i = 0; i < AD9910_NUM_PROFILES; i++) {
+ tmp64 = st->reg[AD9910_REG_PROFILE(i)].val64;
+ ret = ad9910_reg64_write(st,
+ AD9910_REG_PROFILE(i),
+ st->reg_profile[i],
+ false);
+ if (ret)
+ return ret;
+ st->reg_profile[i] = tmp64;
+ }
+
+ tmp32 = FIELD_PREP(AD9910_CFR1_RAM_ENABLE_MSK, !!val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_RAM_ENABLE_MSK,
+ tmp32, true);
default:
return -EINVAL;
}
@@ -1128,6 +1247,11 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
switch (chan->channel) {
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
+ if (AD9910_RAM_ENABLED(st)) {
+ FIELD_MODIFY(AD9910_PROFILE_ST_FTW_MSK,
+ &st->reg_profile[tmp32], tmp64);
+ return 0;
+ }
tmp64 = FIELD_PREP(AD9910_PROFILE_ST_FTW_MSK, tmp64);
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_FTW_MSK,
@@ -1154,6 +1278,8 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
AD9910_DRG_LIMIT_LOWER_MSK,
tmp64, true);
+ case AD9910_CHANNEL_RAM:
+ return ad9910_reg32_write(st, AD9910_REG_FTW, tmp64, true);
default:
return -EINVAL;
}
@@ -1171,6 +1297,13 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
tmp64 <<= 16;
tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_MAX_PHASE_MICRORAD);
tmp64 = min(tmp64, AD9910_POW_MAX);
+
+ if (AD9910_RAM_ENABLED(st)) {
+ FIELD_MODIFY(AD9910_PROFILE_ST_POW_MSK,
+ &st->reg_profile[tmp32], tmp64);
+ return 0;
+ }
+
tmp64 = FIELD_PREP(AD9910_PROFILE_ST_POW_MSK, tmp64);
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_POW_MSK,
@@ -1209,6 +1342,15 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
AD9910_DRG_LIMIT_LOWER_MSK,
tmp64, true);
+ case AD9910_CHANNEL_RAM:
+ tmp64 = (u64)val * MICRO + val2;
+ if (tmp64 >= AD9910_MAX_PHASE_MICRORAD)
+ return -EINVAL;
+
+ tmp64 <<= 16;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, AD9910_MAX_PHASE_MICRORAD);
+ tmp64 = min(tmp64, AD9910_POW_MAX);
+ return ad9910_reg16_write(st, AD9910_REG_POW, tmp64, true);
default:
return -EINVAL;
}
@@ -1222,6 +1364,13 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
tmp64 = ((u64)val * MICRO + val2) << 14;
tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, MICRO);
tmp64 = min(tmp64, AD9910_ASF_MAX);
+
+ if (AD9910_RAM_ENABLED(st)) {
+ FIELD_MODIFY(AD9910_PROFILE_ST_ASF_MSK,
+ &st->reg_profile[tmp32], tmp64);
+ return 0;
+ }
+
tmp64 = FIELD_PREP(AD9910_PROFILE_ST_ASF_MSK, tmp64);
return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
AD9910_PROFILE_ST_ASF_MSK,
@@ -1279,6 +1428,18 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg32_update(st, AD9910_REG_DRG_RATE,
AD9910_DRG_RATE_DEC_MSK,
tmp32, true);
+ case AD9910_CHANNEL_RAM:
+ if (!AD9910_RAM_ENABLED(st)) {
+ FIELD_MODIFY(AD9910_PROFILE_RAM_STEP_RATE_MSK,
+ &st->reg_profile[st->profile], tmp32);
+ return 0;
+ }
+
+ tmp64 = FIELD_PREP(AD9910_PROFILE_RAM_STEP_RATE_MSK, tmp32);
+ return ad9910_reg64_update(st, AD9910_REG_PROFILE(st->profile),
+ AD9910_PROFILE_RAM_STEP_RATE_MSK,
+ tmp64, true);
+
default:
return -EINVAL;
}
@@ -1300,6 +1461,7 @@ static int ad9910_write_raw_get_fmt(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
switch (chan->channel) {
case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
+ case AD9910_CHANNEL_RAM:
return IIO_VAL_INT_PLUS_MICRO;
case AD9910_CHANNEL_DRG_RAMP_UP:
case AD9910_CHANNEL_DRG_RAMP_DOWN:
@@ -1392,6 +1554,123 @@ static int ad9910_debugfs_reg_access(struct iio_dev *indio_dev,
return ad9910_debugfs_reg_write(st, high32, reg, writeval);
}
+static enum fw_upload_err ad9910_ram_fwu_prepare(struct fw_upload *fw_upload,
+ const u8 *data, u32 size)
+{
+ struct ad9910_state *st = fw_upload->dd_handle;
+ const struct ad9910_ram_fw *fw_data = (const struct ad9910_ram_fw *)data;
+ u32 wcount, bcount;
+
+ if (size < sizeof(struct ad9910_ram_fw))
+ return FW_UPLOAD_ERR_INVALID_SIZE;
+
+ if (get_unaligned_be32(&fw_data->magic) != AD9910_RAM_FW_MAGIC)
+ return FW_UPLOAD_ERR_FW_INVALID;
+
+ wcount = get_unaligned_be32(&fw_data->wcount);
+ bcount = size - sizeof(struct ad9910_ram_fw);
+ if (wcount > AD9910_RAM_SIZE_MAX_WORDS ||
+ bcount != (wcount * AD9910_RAM_WORD_SIZE))
+ return FW_UPLOAD_ERR_INVALID_SIZE;
+
+ guard(mutex)(&st->lock);
+ st->ram_fwu_cancel = false;
+
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err ad9910_ram_fwu_write(struct fw_upload *fw_upload,
+ const u8 *data, u32 offset,
+ u32 size, u32 *written)
+{
+ struct ad9910_state *st = fw_upload->dd_handle;
+ const struct ad9910_ram_fw *fw_data = (const struct ad9910_ram_fw *)data;
+ int ret, ret2, idx, wcount;
+ u64 tmp64, backup;
+
+ if (offset != 0)
+ return FW_UPLOAD_ERR_INVALID_SIZE;
+
+ guard(mutex)(&st->lock);
+
+ if (st->ram_fwu_cancel)
+ return FW_UPLOAD_ERR_CANCELED;
+
+ if (AD9910_RAM_ENABLED(st))
+ return FW_UPLOAD_ERR_HW_ERROR;
+
+ /* copy ram profiles */
+ for (idx = 0; idx < AD9910_NUM_PROFILES; idx++)
+ st->reg_profile[idx] = get_unaligned_be64(&fw_data->profiles[idx]) |
+ AD9910_PROFILE_RAM_OPEN_MSK;
+
+ /* update CFR1 */
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_RAM_PLAYBACK_DEST_MSK |
+ AD9910_CFR1_INT_PROFILE_CTL_MSK,
+ get_unaligned_be32(&fw_data->cfr1), true);
+ if (ret)
+ return FW_UPLOAD_ERR_RW_ERROR;
+
+ wcount = get_unaligned_be32(&fw_data->wcount);
+ if (!wcount) {
+ *written = size;
+ return FW_UPLOAD_ERR_NONE; /* nothing else to write */
+ }
+
+ /* ensure profile is selected */
+ ret = ad9910_profile_set(st, st->profile);
+ if (ret)
+ return FW_UPLOAD_ERR_HW_ERROR;
+
+ /* backup profile register and update it with required address range */
+ backup = st->reg[AD9910_REG_PROFILE(st->profile)].val64;
+ tmp64 = AD9910_PROFILE_RAM_STEP_RATE_MSK |
+ FIELD_PREP(AD9910_PROFILE_RAM_START_ADDR_MSK, 0) |
+ FIELD_PREP(AD9910_PROFILE_RAM_END_ADDR_MSK, wcount - 1);
+ ret = ad9910_reg64_write(st, AD9910_REG_PROFILE(st->profile), tmp64, true);
+ if (ret)
+ return FW_UPLOAD_ERR_RW_ERROR;
+
+ /* populate words into tx_buf[1:] */
+ memcpy(&st->tx_buf[1], fw_data->words, wcount * AD9910_RAM_WORD_SIZE);
+
+ /* write ram data and restore profile register */
+ ret = ad9910_spi_write(st, AD9910_REG_RAM,
+ wcount * AD9910_RAM_WORD_SIZE, false);
+ ret2 = ad9910_reg64_write(st, AD9910_REG_PROFILE(st->profile), backup, true);
+ if (ret || ret2)
+ return FW_UPLOAD_ERR_RW_ERROR;
+
+ *written = size;
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err ad9910_ram_fwu_poll_complete(struct fw_upload *fw_upload)
+{
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static void ad9910_ram_fwu_cancel(struct fw_upload *fw_upload)
+{
+ struct ad9910_state *st = fw_upload->dd_handle;
+
+ guard(mutex)(&st->lock);
+ st->ram_fwu_cancel = true;
+}
+
+static void ad9910_ram_fwu_unregister(void *data)
+{
+ firmware_upload_unregister(data);
+}
+
+static const struct fw_upload_ops ad9910_ram_fwu_ops = {
+ .prepare = ad9910_ram_fwu_prepare,
+ .write = ad9910_ram_fwu_write,
+ .poll_complete = ad9910_ram_fwu_poll_complete,
+ .cancel = ad9910_ram_fwu_cancel
+};
+
static const struct iio_info ad9910_info = {
.read_raw = ad9910_read_raw,
.write_raw = ad9910_write_raw,
@@ -1496,6 +1775,13 @@ static int ad9910_setup(struct ad9910_state *st, struct reset_control *dev_rst)
if (ret)
return ret;
+ for (int i = 0; i < AD9910_NUM_PROFILES; i++) {
+ st->reg_profile[i] = AD9910_PROFILE_RAM_OPEN_MSK;
+ st->reg_profile[i] |= FIELD_PREP(AD9910_PROFILE_RAM_STEP_RATE_MSK, 1);
+ st->reg_profile[i] |= FIELD_PREP(AD9910_PROFILE_RAM_END_ADDR_MSK,
+ AD9910_RAM_ADDR_MAX);
+ }
+
return ad9910_io_update(st);
}
@@ -1512,6 +1798,22 @@ static void ad9910_release(void *data)
true);
}
+static inline void ad9910_debugfs_init(struct ad9910_state *st,
+ struct iio_dev *indio_dev)
+{
+ char buf[64];
+
+ /*
+ * symlinks are created here so iio userspace tools can refer to them
+ * as debug attributes.
+ */
+ snprintf(buf, sizeof(buf), "/sys/class/firmware/%s/loading", st->ram_fwu_name);
+ debugfs_create_symlink("ram_loading", iio_get_debugfs_dentry(indio_dev), buf);
+
+ snprintf(buf, sizeof(buf), "/sys/class/firmware/%s/data", st->ram_fwu_name);
+ debugfs_create_symlink("ram_data", iio_get_debugfs_dentry(indio_dev), buf);
+}
+
static int ad9910_probe(struct spi_device *spi)
{
static const char * const supplies[] = {
@@ -1595,7 +1897,21 @@ static int ad9910_probe(struct spi_device *spi)
if (ret)
return dev_err_probe(dev, ret, "failed to add release action\n");
- return devm_iio_device_register(dev, indio_dev);
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ snprintf(st->ram_fwu_name, sizeof(st->ram_fwu_name), "%s:ram",
+ dev_name(&indio_dev->dev));
+ st->ram_fwu = firmware_upload_register(THIS_MODULE, dev, st->ram_fwu_name,
+ &ad9910_ram_fwu_ops, st);
+ if (IS_ERR(st->ram_fwu))
+ return dev_err_probe(dev, PTR_ERR(st->ram_fwu),
+ "failed to register to the RAM Upload\n");
+
+ ad9910_debugfs_init(st, indio_dev);
+
+ return devm_add_action_or_reset(dev, ad9910_ram_fwu_unregister, st->ram_fwu);
}
static const struct spi_device_id ad9910_id[] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 6/9] iio: frequency: ad9910: add output shift keying support
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (4 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 5/9] iio: frequency: ad9910: add RAM mode support Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels Rodrigo Alencar via B4 Relay
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add OSK channel with amplitude envelope control capabilities:
- OSK enable/disable via IIO_CHAN_INFO_ENABLE;
- Amplitude ramp rate control via IIO_CHAN_INFO_SAMP_FREQ;
- Amplitude scale readback via IIO_CHAN_INFO_SCALE (ASF register);
- Manual/external pin control via pinctrl_en ext_info attribute;
- Automatic OSK step size configuration via scale_increment ext_info;
attribute with selectable step sizes (61, 122, 244, 488 micro-units)
The ASF register is initialized with a default amplitude ramp rate during
device setup to ensure valid readback.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/frequency/ad9910.c | 153 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 152 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index 7880beaa0bc4..e43df6265fd4 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -235,6 +235,7 @@
* @AD9910_CHANNEL_DRG_RAMP_UP: DRG ramp up channel
* @AD9910_CHANNEL_DRG_RAMP_DOWN: DRG ramp down channel
* @AD9910_CHANNEL_RAM: RAM control output channel
+ * @AD9910_CHANNEL_OSK: Output Shift Keying output channel
*/
enum ad9910_channel {
AD9910_CHANNEL_PHY = 100,
@@ -251,6 +252,7 @@ enum ad9910_channel {
AD9910_CHANNEL_DRG_RAMP_UP = 121,
AD9910_CHANNEL_DRG_RAMP_DOWN = 122,
AD9910_CHANNEL_RAM = 130,
+ AD9910_CHANNEL_OSK = 140,
};
/**
@@ -316,6 +318,8 @@ enum {
AD9910_DRG_FREQ_STEP,
AD9910_DRG_PHASE_STEP,
AD9910_DRG_AMP_STEP,
+ AD9910_OSK_MANUAL_EXTCTL,
+ AD9910_OSK_AUTO_STEP,
};
struct ad9910_data {
@@ -611,6 +615,10 @@ static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
val = BIT(FIELD_GET(AD9910_CFR2_FM_GAIN_MSK,
st->reg[AD9910_REG_CFR2].val32));
break;
+ case AD9910_OSK_MANUAL_EXTCTL:
+ val = FIELD_GET(AD9910_CFR1_OSK_MANUAL_EXT_CTL_MSK,
+ st->reg[AD9910_REG_CFR1].val32);
+ break;
default:
return -EINVAL;
}
@@ -650,6 +658,12 @@ static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
AD9910_CFR2_FM_GAIN_MSK,
val32, true);
break;
+ case AD9910_OSK_MANUAL_EXTCTL:
+ val32 = val32 ? AD9910_CFR1_OSK_MANUAL_EXT_CTL_MSK : 0;
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_OSK_MANUAL_EXT_CTL_MSK,
+ val32, true);
+ break;
default:
return -EINVAL;
}
@@ -889,6 +903,84 @@ static ssize_t ad9910_drg_attrs_write(struct iio_dev *indio_dev,
return ret ?: len;
}
+static const u16 ad9910_osk_ustep[] = {
+ 0, 61, 122, 244, 488,
+};
+
+static ssize_t ad9910_osk_attrs_read(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ int vals[2];
+ bool auto_en;
+ u32 raw_val;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_OSK_AUTO_STEP:
+ auto_en = FIELD_GET(AD9910_CFR1_SELECT_AUTO_OSK_MSK,
+ st->reg[AD9910_REG_CFR1].val32);
+ raw_val = FIELD_GET(AD9910_ASF_STEP_SIZE_MSK,
+ st->reg[AD9910_REG_ASF].val32);
+ vals[0] = 0;
+ vals[1] = auto_en ? ad9910_osk_ustep[raw_val + 1] : 0;
+
+ return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t ad9910_osk_attrs_write(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct ad9910_state *st = iio_priv(indio_dev);
+ int val, val2;
+ int ret;
+ u32 raw_val;
+
+ ret = iio_str_to_fixpoint(buf, MICRO / 10, &val, &val2);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&st->lock);
+
+ switch (private) {
+ case AD9910_OSK_AUTO_STEP:
+ if (val != 0)
+ return -EINVAL;
+
+ raw_val = find_closest(val2, ad9910_osk_ustep,
+ ARRAY_SIZE(ad9910_osk_ustep));
+ if (raw_val) {
+ /* set OSK step and get automatic OSK enabled */
+ raw_val = FIELD_PREP(AD9910_ASF_STEP_SIZE_MSK,
+ raw_val - 1);
+ ret = ad9910_reg32_update(st, AD9910_REG_ASF,
+ AD9910_ASF_STEP_SIZE_MSK,
+ raw_val, true);
+ if (ret)
+ return ret;
+
+ raw_val = AD9910_CFR1_SELECT_AUTO_OSK_MSK;
+ }
+
+ ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_SELECT_AUTO_OSK_MSK,
+ raw_val, true);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret ?: len;
+}
+
#define AD9910_EXT_INFO_TMPL(_name, _ident, _shared, _fn_desc) { \
.name = _name, \
.read = ad9910_ ## _fn_desc ## _read, \
@@ -906,6 +998,9 @@ static ssize_t ad9910_drg_attrs_write(struct iio_dev *indio_dev,
#define AD9910_DRG_EXT_INFO(_name, _ident) \
AD9910_EXT_INFO_TMPL(_name, _ident, IIO_SEPARATE, drg_attrs)
+#define AD9910_OSK_EXT_INFO(_name, _ident) \
+ AD9910_EXT_INFO_TMPL(_name, _ident, IIO_SEPARATE, osk_attrs)
+
static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
AD9910_EXT_INFO("powerdown", AD9910_POWERDOWN, IIO_SEPARATE),
{ }
@@ -926,6 +1021,12 @@ static const struct iio_chan_spec_ext_info ad9910_drg_ramp_ext_info[] = {
{ }
};
+static const struct iio_chan_spec_ext_info ad9910_osk_ext_info[] = {
+ AD9910_EXT_INFO("pinctrl_en", AD9910_OSK_MANUAL_EXTCTL, IIO_SEPARATE),
+ AD9910_OSK_EXT_INFO("scale_step", AD9910_OSK_AUTO_STEP),
+ { }
+};
+
#define AD9910_PROFILE_CHAN(idx) { \
.type = IIO_ALTVOLTAGE, \
.indexed = 1, \
@@ -1016,6 +1117,18 @@ static const struct iio_chan_spec ad9910_channels[] = {
BIT(IIO_CHAN_INFO_PHASE) |
BIT(IIO_CHAN_INFO_SAMP_FREQ),
},
+ [AD9910_CHAN_IDX_OSK] = {
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .output = 1,
+ .channel = AD9910_CHANNEL_OSK,
+ .address = AD9910_CHAN_IDX_OSK,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .ext_info = ad9910_osk_ext_info,
+ },
};
static int ad9910_read_raw(struct iio_dev *indio_dev,
@@ -1055,6 +1168,10 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = FIELD_GET(AD9910_CFR1_RAM_ENABLE_MSK,
st->reg[AD9910_REG_CFR1].val32);
break;
+ case AD9910_CHANNEL_OSK:
+ *val = FIELD_GET(AD9910_CFR1_OSK_ENABLE_MSK,
+ st->reg[AD9910_REG_CFR1].val32);
+ break;
default:
return -EINVAL;
}
@@ -1136,6 +1253,12 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
*val = 0;
*val2 = tmp64 * NANO >> 32;
return IIO_VAL_INT_PLUS_NANO;
+ case AD9910_CHANNEL_OSK:
+ tmp64 = FIELD_GET(AD9910_ASF_SCALE_FACTOR_MSK,
+ st->reg[AD9910_REG_ASF].val32);
+ *val = 0;
+ *val2 = tmp64 * MICRO >> 14;
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -1156,6 +1279,10 @@ static int ad9910_read_raw(struct iio_dev *indio_dev,
tmp32 = FIELD_GET(AD9910_PROFILE_RAM_STEP_RATE_MSK,
ad9910_ram_profile_val(st));
break;
+ case AD9910_CHANNEL_OSK:
+ tmp32 = FIELD_GET(AD9910_ASF_RAMP_RATE_MSK,
+ st->reg[AD9910_REG_ASF].val32);
+ break;
default:
return -EINVAL;
}
@@ -1234,6 +1361,11 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg32_update(st, AD9910_REG_CFR1,
AD9910_CFR1_RAM_ENABLE_MSK,
tmp32, true);
+ case AD9910_CHANNEL_OSK:
+ tmp32 = FIELD_PREP(AD9910_CFR1_OSK_ENABLE_MSK, val);
+ return ad9910_reg32_update(st, AD9910_REG_CFR1,
+ AD9910_CFR1_OSK_ENABLE_MSK,
+ tmp32, true);
default:
return -EINVAL;
}
@@ -1403,6 +1535,14 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_DRG_LIMIT,
AD9910_DRG_LIMIT_LOWER_MSK,
tmp64, true);
+ case AD9910_CHANNEL_OSK:
+ tmp64 = ((u64)val * MICRO + val2) << 14;
+ tmp64 = DIV_U64_ROUND_CLOSEST(tmp64, MICRO);
+ tmp32 = min(tmp64, AD9910_ASF_MAX);
+ tmp32 = FIELD_PREP(AD9910_ASF_SCALE_FACTOR_MSK, tmp32);
+ return ad9910_reg32_update(st, AD9910_REG_ASF,
+ AD9910_ASF_SCALE_FACTOR_MSK,
+ tmp32, true);
default:
return -EINVAL;
}
@@ -1439,7 +1579,12 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
return ad9910_reg64_update(st, AD9910_REG_PROFILE(st->profile),
AD9910_PROFILE_RAM_STEP_RATE_MSK,
tmp64, true);
-
+ break;
+ case AD9910_CHANNEL_OSK:
+ return ad9910_reg32_update(st, AD9910_REG_ASF,
+ AD9910_ASF_RAMP_RATE_MSK,
+ FIELD_PREP(AD9910_ASF_RAMP_RATE_MSK, tmp32),
+ true);
default:
return -EINVAL;
}
@@ -1769,6 +1914,12 @@ static int ad9910_setup(struct ad9910_state *st, struct reset_control *dev_rst)
return ret;
/* configure step rate with default values */
+ ret = ad9910_reg32_write(st, AD9910_REG_ASF,
+ FIELD_PREP(AD9910_ASF_RAMP_RATE_MSK, 1),
+ false);
+ if (ret)
+ return ret;
+
reg32 = FIELD_PREP(AD9910_DRG_RATE_DEC_MSK, 1) |
FIELD_PREP(AD9910_DRG_RATE_INC_MSK, 1);
ret = ad9910_reg32_write(st, AD9910_REG_DRG_RATE, reg32, false);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (5 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 6/9] iio: frequency: ad9910: add output shift keying support Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 12:12 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 8/9] Documentation: ABI: testing: add docs for ad9910 sysfs entries Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver Rodrigo Alencar via B4 Relay
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add human-readable labels for all AD9910 IIO channels via the read_label
callback.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/frequency/ad9910.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index e43df6265fd4..50d97ce937c7 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -1816,10 +1816,36 @@ static const struct fw_upload_ops ad9910_ram_fwu_ops = {
.cancel = ad9910_ram_fwu_cancel
};
+static const char * const ad9910_channel_str[] = {
+ [AD9910_CHAN_IDX_PHY] = "phy",
+ [AD9910_CHAN_IDX_PROFILE_0] = "profile[0]",
+ [AD9910_CHAN_IDX_PROFILE_1] = "profile[1]",
+ [AD9910_CHAN_IDX_PROFILE_2] = "profile[2]",
+ [AD9910_CHAN_IDX_PROFILE_3] = "profile[3]",
+ [AD9910_CHAN_IDX_PROFILE_4] = "profile[4]",
+ [AD9910_CHAN_IDX_PROFILE_5] = "profile[5]",
+ [AD9910_CHAN_IDX_PROFILE_6] = "profile[6]",
+ [AD9910_CHAN_IDX_PROFILE_7] = "profile[7]",
+ [AD9910_CHAN_IDX_PARALLEL_PORT] = "parallel_port",
+ [AD9910_CHAN_IDX_DRG] = "digital_ramp_generator",
+ [AD9910_CHAN_IDX_DRG_RAMP_UP] = "digital_ramp_up",
+ [AD9910_CHAN_IDX_DRG_RAMP_DOWN] = "digital_ramp_down",
+ [AD9910_CHAN_IDX_RAM] = "ram_control",
+ [AD9910_CHAN_IDX_OSK] = "output_shift_keying",
+};
+
+static int ad9910_read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ char *label)
+{
+ return sysfs_emit(label, "%s\n", ad9910_channel_str[chan->address]);
+}
+
static const struct iio_info ad9910_info = {
.read_raw = ad9910_read_raw,
.write_raw = ad9910_write_raw,
.write_raw_get_fmt = ad9910_write_raw_get_fmt,
+ .read_label = ad9910_read_label,
.debugfs_reg_access = &ad9910_debugfs_reg_access,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 8/9] Documentation: ABI: testing: add docs for ad9910 sysfs entries
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (6 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver Rodrigo Alencar via B4 Relay
8 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add custom ABI documentation file for the DDS AD9910 with sysfs entries to
control Parallel Port, Digital Ramp Generator and OSK parameters.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../ABI/testing/sysfs-bus-iio-frequency-ad9910 | 62 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 63 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9910 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9910
new file mode 100644
index 000000000000..fabc2a5417d1
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9910
@@ -0,0 +1,62 @@
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_offset
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ For a channel that allows frequency control through buffers, this
+ represents the base frequency value in Hz. The actual output frequency
+ is a result with the sum of this value.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_scale
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ For a channel that allows frequency control through buffers, this
+ represents the frequency modulation gain. This value multiplies the
+ buffer input sample value before it is added to a frequency offset.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_phase_offset
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ For a channel that allows phase control through buffers, this
+ represents the base phase value in radians. The actual output phase
+ is a result with the sum of this value.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale_offset
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ For a channel that allows amplitude control through buffers, this
+ represents the value for a base amplitude scale. The actual output
+ amplitude scale is a result with the sum of this value.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_step
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ Channels that sweep frequency values at determined rate use this value
+ to set the frequency step size in Hz.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_phase_step
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ Channels that sweep phase values at determined rate use this value
+ to set the phase step size in radians.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_scale_step
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ Channels that sweep amplitude values at determined rate use this value
+ to set the amplite scale step.
+
+What: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_pinctrl_en
+KernelVersion:
+Contact: linux-iio@vger.kernel.org
+Description:
+ Channels that supports pin control to enable/disable its output use
+ this attribute to set the pin control mode. When set to 1, the output
+ state is controlled by a physical pin, and the channel is enabled when
+ the pin is active. When set to 0, only software control is used to
+ enable/disable the channel output.
diff --git a/MAINTAINERS b/MAINTAINERS
index 6403439b530d..edd87ee7da5f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1635,6 +1635,7 @@ M: Rodrigo Alencar <rodrigo.alencar@analog.com>
L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
+F: Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9910
F: Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
F: drivers/iio/frequency/ad9910.c
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
` (7 preceding siblings ...)
2026-04-17 8:17 ` [PATCH RFC v3 8/9] Documentation: ABI: testing: add docs for ad9910 sysfs entries Rodrigo Alencar via B4 Relay
@ 2026-04-17 8:17 ` Rodrigo Alencar via B4 Relay
2026-04-26 13:10 ` Jonathan Cameron
8 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-17 8:17 UTC (permalink / raw)
To: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add documentation for the AD9910 DDS IIO driver, which describes channels,
DDS modes, attributes and ABI usage examples.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Documentation/iio/ad9910.rst | 586 +++++++++++++++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 1 +
3 files changed, 588 insertions(+)
diff --git a/Documentation/iio/ad9910.rst b/Documentation/iio/ad9910.rst
new file mode 100644
index 000000000000..a79819b5afe5
--- /dev/null
+++ b/Documentation/iio/ad9910.rst
@@ -0,0 +1,586 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+AD9910 driver
+=============
+
+DDS (Direct Digital Synthesizer) driver for the Analog Devices Inc. AD9910.
+The module name is ``ad9910``.
+
+* `AD9910 <https://www.analog.com/en/products/ad9910.html>`_
+
+The AD9910 is a 1 GSPS DDS with a 14-bit DAC, driven over SPI. The driver
+exposes the device through the IIO ``altvoltage`` channel type and supports
+five DDS operating modes: single tone, parallel port modulation, digital ramp
+generation (DRG), RAM playback and output shift keying (OSK). The device has
+8 hardware profiles, each capable of storing independent single tone and RAM
+playback parameters.
+
+
+Channel hierarchy
+=================
+
+The driver exposes the following IIO output channels, each identified by a
+unique channel number and a human-readable label:
+
+* ``out_altvoltage100``: ``phy``: Physical output: system clock and profile control
+
+ * ``out_altvoltage101``: ``profile[0]``: Single tone control for profile 0:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage102``: ``profile[1]``: Single tone control for profile 1:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage103``: ``profile[2]``: Single tone control for profile 2:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage104``: ``profile[3]``: Single tone control for profile 3:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage105``: ``profile[4]``: Single tone control for profile 4:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage106``: ``profile[5]``: Single tone control for profile 5:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage107``: ``profile[6]``: Single tone control for profile 6:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage108``: ``profile[7]``: Single tone control for profile 7:
+ frequency, phase, amplitude
+
+ * ``out_altvoltage110``: ``parallel_port``: Parallel port modulation: enable
+ and offset/scale parameters
+
+ * ``out_altvoltage120``: ``digital_ramp_generator``: DRG control: enable
+
+ * ``out_altvoltage121``: ``digital_ramp_up``: DRG ramp-up parameters:
+ no-dwell enable, limits, step sizes, ramp rate
+ * ``out_altvoltage122``: ``digital_ramp_down``: DRG ramp-down parameters:
+ no-dwell enable, limits, step sizes, ramp rate
+
+ * ``out_altvoltage130``: ``ram_control``: RAM playback: enable, frequency,
+ phase and sampling frequency for active profile. Other configurations are
+ provided through a firmware upload interface.
+
+ * ``out_altvoltage150``: ``output_shift_keying``: OSK: enable, amplitude
+ scale, ramp rate, auto/manual control
+
+The ``phy`` channel is the root of the hierarchy. Changing its
+``sampling_frequency`` reconfigures the system clock (SYSCLK) which affects all
+other channels.
+
+All mode-specific channels (single-tone, parallel port, DRG, RAM, OSK) have an
+``enable`` attribute that turns the mode on/off.
+
+DDS modes
+=========
+
+The AD9910 supports multiple modes of operation that can be configured
+independently or in combination. Such modes and their corresponding IIO channels
+are described in this section. The following tables are extracted from the
+AD9910 datasheet and summarizes the control parameters for each mode and their
+priority when multiple sources are enabled simultaneously:
+
+.. flat-table:: DDS Frequency Control
+ :header-rows: 1
+
+ * - Priority
+ - Data Source
+ - Conditions
+
+ * - Highest Priority
+ - RAM
+ - RAM enabled and data destination is frequency
+
+ * -
+ - DRG
+ - DRG enabled and data destination is frequency
+
+ * -
+ - Parallel data and FTW (frequency_offset)
+ - Parallel data port enabled and data destination is frequency
+
+ * -
+ - FTW (frequency)
+ - RAM enabled and data destination is not frequency
+
+ * -
+ - FTW (frequency) in single tone channel for the active profile
+ - DRG enabled and data destination is not frequency
+
+ * -
+ - FTW (frequency) in single tone channel for the active profile
+ - Parallel data port enabled and data destination is not frequency
+
+ * - Lowest Priority
+ - FTW (frequency) in single tone channel for the active profile
+ - None
+
+.. flat-table:: DDS Phase Control
+ :header-rows: 1
+
+ * - Priority
+ - Data Source
+ - Conditions
+
+ * - Highest Priority
+ - RAM
+ - RAM enabled and data destination is phase or polar
+
+ * -
+ - DRG
+ - DRG enabled and data destination is phase
+
+ * -
+ - Parallel data port
+ - Parallel data port enabled and data destination is phase
+
+ * -
+ - Parallel data port and POW register LSBs (phase_offset)
+ - Parallel data port enabled and data destination is polar
+
+ * -
+ - POW (phase)
+ - RAM enabled and destination is not phase nor polar
+
+ * -
+ - POW (phase) in single tone channel for the active profile
+ - DRG enabled and data destination is not phase
+
+ * -
+ - POW (phase) in single tone channel for the active profile
+ - Parallel data port enabled and data destination is not phase nor polar
+
+ * - Lowest Priority
+ - POW (phase) in single tone channel for the active profile
+ - None
+
+.. flat-table:: DDS Amplitude Control
+ :header-rows: 1
+
+ * - Priority
+ - Data Source
+ - Conditions
+
+ * - Highest Priority
+ - OSK generator
+ - OSK enabled (auto mode)
+
+ * -
+ - ASF register
+ - OSK enabled (manual mode)
+
+ * -
+ - RAM
+ - RAM enabled and data destination is amplitude or polar
+
+ * -
+ - DRG
+ - DRG enabled and data destination is amplitude
+
+ * -
+ - Parallel data port
+ - Parallel data port enabled and data destination is amplitude
+
+ * -
+ - Parallel data port and ASF register LSBs (scale_offset)
+ - Parallel data port enabled and data destination is polar
+
+ * - Lowest Priority
+ - ASF (scale) in single tone channel for the active profile
+ - (Amplitude scale is already enabled by default)
+
+Single tone mode
+----------------
+
+Single tone is the baseline operating mode. The ``profile[Y]`` channels
+provides enable, frequency, phase and amplitude control:
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable profile Y. Only one profile can be active at a
+ time. Then enabling a profile disables the current active profile.
+ Disabling an active profile enables the next profile in ascending order,
+ wrapping around from 7 to 0.
+
+ * - ``frequency``
+ - Hz
+ - Output frequency. Range [0, SYSCLK/2). Stored in the profile's frequency
+ tuning word (FTW).
+
+ * - ``phase``
+ - rad
+ - Phase offset. Range [0, 2*pi). Stored in the profile's phase offset word
+ (POW).
+
+ * - ``scale``
+ - fractional
+ - Amplitude scale factor. Range [0, 1]. Stored in the profile's amplitude
+ scale factor (ASF).
+
+Profile switching is allowed while RAM mode is enabled. In that case single tone
+parameters are stored in a shadow register and are not written to hardware until
+RAM mode is disabled.
+
+Usage examples
+^^^^^^^^^^^^^^
+
+Set the active profile to 2 and configure a 100 MHz tone:
+
+.. code-block:: bash
+
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_en
+ echo 100000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_frequency
+ echo 0.5 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_scale
+ echo 0 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_phase
+
+Read back the current single tone frequency:
+
+.. code-block:: bash
+
+ cat /sys/bus/iio/devices/iio:device0/out_altvoltage103_frequency
+
+Parallel port mode
+------------------
+
+When enabled, the parallel port allows real-time modulation of DDS parameters
+through a 16-bit external data bus.
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable the parallel data port.
+
+ * - ``frequency_scale``
+ - power-of-2
+ - FM gain multiplier applied to 16-bit parallel input. Range [1, 32768],
+ must be a power of 2.
+
+ * - ``frequency_offset``
+ - Hz
+ - Base FTW to which scaled parallel data is added. Range [0, SYSCLK/2).
+
+ * - ``phase_offset``
+ - rad
+ - Base phase for polar modulation. Lower 8 bits of POW register.
+ Range [0, 2*pi/256).
+
+ * - ``scale_offset``
+ - fractional
+ - Base amplitude for polar modulation. Lower 6 bits of ASF register.
+ Range [0, 1/256).
+
+Usage examples
+^^^^^^^^^^^^^^
+
+Enable parallel port with a frequency scale of 16 and a 50 MHz offset:
+
+.. code-block:: bash
+
+ echo 16 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_frequency_scale
+ echo 50000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_frequency_offset
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_en
+
+Digital ramp generator (DRG)
+----------------------------
+
+The DRG produces linear frequency, phase or amplitude sweeps using dedicated
+hardware. It is controlled through three channels: a parent control channel
+(``digital_ramp_generator``) and two child ramp channels
+(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
+ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
+sets the destination to frequency.
+
+Control channel attributes
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable the DRG.
+
+Ramp channel attributes
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
+attribute set but configure ascending and descending ramp parameters
+independently:
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable the ramp no-dwell behavior. Enabling both creates a
+ bidirectional continuous ramp (Triangular pattern). Other configurations
+ creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
+ only, ramp-down only or bidirectional with dwell at the limits.
+
+ * - ``frequency``
+ - Hz
+ - Frequency ramp limit. Range [0, SYSCLK/2).
+
+ * - ``phase``
+ - rad
+ - Phase ramp limit. Range [0, 2*pi).
+
+ * - ``scale``
+ - fractional
+ - Amplitude scale ramp limit. Range [0, 1).
+
+ * - ``sampling_frequency``
+ - Hz
+ - Ramp clock rate: SYSCLK / (4 * divider).
+
+ * - ``frequency_step``
+ - Hz
+ - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
+
+ * - ``phase_step``
+ - rad
+ - Per-tick phase increment/decrement. Range [0, 2*pi).
+
+ * - ``scale_step``
+ - fractional
+ - Per-tick amplitude scale increment/decrement. Range [0, 1).
+
+Usage examples
+^^^^^^^^^^^^^^
+
+Configure a frequency sweep from 40 MHz to 60 MHz at a 1 kHz step:
+
+.. code-block:: bash
+
+ # Enable both no-dwell modes for a bidirectional ramp
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_en
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_en
+
+ # Set ramp limits
+ echo 60000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency
+ echo 40000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency
+
+ # Set ramp step size to 1 kHz
+ echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency_step
+ echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency_step
+
+ # Set ramp rate at 25 MHz
+ echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_sampling_frequency
+ echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_sampling_frequency
+
+ # Enable the DRG
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage120_en
+
+RAM mode
+--------
+
+The AD9910 contains a 1024 x 32-bit RAM that can be loaded with waveform data
+and played back to modulate frequency, phase, amplitude, or polar (phase +
+amplitude) parameters.
+
+RAM control channel attributes
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable RAM playback. Toggling swaps profile registers between
+ single tone and RAM configurations across all 8 profiles.
+
+ * - ``frequency``
+ - Hz
+ - Frequency tuning word used as the single tone frequency when
+ RAM destination is not ``frequency``. Range [0, SYSCLK/2).
+
+ * - ``phase``
+ - rad
+ - Phase offset word used as the single tone phase when RAM destination
+ is not ``phase``. Range [0, 2*pi).
+
+ * - ``sampling_frequency``
+ - Hz
+ - RAM playback step rate of the active profile, which controls how fast the
+ address counter advances: SYSCLK / (4 * step_rate).
+
+Loading RAM data
+^^^^^^^^^^^^^^^^
+
+RAM data is loaded through the firmware upload framework. The driver registers
+a firmware upload sysfs entry named ``iio_deviceX:ram``. The FW data follows
+a simple binary format:
+
+- 72-byte header:
+ - 4-byte big-endian word count: number of 32-bit words to be loaded (0-1024)
+ - 4-byte big-endian CFR1 value: configuration for the CFR1 register. Only
+ bits relevant to RAM mode (data destination and internal profile control)
+ are considered. Other bits are ignored and have no effect.
+ - Bits [30:29]: RAM data destination:
+ - 00: frequency
+ - 01: phase
+ - 10: amplitude
+ - 11: polar
+ - Bits [20:17]: Internal profile control (see Table 14 of the datasheet).
+ - 8 sets of 8-byte big-endian profile data for profiles 0-7. Each set contains:
+ - Bits [55:40]: Address step rate value
+ - Bits [39:30]: End address for the profile
+ - Bits [23:14]: Start address for the profile
+ - Bit [5]: no-dwell high for ramp-up mode
+ - Bit [3]: zero-crossing for direct-switch mode
+ - Bits [2:0]: operating mode:
+ - 000: direct switch
+ - 001: ramp-up
+ - 010: bidirectional
+ - 011: bidirectional continuous
+ - 100: ramp-up continuous
+- Followed by the specified number of 32-bit big-endian data words.
+
+Usage examples
+^^^^^^^^^^^^^^
+
+Configure RAM mode with frequency destination and load a waveform:
+
+.. code-block:: bash
+
+ # Load RAM data via firmware upload
+ echo 1 > /sys/class/firmware/iio\:device0\:ram/loading
+ cat waveform.bin > /sys/class/firmware/iio\:device0\:ram/data
+ echo 0 > /sys/class/firmware/iio\:device0\:ram/loading
+
+ # Enable RAM mode
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage130_en
+
+Output shift keying (OSK)
+-------------------------
+
+OSK controls the output amplitude envelope, allowing the output to be ramped
+on/off rather than switched abruptly.
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``en``
+ - boolean
+ - Enable/disable OSK.
+
+ * - ``scale``
+ - fractional
+ - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
+
+ * - ``sampling_frequency``
+ - Hz
+ - OSK ramp rate: SYSCLK / (4 * divider).
+
+ * - ``pinctrl_en``
+ - boolean
+ - Enable manual external pin control. When enabled, the OSK pin directly
+ gates the output on/off instead of using the automatic ramp.
+
+ * - ``scale_step``
+ - fractional
+ - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
+ and sets the per-tick increment. Writing ``0`` disables it. Rounded to
+ nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
+
+Usage examples
+^^^^^^^^^^^^^^
+
+Enable OSK with automatic ramping:
+
+.. code-block:: bash
+
+ # Set ramp rate
+ echo 1000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_sampling_frequency
+
+ # Enable automatic OSK with step size
+ echo 0.000244 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_scale_step
+
+ # Enable OSK
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_en
+
+Enable manual pin-controlled OSK:
+
+.. code-block:: bash
+
+ # Enable manual pin control
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_pinctrl_en
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_en
+
+ # Set target amplitude to full scale
+ echo 1.0 > /sys/bus/iio/devices/iio:device0/out_altvoltage150_scale
+
+Physical channel
+================
+
+The ``phy`` channel provides device-level control:
+
+.. flat-table::
+ :header-rows: 1
+
+ * - Attribute
+ - Unit
+ - Description
+
+ * - ``sampling_frequency``
+ - Hz
+ - System clock (SYSCLK) frequency. With PLL enabled, configures the PLL
+ multiplier (range 420-1000 MHz). Without PLL, ref clock can only be
+ divided by 2.
+
+ * - ``powerdown``
+ - boolean
+ - Software power-down. Writing 1 powers down the digital core, DAC,
+ reference clock input and auxiliary DAC simultaneously.
+
+Usage examples
+--------------
+
+Set the system clock to 1 GHz:
+
+.. code-block:: bash
+
+ echo 1000000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage100_sampling_frequency
+
+Read current system clock frequency:
+
+.. code-block:: bash
+
+ cat /sys/bus/iio/devices/iio:device0/out_altvoltage100_sampling_frequency
+
+Power down the device:
+
+.. code-block:: bash
+
+ echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage100_powerdown
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index ba3e609c6a13..55cb1ce84ba8 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -29,6 +29,7 @@ Industrial I/O Kernel Drivers
ad7606
ad7625
ad7944
+ ad9910
ade9000
adis16475
adis16480
diff --git a/MAINTAINERS b/MAINTAINERS
index edd87ee7da5f..14e4272357ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1637,6 +1637,7 @@ S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9910
F: Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
+F: Documentation/iio/ad9910.rst
F: drivers/iio/frequency/ad9910.c
ANALOG DEVICES INC MAX22007 DRIVER
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910
2026-04-17 8:17 ` [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
@ 2026-04-26 11:01 ` Jonathan Cameron
2026-04-26 12:48 ` Rodrigo Alencar
0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 11:01 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:30 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> configurations for clocks, DAC current, reset and basic GPIO control.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
> .../bindings/iio/frequency/adi,ad9910.yaml | 189 +++++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 196 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
> new file mode 100644
> index 000000000000..61e879bca5c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
...
> +
> + reset-gpios:
> + description:
> + GPIOs controlling the Main Device reset.
> +
> + io-reset-gpios:
> + maxItems: 1
> + description:
> + GPIO controlling the I/O_RESET pin.
> +
> + powerdown-gpios:
> + maxItems: 1
> + description:
> + GPIO controlling the EXT_PWR_DWN pin.
> +
> + update-gpios:
> + maxItems: 1
> + description:
> + GPIO controlling the I/O_UPDATE pin.
> +
> + profile-gpios:
> + minItems: 3
> + maxItems: 3
> + description:
> + GPIOs controlling the PROFILE[2:0] pins for profile selection.
> +
> + sync-err-gpios:
> + maxItems: 1
> + description:
> + GPIO used to read SYNC_SMP_ERR pin status.
Looking at the datasheet there are a few other things that might want to be here.
pll-lock for example might be wired to a gpio to allow a check that lock has
occurred. Maybe sync-samp-err as well though possibly that one wants to be an
interrupt?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation
2026-04-17 8:17 ` [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation Rodrigo Alencar via B4 Relay
@ 2026-04-26 11:42 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 11:42 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:31 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add the core AD9910 DDS driver infrastructure with single tone mode
> support. This includes SPI register access, profile management via GPIO
> pins, PLL/DAC configuration from firmware properties, and single tone
> frequency/phase/amplitude control through IIO attributes.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Hi Rodrigo
Minor stuff inline. This looks pretty clean on the whole.
I'll leave ABI discussion for documentation patches.
Thanks,
Jonathan
> diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> new file mode 100644
> index 000000000000..e9005037db1a
> --- /dev/null
> +++ b/drivers/iio/frequency/ad9910.c
...
> +struct ad9910_state {
> + struct spi_device *spi;
> + struct clk *refclk;
> +
> + struct gpio_desc *gpio_pwdown;
> + struct gpio_desc *gpio_update;
> + struct gpio_descs *gpio_profile;
> +
> + /* cached registers */
> + union ad9910_reg reg[AD9910_REG_NUM_CACHED];
> +
> + /* Lock for accessing device registers and state variables */
> + struct mutex lock;
> +
> + struct ad9910_data data;
> + u8 profile;
> +
> + union {
> + __be64 be64;
> + __be32 be32;
> + __be16 be16;
> + } rx_buf;
By tweaking the macros a little you should be able to use a suitably sized
__bexx on the stack instead of messing around with a union in here.
I think that'll end up simpler.
> + /*
> + * RAM loading requires a reasonable amount of bytes, at the same time
> + * DMA capable SPI drivers requires the transfer buffers to live in
> + * their own cache lines.
> + */
> + u8 tx_buf[AD9910_SPI_MESSAGE_LEN_MAX] __aligned(IIO_DMA_MINALIGN);
> +};
> +
> +/**
> + * ad9910_rational_scale() - Perform scaling of input given a reference.
> + * @input: The input value to be scaled.
> + * @scale: The numerator of the scaling factor.
> + * @reference: The denominator of the scaling factor.
> + *
> + * Closest rounding with mul_u64_add_u64_div_u64
> + *
> + * Return: The scaled value.
> + */
> +#define ad9910_rational_scale(input, scale, reference) ({ \
> + u64 _tmp = (reference); \
> + mul_u64_add_u64_div_u64(input, scale, _tmp >> 1, _tmp); \
Why a macro? I'd just do a helper function and let the compiler figure
out if it should inline.
> +})
> +
> +static int ad9910_io_update(struct ad9910_state *st)
> +{
> + if (st->gpio_update) {
> + gpiod_set_value_cansleep(st->gpio_update, 1);
> + udelay(1);
> + gpiod_set_value_cansleep(st->gpio_update, 0);
> + }
> +
> + return 0;
> +}
> +
> +static inline int ad9910_spi_read(struct ad9910_state *st, u8 reg, size_t len)
> +{
> + st->tx_buf[0] = AD9910_SPI_READ_MSK |
> + FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
Given spi_write_then_read() bounces everything might as well use a local u8 on
the stack for tx.
> + return spi_write_then_read(st->spi, st->tx_buf, 1, &st->rx_buf, len);
> +}
> +
> +static inline int ad9910_spi_write(struct ad9910_state *st, u8 reg, size_t len,
> + bool update)
> +{
> + int ret;
> +
> + st->tx_buf[0] = FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
> + ret = spi_write(st->spi, st->tx_buf, AD9910_SPI_DATA_IDX + len);
> + if (!ret && update)
> + return ad9910_io_update(st);
if (ret)
return ret;
if (!update)
return 0;
return ad9910_io_update(st);
Takes more lines but easier to follow. Tweak the !update bit to go the
other way if that makes more sense once more of this series is applied.
> +
> + return ret;
> +}
> +static int ad9910_set_dac_current(struct ad9910_state *st, bool update)
> +{
> + u32 fsc_code;
> +
> + /* FSC = (86.4 / Rset) * (1 + CODE/256) where Rset = 10k ohms */
> + fsc_code = DIV_ROUND_CLOSEST(st->data.dac_output_current, 90) - 96;
> + fsc_code &= 0xFFU;
Given it's so small why is the U helpful? It's not going to get sign extended
anyway (unless I need more coffee).
> +
> + return ad9910_reg32_write(st, AD9910_REG_AUX_DAC, fsc_code, update);
> +}
> +
> +static int ad9910_set_sysclk_freq(struct ad9910_state *st, u32 freq_hz,
> + bool update)
> +{
> + u32 sysclk_freq_hz, refclk_freq_hz = clk_get_rate(st->refclk);
> + u32 tmp32, vco_sel;
> + int ret;
> +
> + if (st->data.pll_enabled) {
> + if (refclk_freq_hz < AD9910_PLL_IN_MIN_FREQ_HZ ||
> + refclk_freq_hz > AD9910_PLL_IN_MAX_FREQ_HZ) {
> + dev_err(&st->spi->dev,
Use a local struct device as several copies of this in the function.
> + "REF_CLK frequency %u Hz is out of PLL input range\n",
> + refclk_freq_hz);
> + return -ERANGE;
> + }
> +
> + tmp32 = DIV_ROUND_CLOSEST(freq_hz, refclk_freq_hz);
> + tmp32 = clamp(tmp32, AD9910_PLL_MIN_N, AD9910_PLL_MAX_N);
> + sysclk_freq_hz = refclk_freq_hz * tmp32;
> +
> + if (sysclk_freq_hz < AD9910_PLL_OUT_MIN_FREQ_HZ ||
> + sysclk_freq_hz > AD9910_PLL_OUT_MAX_FREQ_HZ) {
> + dev_err(&st->spi->dev,
> + "PLL output frequency %u Hz is out of range\n",
> + sysclk_freq_hz);
> + return -ERANGE;
> + }
> +
> + if (sysclk_freq_hz <= AD9910_VCO0_RANGE_AUTO_MAX_HZ)
> + vco_sel = 0;
> + else if (sysclk_freq_hz <= AD9910_VCO1_RANGE_AUTO_MAX_HZ)
> + vco_sel = 1;
> + else if (sysclk_freq_hz <= AD9910_VCO2_RANGE_AUTO_MAX_HZ)
> + vco_sel = 2;
> + else if (sysclk_freq_hz <= AD9910_VCO3_RANGE_AUTO_MAX_HZ)
> + vco_sel = 3;
> + else if (sysclk_freq_hz <= AD9910_VCO4_RANGE_AUTO_MAX_HZ)
> + vco_sel = 4;
> + else
> + vco_sel = 5;
> +
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
> + AD9910_CFR3_N_MSK | AD9910_CFR3_VCO_SEL_MSK,
> + FIELD_PREP(AD9910_CFR3_N_MSK, tmp32) |
> + FIELD_PREP(AD9910_CFR3_VCO_SEL_MSK, vco_sel),
> + update);
> + if (ret)
> + return ret;
> + } else {
> + tmp32 = DIV_ROUND_CLOSEST(refclk_freq_hz, freq_hz);
> + tmp32 = clamp(tmp32, 1, 2);
> + sysclk_freq_hz = refclk_freq_hz / tmp32;
> + tmp32 = FIELD_PREP(AD9910_CFR3_REFCLK_DIV_BYPASS_MSK, tmp32 % 2);
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
> + AD9910_CFR3_REFCLK_DIV_BYPASS_MSK,
> + tmp32, update);
> + if (ret)
> + return ret;
> + }
> +
> + st->data.sysclk_freq_hz = sysclk_freq_hz;
> +
> + return 0;
> +}
> +
> +static int ad9910_powerdown_set(struct ad9910_state *st, bool enable)
I'd just put the gpio call in line.
If not rename this a little as at one place where this is used I thought
it was checking if it was set rather than setting it. set_powerdown
would be fine.
> +{
> + return gpiod_set_value_cansleep(st->gpio_pwdown, enable);
> +}
> +
> +static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + int val;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (private) {
> + case AD9910_POWERDOWN:
> + val = !!FIELD_GET(AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> + st->reg[AD9910_REG_CFR1].val32);
I think Linus came out against the magic !! a few years ago in favour
of a ternary. Whilst we have a lot of !! in IIO ? 1 : 0; is probably
the slightly better way to go.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> +
Why this blank line?
> + u32 val32;
> + int ret;
> +
> + ret = kstrtou32(buf, 10, &val32);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (private) {
> + case AD9910_POWERDOWN:
> + val32 = val32 ? AD9910_CFR1_SOFT_POWER_DOWN_MSK : 0;
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> + val32, true);
> + break;
I'd prefer a check here on ret then unconditional return of len below.
It's nice to know on error path that nothing else is to be done. Tends to
scale better as code grows.
> + default:
> + return -EINVAL;
> + }
> +
> + return ret ?: len;
> +}
> +
> +#define AD9910_EXT_INFO_TMPL(_name, _ident, _shared, _fn_desc) { \
> + .name = _name, \
> + .read = ad9910_ ## _fn_desc ## _read, \
> + .write = ad9910_ ## _fn_desc ## _write, \
> + .private = _ident, \
> + .shared = _shared, \
> +}
> +
> +#define AD9910_EXT_INFO(_name, _ident, _shared) \
> + AD9910_EXT_INFO_TMPL(_name, _ident, _shared, ext_info)
> +
> +static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
> + AD9910_EXT_INFO("powerdown", AD9910_POWERDOWN, IIO_SEPARATE),
I haven't checked yet, but if any of these macros are only used once or
twice, just squash the definition inline. Every time these are used
they mean looking at what the implementation is. If there are loads
of uses then fair enough.
> + { }
> +};
> +
> +#define AD9910_PROFILE_CHAN(idx) { \
> + .type = IIO_ALTVOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = AD9910_CHANNEL_PROFILE_ ## idx, \
> + .address = AD9910_CHAN_IDX_PROFILE_ ## idx, \
> + .scan_index = -1, \
Normally we only bring .scan_index in at all on adding buffered / chrdev
support. So I'd drop it for now as it ends up being confusing to
indicate a channel is not being used with the buffered interface that
doesn't exist (yet)
> + .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) | \
> + BIT(IIO_CHAN_INFO_FREQUENCY) | \
> + BIT(IIO_CHAN_INFO_PHASE) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +static const struct iio_chan_spec ad9910_channels[] = {
> + [AD9910_CHAN_IDX_PHY] = {
> + .type = IIO_ALTVOLTAGE,
> + .indexed = 1,
> + .output = 1,
> + .channel = AD9910_CHANNEL_PHY,
> + .address = AD9910_CHAN_IDX_PHY,
> + .scan_index = -1,
As above.
> + .info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .ext_info = ad9910_phy_ext_info,
> + },
> + [AD9910_CHAN_IDX_PROFILE_0] = AD9910_PROFILE_CHAN(0),
> + [AD9910_CHAN_IDX_PROFILE_1] = AD9910_PROFILE_CHAN(1),
> + [AD9910_CHAN_IDX_PROFILE_2] = AD9910_PROFILE_CHAN(2),
> + [AD9910_CHAN_IDX_PROFILE_3] = AD9910_PROFILE_CHAN(3),
> + [AD9910_CHAN_IDX_PROFILE_4] = AD9910_PROFILE_CHAN(4),
> + [AD9910_CHAN_IDX_PROFILE_5] = AD9910_PROFILE_CHAN(5),
> + [AD9910_CHAN_IDX_PROFILE_6] = AD9910_PROFILE_CHAN(6),
> + [AD9910_CHAN_IDX_PROFILE_7] = AD9910_PROFILE_CHAN(7),
> +};
> +
> +static int ad9910_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long info)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + u64 tmp64;
> + u32 tmp32;
Similar to the write_raw() comment below. If you can give these
more useful names than tmp that would be good.
> +
> + guard(mutex)(&st->lock);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_ENABLE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = (chan->channel - AD9910_CHANNEL_PROFILE_0);
No need for brackets on that one - particularly as it is locally inconsistent.
> + *val = (tmp32 == st->profile);
> + break;
> + default:
> + return -EINVAL;
> + }
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_FREQUENCY:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp32 = FIELD_GET(AD9910_PROFILE_ST_FTW_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + break;
> + default:
> + return -EINVAL;
> + }
> + tmp64 = (u64)tmp32 * st->data.sysclk_freq_hz;
> + *val = tmp64 >> 32;
> + *val2 = ((tmp64 & GENMASK_ULL(31, 0)) * MICRO) >> 32;
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_PHASE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_GET(AD9910_PROFILE_ST_POW_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + tmp32 = (tmp64 * AD9910_MAX_PHASE_MICRORAD) >> 16;
> + *val = tmp32 / MICRO;
> + *val2 = tmp32 % MICRO;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_GET(AD9910_PROFILE_ST_ASF_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + *val = 0;
> + *val2 = tmp64 * MICRO >> 14;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PHY:
> + *val = st->data.sysclk_freq_hz;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad9910_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long info)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + u64 tmp64;
> + u32 tmp32;
Seems tmp32 is always some type of index, perhaps we can
use naming to reflect that?
> +
> + guard(mutex)(&st->lock);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_ENABLE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + if (!val) {
> + if (tmp32 != st->profile)
> + return 0;
> +
> + tmp32 = (tmp32 + 1) % AD9910_NUM_PROFILES;
This is a little odd looking. Perhaps a comment on why we are rolling by 1?
> + }
> +
> + return ad9910_profile_set(st, tmp32);
> + default:
> +
> +static int ad9910_debugfs_reg_read(struct ad9910_state *st, bool high32,
> + unsigned int reg, unsigned int *readval)
> +{
> + union ad9910_reg tmp;
> + int ret;
> +
> + switch (reg) {
> + case AD9910_REG_DRG_LIMIT:
> + case AD9910_REG_DRG_STEP:
> + case AD9910_REG_PROFILE0 ... AD9910_REG_PROFILE7:
> + ret = ad9910_reg64_read(st, reg, &tmp.val64);
> + if (ret)
> + return ret;
> + *readval = high32 ? upper_32_bits(tmp.val64) :
> + lower_32_bits(tmp.val64);
> + return 0;
> + case AD9910_REG_POW:
> + ret = ad9910_reg16_read(st, reg, &tmp.val16);
> + if (ret)
> + return ret;
> + *readval = tmp.val16;
> + return 0;
> + default:
> + ret = ad9910_reg32_read(st, reg, &tmp.val32);
> + if (ret)
> + return ret;
> + *readval = tmp.val32;
> + return 0;
> + }
> +}
> +
> +static int ad9910_debugfs_reg_write(struct ad9910_state *st, bool high32,
> + unsigned int reg, unsigned int writeval)
> +{
> + switch (reg) {
> + case AD9910_REG_DRG_LIMIT:
> + case AD9910_REG_DRG_STEP:
> + case AD9910_REG_PROFILE0 ... AD9910_REG_PROFILE7:
> + if (high32)
> + return ad9910_reg64_update(st, reg, GENMASK_ULL(63, 32),
> + FIELD_PREP(GENMASK_ULL(63, 32), writeval),
> + true);
> +
> + return ad9910_reg64_update(st, reg, GENMASK_ULL(31, 0),
> + writeval, true);
> + case AD9910_REG_POW:
> + return ad9910_reg16_write(st, reg, writeval, true);
> + default:
> + return ad9910_reg32_write(st, reg, writeval, true);
> + }
> +}
> +
> +static int ad9910_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg, unsigned int writeval,
> + unsigned int *readval)
> +{
> + bool high32 = FIELD_GET(AD9910_REG_HIGH32_FLAG_MSK, reg);
> + struct ad9910_state *st = iio_priv(indio_dev);
> +
> + /*
> + * REG_HIGH32_FLAG is only used for regiter access to indicate upper 32
> + * bits of 64-bit registers. It is a workaround for debugfs_reg_access()
> + * limitation which only supports 32-bit values.
Feels like we should just fix this rather than papering over it.
Add a new callback to the the core to handle larger registers and then
use it here. I'm not a huge fan of the debugfs interface in general
but given we have it, why not make it work for ever use case?
> + */
> + reg &= ~AD9910_REG_HIGH32_FLAG_MSK;
> + if (reg >= AD9910_REG_RAM)
> + return -EINVAL;
> +
> + guard(mutex)(&st->lock);
> +
> + if (readval)
> + return ad9910_debugfs_reg_read(st, high32, reg, readval);
> + else
> + return ad9910_debugfs_reg_write(st, high32, reg, writeval);
> +}
> +static int ad9910_cfg_sysclk(struct ad9910_state *st, bool update)
> +{
> + u32 tmp32, cfr3 = AD9910_CFR3_OPEN_MSK;
Don't mix declarations with assignment with those that don't assign.
It isn't great for readability.
> +
> + cfr3 |= AD9910_CFR3_VCO_SEL_MSK |
> + FIELD_PREP(AD9910_CFR3_DRV0_MSK, st->data.refclk_out_drv);
> +
> + if (st->data.pll_enabled) {
> + tmp32 = st->data.pll_charge_pump_current - AD9910_ICP_MIN_uA;
> + tmp32 = DIV_ROUND_CLOSEST(tmp32, AD9910_ICP_STEP_uA);
> + cfr3 |= FIELD_PREP(AD9910_CFR3_ICP_MSK, tmp32) |
> + AD9910_CFR3_PLL_EN_MSK;
> + } else {
> + cfr3 |= AD9910_CFR3_REFCLK_DIV_RESETB_MSK |
> + AD9910_CFR3_PFD_RESET_MSK;
> + }
> + st->reg[AD9910_REG_CFR3].val32 = cfr3;
> +
> + return ad9910_set_sysclk_freq(st, AD9910_PLL_OUT_MAX_FREQ_HZ, update);
> +}
> +
> +static int ad9910_parse_fw(struct ad9910_state *st)
> +{
> + static const char * const refclk_out_drv0[] = {
> + "disabled", "low", "medium", "high",
> + };
> + struct device *dev = &st->spi->dev;
> + u32 tmp;
> + int ret;
> +
> + st->data.pll_enabled = device_property_read_bool(dev, "adi,pll-enable");
> + if (st->data.pll_enabled) {
> + tmp = AD9910_ICP_MIN_uA;
> + device_property_read_u32(dev, "adi,charge-pump-current-microamp", &tmp);
> + if (tmp < AD9910_ICP_MIN_uA || tmp > AD9910_ICP_MAX_uA)
> + return dev_err_probe(dev, -ERANGE,
> + "invalid charge pump current %u\n", tmp);
> + st->data.pll_charge_pump_current = tmp;
> +
> + st->data.refclk_out_drv = AD9910_REFCLK_OUT_DRV_DISABLED;
Given the array stuff means you aren't reading directly into here having this
preset to default doesn't make much sense - instead I'd have this as...
> + ret = device_property_match_property_string(dev,
> + "adi,refclk-out-drive-strength",
> + refclk_out_drv0,
> + ARRAY_SIZE(refclk_out_drv0));
> + if (ret >= 0)
if (ret < 0)
st->data.refclk_out_drv = AD9910_REFCLK_OUT_DRV_DISABLED;
else
st->data.refclk_out_drv = ret;
> + st->data.refclk_out_drv = ret;
> + }
> +
> + tmp = AD9910_DAC_IOUT_DEFAULT_uA;
> + device_property_read_u32(dev, "adi,dac-output-current-microamp", &tmp);
> + if (tmp < AD9910_DAC_IOUT_MIN_uA || tmp > AD9910_DAC_IOUT_MAX_uA)
> + return dev_err_probe(dev, -ERANGE,
> + "Invalid DAC output current %u uA\n", tmp);
> + st->data.dac_output_current = tmp;
> +
> + return 0;
> +}
> +
> +static int ad9910_setup(struct ad9910_state *st, struct reset_control *dev_rst)
> +{
> + u32 reg32;
> + int ret;
> +
> + ret = reset_control_deassert(dev_rst);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_reg32_write(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SDIO_INPUT_ONLY_MSK, false);
> + if (ret)
> + return ret;
I assume this is the the point where the stuff undone in ad9910_release()
is set as soft powerdown is cleared.
If so, it's here that you should be registering the devm action,
or you should be undoing this in any error path later in this function
so that it has no side effects.
> +
> + reg32 = AD9910_CFR2_AMP_SCALE_SINGLE_TONE_MSK |
> + AD9910_CFR2_SYNC_TIMING_VAL_DISABLE_MSK |
> + AD9910_CFR2_DRG_NO_DWELL_MSK |
> + AD9910_CFR2_DATA_ASM_HOLD_LAST_MSK |
> + AD9910_CFR2_SYNC_CLK_EN_MSK |
> + AD9910_CFR2_PDCLK_ENABLE_MSK;
> + ret = ad9910_reg32_write(st, AD9910_REG_CFR2, reg32, false);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_cfg_sysclk(st, false);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_set_dac_current(st, false);
> + if (ret)
> + return ret;
> +
> + return ad9910_io_update(st);
> +}
> +
> +static void ad9910_release(void *data)
> +{
> + struct ad9910_state *st = data;
> +
> + if (!ad9910_powerdown_set(st, true))
> + return;
> +
> + ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> + AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> + true);
> +}
> +
> +static int ad9910_probe(struct spi_device *spi)
> +{
> + static const char * const supplies[] = {
> + "dvdd-io33", "avdd33", "dvdd18", "avdd18",
> + };
> + struct reset_control *dev_rst;
> + struct gpio_desc *io_rst_gpio;
> + struct device *dev = &spi->dev;
> + struct iio_dev *indio_dev;
> + struct ad9910_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> +
> + st->refclk = devm_clk_get_enabled(dev, "ref_clk");
> + if (IS_ERR(st->refclk))
> + return dev_err_probe(dev, PTR_ERR(st->refclk),
> + "Failed to get reference clock\n");
> +
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
> +
> + ret = devm_mutex_init(dev, &st->lock);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = "ad9910";
> + indio_dev->info = &ad9910_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = ad9910_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ad9910_channels);
> +
> + dev_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> + if (IS_ERR(dev_rst))
> + return dev_err_probe(dev, PTR_ERR(dev_rst),
> + "failed to get device reset control\n");
> +
> + /*
> + * The IO RESET pin is not used in this driver, as we assume that all
> + * SPI transfers are complete, but if it is wired up, we need to make
> + * sure it is not floating. We can use either a reset controller or a
> + * GPIO for this.
> + */
> + io_rst_gpio = devm_gpiod_get_optional(dev, "io-reset", GPIOD_OUT_LOW);
> + if (IS_ERR(io_rst_gpio))
> + return dev_err_probe(dev, PTR_ERR(io_rst_gpio),
> + "failed to get io reset gpio\n");
> +
> + st->gpio_pwdown = devm_gpiod_get_optional(dev, "powerdown",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_pwdown))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_pwdown),
> + "failed to get powerdown gpio\n");
So at this point you've powered up the chip. That's fine but I think
the matching power down (assuming the unwind of this doesn't do it -
and I think it doesn't?) is not registered until you add the
ad9910_release devm action below. It should be immediately after this
- hence it needs it's own devm action.
> +
> + st->gpio_update = devm_gpiod_get_optional(dev, "update", GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_update))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_update),
> + "failed to get update gpio\n");
> +
> + st->gpio_profile = devm_gpiod_get_array_optional(dev, "profile",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_profile))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_profile),
> + "failed to get profile gpios\n");
> +
> + ret = ad9910_parse_fw(st);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_setup(st, dev_rst);
> + if (ret)
> + return dev_err_probe(dev, ret, "device setup failed\n");
> +
> + ret = devm_add_action_or_reset(dev, ad9910_release, st);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add release action\n");
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support
2026-04-17 8:17 ` [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support Rodrigo Alencar via B4 Relay
@ 2026-04-26 11:59 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 11:59 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:32 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add parallel port channel with frequency scale, frequency offset, phase
> offset, and amplitude offset extended attributes for configuring the
> parallel data path.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Really minor stuff - mostly follow on from review of previous patch.
> ---
> drivers/iio/frequency/ad9910.c | 152 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
> diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> index e9005037db1a..5b4076028a29 100644
> --- a/drivers/iio/frequency/ad9910.c
> +++ b/drivers/iio/frequency/ad9910.c
> struct ad9910_data {
> @@ -478,6 +490,10 @@ static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
> val = !!FIELD_GET(AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> st->reg[AD9910_REG_CFR1].val32);
> break;
> + case AD9910_PP_FREQ_SCALE:
> + val = BIT(FIELD_GET(AD9910_CFR2_FM_GAIN_MSK,
> + st->reg[AD9910_REG_CFR2].val32));
> + break;
> default:
> return -EINVAL;
> }
> @@ -508,6 +524,113 @@ static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
> AD9910_CFR1_SOFT_POWER_DOWN_MSK,
> val32, true);
> break;
> + case AD9910_PP_FREQ_SCALE:
> + if (val32 > BIT(15) || !is_power_of_2(val32))
> + return -EINVAL;
> +
> + val32 = FIELD_PREP(AD9910_CFR2_FM_GAIN_MSK, ilog2(val32));
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR2,
> + AD9910_CFR2_FM_GAIN_MSK,
> + val32, true);
As in previous, I'd prefer the more verbose
if (ret)
return ret;
break;
Same for all the similar cases.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret ?: len;
> +}
> @@ -661,6 +808,11 @@ static int ad9910_write_raw(struct iio_dev *indio_dev,
> }
>
> return ad9910_profile_set(st, tmp32);
> + case AD9910_CHANNEL_PARALLEL_PORT:
> + tmp32 = FIELD_PREP(AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK, !!val);
> + return ad9910_reg32_update(st, AD9910_REG_CFR2,
> + AD9910_CFR2_PARALLEL_DATA_PORT_EN_MSK,
> + tmp32, true);
Ah. So tmp32 isn't always an index. Maybe just use local clearer named variables?
> default:
> return -EINVAL;
> }
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support
2026-04-17 8:17 ` [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support Rodrigo Alencar via B4 Relay
@ 2026-04-26 12:05 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 12:05 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:33 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add DRG channels with destination selection (frequency, phase, or
I'd spell out Digital Ramp Generator here as well rather than
just in the patch title.
I'll come back to the comment below (probably) when looking at the
ABI documentation.
> amplitude) based on attribute writes, dwell mode control,
> configurable upper/lower limits, increment/decrement step sizes, and
> step rate settings.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
> drivers/iio/frequency/ad9910.c | 425 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 423 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> index 5b4076028a29..c9ec677cd63a 100644
> --- a/drivers/iio/frequency/ad9910.c
> +++ b/drivers/iio/frequency/ad9910.c
>
> +static const struct iio_chan_spec_ext_info ad9910_drg_ramp_ext_info[] = {
> + AD9910_DRG_EXT_INFO("frequency_step", AD9910_DRG_FREQ_STEP),
> + AD9910_DRG_EXT_INFO("phase_step", AD9910_DRG_PHASE_STEP),
> + AD9910_DRG_EXT_INFO("scale_step", AD9910_DRG_AMP_STEP),
For things that are tidied to a clock rate we normally try to express
then in terms of time rather than ticks / steps as that's what the
user normally cares about. Anyhow I'll come back to this with the
documentation patch review.
> + { }
> +};
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels
2026-04-17 8:17 ` [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels Rodrigo Alencar via B4 Relay
@ 2026-04-26 12:12 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 12:12 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:36 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add human-readable labels for all AD9910 IIO channels via the read_label
> callback.
Given how important they are for this driver, I'd add the labels as you
go along rather than doing it at the end like this.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910
2026-04-26 11:01 ` Jonathan Cameron
@ 2026-04-26 12:48 ` Rodrigo Alencar
0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Alencar @ 2026-04-26 12:48 UTC (permalink / raw)
To: Jonathan Cameron, Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On 26/04/26 12:01PM, Jonathan Cameron wrote:
> On Fri, 17 Apr 2026 09:17:30 +0100
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
>
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> > configurations for clocks, DAC current, reset and basic GPIO control.
> >
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > ---
> > .../bindings/iio/frequency/adi,ad9910.yaml | 189 +++++++++++++++++++++
> > MAINTAINERS | 7 +
> > 2 files changed, 196 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
> > new file mode 100644
> > index 000000000000..61e879bca5c2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
>
> ...
>
> > +
> > + reset-gpios:
> > + description:
> > + GPIOs controlling the Main Device reset.
> > +
> > + io-reset-gpios:
> > + maxItems: 1
> > + description:
> > + GPIO controlling the I/O_RESET pin.
> > +
> > + powerdown-gpios:
> > + maxItems: 1
> > + description:
> > + GPIO controlling the EXT_PWR_DWN pin.
> > +
> > + update-gpios:
> > + maxItems: 1
> > + description:
> > + GPIO controlling the I/O_UPDATE pin.
> > +
> > + profile-gpios:
> > + minItems: 3
> > + maxItems: 3
> > + description:
> > + GPIOs controlling the PROFILE[2:0] pins for profile selection.
> > +
> > + sync-err-gpios:
> > + maxItems: 1
> > + description:
> > + GPIO used to read SYNC_SMP_ERR pin status.
> Looking at the datasheet there are a few other things that might want to be here.
>
> pll-lock for example might be wired to a gpio to allow a check that lock has
> occurred. Maybe sync-samp-err as well though possibly that one wants to be an
> interrupt?
I can add the lock-detect gpio. sync-samp-err must be a gpio, because it must be
used by a calibration routine that modifies sync delays and reads the gpio value
to check the sync state. The goal is to find the best delay configs. More details:
https://ez.analog.com/cfs-file/__key/communityserver-discussions-components-files/318/3426.Synchronizing-Multiple-AD9910s.pdf
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
2026-04-17 8:17 ` [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver Rodrigo Alencar via B4 Relay
@ 2026-04-26 13:10 ` Jonathan Cameron
2026-04-26 20:42 ` Rodrigo Alencar
0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-26 13:10 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On Fri, 17 Apr 2026 09:17:38 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add documentation for the AD9910 DDS IIO driver, which describes channels,
> DDS modes, attributes and ABI usage examples.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Hi Rodrigo,
I think this is getting close to something workable subject to some tweaks
to not make the priority thing visible and use rate of change parameters
so /Sec rather than steps.
Given this defines the ABI for a whole class of new devices that are
rather complex, one concern is whether whatever we define here is general
enough to be useful.
Do you have any other DDS in your queue to upstream? Maybe it's worth
sanity checking the ABI against them to see if it is fit for purpose?
Thanks,
Jonathan
> ---
> Documentation/iio/ad9910.rst | 586 +++++++++++++++++++++++++++++++++++++++++++
> Documentation/iio/index.rst | 1 +
> MAINTAINERS | 1 +
> 3 files changed, 588 insertions(+)
>
> diff --git a/Documentation/iio/ad9910.rst b/Documentation/iio/ad9910.rst
> new file mode 100644
> index 000000000000..a79819b5afe5
> --- /dev/null
> +++ b/Documentation/iio/ad9910.rst
> @@ -0,0 +1,586 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +AD9910 driver
> +=============
> +
> +DDS (Direct Digital Synthesizer) driver for the Analog Devices Inc. AD9910.
More conventional to do term then acronym.
Direct Digital Synthesizer (DDS) driver ..
> +The module name is ``ad9910``.
> +
> +* `AD9910 <https://www.analog.com/en/products/ad9910.html>`_
> +
> +The AD9910 is a 1 GSPS DDS with a 14-bit DAC, driven over SPI. The driver
Controlled maybe rather than driven? Driven sort of implies that the SPI bus
is driving the output at 1G.
> +exposes the device through the IIO ``altvoltage`` channel type and supports
...
> +DDS modes
> +=========
> +
> +The AD9910 supports multiple modes of operation that can be configured
> +independently or in combination. Such modes and their corresponding IIO channels
> +are described in this section. The following tables are extracted from the
> +AD9910 datasheet and summarizes the control parameters for each mode and their
> +priority when multiple sources are enabled simultaneously:
Maybe add a bit on what priority means. Does it mean that only the highest
priority one is acted on? If so why do we need to expose that others are
enabled? Just report only the highest priority one as enabled.
I can see the hardware needs to do priority so it knows where to go when
a given source is disabled but from a software point of view that
can be controlled by us enabling that next item (and the driver does
things in the right order to get the appropriate transition)
That may mean that if all modes are disabled, we have to disable any output
but seems doable.
> +
> +.. flat-table:: DDS Frequency Control
> + :header-rows: 1
> +
> + * - Priority
> + - Data Source
> + - Conditions
> +
> + * - Highest Priority
> + - RAM
> + - RAM enabled and data destination is frequency
> +
> + * -
> + - DRG
> + - DRG enabled and data destination is frequency
> +
> + * -
> + - Parallel data and FTW (frequency_offset)
> + - Parallel data port enabled and data destination is frequency
> +
> + * -
> + - FTW (frequency)
> + - RAM enabled and data destination is not frequency
> +
> + * -
> + - FTW (frequency) in single tone channel for the active profile
> + - DRG enabled and data destination is not frequency
> +
> + * -
> + - FTW (frequency) in single tone channel for the active profile
> + - Parallel data port enabled and data destination is not frequency
> +
> + * - Lowest Priority
> + - FTW (frequency) in single tone channel for the active profile
> + - None
> +
> +Single tone mode
> +----------------
> +
> +Single tone is the baseline operating mode. The ``profile[Y]`` channels
> +provides enable, frequency, phase and amplitude control:
> +
> +.. flat-table::
> + :header-rows: 1
> +
> + * - Attribute
> + - Unit
> + - Description
> +
> + * - ``en``
> + - boolean
> + - Enable/disable profile Y. Only one profile can be active at a
> + time. Then enabling a profile disables the current active profile.
> + Disabling an active profile enables the next profile in ascending order,
> + wrapping around from 7 to 0.
That passing on to the next one seems rather non user friendly. Can we just
disable the whole unit under those conditions instead? As above that may mean
turning of the output entirely. So to change mode it would always be transition
to the one that is enabled. A disable of a given channel results in no output.
> +
> + * - ``frequency``
> + - Hz
> + - Output frequency. Range [0, SYSCLK/2). Stored in the profile's frequency
> + tuning word (FTW).
> +
> + * - ``phase``
> + - rad
> + - Phase offset. Range [0, 2*pi). Stored in the profile's phase offset word
> + (POW).
> +
> + * - ``scale``
> + - fractional
> + - Amplitude scale factor. Range [0, 1]. Stored in the profile's amplitude
> + scale factor (ASF).
> +
> +Profile switching is allowed while RAM mode is enabled. In that case single tone
> +parameters are stored in a shadow register and are not written to hardware until
> +RAM mode is disabled.
This is only visible to userspace because of the priority thing? If we hide
that away to transition from RAM to this mode would just mean enabling this mode.
> +
> +Usage examples
> +^^^^^^^^^^^^^^
> +
> +Set the active profile to 2 and configure a 100 MHz tone:
> +
> +.. code-block:: bash
> +
> + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_en
> + echo 100000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_frequency
> + echo 0.5 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_scale
> + echo 0 > /sys/bus/iio/devices/iio:device0/out_altvoltage103_phase
I'd expect to set frequency scale and phase before moving to the tone. Other wise
we get random garbage until those are set. So probably reorder the example.
Fine to say you can also modify them live, but that isn't the most common thing
to do.
> +
> +Read back the current single tone frequency:
> +
> +.. code-block:: bash
> +
> + cat /sys/bus/iio/devices/iio:device0/out_altvoltage103_frequency
> +
> +Parallel port mode
> +------------------
> +
...
> +
> +Usage examples
> +^^^^^^^^^^^^^^
> +
> +Enable parallel port with a frequency scale of 16 and a 50 MHz offset:
> +
> +.. code-block:: bash
> +
> + echo 16 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_frequency_scale
> + echo 50000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_frequency_offset
> + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage110_en
Odd indent.
> +
> +Digital ramp generator (DRG)
> +----------------------------
> +
> +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> +hardware. It is controlled through three channels: a parent control channel
> +(``digital_ramp_generator``) and two child ramp channels
> +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> +sets the destination to frequency.
> +
> +Control channel attributes
> +^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +.. flat-table::
> + :header-rows: 1
> +
> + * - Attribute
> + - Unit
> + - Description
> +
> + * - ``en``
> + - boolean
> + - Enable/disable the DRG.
> +
> +Ramp channel attributes
> +^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> +attribute set but configure ascending and descending ramp parameters
> +independently:
> +
> +.. flat-table::
> + :header-rows: 1
> +
> + * - Attribute
> + - Unit
> + - Description
> +
> + * - ``en``
> + - boolean
> + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> + bidirectional continuous ramp (Triangular pattern). Other configurations
> + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
transition
> + only, ramp-down only or bidirectional with dwell at the limits.
Feels a little unintuitive to use the generic enable for this.
We might need a specific control for this one.
> +
> + * - ``frequency``
> + - Hz
> + - Frequency ramp limit. Range [0, SYSCLK/2).
> +
> + * - ``phase``
> + - rad
> + - Phase ramp limit. Range [0, 2*pi).
> +
> + * - ``scale``
> + - fractional
> + - Amplitude scale ramp limit. Range [0, 1).
> +
> + * - ``sampling_frequency``
> + - Hz
> + - Ramp clock rate: SYSCLK / (4 * divider).
> +
> + * - ``frequency_step``
> + - Hz
> + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
So this was the bit I referred to earlier. Normally we do
rate of change measurements for this stuff rather than what happens on
each tick (based on how we handle things like ROC events)
So could we make these
``frequency_roc`` units HZ/Sec
etc? Then from the mix configured would need to work out the optimum
tick to deliver it.
I suppose it's possible that someone might want a stepped frequency
though which would break this approach? Does anyone actually do that?
If so we'd need to keep the samping_frequency but then control _roc
with that in mind.
> +
> + * - ``phase_step``
> + - rad
> + - Per-tick phase increment/decrement. Range [0, 2*pi).
> +
> + * - ``scale_step``
> + - fractional
> + - Per-tick amplitude scale increment/decrement. Range [0, 1).
> +
> +Usage examples
> +^^^^^^^^^^^^^^
> +
> +Configure a frequency sweep from 40 MHz to 60 MHz at a 1 kHz step:
> +
> +.. code-block:: bash
> +
> + # Enable both no-dwell modes for a bidirectional ramp
> + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_en
> + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_en
Fix indents as mix of tabs and spaces. As above I think using this enable
for no dwell is not going to generalize well. I think we need new ABI for this
though I'm open to anyone suggesting something we can reuse.
> +
> + # Set ramp limits
> + echo 60000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency
> + echo 40000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency
> +
> + # Set ramp step size to 1 kHz
> + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency_step
> + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency_step
> +
> + # Set ramp rate at 25 MHz
> + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_sampling_frequency
> + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_sampling_frequency
> +
> + # Enable the DRG
> + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage120_en
> +
> +RAM mode
> +--------
> +
> +The AD9910 contains a 1024 x 32-bit RAM that can be loaded with waveform data
> +and played back to modulate frequency, phase, amplitude, or polar (phase +
> +amplitude) parameters.
> +
> +RAM control channel attributes
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +.. flat-table::
> + :header-rows: 1
> +
> + * - Attribute
> + - Unit
> + - Description
> +
> + * - ``en``
> + - boolean
> + - Enable/disable RAM playback. Toggling swaps profile registers between
> + single tone and RAM configurations across all 8 profiles.
So this might be a fly in the ointment of my previous comment about using
enable of profile to turn off ram. I guess disabling RAM drops into the
matched number tone profile? That's a pain but not disastrous. We'd have
to only allow transitions by enabling the match number tone profile.
So transitions allowed would be
tone_profileX -> ram_profileX
tone_profileX -> tone_profileY
ram_profileX -> tone_profileX
ram_profileX -> ram_profileY
But not
tone_profileX -> ram_profileY
where X!=Y
> +
> + * - ``frequency``
> + - Hz
> + - Frequency tuning word used as the single tone frequency when
> + RAM destination is not ``frequency``. Range [0, SYSCLK/2).
> +
> + * - ``phase``
> + - rad
> + - Phase offset word used as the single tone phase when RAM destination
> + is not ``phase``. Range [0, 2*pi).
> +
> + * - ``sampling_frequency``
> + - Hz
> + - RAM playback step rate of the active profile, which controls how fast the
> + address counter advances: SYSCLK / (4 * step_rate).
Why do we care what the sysclk relationship is? It's ticking in HZ.
> +
> +Output shift keying (OSK)
This is a new one on me...
> +-------------------------
> +
> +OSK controls the output amplitude envelope, allowing the output to be ramped
> +on/off rather than switched abruptly.
> +
> +.. flat-table::
> + :header-rows: 1
> +
> + * - Attribute
> + - Unit
> + - Description
> +
> + * - ``en``
> + - boolean
> + - Enable/disable OSK.
> +
> + * - ``scale``
> + - fractional
> + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> +
> + * - ``sampling_frequency``
> + - Hz
> + - OSK ramp rate: SYSCLK / (4 * divider).
> +
> + * - ``pinctrl_en``
> + - boolean
> + - Enable manual external pin control. When enabled, the OSK pin directly
> + gates the output on/off instead of using the automatic ramp.
I wonder if we should split the various OSK modes into different channels given
only some properties apply to each of automatic and manual modes. Also I think
automatic mode is meaningless without pinctrl_en (so that can be replaced
by simply enabling that mode). I have no idea if anyone cares about pin ctrl
with manual mode or not? That one seems even more odd.
> +
> + * - ``scale_step``
> + - fractional
> + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
Similar thing about rate of change of amplitude fitting better with current ABI
than step does.
...
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
2026-04-26 13:10 ` Jonathan Cameron
@ 2026-04-26 20:42 ` Rodrigo Alencar
2026-04-27 9:46 ` Jonathan Cameron
0 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Alencar @ 2026-04-26 20:42 UTC (permalink / raw)
To: Jonathan Cameron, Rodrigo Alencar via B4 Relay
Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
linux-hardening, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
On 26/04/26 02:10PM, Jonathan Cameron wrote:
> On Fri, 17 Apr 2026 09:17:38 +0100
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
>
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > DDS modes, attributes and ABI usage examples.
> >
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Hi Rodrigo,
>
> I think this is getting close to something workable subject to some tweaks
> to not make the priority thing visible and use rate of change parameters
> so /Sec rather than steps.
I am not sure about this one. Getting the value into units per seconds will
increase the range of values by a lot, e.g., for the frequency case the step
size can range from a few Hz up to the entire supported range (hundreds of
MHz), and if you consider that one would often have the sampling_frequency
at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
and I am not sure how practical is that, although it can have a physical meaning,
like a "chirp slope".
>
> Given this defines the ABI for a whole class of new devices that are
> rather complex, one concern is whether whatever we define here is general
> enough to be useful.
>
> Do you have any other DDS in your queue to upstream? Maybe it's worth
> sanity checking the ABI against them to see if it is fit for purpose?
Not really, still the only DDS. Other DDS of the same family have a similar
Digital Ramp Generator with controls over ramp limits, rates and step.
...
> > +DDS modes
> > +=========
> > +
> > +The AD9910 supports multiple modes of operation that can be configured
> > +independently or in combination. Such modes and their corresponding IIO channels
> > +are described in this section. The following tables are extracted from the
> > +AD9910 datasheet and summarizes the control parameters for each mode and their
> > +priority when multiple sources are enabled simultaneously:
>
> Maybe add a bit on what priority means. Does it mean that only the highest
> priority one is acted on? If so why do we need to expose that others are
> enabled? Just report only the highest priority one as enabled.
>
> I can see the hardware needs to do priority so it knows where to go when
> a given source is disabled but from a software point of view that
> can be controlled by us enabling that next item (and the driver does
> things in the right order to get the appropriate transition)
>
> That may mean that if all modes are disabled, we have to disable any output
> but seems doable.
That is a bit complicated, as you can see, this part has modes that target
one DDS parameter or multiple (destinations: phase, frequency, amplitude).
Also, multiple modes can coexist, when they target different parameters/destination.
At the same time, RAM mode complicates everything because even though it targets
one specific parameter, once it is enabled influences the base mode for the
other parameters because single-tone is off. I have ordered the mode channels so
that higher index have higher priority, so that can be a bit clearer.
Right now, all the controls are provided, what might be missing is a way
to query which level of those priorities is currently active, but those levels
are not the same thing as the controls. If we turn the priority levels into the
controls themselves it would be a different ABI and it would get a lot messy.
That is why I am dumping this priority table in this document! =(
> > +
> > +.. flat-table:: DDS Frequency Control
> > + :header-rows: 1
> > +
> > + * - Priority
> > + - Data Source
> > + - Conditions
> > +
> > + * - Highest Priority
> > + - RAM
> > + - RAM enabled and data destination is frequency
> > +
> > + * -
> > + - DRG
> > + - DRG enabled and data destination is frequency
> > +
> > + * -
> > + - Parallel data and FTW (frequency_offset)
> > + - Parallel data port enabled and data destination is frequency
> > +
> > + * -
> > + - FTW (frequency)
> > + - RAM enabled and data destination is not frequency
> > +
> > + * -
> > + - FTW (frequency) in single tone channel for the active profile
> > + - DRG enabled and data destination is not frequency
> > +
> > + * -
> > + - FTW (frequency) in single tone channel for the active profile
> > + - Parallel data port enabled and data destination is not frequency
> > +
> > + * - Lowest Priority
> > + - FTW (frequency) in single tone channel for the active profile
> > + - None
>
> > +
> > +Single tone mode
> > +----------------
> > +
> > +Single tone is the baseline operating mode. The ``profile[Y]`` channels
> > +provides enable, frequency, phase and amplitude control:
> > +
> > +.. flat-table::
> > + :header-rows: 1
> > +
> > + * - Attribute
> > + - Unit
> > + - Description
> > +
> > + * - ``en``
> > + - boolean
> > + - Enable/disable profile Y. Only one profile can be active at a
> > + time. Then enabling a profile disables the current active profile.
> > + Disabling an active profile enables the next profile in ascending order,
> > + wrapping around from 7 to 0.
>
> That passing on to the next one seems rather non user friendly. Can we just
> disable the whole unit under those conditions instead? As above that may mean
> turning of the output entirely. So to change mode it would always be transition
> to the one that is enabled. A disable of a given channel results in no output.
Yes, I can go for the software powerdown in that case! and the powerdown attribute
could be removed?
>
> > +
> > + * - ``frequency``
> > + - Hz
> > + - Output frequency. Range [0, SYSCLK/2). Stored in the profile's frequency
> > + tuning word (FTW).
> > +
> > + * - ``phase``
> > + - rad
> > + - Phase offset. Range [0, 2*pi). Stored in the profile's phase offset word
> > + (POW).
> > +
> > + * - ``scale``
> > + - fractional
> > + - Amplitude scale factor. Range [0, 1]. Stored in the profile's amplitude
> > + scale factor (ASF).
> > +
> > +Profile switching is allowed while RAM mode is enabled. In that case single tone
> > +parameters are stored in a shadow register and are not written to hardware until
> > +RAM mode is disabled.
>
> This is only visible to userspace because of the priority thing? If we hide
> that away to transition from RAM to this mode would just mean enabling this mode.
Partially, but the real reason is that single-tone and RAM shares the same profile
registers. So I have things cached, which allows user to change single-tone stuff
while RAM is enabled.
...
> > +Digital ramp generator (DRG)
> > +----------------------------
> > +
> > +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> > +hardware. It is controlled through three channels: a parent control channel
> > +(``digital_ramp_generator``) and two child ramp channels
> > +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> > +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> > +sets the destination to frequency.
> > +
> > +Control channel attributes
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +.. flat-table::
> > + :header-rows: 1
> > +
> > + * - Attribute
> > + - Unit
> > + - Description
> > +
> > + * - ``en``
> > + - boolean
> > + - Enable/disable the DRG.
> > +
> > +Ramp channel attributes
> > +^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> > +attribute set but configure ascending and descending ramp parameters
> > +independently:
> > +
> > +.. flat-table::
> > + :header-rows: 1
> > +
> > + * - Attribute
> > + - Unit
> > + - Description
> > +
> > + * - ``en``
> > + - boolean
> > + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> > + bidirectional continuous ramp (Triangular pattern). Other configurations
> > + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
>
> transition
>
> > + only, ramp-down only or bidirectional with dwell at the limits.
>
> Feels a little unintuitive to use the generic enable for this.
> We might need a specific control for this one.
How about dwell_en, but it might not sound that generic. I used "enable" because:
- no-dwell high means a ramp-up pattern (only enabling the ramp-up channel)
- no-dwell low means a ramp-down pattern (only enabling the ramp-down channel)
- both no-dwell is a continuous ramp that goes up and down. (both enabled)
The last case is a bit off though, when both are disabled we get the normal mode, which
is also a ramps up and down, but dwelling in the limits.
> > +
> > + * - ``frequency``
> > + - Hz
> > + - Frequency ramp limit. Range [0, SYSCLK/2).
> > +
> > + * - ``phase``
> > + - rad
> > + - Phase ramp limit. Range [0, 2*pi).
> > +
> > + * - ``scale``
> > + - fractional
> > + - Amplitude scale ramp limit. Range [0, 1).
> > +
> > + * - ``sampling_frequency``
> > + - Hz
> > + - Ramp clock rate: SYSCLK / (4 * divider).
> > +
> > + * - ``frequency_step``
> > + - Hz
> > + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
>
> So this was the bit I referred to earlier. Normally we do
> rate of change measurements for this stuff rather than what happens on
> each tick (based on how we handle things like ROC events)
>
> So could we make these
> ``frequency_roc`` units HZ/Sec
> etc? Then from the mix configured would need to work out the optimum
> tick to deliver it.
>
> I suppose it's possible that someone might want a stepped frequency
> though which would break this approach? Does anyone actually do that?
> If so we'd need to keep the samping_frequency but then control _roc
> with that in mind.
yeah... frequency steps would make sense when the user controls when to
perform the updates, or when it comes from certain events.
sampling frequency defines the timing and this roc attr would
also depend on timing... there would be two options:
* ignore updating ramp step when sampling freq is updated. Here roc
would have a different value when readback.
* cache the "requested" roc and use it update ramp step when sampling
freq is updated, so roc remains with the value initially configured.
>
> > +
> > + * - ``phase_step``
> > + - rad
> > + - Per-tick phase increment/decrement. Range [0, 2*pi).
> > +
> > + * - ``scale_step``
> > + - fractional
> > + - Per-tick amplitude scale increment/decrement. Range [0, 1).
> > +
> > +Usage examples
> > +^^^^^^^^^^^^^^
> > +
> > +Configure a frequency sweep from 40 MHz to 60 MHz at a 1 kHz step:
> > +
> > +.. code-block:: bash
> > +
> > + # Enable both no-dwell modes for a bidirectional ramp
> > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_en
> > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_en
>
> Fix indents as mix of tabs and spaces. As above I think using this enable
> for no dwell is not going to generalize well. I think we need new ABI for this
> though I'm open to anyone suggesting something we can reuse.
I suggested dwell_en, and I am not sure what else could be used here.
Something like hold_en could work and sounds more generic.. not sure.
> > +
> > + # Set ramp limits
> > + echo 60000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency
> > + echo 40000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency
> > +
> > + # Set ramp step size to 1 kHz
> > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency_step
> > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency_step
> > +
> > + # Set ramp rate at 25 MHz
> > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_sampling_frequency
> > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_sampling_frequency
> > +
> > + # Enable the DRG
> > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage120_en
> > +
> > +RAM mode
> > +--------
> > +
> > +The AD9910 contains a 1024 x 32-bit RAM that can be loaded with waveform data
> > +and played back to modulate frequency, phase, amplitude, or polar (phase +
> > +amplitude) parameters.
> > +
> > +RAM control channel attributes
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +.. flat-table::
> > + :header-rows: 1
> > +
> > + * - Attribute
> > + - Unit
> > + - Description
> > +
> > + * - ``en``
> > + - boolean
> > + - Enable/disable RAM playback. Toggling swaps profile registers between
> > + single tone and RAM configurations across all 8 profiles.
>
> So this might be a fly in the ointment of my previous comment about using
> enable of profile to turn off ram. I guess disabling RAM drops into the
> matched number tone profile? That's a pain but not disastrous. We'd have
> to only allow transitions by enabling the match number tone profile.
>
> So transitions allowed would be
>
> tone_profileX -> ram_profileX
> tone_profileX -> tone_profileY
> ram_profileX -> tone_profileX
> ram_profileX -> ram_profileY
>
> But not
> tone_profileX -> ram_profileY
> where X!=Y
No, there is only one RAM enable bit, once it is on, all single tone
profiles go away (all profile registers are repurposed for RAM mode
usage).
> > +
> > + * - ``frequency``
> > + - Hz
> > + - Frequency tuning word used as the single tone frequency when
> > + RAM destination is not ``frequency``. Range [0, SYSCLK/2).
> > +
> > + * - ``phase``
> > + - rad
> > + - Phase offset word used as the single tone phase when RAM destination
> > + is not ``phase``. Range [0, 2*pi).
> > +
> > + * - ``sampling_frequency``
> > + - Hz
> > + - RAM playback step rate of the active profile, which controls how fast the
> > + address counter advances: SYSCLK / (4 * step_rate).
>
> Why do we care what the sysclk relationship is? It's ticking in HZ.
Can be removed, but just to point out that the configured value will adjust to the
value where the divider is an integer. Maybe the user should be aware of that.
> > +
> > +Output shift keying (OSK)
> This is a new one on me...
> > +-------------------------
> > +
> > +OSK controls the output amplitude envelope, allowing the output to be ramped
> > +on/off rather than switched abruptly.
>
> > +
> > +.. flat-table::
> > + :header-rows: 1
> > +
> > + * - Attribute
> > + - Unit
> > + - Description
> > +
> > + * - ``en``
> > + - boolean
> > + - Enable/disable OSK.
> > +
> > + * - ``scale``
> > + - fractional
> > + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> > +
> > + * - ``sampling_frequency``
> > + - Hz
> > + - OSK ramp rate: SYSCLK / (4 * divider).
> > +
> > + * - ``pinctrl_en``
> > + - boolean
> > + - Enable manual external pin control. When enabled, the OSK pin directly
> > + gates the output on/off instead of using the automatic ramp.
>
> I wonder if we should split the various OSK modes into different channels given
> only some properties apply to each of automatic and manual modes. Also I think
> automatic mode is meaningless without pinctrl_en (so that can be replaced
> by simply enabling that mode). I have no idea if anyone cares about pin ctrl
> with manual mode or not? That one seems even more odd.
OSK is either in manual or auto:
* In manual mode the OSK pin enables and disables the output based on its level.
* In auto, the OSK pin controls the direction the amplitude updates.
If we enable RAM mode, and other modes do not target amplitude, the only way to
manually configure the amplitude in software (i.e. without using an OSK gpio)
is going manual mode (scale_step == 0), disable this pinctrl_en and then set the
scale property (ASF register). That is the only reason I added this property.
> > +
> > + * - ``scale_step``
> > + - fractional
> > + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> > + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> > + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
>
> Similar thing about rate of change of amplitude fitting better with current ABI
> than step does.
ok... and this one is still missing the correspondent available attr.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
2026-04-26 20:42 ` Rodrigo Alencar
@ 2026-04-27 9:46 ` Jonathan Cameron
2026-04-27 10:31 ` Nuno Sá
0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2026-04-27 9:46 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-iio,
devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
On Sun, 26 Apr 2026 21:42:15 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/04/26 02:10PM, Jonathan Cameron wrote:
> > On Fri, 17 Apr 2026 09:17:38 +0100
> > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> >
> > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > >
> > > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > > DDS modes, attributes and ABI usage examples.
> > >
> > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > Hi Rodrigo,
> >
> > I think this is getting close to something workable subject to some tweaks
> > to not make the priority thing visible and use rate of change parameters
> > so /Sec rather than steps.
>
> I am not sure about this one. Getting the value into units per seconds will
> increase the range of values by a lot, e.g., for the frequency case the step
> size can range from a few Hz up to the entire supported range (hundreds of
> MHz), and if you consider that one would often have the sampling_frequency
> at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
> and I am not sure how practical is that, although it can have a physical meaning,
> like a "chirp slope".
That scaling is indeed a bit of a pain though it will go in a 64 bit int
however, seems likely we'll get higher frequency devices one day that will
limb even faster.
Maybe wait and see if anyone else has input on this.
>
> >
> > Given this defines the ABI for a whole class of new devices that are
> > rather complex, one concern is whether whatever we define here is general
> > enough to be useful.
> >
> > Do you have any other DDS in your queue to upstream? Maybe it's worth
> > sanity checking the ABI against them to see if it is fit for purpose?
>
> Not really, still the only DDS. Other DDS of the same family have a similar
> Digital Ramp Generator with controls over ramp limits, rates and step.
There are two in staging that have been there a very long time...
ad9832 and ad9834. I haven't looked at how they correspond to this.
We should think hard about whether to bring them inline with this
and out of staging, or just delete them.
>
> ...
>
> > > +DDS modes
> > > +=========
> > > +
> > > +The AD9910 supports multiple modes of operation that can be configured
> > > +independently or in combination. Such modes and their corresponding IIO channels
> > > +are described in this section. The following tables are extracted from the
> > > +AD9910 datasheet and summarizes the control parameters for each mode and their
> > > +priority when multiple sources are enabled simultaneously:
> >
> > Maybe add a bit on what priority means. Does it mean that only the highest
> > priority one is acted on? If so why do we need to expose that others are
> > enabled? Just report only the highest priority one as enabled.
> >
> > I can see the hardware needs to do priority so it knows where to go when
> > a given source is disabled but from a software point of view that
> > can be controlled by us enabling that next item (and the driver does
> > things in the right order to get the appropriate transition)
> >
> > That may mean that if all modes are disabled, we have to disable any output
> > but seems doable.
>
> That is a bit complicated, as you can see, this part has modes that target
> one DDS parameter or multiple (destinations: phase, frequency, amplitude).
> Also, multiple modes can coexist, when they target different parameters/destination.
> At the same time, RAM mode complicates everything because even though it targets
> one specific parameter, once it is enabled influences the base mode for the
> other parameters because single-tone is off. I have ordered the mode channels so
> that higher index have higher priority, so that can be a bit clearer.
>
> Right now, all the controls are provided, what might be missing is a way
> to query which level of those priorities is currently active, but those levels
> are not the same thing as the controls. If we turn the priority levels into the
> controls themselves it would be a different ABI and it would get a lot messy.
> That is why I am dumping this priority table in this document! =(
I understand (at least some) of the hardware complexity but I don't like
the fact this is effectively exposing it to userspace. + I really don't want
more ABI to indicate whether a mode is actively doing anything or not.
Whilst I agree the code will be more complex, having clarity on what is
enabled at any given time is definitely something we ant to aim for.
Can we work out a transition diagram? That might make it easier to
tell whether it's possible to map it as single enables at a time and
incorporate weird corners like the RAM one. Maybe not needed if the
RAM one is the only real oddity and otherwise it's just going up
and down the priority lists.
One complexity I can see with single enables is that they'd need
to be separate for each of frequency, phase and amplitude to reflect
the transitions that can occur.
Also the fun of profiles, where those profile pins are basically picking
symbols - could be used for multi level PSK or FSK for example if wired
up to an external symbol source. I'd be a bit surprised if those are
always wired up to a host CPU.
*sigh* I'm talking my self around to needing ABI to indicate a channel
is active. The symbol stuff gets us some of the way there (and would
work for the tones) but doesn't cover the added complexity of RAM etc.
So 'maybe' new ABI for _isactive or something like that?
Perhaps the boundary we put on this is the ABI should be such that
simple choices such as enabling a single tone, or single RAM mode
setting are intuitive.
Why do we only have one ram channel? I'd kind of expect the firmware
to fill all 8 RAM profiles because of that 'external' profile pins
use case.
>
> > > +
> > > +.. flat-table:: DDS Frequency Control
> > > + :header-rows: 1
> > > +
> > > + * - Priority
> > > + - Data Source
> > > + - Conditions
> > > +
> > > + * - Highest Priority
> > > + - RAM
> > > + - RAM enabled and data destination is frequency
> > > +
> > > + * -
> > > + - DRG
> > > + - DRG enabled and data destination is frequency
> > > +
> > > + * -
> > > + - Parallel data and FTW (frequency_offset)
> > > + - Parallel data port enabled and data destination is frequency
> > > +
> > > + * -
> > > + - FTW (frequency)
> > > + - RAM enabled and data destination is not frequency
> > > +
> > > + * -
> > > + - FTW (frequency) in single tone channel for the active profile
> > > + - DRG enabled and data destination is not frequency
> > > +
> > > + * -
> > > + - FTW (frequency) in single tone channel for the active profile
> > > + - Parallel data port enabled and data destination is not frequency
> > > +
> > > + * - Lowest Priority
> > > + - FTW (frequency) in single tone channel for the active profile
> > > + - None
> >
> > > +
> > > +Single tone mode
> > > +----------------
> > > +
> > > +Single tone is the baseline operating mode. The ``profile[Y]`` channels
> > > +provides enable, frequency, phase and amplitude control:
> > > +
> > > +.. flat-table::
> > > + :header-rows: 1
> > > +
> > > + * - Attribute
> > > + - Unit
> > > + - Description
> > > +
> > > + * - ``en``
> > > + - boolean
> > > + - Enable/disable profile Y. Only one profile can be active at a
> > > + time. Then enabling a profile disables the current active profile.
> > > + Disabling an active profile enables the next profile in ascending order,
> > > + wrapping around from 7 to 0.
> >
> > That passing on to the next one seems rather non user friendly. Can we just
> > disable the whole unit under those conditions instead? As above that may mean
> > turning of the output entirely. So to change mode it would always be transition
> > to the one that is enabled. A disable of a given channel results in no output.
>
> Yes, I can go for the software powerdown in that case! and the powerdown attribute
> could be removed?
Yes, I think that works. If all sources are disabled, then powerdown.
Maybe we keep the powerdown as well though as that's standard DAC ABI.
>
> >
> > > +
> > > + * - ``frequency``
> > > + - Hz
> > > + - Output frequency. Range [0, SYSCLK/2). Stored in the profile's frequency
> > > + tuning word (FTW).
> > > +
> > > + * - ``phase``
> > > + - rad
> > > + - Phase offset. Range [0, 2*pi). Stored in the profile's phase offset word
> > > + (POW).
> > > +
> > > + * - ``scale``
> > > + - fractional
> > > + - Amplitude scale factor. Range [0, 1]. Stored in the profile's amplitude
> > > + scale factor (ASF).
> > > +
> > > +Profile switching is allowed while RAM mode is enabled. In that case single tone
> > > +parameters are stored in a shadow register and are not written to hardware until
> > > +RAM mode is disabled.
> >
> > This is only visible to userspace because of the priority thing? If we hide
> > that away to transition from RAM to this mode would just mean enabling this mode.
>
> Partially, but the real reason is that single-tone and RAM shares the same profile
> registers. So I have things cached, which allows user to change single-tone stuff
> while RAM is enabled.
Ah. So there is no smooth (e.g. race free) path to transition from RAM mode to
single tone? That is annoying.
>
> ...
>
> > > +Digital ramp generator (DRG)
> > > +----------------------------
> > > +
> > > +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> > > +hardware. It is controlled through three channels: a parent control channel
> > > +(``digital_ramp_generator``) and two child ramp channels
> > > +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> > > +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> > > +sets the destination to frequency.
> > > +
> > > +Control channel attributes
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +.. flat-table::
> > > + :header-rows: 1
> > > +
> > > + * - Attribute
> > > + - Unit
> > > + - Description
> > > +
> > > + * - ``en``
> > > + - boolean
> > > + - Enable/disable the DRG.
> > > +
> > > +Ramp channel attributes
> > > +^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> > > +attribute set but configure ascending and descending ramp parameters
> > > +independently:
> > > +
> > > +.. flat-table::
> > > + :header-rows: 1
> > > +
> > > + * - Attribute
> > > + - Unit
> > > + - Description
> > > +
> > > + * - ``en``
> > > + - boolean
> > > + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> > > + bidirectional continuous ramp (Triangular pattern). Other configurations
> > > + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
> >
> > transition
> >
> > > + only, ramp-down only or bidirectional with dwell at the limits.
> >
> > Feels a little unintuitive to use the generic enable for this.
> > We might need a specific control for this one.
>
> How about dwell_en, but it might not sound that generic. I used "enable" because:
> - no-dwell high means a ramp-up pattern (only enabling the ramp-up channel)
> - no-dwell low means a ramp-down pattern (only enabling the ramp-down channel)
> - both no-dwell is a continuous ramp that goes up and down. (both enabled)
> The last case is a bit off though, when both are disabled we get the normal mode, which
> is also a ramps up and down, but dwelling in the limits.
>
> > > +
> > > + * - ``frequency``
> > > + - Hz
> > > + - Frequency ramp limit. Range [0, SYSCLK/2).
> > > +
> > > + * - ``phase``
> > > + - rad
> > > + - Phase ramp limit. Range [0, 2*pi).
Looking at this again, how do we set the DRG mode? E.g. if it effects
only phase?
> > > +
> > > + * - ``scale``
> > > + - fractional
> > > + - Amplitude scale ramp limit. Range [0, 1).
> > > +
> > > + * - ``sampling_frequency``
> > > + - Hz
> > > + - Ramp clock rate: SYSCLK / (4 * divider).
> > > +
> > > + * - ``frequency_step``
> > > + - Hz
> > > + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
> >
> > So this was the bit I referred to earlier. Normally we do
> > rate of change measurements for this stuff rather than what happens on
> > each tick (based on how we handle things like ROC events)
> >
> > So could we make these
> > ``frequency_roc`` units HZ/Sec
> > etc? Then from the mix configured would need to work out the optimum
> > tick to deliver it.
> >
> > I suppose it's possible that someone might want a stepped frequency
> > though which would break this approach? Does anyone actually do that?
> > If so we'd need to keep the samping_frequency but then control _roc
> > with that in mind.
>
> yeah... frequency steps would make sense when the user controls when to
> perform the updates, or when it comes from certain events.
You've lost me here. How can they do that? Some external clocking
or event?
>
> sampling frequency defines the timing and this roc attr would
> also depend on timing... there would be two options:
> * ignore updating ramp step when sampling freq is updated. Here roc
> would have a different value when readback.
> * cache the "requested" roc and use it update ramp step when sampling
> freq is updated, so roc remains with the value initially configured.
If we do end up going the roc route both are valid ABI, but the second is
nicer from useability point of view.
>
> >
> > > +
> > > + * - ``phase_step``
> > > + - rad
> > > + - Per-tick phase increment/decrement. Range [0, 2*pi).
> > > +
> > > + * - ``scale_step``
> > > + - fractional
> > > + - Per-tick amplitude scale increment/decrement. Range [0, 1).
> > > +
> > > +Usage examples
> > > +^^^^^^^^^^^^^^
> > > +
> > > +Configure a frequency sweep from 40 MHz to 60 MHz at a 1 kHz step:
> > > +
> > > +.. code-block:: bash
> > > +
> > > + # Enable both no-dwell modes for a bidirectional ramp
> > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_en
> > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_en
> >
> > Fix indents as mix of tabs and spaces. As above I think using this enable
> > for no dwell is not going to generalize well. I think we need new ABI for this
> > though I'm open to anyone suggesting something we can reuse.
>
> I suggested dwell_en, and I am not sure what else could be used here.
> Something like hold_en could work and sounds more generic.. not sure.
likewise :(
>
> > > +
> > > + # Set ramp limits
> > > + echo 60000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency
> > > + echo 40000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency
> > > +
> > > + # Set ramp step size to 1 kHz
> > > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency_step
> > > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency_step
> > > +
> > > + # Set ramp rate at 25 MHz
> > > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_sampling_frequency
> > > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_sampling_frequency
> > > +
> > > + # Enable the DRG
> > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage120_en
> > > +
> > > +RAM mode
> > > +--------
> > > +
> > > +The AD9910 contains a 1024 x 32-bit RAM that can be loaded with waveform data
> > > +and played back to modulate frequency, phase, amplitude, or polar (phase +
> > > +amplitude) parameters.
> > > +
> > > +RAM control channel attributes
> > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +
> > > +.. flat-table::
> > > + :header-rows: 1
> > > +
> > > + * - Attribute
> > > + - Unit
> > > + - Description
> > > +
> > > + * - ``en``
> > > + - boolean
> > > + - Enable/disable RAM playback. Toggling swaps profile registers between
> > > + single tone and RAM configurations across all 8 profiles.
> >
> > So this might be a fly in the ointment of my previous comment about using
> > enable of profile to turn off ram. I guess disabling RAM drops into the
> > matched number tone profile? That's a pain but not disastrous. We'd have
> > to only allow transitions by enabling the match number tone profile.
> >
> > So transitions allowed would be
> >
> > tone_profileX -> ram_profileX
> > tone_profileX -> tone_profileY
> > ram_profileX -> tone_profileX
> > ram_profileX -> ram_profileY
> >
> > But not
> > tone_profileX -> ram_profileY
> > where X!=Y
>
> No, there is only one RAM enable bit, once it is on, all single tone
> profiles go away (all profile registers are repurposed for RAM mode
> usage).
I'd missed the repurposing that basically means you never transition from
tone to RAM without going via some intermediate point (maybe power down).
>
> > > +
> > > + * - ``frequency``
> > > + - Hz
> > > + - Frequency tuning word used as the single tone frequency when
> > > + RAM destination is not ``frequency``. Range [0, SYSCLK/2).
> > > +
> > > + * - ``phase``
> > > + - rad
> > > + - Phase offset word used as the single tone phase when RAM destination
> > > + is not ``phase``. Range [0, 2*pi).
> > > +
> > > + * - ``sampling_frequency``
> > > + - Hz
> > > + - RAM playback step rate of the active profile, which controls how fast the
> > > + address counter advances: SYSCLK / (4 * step_rate).
> >
> > Why do we care what the sysclk relationship is? It's ticking in HZ.
>
> Can be removed, but just to point out that the configured value will adjust to the
> value where the divider is an integer. Maybe the user should be aware of that.
If you keep it just add something to note that it takes discrete values
according to that formula. I was reading more into it than that and
getting confused to where step_rate was coming from!
>
> > > +
> > > +Output shift keying (OSK)
> > This is a new one on me...
> > > +-------------------------
> > > +
> > > +OSK controls the output amplitude envelope, allowing the output to be ramped
> > > +on/off rather than switched abruptly.
> >
> > > +
> > > +.. flat-table::
> > > + :header-rows: 1
> > > +
> > > + * - Attribute
> > > + - Unit
> > > + - Description
> > > +
> > > + * - ``en``
> > > + - boolean
> > > + - Enable/disable OSK.
> > > +
> > > + * - ``scale``
> > > + - fractional
> > > + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> > > +
> > > + * - ``sampling_frequency``
> > > + - Hz
> > > + - OSK ramp rate: SYSCLK / (4 * divider).
> > > +
> > > + * - ``pinctrl_en``
> > > + - boolean
> > > + - Enable manual external pin control. When enabled, the OSK pin directly
> > > + gates the output on/off instead of using the automatic ramp.
> >
> > I wonder if we should split the various OSK modes into different channels given
> > only some properties apply to each of automatic and manual modes. Also I think
> > automatic mode is meaningless without pinctrl_en (so that can be replaced
> > by simply enabling that mode). I have no idea if anyone cares about pin ctrl
> > with manual mode or not? That one seems even more odd.
>
> OSK is either in manual or auto:
> * In manual mode the OSK pin enables and disables the output based on its level.
> * In auto, the OSK pin controls the direction the amplitude updates.
>
> If we enable RAM mode, and other modes do not target amplitude, the only way to
> manually configure the amplitude in software (i.e. without using an OSK gpio)
> is going manual mode (scale_step == 0), disable this pinctrl_en and then set the
> scale property (ASF register). That is the only reason I added this property.
Ah. Maybe we hide that away and make the amplitude a property of RAM channel?
It can do this magic under the hood. I don't mind the attributes for OSK changing
if this trick is in use (they won't be active anyway).
>
> > > +
> > > + * - ``scale_step``
> > > + - fractional
> > > + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> > > + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> > > + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
> >
> > Similar thing about rate of change of amplitude fitting better with current ABI
> > than step does.
>
> ok... and this one is still missing the correspondent available attr.
Available is a bit tricky when there is an inverse relationship involved in the maths
as what do we put the step as. Maybe we should add a note on that to the ABI
docs. [min step max] where step gives the minimum step that due to non linearity
may not be applicable between discrete values that may be taken away from that
minimum granularity base value. If that occurs the driver will round to the
nearest possible value.
Something like that.
J
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
2026-04-27 9:46 ` Jonathan Cameron
@ 2026-04-27 10:31 ` Nuno Sá
0 siblings, 0 replies; 20+ messages in thread
From: Nuno Sá @ 2026-04-27 10:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rodrigo Alencar, Rodrigo Alencar via B4 Relay, rodrigo.alencar,
linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
On Mon, Apr 27, 2026 at 10:46:08AM +0100, Jonathan Cameron wrote:
> On Sun, 26 Apr 2026 21:42:15 +0100
> Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
>
> > On 26/04/26 02:10PM, Jonathan Cameron wrote:
> > > On Fri, 17 Apr 2026 09:17:38 +0100
> > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > >
> > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > >
> > > > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > > > DDS modes, attributes and ABI usage examples.
> > > >
> > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > >
> > > Hi Rodrigo,
> > >
> > > I think this is getting close to something workable subject to some tweaks
> > > to not make the priority thing visible and use rate of change parameters
> > > so /Sec rather than steps.
> >
> > I am not sure about this one. Getting the value into units per seconds will
> > increase the range of values by a lot, e.g., for the frequency case the step
> > size can range from a few Hz up to the entire supported range (hundreds of
> > MHz), and if you consider that one would often have the sampling_frequency
> > at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
> > and I am not sure how practical is that, although it can have a physical meaning,
> > like a "chirp slope".
>
> That scaling is indeed a bit of a pain though it will go in a 64 bit int
> however, seems likely we'll get higher frequency devices one day that will
> limb even faster.
>
> Maybe wait and see if anyone else has input on this.
If we think things like RF DACs (which internally - typically - make use
of things like DDS), we can already go to the GHz "world".
Not saying we already have such a device that would map the new ABI with
bigger values (we might have but nothing I'm aware of from the top of my
head) but just saying the above is, indeed, very likely to pop up at some
time.
Just something to bear in mind :)
- Nuno Sá
> >
> > >
> > > Given this defines the ABI for a whole class of new devices that are
> > > rather complex, one concern is whether whatever we define here is general
> > > enough to be useful.
> > >
> > > Do you have any other DDS in your queue to upstream? Maybe it's worth
> > > sanity checking the ABI against them to see if it is fit for purpose?
> >
> > Not really, still the only DDS. Other DDS of the same family have a similar
> > Digital Ramp Generator with controls over ramp limits, rates and step.
>
> There are two in staging that have been there a very long time...
> ad9832 and ad9834. I haven't looked at how they correspond to this.
>
> We should think hard about whether to bring them inline with this
> and out of staging, or just delete them.
>
> >
> > ...
> >
> > > > +DDS modes
> > > > +=========
> > > > +
> > > > +The AD9910 supports multiple modes of operation that can be configured
> > > > +independently or in combination. Such modes and their corresponding IIO channels
> > > > +are described in this section. The following tables are extracted from the
> > > > +AD9910 datasheet and summarizes the control parameters for each mode and their
> > > > +priority when multiple sources are enabled simultaneously:
> > >
> > > Maybe add a bit on what priority means. Does it mean that only the highest
> > > priority one is acted on? If so why do we need to expose that others are
> > > enabled? Just report only the highest priority one as enabled.
> > >
> > > I can see the hardware needs to do priority so it knows where to go when
> > > a given source is disabled but from a software point of view that
> > > can be controlled by us enabling that next item (and the driver does
> > > things in the right order to get the appropriate transition)
> > >
> > > That may mean that if all modes are disabled, we have to disable any output
> > > but seems doable.
> >
> > That is a bit complicated, as you can see, this part has modes that target
> > one DDS parameter or multiple (destinations: phase, frequency, amplitude).
> > Also, multiple modes can coexist, when they target different parameters/destination.
> > At the same time, RAM mode complicates everything because even though it targets
> > one specific parameter, once it is enabled influences the base mode for the
> > other parameters because single-tone is off. I have ordered the mode channels so
> > that higher index have higher priority, so that can be a bit clearer.
> >
> > Right now, all the controls are provided, what might be missing is a way
> > to query which level of those priorities is currently active, but those levels
> > are not the same thing as the controls. If we turn the priority levels into the
> > controls themselves it would be a different ABI and it would get a lot messy.
> > That is why I am dumping this priority table in this document! =(
>
> I understand (at least some) of the hardware complexity but I don't like
> the fact this is effectively exposing it to userspace. + I really don't want
> more ABI to indicate whether a mode is actively doing anything or not.
> Whilst I agree the code will be more complex, having clarity on what is
> enabled at any given time is definitely something we ant to aim for.
>
> Can we work out a transition diagram? That might make it easier to
> tell whether it's possible to map it as single enables at a time and
> incorporate weird corners like the RAM one. Maybe not needed if the
> RAM one is the only real oddity and otherwise it's just going up
> and down the priority lists.
>
> One complexity I can see with single enables is that they'd need
> to be separate for each of frequency, phase and amplitude to reflect
> the transitions that can occur.
>
> Also the fun of profiles, where those profile pins are basically picking
> symbols - could be used for multi level PSK or FSK for example if wired
> up to an external symbol source. I'd be a bit surprised if those are
> always wired up to a host CPU.
>
> *sigh* I'm talking my self around to needing ABI to indicate a channel
> is active. The symbol stuff gets us some of the way there (and would
> work for the tones) but doesn't cover the added complexity of RAM etc.
> So 'maybe' new ABI for _isactive or something like that?
>
> Perhaps the boundary we put on this is the ABI should be such that
> simple choices such as enabling a single tone, or single RAM mode
> setting are intuitive.
>
> Why do we only have one ram channel? I'd kind of expect the firmware
> to fill all 8 RAM profiles because of that 'external' profile pins
> use case.
>
>
> >
> > > > +
> > > > +.. flat-table:: DDS Frequency Control
> > > > + :header-rows: 1
> > > > +
> > > > + * - Priority
> > > > + - Data Source
> > > > + - Conditions
> > > > +
> > > > + * - Highest Priority
> > > > + - RAM
> > > > + - RAM enabled and data destination is frequency
> > > > +
> > > > + * -
> > > > + - DRG
> > > > + - DRG enabled and data destination is frequency
> > > > +
> > > > + * -
> > > > + - Parallel data and FTW (frequency_offset)
> > > > + - Parallel data port enabled and data destination is frequency
> > > > +
> > > > + * -
> > > > + - FTW (frequency)
> > > > + - RAM enabled and data destination is not frequency
> > > > +
> > > > + * -
> > > > + - FTW (frequency) in single tone channel for the active profile
> > > > + - DRG enabled and data destination is not frequency
> > > > +
> > > > + * -
> > > > + - FTW (frequency) in single tone channel for the active profile
> > > > + - Parallel data port enabled and data destination is not frequency
> > > > +
> > > > + * - Lowest Priority
> > > > + - FTW (frequency) in single tone channel for the active profile
> > > > + - None
> > >
> > > > +
> > > > +Single tone mode
> > > > +----------------
> > > > +
> > > > +Single tone is the baseline operating mode. The ``profile[Y]`` channels
> > > > +provides enable, frequency, phase and amplitude control:
> > > > +
> > > > +.. flat-table::
> > > > + :header-rows: 1
> > > > +
> > > > + * - Attribute
> > > > + - Unit
> > > > + - Description
> > > > +
> > > > + * - ``en``
> > > > + - boolean
> > > > + - Enable/disable profile Y. Only one profile can be active at a
> > > > + time. Then enabling a profile disables the current active profile.
> > > > + Disabling an active profile enables the next profile in ascending order,
> > > > + wrapping around from 7 to 0.
> > >
> > > That passing on to the next one seems rather non user friendly. Can we just
> > > disable the whole unit under those conditions instead? As above that may mean
> > > turning of the output entirely. So to change mode it would always be transition
> > > to the one that is enabled. A disable of a given channel results in no output.
> >
> > Yes, I can go for the software powerdown in that case! and the powerdown attribute
> > could be removed?
>
> Yes, I think that works. If all sources are disabled, then powerdown.
> Maybe we keep the powerdown as well though as that's standard DAC ABI.
>
> >
> > >
> > > > +
> > > > + * - ``frequency``
> > > > + - Hz
> > > > + - Output frequency. Range [0, SYSCLK/2). Stored in the profile's frequency
> > > > + tuning word (FTW).
> > > > +
> > > > + * - ``phase``
> > > > + - rad
> > > > + - Phase offset. Range [0, 2*pi). Stored in the profile's phase offset word
> > > > + (POW).
> > > > +
> > > > + * - ``scale``
> > > > + - fractional
> > > > + - Amplitude scale factor. Range [0, 1]. Stored in the profile's amplitude
> > > > + scale factor (ASF).
> > > > +
> > > > +Profile switching is allowed while RAM mode is enabled. In that case single tone
> > > > +parameters are stored in a shadow register and are not written to hardware until
> > > > +RAM mode is disabled.
> > >
> > > This is only visible to userspace because of the priority thing? If we hide
> > > that away to transition from RAM to this mode would just mean enabling this mode.
> >
> > Partially, but the real reason is that single-tone and RAM shares the same profile
> > registers. So I have things cached, which allows user to change single-tone stuff
> > while RAM is enabled.
>
> Ah. So there is no smooth (e.g. race free) path to transition from RAM mode to
> single tone? That is annoying.
>
> >
> > ...
> >
> > > > +Digital ramp generator (DRG)
> > > > +----------------------------
> > > > +
> > > > +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> > > > +hardware. It is controlled through three channels: a parent control channel
> > > > +(``digital_ramp_generator``) and two child ramp channels
> > > > +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> > > > +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> > > > +sets the destination to frequency.
> > > > +
> > > > +Control channel attributes
> > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > +
> > > > +.. flat-table::
> > > > + :header-rows: 1
> > > > +
> > > > + * - Attribute
> > > > + - Unit
> > > > + - Description
> > > > +
> > > > + * - ``en``
> > > > + - boolean
> > > > + - Enable/disable the DRG.
> > > > +
> > > > +Ramp channel attributes
> > > > +^^^^^^^^^^^^^^^^^^^^^^^^
> > > > +
> > > > +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> > > > +attribute set but configure ascending and descending ramp parameters
> > > > +independently:
> > > > +
> > > > +.. flat-table::
> > > > + :header-rows: 1
> > > > +
> > > > + * - Attribute
> > > > + - Unit
> > > > + - Description
> > > > +
> > > > + * - ``en``
> > > > + - boolean
> > > > + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> > > > + bidirectional continuous ramp (Triangular pattern). Other configurations
> > > > + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
> > >
> > > transition
> > >
> > > > + only, ramp-down only or bidirectional with dwell at the limits.
> > >
> > > Feels a little unintuitive to use the generic enable for this.
> > > We might need a specific control for this one.
> >
> > How about dwell_en, but it might not sound that generic. I used "enable" because:
> > - no-dwell high means a ramp-up pattern (only enabling the ramp-up channel)
> > - no-dwell low means a ramp-down pattern (only enabling the ramp-down channel)
> > - both no-dwell is a continuous ramp that goes up and down. (both enabled)
> > The last case is a bit off though, when both are disabled we get the normal mode, which
> > is also a ramps up and down, but dwelling in the limits.
> >
> > > > +
> > > > + * - ``frequency``
> > > > + - Hz
> > > > + - Frequency ramp limit. Range [0, SYSCLK/2).
> > > > +
> > > > + * - ``phase``
> > > > + - rad
> > > > + - Phase ramp limit. Range [0, 2*pi).
>
> Looking at this again, how do we set the DRG mode? E.g. if it effects
> only phase?
>
> > > > +
> > > > + * - ``scale``
> > > > + - fractional
> > > > + - Amplitude scale ramp limit. Range [0, 1).
> > > > +
> > > > + * - ``sampling_frequency``
> > > > + - Hz
> > > > + - Ramp clock rate: SYSCLK / (4 * divider).
> > > > +
> > > > + * - ``frequency_step``
> > > > + - Hz
> > > > + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
> > >
> > > So this was the bit I referred to earlier. Normally we do
> > > rate of change measurements for this stuff rather than what happens on
> > > each tick (based on how we handle things like ROC events)
> > >
> > > So could we make these
> > > ``frequency_roc`` units HZ/Sec
> > > etc? Then from the mix configured would need to work out the optimum
> > > tick to deliver it.
> > >
> > > I suppose it's possible that someone might want a stepped frequency
> > > though which would break this approach? Does anyone actually do that?
> > > If so we'd need to keep the samping_frequency but then control _roc
> > > with that in mind.
> >
> > yeah... frequency steps would make sense when the user controls when to
> > perform the updates, or when it comes from certain events.
>
> You've lost me here. How can they do that? Some external clocking
> or event?
>
> >
> > sampling frequency defines the timing and this roc attr would
> > also depend on timing... there would be two options:
> > * ignore updating ramp step when sampling freq is updated. Here roc
> > would have a different value when readback.
> > * cache the "requested" roc and use it update ramp step when sampling
> > freq is updated, so roc remains with the value initially configured.
>
> If we do end up going the roc route both are valid ABI, but the second is
> nicer from useability point of view.
>
> >
> > >
> > > > +
> > > > + * - ``phase_step``
> > > > + - rad
> > > > + - Per-tick phase increment/decrement. Range [0, 2*pi).
> > > > +
> > > > + * - ``scale_step``
> > > > + - fractional
> > > > + - Per-tick amplitude scale increment/decrement. Range [0, 1).
> > > > +
> > > > +Usage examples
> > > > +^^^^^^^^^^^^^^
> > > > +
> > > > +Configure a frequency sweep from 40 MHz to 60 MHz at a 1 kHz step:
> > > > +
> > > > +.. code-block:: bash
> > > > +
> > > > + # Enable both no-dwell modes for a bidirectional ramp
> > > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_en
> > > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_en
> > >
> > > Fix indents as mix of tabs and spaces. As above I think using this enable
> > > for no dwell is not going to generalize well. I think we need new ABI for this
> > > though I'm open to anyone suggesting something we can reuse.
> >
> > I suggested dwell_en, and I am not sure what else could be used here.
> > Something like hold_en could work and sounds more generic.. not sure.
>
> likewise :(
>
> >
> > > > +
> > > > + # Set ramp limits
> > > > + echo 60000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency
> > > > + echo 40000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency
> > > > +
> > > > + # Set ramp step size to 1 kHz
> > > > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_frequency_step
> > > > + echo 1000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_frequency_step
> > > > +
> > > > + # Set ramp rate at 25 MHz
> > > > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage121_sampling_frequency
> > > > + echo 25000000 > /sys/bus/iio/devices/iio:device0/out_altvoltage122_sampling_frequency
> > > > +
> > > > + # Enable the DRG
> > > > + echo 1 > /sys/bus/iio/devices/iio:device0/out_altvoltage120_en
> > > > +
> > > > +RAM mode
> > > > +--------
> > > > +
> > > > +The AD9910 contains a 1024 x 32-bit RAM that can be loaded with waveform data
> > > > +and played back to modulate frequency, phase, amplitude, or polar (phase +
> > > > +amplitude) parameters.
> > > > +
> > > > +RAM control channel attributes
> > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > +
> > > > +.. flat-table::
> > > > + :header-rows: 1
> > > > +
> > > > + * - Attribute
> > > > + - Unit
> > > > + - Description
> > > > +
> > > > + * - ``en``
> > > > + - boolean
> > > > + - Enable/disable RAM playback. Toggling swaps profile registers between
> > > > + single tone and RAM configurations across all 8 profiles.
> > >
> > > So this might be a fly in the ointment of my previous comment about using
> > > enable of profile to turn off ram. I guess disabling RAM drops into the
> > > matched number tone profile? That's a pain but not disastrous. We'd have
> > > to only allow transitions by enabling the match number tone profile.
> > >
> > > So transitions allowed would be
> > >
> > > tone_profileX -> ram_profileX
> > > tone_profileX -> tone_profileY
> > > ram_profileX -> tone_profileX
> > > ram_profileX -> ram_profileY
> > >
> > > But not
> > > tone_profileX -> ram_profileY
> > > where X!=Y
> >
> > No, there is only one RAM enable bit, once it is on, all single tone
> > profiles go away (all profile registers are repurposed for RAM mode
> > usage).
>
> I'd missed the repurposing that basically means you never transition from
> tone to RAM without going via some intermediate point (maybe power down).
>
> >
> > > > +
> > > > + * - ``frequency``
> > > > + - Hz
> > > > + - Frequency tuning word used as the single tone frequency when
> > > > + RAM destination is not ``frequency``. Range [0, SYSCLK/2).
> > > > +
> > > > + * - ``phase``
> > > > + - rad
> > > > + - Phase offset word used as the single tone phase when RAM destination
> > > > + is not ``phase``. Range [0, 2*pi).
> > > > +
> > > > + * - ``sampling_frequency``
> > > > + - Hz
> > > > + - RAM playback step rate of the active profile, which controls how fast the
> > > > + address counter advances: SYSCLK / (4 * step_rate).
> > >
> > > Why do we care what the sysclk relationship is? It's ticking in HZ.
> >
> > Can be removed, but just to point out that the configured value will adjust to the
> > value where the divider is an integer. Maybe the user should be aware of that.
>
> If you keep it just add something to note that it takes discrete values
> according to that formula. I was reading more into it than that and
> getting confused to where step_rate was coming from!
>
> >
> > > > +
> > > > +Output shift keying (OSK)
> > > This is a new one on me...
> > > > +-------------------------
> > > > +
> > > > +OSK controls the output amplitude envelope, allowing the output to be ramped
> > > > +on/off rather than switched abruptly.
> > >
> > > > +
> > > > +.. flat-table::
> > > > + :header-rows: 1
> > > > +
> > > > + * - Attribute
> > > > + - Unit
> > > > + - Description
> > > > +
> > > > + * - ``en``
> > > > + - boolean
> > > > + - Enable/disable OSK.
> > > > +
> > > > + * - ``scale``
> > > > + - fractional
> > > > + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> > > > +
> > > > + * - ``sampling_frequency``
> > > > + - Hz
> > > > + - OSK ramp rate: SYSCLK / (4 * divider).
> > > > +
> > > > + * - ``pinctrl_en``
> > > > + - boolean
> > > > + - Enable manual external pin control. When enabled, the OSK pin directly
> > > > + gates the output on/off instead of using the automatic ramp.
> > >
> > > I wonder if we should split the various OSK modes into different channels given
> > > only some properties apply to each of automatic and manual modes. Also I think
> > > automatic mode is meaningless without pinctrl_en (so that can be replaced
> > > by simply enabling that mode). I have no idea if anyone cares about pin ctrl
> > > with manual mode or not? That one seems even more odd.
> >
> > OSK is either in manual or auto:
> > * In manual mode the OSK pin enables and disables the output based on its level.
> > * In auto, the OSK pin controls the direction the amplitude updates.
> >
> > If we enable RAM mode, and other modes do not target amplitude, the only way to
> > manually configure the amplitude in software (i.e. without using an OSK gpio)
> > is going manual mode (scale_step == 0), disable this pinctrl_en and then set the
> > scale property (ASF register). That is the only reason I added this property.
>
> Ah. Maybe we hide that away and make the amplitude a property of RAM channel?
> It can do this magic under the hood. I don't mind the attributes for OSK changing
> if this trick is in use (they won't be active anyway).
>
> >
> > > > +
> > > > + * - ``scale_step``
> > > > + - fractional
> > > > + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> > > > + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> > > > + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
> > >
> > > Similar thing about rate of change of amplitude fitting better with current ABI
> > > than step does.
> >
> > ok... and this one is still missing the correspondent available attr.
>
> Available is a bit tricky when there is an inverse relationship involved in the maths
> as what do we put the step as. Maybe we should add a note on that to the ABI
> docs. [min step max] where step gives the minimum step that due to non linearity
> may not be applicable between discrete values that may be taken away from that
> minimum granularity base value. If that occurs the driver will round to the
> nearest possible value.
>
> Something like that.
>
> J
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-04-27 10:31 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 8:17 [PATCH RFC v3 0/9] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 1/9] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
2026-04-26 11:01 ` Jonathan Cameron
2026-04-26 12:48 ` Rodrigo Alencar
2026-04-17 8:17 ` [PATCH RFC v3 2/9] iio: frequency: ad9910: initial driver implementation Rodrigo Alencar via B4 Relay
2026-04-26 11:42 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 3/9] iio: frequency: ad9910: add simple parallel port mode support Rodrigo Alencar via B4 Relay
2026-04-26 11:59 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support Rodrigo Alencar via B4 Relay
2026-04-26 12:05 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 5/9] iio: frequency: ad9910: add RAM mode support Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 6/9] iio: frequency: ad9910: add output shift keying support Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 7/9] iio: frequency: ad9910: add channel labels Rodrigo Alencar via B4 Relay
2026-04-26 12:12 ` Jonathan Cameron
2026-04-17 8:17 ` [PATCH RFC v3 8/9] Documentation: ABI: testing: add docs for ad9910 sysfs entries Rodrigo Alencar via B4 Relay
2026-04-17 8:17 ` [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver Rodrigo Alencar via B4 Relay
2026-04-26 13:10 ` Jonathan Cameron
2026-04-26 20:42 ` Rodrigo Alencar
2026-04-27 9:46 ` Jonathan Cameron
2026-04-27 10:31 ` Nuno Sá
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox