public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers
@ 2025-12-19 12:34 Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

This patch series adds support for the Analog Devices ADF41513 and ADF41510
ultralow noise PLL frequency synthesizers. These devices are designed for
implementing local oscillators (LOs) in high-frequency applications.
The ADF41513 covers frequencies from 1 GHz to 26.5 GHz, while the ADF41510
operates from 1 GHz to 10 GHz.

Key features supported by this driver:
- Integer-N and fractional-N operation modes
- High maximum PFD frequency (250 MHz integer-N, 125 MHz fractional-N)
- 25-bit fixed modulus or 49-bit variable modulus fractional modes
- Digital lock detect functionality
- Phase resync capability for consistent output phase
- Load Enable vs Reference signal syncronization

The series includes:
1. PLL driver implementation
2. Device tree bindings documentation
3. IIO ABI documentation

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v2:
- separate driver implementation from extra features and improve commit messages
- use macros from units.h
- explanation of custom parse function: adf41513_parse_uhz
- reorganize driver data structures
- drop clock framework support for now
- reorganize documentation
- Link to v1: https://lore.kernel.org/r/20251110-adf41513-iio-driver-v1-0-2df8be0fdc6e@analog.com

---
Rodrigo Alencar (6):
      dt-bindings: iio: frequency: add adf41513
      iio: frequency: adf41513: driver implementation
      iio: frequency: adf41513: handle LE synchronization feature
      iio: frequency: adf41513: features on frequency change
      docs: iio: add documentation for adf41513 driver
      Documentation: ABI: testing: add support for ADF41513

 .../ABI/testing/sysfs-bus-iio-frequency-adf41513   |   27 +
 .../bindings/iio/frequency/adi,adf41513.yaml       |  246 ++++
 Documentation/iio/adf41513.rst                     |  255 ++++
 Documentation/iio/index.rst                        |    1 +
 MAINTAINERS                                        |   10 +
 drivers/iio/frequency/Kconfig                      |   10 +
 drivers/iio/frequency/Makefile                     |    1 +
 drivers/iio/frequency/adf41513.c                   | 1378 ++++++++++++++++++++
 8 files changed, 1928 insertions(+)
---
base-commit: a7b10f0963c651a6406d958a5f64b9c5594f84da
change-id: 20251110-adf41513-iio-driver-aaca8a7f808e

Best regards,
-- 
Rodrigo Alencar <rodrigo.alencar@analog.com>



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

* [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-20  9:21   ` Krzysztof Kozlowski
  2025-12-21 16:59   ` Jonathan Cameron
  2025-12-19 12:34 ` [PATCH v2 2/6] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
can be used to implement local oscillators (LOs) as high as 26.5 GHz.
Most properties refer to existing PLL driver properties (e.g. ADF4350).

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../bindings/iio/frequency/adi,adf41513.yaml       | 246 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 2 files changed, 253 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
new file mode 100644
index 000000000000..01ceb2a7d21b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
@@ -0,0 +1,246 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,adf41513.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADF41513 PLL Frequency Synthesizer
+
+maintainers:
+  - Rodrigo Alencar <rodrigo.alencar@analog.com>
+
+description:
+  The ADF41513 is an ultralow noise frequency synthesizer that can be used to
+  implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
+  downconversion sections of wireless receivers and transmitters. The ADF41510
+  supports frequencies up to 10 GHz.
+
+  https://www.analog.com/en/products/adf41513.html
+  https://www.analog.com/en/products/adf41510.html
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,adf41510
+      - adi,adf41513
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 25000000
+
+  clocks:
+    maxItems: 1
+    description: Clock that provides the reference input frequency.
+
+  avdd1-supply:
+    description: PFD and Up and Down Digital Driver Power Supply (3.3 V)
+
+  avdd2-supply:
+    description: RF Buffer and Prescaler Power Supply (3.3 V)
+
+  avdd3-supply:
+    description: N Divider Power Supply (3.3 V)
+
+  avdd4-supply:
+    description: R Divider and Lock Detector Power Supply (3.3 V)
+
+  avdd5-supply:
+    description: Sigma-Delta Modulator and SPI Power Supply (3.3 V)
+
+  vp-supply:
+    description: Charge Pump Power Supply (3.3 V)
+
+  enable-gpios:
+    description:
+      GPIO that controls the chip enable pin. A logic low on this pin
+      powers down the device and puts the charge pump output into
+      three-state mode.
+    maxItems: 1
+
+  lock-detect-gpios:
+    description:
+      GPIO for lock detect functionality. When configured for digital lock
+      detect, this pin will output a logic high when the PLL is locked.
+    maxItems: 1
+
+  adi,power-up-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint64
+    minimum: 1000000000
+    maximum: 26500000000
+    default: 10000000000
+    description:
+      The PLL tunes to this frequency (in Hz) during the initialization
+      sequence. This property should be set to a frequency supported by the
+      loop filter and VCO used in the design. Range is 1 GHz to 26.5 GHz for
+      ADF41513, and 1 GHz to 10 GHz for ADF41510.
+
+  adi,reference-div-factor:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 32
+    default: 1
+    description:
+      Value for the reference division factor (R Counter). The driver will
+      increment R Counter as needed to achieve a PFD frequency within the
+      allowed range. High R counter values will reduce the PFD frequency, which
+      lowers the frequency resolution, and affects phase noise performance.
+
+  adi,reference-doubler-enable:
+    description:
+      Enables the reference doubler. The maximum reference frequency when
+      the doubler is enabled is 225 MHz.
+    type: boolean
+
+  adi,reference-div2-enable:
+    description:
+      Enables the reference divide-by-2 function. This provides a 50%
+      duty cycle signal to the PFD.
+    type: boolean
+
+  adi,charge-pump-resistor-ohms:
+    minimum: 1800
+    maximum: 10000
+    default: 2700
+    description:
+      External charge pump resistor (R_SET) value in ohms. This sets the maximum
+      charge pump current along with the charge pump current setting.
+
+  adi,charge-pump-current-microamp:
+    description:
+      Charge pump current (I_CP) in microamps. The value will be rounded to the
+      nearest supported value. Range of acceptable values depends on the
+      charge pump resistor value, such that 810 mV <= I_CP * R_SET <= 12960 mV.
+      This value depends on the loop filter design.
+
+  adi,muxout-select:
+    description:
+      On chip multiplexer output selection.
+      high_z - MUXOUT Pin set to high-Z. (default)
+      muxout_high - MUXOUT Pin set to high.
+      muxout_low - MUXOUT Pin set to low.
+      f_div_rclk - MUXOUT Pin set to R divider output
+      f_div_nclk - MUXOUT Pin set to N divider output
+      lock_detect - MUXOUT Pin set to Digital lock detect
+      serial_data - MUXOUT Pin set to Serial data output
+      readback - MUXOUT Pin set to Readback mode
+      f_div_clk1 - MUXOUT Pin set to CLK1 divider output
+      f_div_rclk_2 - MUXOUT Pin set to R divider/2 output
+      f_div_nclk_2 - MUXOUT Pin set to N divider/2 output
+    enum: [high_z, muxout_high, muxout_low, f_div_rclk, f_div_nclk, lock_detect,
+           serial_data, readback, f_div_clk1, f_div_rclk_2, f_div_nclk_2]
+
+  adi,muxout-level-1v8-enable:
+    description:
+      Set MUXOUT and DLD logic levels to 1.8V. Default is 3.3V.
+    type: boolean
+
+  adi,phase-detector-polarity-positive-enable:
+    description:
+      Set phase detector polarity to positive. Default is negative.
+      Use positive polarity with non-inverting loop filter and VCO with
+      positive tuning slope, or with inverting loop filter and VCO with
+      negative tuning slope.
+    type: boolean
+
+  adi,lock-detector-count:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 64
+    description:
+      Sets the value for Lock Detector count of the PLL, which determines the
+      number of consecutive phase detector cycles that must be within the lock
+      detector window before lock is declared. Lower values increase the lock
+      detection sensitivity.
+    enum: [2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
+
+  adi,phase-resync-period-ns:
+    default: 0
+    description:
+      When this value is non-zero, enable phase resync functionality, which
+      produces a consistent output phase offset with respect to the input
+      reference. The value specifies the resync period in nanoseconds, used
+      to configure clock dividers with respect to the PFD frequency. This value
+      should be set to a value that is at least as long as the worst case lock
+      time, i.e., it depends mostly on the loop filter design.
+
+  adi,le-sync-enable:
+    description:
+      Synchronizes Load Enable (LE) transitions with the reference signal to
+      avoid asynchronous glitches in the output. This is recommended when using
+      the PLL as a frequency synthesizer, where reference signal will always be
+      present while the device is being configured. When using the PLL as a
+      frequency tracker, where the reference signal may be absent for long
+      periods of time, LE sync should be disabled.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - avdd1-supply
+  - avdd2-supply
+  - avdd3-supply
+  - avdd4-supply
+  - avdd5-supply
+  - vp-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pll@0 {
+            compatible = "adi,adf41513";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+            clocks = <&ref_clk>;
+            avdd1-supply = <&vdd_3v3>;
+            avdd2-supply = <&vdd_3v3>;
+            avdd3-supply = <&vdd_3v3>;
+            avdd4-supply = <&vdd_3v3>;
+            avdd5-supply = <&vdd_3v3>;
+            vp-supply = <&vdd_3v3>;
+
+            adi,power-up-frequency = /bits/ 64 <12000000000>;
+            adi,charge-pump-current-microamp = <2400>;
+            adi,phase-detector-polarity-positive-enable;
+        };
+    };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pll@0 {
+            compatible = "adi,adf41513";
+            reg = <0>;
+            spi-max-frequency = <25000000>;
+            clocks = <&ref_clk>;
+            avdd1-supply = <&avdd1_3v3>;
+            avdd2-supply = <&avdd2_3v3>;
+            avdd3-supply = <&avdd3_3v3>;
+            avdd4-supply = <&avdd4_3v3>;
+            avdd5-supply = <&avdd5_3v3>;
+            vp-supply = <&vp_3v3>;
+            enable-gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
+            lock-detect-gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
+
+            adi,power-up-frequency = /bits/ 64 <15500000000>;
+            adi,charge-pump-current-microamp = <3600>;
+            adi,charge-pump-resistor-ohms = <2700>;
+            adi,reference-doubler-enable;
+            adi,muxout-select = "lock_detect";
+            adi,lock-detector-count = <64>;
+            adi,phase-resync-period-ns = <0>;
+            adi,phase-detector-polarity-positive-enable;
+            adi,le-sync-enable;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 29340394ac9d..1c1343f21160 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1601,6 +1601,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ada4250.yaml
 F:	drivers/iio/amplifiers/ada4250.c
 
+ANALOG DEVICES INC ADF41513 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,adf41513.yaml
+
 ANALOG DEVICES INC ADF4377 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.43.0



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

* [PATCH v2 2/6] iio: frequency: adf41513: driver implementation
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-21 17:49   ` Jonathan Cameron
  2025-12-19 12:34 ` [PATCH v2 3/6] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

The driver is based on existing PLL drivers in the IIO subsystem and
implements the following key features:

- Integer-N and fractional-N (fixed/variable modulus) synthesis modes
- High-resolution frequency calculations using microhertz (µHz) precision
  to handle sub-Hz resolution across multi-GHz frequency ranges
- IIO debugfs interface for direct register access
- FW property parsing from devicetree including charge pump settings,
  reference path configuration and muxout options
- Power management support with suspend/resume callbacks
- Lock detect GPIO monitoring

The driver uses 64-bit microhertz values throughout PLL calculations to
maintain precision when working with frequencies that exceed 32-bit Hz
representation while requiring fractional Hz resolution.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 MAINTAINERS                      |    1 +
 drivers/iio/frequency/Kconfig    |   10 +
 drivers/iio/frequency/Makefile   |    1 +
 drivers/iio/frequency/adf41513.c | 1249 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 1261 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c1343f21160..06ba879a248a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1607,6 +1607,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
+F:	drivers/iio/frequency/adf41513.c
 
 ANALOG DEVICES INC ADF4377 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/iio/frequency/Kconfig b/drivers/iio/frequency/Kconfig
index 583cbdf4e8cd..90c6304c4bcd 100644
--- a/drivers/iio/frequency/Kconfig
+++ b/drivers/iio/frequency/Kconfig
@@ -29,6 +29,16 @@ endmenu
 
 menu "Phase-Locked Loop (PLL) frequency synthesizers"
 
+config ADF41513
+	tristate "Analog Devices ADF41513 PLL Frequency Synthesizer"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices ADF41513
+	  26.5 GHz Integer-N/Fractional-N PLL Frequency Synthesizer.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called adf41513.
+
 config ADF4350
 	tristate "Analog Devices ADF4350/ADF4351 Wideband Synthesizers"
 	depends on SPI
diff --git a/drivers/iio/frequency/Makefile b/drivers/iio/frequency/Makefile
index 70d0e0b70e80..53b4d01414d8 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_ADF41513) += adf41513.o
 obj-$(CONFIG_ADF4350) += adf4350.o
 obj-$(CONFIG_ADF4371) += adf4371.o
 obj-$(CONFIG_ADF4377) += adf4377.o
diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
new file mode 100644
index 000000000000..a967dc4479e7
--- /dev/null
+++ b/drivers/iio/frequency/adf41513.c
@@ -0,0 +1,1249 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADF41513 SPI PLL Frequency Synthesizer driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.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/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+/* Registers */
+#define ADF41513_REG0		0
+#define ADF41513_REG1		1
+#define ADF41513_REG2		2
+#define ADF41513_REG3		3
+#define ADF41513_REG4		4
+#define ADF41513_REG5		5
+#define ADF41513_REG6		6
+#define ADF41513_REG7		7
+#define ADF41513_REG8		8
+#define ADF41513_REG9		9
+#define ADF41513_REG10		10
+#define ADF41513_REG11		11
+#define ADF41513_REG12		12
+#define ADF41513_REG13		13
+#define ADF41513_REG_NUM	14
+
+#define ADF41513_SYNC_REG0	BIT(ADF41513_REG0)
+#define ADF41513_SYNC_REG1	BIT(ADF41513_REG1)
+#define ADF41513_SYNC_REG2	BIT(ADF41513_REG2)
+#define ADF41513_SYNC_REG3	BIT(ADF41513_REG3)
+#define ADF41513_SYNC_REG4	BIT(ADF41513_REG4)
+#define ADF41513_SYNC_REG5	BIT(ADF41513_REG5)
+#define ADF41513_SYNC_REG6	BIT(ADF41513_REG6)
+#define ADF41513_SYNC_REG7	BIT(ADF41513_REG7)
+#define ADF41513_SYNC_REG9	BIT(ADF41513_REG9)
+#define ADF41513_SYNC_REG11	BIT(ADF41513_REG11)
+#define ADF41513_SYNC_REG12	BIT(ADF41513_REG12)
+#define ADF41513_SYNC_REG13	BIT(ADF41513_REG13)
+#define ADF41513_SYNC_DIFF	0
+#define ADF41513_SYNC_ALL	GENMASK(ADF41513_REG13, ADF41513_REG0)
+
+/* REG0 Bit Definitions */
+#define ADF41513_REG0_CTRL_BITS_MSK		GENMASK(3, 0)
+#define ADF41513_REG0_INT_MSK			GENMASK(19, 4)
+#define ADF41513_REG0_VAR_MOD_MSK		BIT(28)
+
+/* REG1 Bit Definitions */
+#define ADF41513_REG1_FRAC1_MSK			GENMASK(28, 4)
+#define ADF41513_REG1_DITHER2_MSK		BIT(31)
+
+/* REG2 Bit Definitions */
+#define ADF41513_REG2_PHASE_VAL_MSK		GENMASK(15, 4)
+#define ADF41513_REG2_PHASE_ADJ_MSK		BIT(31)
+
+/* REG3 Bit Definitions */
+#define ADF41513_REG3_FRAC2_MSK			GENMASK(27, 4)
+
+/* REG4 Bit Definitions */
+#define ADF41513_REG4_MOD2_MSK			GENMASK(27, 4)
+
+/* REG5 Bit Definitions */
+#define ADF41513_REG5_CLK1_DIV_MSK		GENMASK(15, 4)
+#define ADF41513_REG5_R_CNT_MSK			GENMASK(20, 16)
+#define ADF41513_REG5_REF_DOUBLER_MSK		BIT(21)
+#define ADF41513_REG5_RDIV2_MSK			BIT(22)
+#define ADF41513_REG5_PRESCALER_MSK		BIT(23)
+#define ADF41513_REG5_LSB_P1_MSK		BIT(24)
+#define ADF41513_REG5_CP_CURRENT_MSK		GENMASK(28, 25)
+#define ADF41513_REG5_DLD_MODES_MSK		GENMASK(31, 30)
+
+/* REG6 Bit Definitions */
+#define ADF41513_REG6_COUNTER_RESET_MSK		BIT(4)
+#define ADF41513_REG6_CP_TRISTATE_MSK		BIT(5)
+#define ADF41513_REG6_POWER_DOWN_MSK		BIT(6)
+#define ADF41513_REG6_PD_POLARITY_MSK		BIT(7)
+#define ADF41513_REG6_LDP_MSK			GENMASK(9, 8)
+#define ADF41513_REG6_CP_TRISTATE_PD_ON_MSK	BIT(16)
+#define ADF41513_REG6_SD_RESET_MSK		BIT(17)
+#define ADF41513_REG6_LOL_ENABLE_MSK		BIT(18)
+#define ADF41513_REG6_ABP_MSK			BIT(19)
+#define ADF41513_REG6_INT_MODE_MSK		BIT(20)
+#define ADF41513_REG6_BLEED_ENABLE_MSK		BIT(22)
+#define ADF41513_REG6_BLEED_POLARITY_MSK	BIT(23)
+#define ADF41513_REG6_BLEED_CURRENT_MSK		GENMASK(31, 24)
+
+/* REG7 Bit Definitions */
+#define ADF41513_REG7_CLK2_DIV_MSK		GENMASK(17, 6)
+#define ADF41513_REG7_CLK_DIV_MODE_MSK		GENMASK(19, 18)
+#define ADF41513_REG7_PS_BIAS_MSK		GENMASK(21, 20)
+#define ADF41513_REG7_N_DELAY_MSK		GENMASK(23, 22)
+#define ADF41513_REG7_LD_CLK_SEL_MSK		BIT(26)
+#define ADF41513_REG7_LD_COUNT_MSK		GENMASK(29, 27)
+
+/* REG9 Bit Definitions */
+#define ADF41513_REG9_LD_BIAS_MSK		GENMASK(31, 30)
+
+/* REG11 Bit Definitions */
+#define ADF41513_REG11_POWER_DOWN_SEL_MSK	BIT(31)
+
+/* REG12 Bit Definitions */
+#define ADF41513_REG12_READBACK_SEL_MSK		GENMASK(19, 14)
+#define ADF41513_REG12_LE_SELECT_MSK		BIT(20)
+#define ADF41513_REG12_MASTER_RESET_MSK		BIT(22)
+#define ADF41513_REG12_LOGIC_LEVEL_MSK		BIT(27)
+#define ADF41513_REG12_MUXOUT_MSK		GENMASK(31, 28)
+
+/* MUXOUT Selection */
+#define ADF41513_MUXOUT_TRISTATE		0x0
+#define ADF41513_MUXOUT_DVDD			0x1
+#define ADF41513_MUXOUT_DGND			0x2
+#define ADF41513_MUXOUT_R_DIV			0x3
+#define ADF41513_MUXOUT_N_DIV			0x4
+#define ADF41513_MUXOUT_DIG_LD			0x6
+#define ADF41513_MUXOUT_SDO			0x7
+#define ADF41513_MUXOUT_READBACK		0x8
+#define ADF41513_MUXOUT_CLK1_DIV		0xA
+#define ADF41513_MUXOUT_R_DIV2			0xD
+#define ADF41513_MUXOUT_N_DIV2			0xE
+
+/* DLD Mode Selection */
+#define ADF41513_DLD_TRISTATE			0x0
+#define ADF41513_DLD_DIG_LD			0x1
+#define ADF41513_DLD_LOW			0x2
+#define ADF41513_DLD_HIGH			0x3
+
+/* Prescaler Selection */
+#define ADF41513_PRESCALER_4_5			0
+#define ADF41513_PRESCALER_8_9			1
+#define ADF41513_PRESCALER_AUTO			2
+
+/* Specifications */
+#define ADF41513_MIN_RF_FREQ			(1000ULL * HZ_PER_MHZ)
+#define ADF41510_MAX_RF_FREQ			(10000ULL * HZ_PER_MHZ)
+#define ADF41513_MAX_RF_FREQ			(26500ULL * HZ_PER_MHZ)
+
+#define ADF41513_MIN_REF_FREQ			(10U * HZ_PER_MHZ)
+#define ADF41513_MAX_REF_FREQ			(800U * HZ_PER_MHZ)
+#define ADF41513_MAX_REF_FREQ_DOUBLER		(225U * HZ_PER_MHZ)
+
+#define ADF41513_MAX_PFD_FREQ_INT_N_UHZ		(250ULL * HZ_PER_MHZ * MICROHZ_PER_HZ)
+#define ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ	(125ULL * HZ_PER_MHZ * MICROHZ_PER_HZ)
+#define ADF41513_MAX_FREQ_RESOLUTION_UHZ	(100ULL * HZ_PER_KHZ * MICROHZ_PER_HZ)
+
+#define ADF41513_MIN_INT_4_5			20
+#define ADF41513_MAX_INT_4_5			511
+#define ADF41513_MIN_INT_8_9			64
+#define ADF41513_MAX_INT_8_9			1023
+
+#define ADF41513_MIN_INT_FRAC_4_5		23
+#define ADF41513_MIN_INT_FRAC_8_9		75
+
+#define ADF41513_MIN_R_CNT			1
+#define ADF41513_MAX_R_CNT			32
+
+#define ADF41513_MIN_R_SET			1800
+#define ADF41513_DEFAULT_R_SET			2700
+#define ADF41513_MAX_R_SET			10000
+
+#define ADF41513_MIN_CP_VOLTAGE_mV		810
+#define ADF41513_DEFAULT_CP_VOLTAGE_mV		6480
+#define ADF41513_MAX_CP_VOLTAGE_mV		12960
+
+#define ADF41513_MAX_CLK_DIVIDER		4095
+#define ADF41513_LD_COUNT_FAST_MIN		2
+#define ADF41513_LD_COUNT_FAST_LIMIT		64
+#define ADF41513_LD_COUNT_MIN			64
+#define ADF41513_LD_COUNT_MAX			8192
+
+#define ADF41513_FIXED_MODULUS			BIT(25)
+#define ADF41513_MAX_MOD2			(BIT(24) - 1)
+
+#define ADF41513_HZ_DECIMAL_PRECISION		6
+
+enum {
+	ADF41513_FREQ,
+	ADF41513_POWER_DOWN,
+	ADF41513_FREQ_RESOLUTION,
+	ADF41513_FREQ_REFIN,
+};
+
+enum adf41513_pll_mode {
+	ADF41513_MODE_INVALID,
+	ADF41513_MODE_INTEGER_N,
+	ADF41513_MODE_FIXED_MODULUS,
+	ADF41513_MODE_VARIABLE_MODULUS,
+};
+
+struct adf41513_chip_info {
+	const char *name;
+	bool has_prescaler_8_9;
+	u64 max_rf_freq_hz;
+};
+
+struct adf41513_data {
+	u64 power_up_frequency_hz;
+	u64 freq_resolution_uhz;
+	u32 charge_pump_voltage_mv;
+	u32 lock_detect_count;
+
+	u8 ref_div_factor;
+	bool ref_doubler_en;
+	bool ref_div2_en;
+	bool phase_detector_polarity;
+
+	u8 muxout_select;
+	bool muxout_1v8_en;
+};
+
+struct adf41513_pll_settings {
+	enum adf41513_pll_mode mode;
+
+	/* reference path parameters */
+	u8 r_counter;
+	u8 ref_doubler;
+	u8 ref_div2;
+	u8 prescaler;
+
+	/* frequency parameters */
+	u64 target_frequency_uhz;
+	u64 actual_frequency_uhz;
+	u64 pfd_frequency_uhz;
+
+	/* pll parameters */
+	u16 int_value;
+	u32 frac1;
+	u32 frac2;
+	u32 mod2;
+};
+
+struct adf41513_state {
+	const struct adf41513_chip_info *chip_info;
+	struct spi_device *spi;
+	struct gpio_desc *lock_detect;
+	struct gpio_desc *chip_enable;
+	struct clk *ref_clk;
+
+	u64 ref_freq_hz;
+
+	/*
+	 * Lock for accessing device registers. Some operations require
+	 * multiple consecutive R/W operations, during which the device
+	 * shouldn't be interrupted. The buffers are also shared across
+	 * all operations so need to be protected on stand alone reads and
+	 * writes.
+	 */
+	struct mutex lock;
+
+	/* Cached register values */
+	u32 regs[ADF41513_REG_NUM];
+	u32 regs_hw[ADF41513_REG_NUM];
+
+	struct adf41513_data data;
+	struct adf41513_pll_settings settings;
+
+	/*
+	 * DMA (thus cache coherency maintenance) may require that
+	 * transfer buffers live in their own cache lines.
+	 */
+	__be32 buf __aligned(IIO_DMA_MINALIGN);
+};
+
+static const char * const adf41513_muxout_modes[] = {
+	[ADF41513_MUXOUT_TRISTATE] = "high_z",
+	[ADF41513_MUXOUT_DVDD] = "muxout_high",
+	[ADF41513_MUXOUT_DGND] = "muxout_low",
+	[ADF41513_MUXOUT_R_DIV] = "f_div_rclk",
+	[ADF41513_MUXOUT_N_DIV] = "f_div_nclk",
+	[ADF41513_MUXOUT_DIG_LD] = "lock_detect",
+	[ADF41513_MUXOUT_SDO] = "serial_data",
+	[ADF41513_MUXOUT_READBACK] = "readback",
+	[ADF41513_MUXOUT_CLK1_DIV] = "f_div_clk1",
+	[ADF41513_MUXOUT_R_DIV2] = "f_div_rclk_2",
+	[ADF41513_MUXOUT_N_DIV2] = "f_div_nclk_2",
+};
+
+static const char * const adf41513_power_supplies[] = {
+	"avdd1", "avdd2", "avdd3", "avdd4", "avdd5", "vp"
+};
+
+/**
+ * adf41513_parse_uhz() - parse fixed point frequency string into microhertz
+ * @str:	input string with frequency in Hz (supports 6 decimal places)
+ * @freq_uhz:	output frequency in microhertz
+ *
+ * This driver supports sub-Hz frequency resolution with frequency ranges
+ * up to several GHz (> 2^32). To achieve this, frequency calculations are
+ * done in microhertz using u64 variables. iio core parse helpers only support
+ * 64-bit integers or 32-bit integers plus fractional part. Here, we need
+ * 64-bit integer plus fractional part (6 decimal places) to achieve lower
+ * frequency resolutions.
+ * See iio_write_channel_info and __iio_str_to_fixpoint in
+ * drivers/iio/industrialio-core.c
+ *
+ * Returns:
+ * 0 on success, -EINVAL on parsing error.
+ */
+static int adf41513_parse_uhz(const char *str, u64 *freq_uhz)
+{
+	u64 uhz = 0;
+	int f_count = ADF41513_HZ_DECIMAL_PRECISION;
+	bool frac_part = false;
+
+	if (str[0] == '+')
+		str++;
+
+	while (*str && f_count > 0) {
+		if ('0' <= *str && *str <= '9') {
+			uhz = uhz * 10 + *str - '0';
+			if (frac_part)
+				f_count--;
+		} else if (*str == '\n') {
+			if (*(str + 1) == '\0')
+				break;
+			return -EINVAL;
+		} else if (*str == '.' && !frac_part) {
+			frac_part = true;
+		} else {
+			return -EINVAL;
+		}
+		str++;
+	}
+
+	for (; f_count > 0; f_count--)
+		uhz *= 10;
+
+	*freq_uhz = uhz;
+
+	return 0;
+}
+
+static int adf41513_uhz_to_str(u64 freq_uhz, char *buf)
+{
+	u32 frac_part;
+	u64 int_part = div_u64_rem(freq_uhz, MICROHZ_PER_HZ, &frac_part);
+
+	return sysfs_emit(buf, "%llu.%06u\n", int_part, frac_part);
+}
+
+static int adf41513_sync_config(struct adf41513_state *st, u16 sync_mask)
+{
+	int ret;
+	int i;
+
+	/* write registers in reverse order (R13 to R0)*/
+	for (i = ADF41513_REG13; i >= ADF41513_REG0; i--) {
+		if (st->regs_hw[i] != st->regs[i] || sync_mask & BIT(i)) {
+			st->buf = cpu_to_be32(st->regs[i] | i);
+			ret = spi_write(st->spi, &st->buf, sizeof(st->buf));
+			if (ret < 0)
+				return ret;
+			st->regs_hw[i] = st->regs[i];
+			dev_dbg(&st->spi->dev, "REG%d <= 0x%08X\n", i, st->regs[i] | i);
+		}
+	}
+
+	return 0;
+}
+
+static u64 adf41513_pll_get_rate(struct adf41513_state *st)
+{
+	struct adf41513_pll_settings *cfg = &st->settings;
+
+	if (cfg->mode != ADF41513_MODE_INVALID)
+		return cfg->actual_frequency_uhz;
+
+	/* get pll settings from regs_hw */
+	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK,
+				   st->regs_hw[ADF41513_REG0]);
+	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK,
+			       st->regs_hw[ADF41513_REG1]);
+	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK,
+			       st->regs_hw[ADF41513_REG3]);
+	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK,
+			      st->regs_hw[ADF41513_REG4]);
+	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK,
+				   st->regs_hw[ADF41513_REG5]);
+	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK,
+				     st->regs_hw[ADF41513_REG5]);
+	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK,
+				  st->regs_hw[ADF41513_REG5]);
+	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,
+				   st->regs_hw[ADF41513_REG5]);
+
+	/* calculate pfd frequency */
+	cfg->pfd_frequency_uhz = st->ref_freq_hz * MICROHZ_PER_HZ;
+	if (cfg->ref_doubler)
+		cfg->pfd_frequency_uhz <<= 1;
+	if (cfg->ref_div2)
+		cfg->pfd_frequency_uhz >>= 1;
+	cfg->pfd_frequency_uhz = div_u64(cfg->pfd_frequency_uhz,
+					 cfg->r_counter);
+	cfg->actual_frequency_uhz = (u64)cfg->int_value * cfg->pfd_frequency_uhz;
+
+	/* check if int mode is selected */
+	if (FIELD_GET(ADF41513_REG6_INT_MODE_MSK, st->regs_hw[ADF41513_REG6])) {
+		cfg->mode = ADF41513_MODE_INTEGER_N;
+	} else {
+		cfg->actual_frequency_uhz += mul_u64_u64_div_u64(cfg->frac1,
+								 cfg->pfd_frequency_uhz,
+								 ADF41513_FIXED_MODULUS);
+
+		/* check if variable modulus is selected */
+		if (FIELD_GET(ADF41513_REG0_VAR_MOD_MSK, st->regs_hw[ADF41513_REG0])) {
+			cfg->actual_frequency_uhz +=
+				mul_u64_u64_div_u64(cfg->frac2,
+						    cfg->pfd_frequency_uhz,
+						    ADF41513_FIXED_MODULUS * cfg->mod2);
+
+			cfg->mode = ADF41513_MODE_VARIABLE_MODULUS;
+		} else {
+			/* LSB_P1 offset */
+			if (!FIELD_GET(ADF41513_REG5_LSB_P1_MSK, st->regs_hw[ADF41513_REG5]))
+				cfg->actual_frequency_uhz +=
+					div_u64(cfg->pfd_frequency_uhz,
+						ADF41513_FIXED_MODULUS * 2);
+			cfg->mode = ADF41513_MODE_FIXED_MODULUS;
+		}
+	}
+
+	cfg->target_frequency_uhz = cfg->actual_frequency_uhz;
+
+	return cfg->actual_frequency_uhz;
+}
+
+static int adf41513_calc_pfd_frequency(struct adf41513_state *st,
+				       struct adf41513_pll_settings *result,
+				       u64 fpfd_limit_uhz)
+{
+	result->ref_div2 = st->data.ref_div2_en ? 1 : 0;
+	result->ref_doubler = st->data.ref_doubler_en ? 1 : 0;
+
+	if (st->data.ref_doubler_en && st->ref_freq_hz > ADF41513_MAX_REF_FREQ_DOUBLER) {
+		result->ref_doubler = 0;
+		dev_warn(&st->spi->dev, "Disabling ref doubler due to high reference frequency\n");
+	}
+
+	result->r_counter = st->data.ref_div_factor - 1;
+	do {
+		result->r_counter++;
+		/* f_PFD = REF_IN × ((1 + D)/(R × (1 + T))) */
+		result->pfd_frequency_uhz = st->ref_freq_hz * MICROHZ_PER_HZ;
+		if (result->ref_doubler)
+			result->pfd_frequency_uhz <<= 1;
+		if (result->ref_div2)
+			result->pfd_frequency_uhz >>= 1;
+		result->pfd_frequency_uhz = div_u64(result->pfd_frequency_uhz,
+						    result->r_counter);
+	} while (result->pfd_frequency_uhz > fpfd_limit_uhz);
+
+	if (result->r_counter > ADF41513_MAX_R_CNT) {
+		dev_err(&st->spi->dev, "Cannot optimize PFD frequency\n");
+		return -ERANGE;
+	}
+
+	return 0;
+}
+
+static int adf41513_calc_integer_n(struct adf41513_state *st,
+				   struct adf41513_pll_settings *result)
+{
+	u16 max_int = (st->chip_info->has_prescaler_8_9) ?
+		      ADF41513_MAX_INT_8_9 : ADF41513_MAX_INT_4_5;
+	u64 freq_error_uhz;
+	u16 int_value = div64_u64_rem(result->target_frequency_uhz, result->pfd_frequency_uhz,
+				      &freq_error_uhz);
+
+	/* check if freq error is within a tolerance of 1/2 resolution */
+	if (freq_error_uhz > (result->pfd_frequency_uhz >> 1) && int_value < max_int) {
+		int_value++;
+		freq_error_uhz = result->pfd_frequency_uhz - freq_error_uhz;
+	}
+
+	if (freq_error_uhz > st->data.freq_resolution_uhz)
+		return -ERANGE;
+
+	/* set prescaler */
+	if (st->chip_info->has_prescaler_8_9 && int_value >= ADF41513_MIN_INT_8_9 &&
+	    int_value <= ADF41513_MAX_INT_8_9)
+		result->prescaler = 1;
+	else if (int_value >= ADF41513_MIN_INT_4_5 && int_value <= ADF41513_MAX_INT_4_5)
+		result->prescaler = 0;
+	else
+		return -ERANGE;
+
+	result->actual_frequency_uhz = (u64)int_value * result->pfd_frequency_uhz;
+	result->mode = ADF41513_MODE_INTEGER_N;
+	result->int_value = int_value;
+	result->frac1 = 0;
+	result->frac2 = 0;
+	result->mod2 = 0;
+
+	return 0;
+}
+
+static int adf41513_calc_fixed_mod(struct adf41513_state *st,
+				   struct adf41513_pll_settings *result)
+{
+	u64 freq_error_uhz;
+	u64 resolution_uhz = div_u64(result->pfd_frequency_uhz, ADF41513_FIXED_MODULUS);
+	u64 target_frequency_uhz = result->target_frequency_uhz;
+	u32 frac1;
+	u16 int_value;
+	bool lsb_p1_offset = !FIELD_GET(ADF41513_REG5_LSB_P1_MSK, st->regs_hw[ADF41513_REG5]);
+
+	/* LSB_P1 adds a frequency offset of f_pfd/2^26 */
+	if (lsb_p1_offset)
+		target_frequency_uhz -= resolution_uhz >> 1;
+
+	int_value = div64_u64_rem(target_frequency_uhz, result->pfd_frequency_uhz,
+				  &freq_error_uhz);
+
+	if (st->chip_info->has_prescaler_8_9 && int_value >= ADF41513_MIN_INT_FRAC_8_9 &&
+	    int_value <= ADF41513_MAX_INT_8_9)
+		result->prescaler = 1;
+	else if (int_value >= ADF41513_MIN_INT_FRAC_4_5 && int_value <= ADF41513_MAX_INT_4_5)
+		result->prescaler = 0;
+	else
+		return -ERANGE;
+
+	/* compute frac1 and fixed modulus error */
+	frac1 = mul_u64_u64_div_u64(freq_error_uhz, ADF41513_FIXED_MODULUS,
+				    result->pfd_frequency_uhz);
+	freq_error_uhz -= mul_u64_u64_div_u64(frac1, result->pfd_frequency_uhz,
+					      ADF41513_FIXED_MODULUS);
+
+	/* check if freq error is within a tolerance of 1/2 resolution */
+	if (freq_error_uhz > (resolution_uhz >> 1) && frac1 < (ADF41513_FIXED_MODULUS - 1)) {
+		frac1++;
+		freq_error_uhz = resolution_uhz - freq_error_uhz;
+	}
+
+	if (freq_error_uhz > st->data.freq_resolution_uhz)
+		return -ERANGE;
+
+	/* integer part */
+	result->actual_frequency_uhz = (u64)int_value * result->pfd_frequency_uhz;
+	/* fractional part */
+	if (lsb_p1_offset)
+		result->actual_frequency_uhz +=	(resolution_uhz >> 1);
+	result->actual_frequency_uhz += mul_u64_u64_div_u64(frac1, result->pfd_frequency_uhz,
+							    ADF41513_FIXED_MODULUS);
+	result->mode = ADF41513_MODE_FIXED_MODULUS;
+	result->int_value = int_value;
+	result->frac1 = frac1;
+	result->frac2 = 0;
+	result->mod2 = 0;
+
+	return 0;
+}
+
+static int adf41513_calc_variable_mod(struct adf41513_state *st,
+				      struct adf41513_pll_settings *result)
+{
+	u64 freq_error_uhz;
+	u32 frac1, frac2, mod2;
+	u16 int_value = div64_u64_rem(result->target_frequency_uhz,
+				      result->pfd_frequency_uhz,
+				      &freq_error_uhz);
+
+	if (st->chip_info->has_prescaler_8_9 && int_value >= ADF41513_MIN_INT_FRAC_8_9 &&
+	    int_value <= ADF41513_MAX_INT_8_9)
+		result->prescaler = 1;
+	else if (int_value >= ADF41513_MIN_INT_FRAC_4_5 && int_value <= ADF41513_MAX_INT_4_5)
+		result->prescaler = 0;
+	else
+		return -ERANGE;
+
+	/* calculate required mod2 based on target resolution / 2 */
+	mod2 = DIV64_U64_ROUND_CLOSEST(result->pfd_frequency_uhz << 1,
+				       st->data.freq_resolution_uhz * ADF41513_FIXED_MODULUS);
+	/* ensure mod2 is at least 2 for meaningful operation */
+	mod2 = clamp(mod2, 2, ADF41513_MAX_MOD2);
+
+	/* calculate frac1 and frac2 */
+	frac1 = mul_u64_u64_div_u64(freq_error_uhz, ADF41513_FIXED_MODULUS,
+				    result->pfd_frequency_uhz);
+	freq_error_uhz -= mul_u64_u64_div_u64(frac1, result->pfd_frequency_uhz,
+					      ADF41513_FIXED_MODULUS);
+	frac2 = mul_u64_u64_div_u64(freq_error_uhz, (u64)mod2 * ADF41513_FIXED_MODULUS,
+				    result->pfd_frequency_uhz);
+
+	/* integer part */
+	result->actual_frequency_uhz = (u64)int_value * result->pfd_frequency_uhz;
+	/* fractional part */
+	result->actual_frequency_uhz += mul_u64_u64_div_u64((u64)frac1 * mod2 + frac2,
+							    result->pfd_frequency_uhz,
+							    (u64)mod2 * ADF41513_FIXED_MODULUS);
+	result->mode = ADF41513_MODE_VARIABLE_MODULUS;
+	result->int_value = int_value;
+	result->frac1 = frac1;
+	result->frac2 = frac2;
+	result->mod2 = mod2;
+
+	return 0;
+}
+
+static int adf41513_calc_pll_settings(struct adf41513_state *st,
+				      struct adf41513_pll_settings *result,
+				      u64 rf_out_uhz)
+{
+	u64 max_rf_freq_uhz = st->chip_info->max_rf_freq_hz * MICROHZ_PER_HZ;
+	u64 min_rf_freq_uhz = ADF41513_MIN_RF_FREQ * MICROHZ_PER_HZ;
+	u64 pfd_freq_limit_uhz;
+	int ret;
+
+	/* input validation */
+	if (rf_out_uhz < min_rf_freq_uhz || rf_out_uhz > max_rf_freq_uhz) {
+		dev_err(&st->spi->dev, "RF frequency %llu uHz out of range [%llu, %llu] uHz\n",
+			rf_out_uhz, min_rf_freq_uhz, max_rf_freq_uhz);
+		return -EINVAL;
+	}
+
+	result->target_frequency_uhz = rf_out_uhz;
+
+	/* try integer-N first (best phase noise performance) */
+	pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_4_5),
+				 ADF41513_MAX_PFD_FREQ_INT_N_UHZ);
+	ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
+	if (ret < 0)
+		return ret;
+
+	ret = adf41513_calc_integer_n(st, result);
+	if (ret < 0) {
+		/* try fractional-N: recompute pfd frequency if necessary */
+		pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_FRAC_4_5),
+					 ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ);
+		if (pfd_freq_limit_uhz < result->pfd_frequency_uhz) {
+			ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
+			if (ret < 0)
+				return ret;
+		}
+
+		/* fixed-modulus attempt */
+		ret = adf41513_calc_fixed_mod(st, result);
+		if (ret < 0) {
+			/* variable-modulus attempt */
+			ret = adf41513_calc_variable_mod(st, result);
+			if (ret < 0) {
+				dev_err(&st->spi->dev,
+					"no valid PLL configuration found for %llu uHz\n",
+					rf_out_uhz);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 sync_mask)
+{
+	struct adf41513_pll_settings result;
+	int ret;
+
+	/* calculate pll settings candidate */
+	ret = adf41513_calc_pll_settings(st, &result, freq_uhz);
+	if (ret < 0)
+		return ret;
+
+	/* apply computed results to pll settings */
+	memcpy(&st->settings, &result, sizeof(struct adf41513_pll_settings));
+
+	dev_dbg(&st->spi->dev,
+		"%s mode: int=%u, frac1=%u, frac2=%u, mod2=%u, fpdf=%llu Hz, prescaler=%s\n",
+		(result.mode == ADF41513_MODE_INTEGER_N) ? "integer-n" :
+		(result.mode == ADF41513_MODE_FIXED_MODULUS) ? "fixed-modulus" : "variable-modulus",
+		result.int_value, result.frac1, result.frac2, result.mod2,
+		div64_u64(result.pfd_frequency_uhz, MICROHZ_PER_HZ),
+		result.prescaler ? "8/9" : "4/5");
+
+	/* int */
+	st->regs[ADF41513_REG0] = FIELD_PREP(ADF41513_REG0_INT_MSK,
+					     st->settings.int_value);
+	if (st->settings.mode == ADF41513_MODE_VARIABLE_MODULUS)
+		st->regs[ADF41513_REG0] |= ADF41513_REG0_VAR_MOD_MSK;
+	/* frac1 */
+	st->regs[ADF41513_REG1] = FIELD_PREP(ADF41513_REG1_FRAC1_MSK,
+					     st->settings.frac1);
+	if (st->settings.mode != ADF41513_MODE_INTEGER_N)
+		st->regs[ADF41513_REG1] |= ADF41513_REG1_DITHER2_MSK;
+
+	/* frac2 */
+	st->regs[ADF41513_REG3] = FIELD_PREP(ADF41513_REG3_FRAC2_MSK,
+					     st->settings.frac2);
+	/* mod2 */
+	st->regs[ADF41513_REG4] &= ADF41513_REG4_MOD2_MSK;
+	st->regs[ADF41513_REG4] |= FIELD_PREP(ADF41513_REG4_MOD2_MSK,
+					      st->settings.mod2);
+
+	/* r-cnt | doubler | rdiv2 | prescaler */
+	st->regs[ADF41513_REG5] &= ~(ADF41513_REG5_R_CNT_MSK |
+				     ADF41513_REG5_REF_DOUBLER_MSK |
+				     ADF41513_REG5_RDIV2_MSK |
+				     ADF41513_REG5_PRESCALER_MSK);
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_R_CNT_MSK,
+					      st->settings.r_counter);
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_REF_DOUBLER_MSK,
+					      st->settings.ref_doubler);
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_RDIV2_MSK,
+					      st->settings.ref_div2);
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_PRESCALER_MSK,
+					      st->settings.prescaler);
+
+	if (st->settings.mode == ADF41513_MODE_INTEGER_N) {
+		st->regs[ADF41513_REG6] |= ADF41513_REG6_INT_MODE_MSK;
+		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_BLEED_ENABLE_MSK;
+	} else {
+		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_INT_MODE_MSK;
+		st->regs[ADF41513_REG6] |= ADF41513_REG6_BLEED_ENABLE_MSK;
+	}
+
+	return adf41513_sync_config(st, sync_mask | ADF41513_SYNC_REG0);
+}
+
+static int adf41513_suspend(struct adf41513_state *st)
+{
+	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_POWER_DOWN_MSK, 1);
+	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+}
+
+static int adf41513_resume(struct adf41513_state *st)
+{
+	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_POWER_DOWN_MSK;
+	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+}
+
+static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
+				 uintptr_t private,
+				 const struct iio_chan_spec *chan,
+				 char *buf)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	u64 freq_uhz;
+
+	guard(mutex)(&st->lock);
+
+	switch ((u32)private) {
+	case ADF41513_FREQ:
+		freq_uhz = adf41513_pll_get_rate(st);
+		if (st->lock_detect)
+			if (!gpiod_get_value_cansleep(st->lock_detect)) {
+				dev_dbg(&st->spi->dev, "PLL un-locked\n");
+				return -EBUSY;
+			}
+		break;
+	case ADF41513_FREQ_RESOLUTION:
+		freq_uhz = st->data.freq_resolution_uhz;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return adf41513_uhz_to_str(freq_uhz, buf);
+}
+
+static ssize_t adf41513_read(struct iio_dev *indio_dev,
+			     uintptr_t private,
+			     const struct iio_chan_spec *chan,
+			     char *buf)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	u32 val;
+
+	guard(mutex)(&st->lock);
+
+	switch ((u32)private) {
+	case ADF41513_FREQ_REFIN:
+		st->ref_freq_hz = clk_get_rate(st->ref_clk);
+		return sysfs_emit(buf, "%llu\n", st->ref_freq_hz);
+	case ADF41513_POWER_DOWN:
+		val = FIELD_GET(ADF41513_REG6_POWER_DOWN_MSK,
+				st->regs_hw[ADF41513_REG6]);
+		return sysfs_emit(buf, "%u\n", val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t adf41513_write_uhz(struct iio_dev *indio_dev,
+				  uintptr_t private,
+				  const struct iio_chan_spec *chan,
+				  const char *buf, size_t len)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	u64 freq_uhz;
+	int ret;
+
+	ret = adf41513_parse_uhz(buf, &freq_uhz);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	switch ((u32)private) {
+	case ADF41513_FREQ:
+		ret = adf41513_set_frequency(st, freq_uhz, ADF41513_SYNC_DIFF);
+		break;
+	case ADF41513_FREQ_RESOLUTION:
+		if (freq_uhz == 0 || freq_uhz > ADF41513_MAX_FREQ_RESOLUTION_UHZ)
+			return -EINVAL;
+		st->data.freq_resolution_uhz = freq_uhz;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret ? ret : len;
+}
+
+static ssize_t adf41513_write(struct iio_dev *indio_dev,
+			      uintptr_t private,
+			      const struct iio_chan_spec *chan,
+			      const char *buf, size_t len)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	unsigned long readin, tmp;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &readin);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	switch ((u32)private) {
+	case ADF41513_FREQ_REFIN:
+		if (readin < ADF41513_MIN_REF_FREQ || readin > ADF41513_MAX_REF_FREQ)
+			return -EINVAL;
+
+		tmp = clk_round_rate(st->ref_clk, readin);
+		if (tmp != readin)
+			return -EINVAL;
+
+		ret = clk_set_rate(st->ref_clk, tmp);
+		if (ret < 0)
+			return ret;
+
+		st->ref_freq_hz = readin;
+		ret = adf41513_set_frequency(st, st->settings.target_frequency_uhz,
+					     ADF41513_SYNC_DIFF);
+		break;
+	case ADF41513_POWER_DOWN:
+		if (readin)
+			ret = adf41513_suspend(st);
+		else
+			ret = adf41513_resume(st);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret ? ret : len;
+}
+
+#define _ADF41513_EXT_INFO(_name, _ident) { \
+	.name = _name, \
+	.read = adf41513_read, \
+	.write = adf41513_write, \
+	.private = _ident, \
+	.shared = IIO_SEPARATE, \
+}
+
+#define _ADF41513_EXT_UHZ_INFO(_name, _ident) { \
+	.name = _name, \
+	.read = adf41513_read_uhz, \
+	.write = adf41513_write_uhz, \
+	.private = _ident, \
+	.shared = IIO_SEPARATE, \
+}
+
+static const struct iio_chan_spec_ext_info adf41513_ext_info[] = {
+	/*
+	 * Ideally we would use IIO_CHAN_INFO_FREQUENCY, but the device supports
+	 * frequency values greater 2^32 with sub-Hz resolution, i.e. 64-bit
+	 * fixed point with 6 decimal places values are used to represent
+	 * frequencies.
+	 */
+	_ADF41513_EXT_UHZ_INFO("frequency", ADF41513_FREQ),
+	_ADF41513_EXT_UHZ_INFO("frequency_resolution", ADF41513_FREQ_RESOLUTION),
+	_ADF41513_EXT_INFO("refin_frequency", ADF41513_FREQ_REFIN),
+	_ADF41513_EXT_INFO("powerdown", ADF41513_POWER_DOWN),
+	{ },
+};
+
+static const struct iio_chan_spec adf41513_chan = {
+	.type = IIO_ALTVOLTAGE,
+	.indexed = 1,
+	.output = 1,
+	.channel = 0,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE),
+	.ext_info = adf41513_ext_info,
+};
+
+static int adf41513_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long info)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	u32 phase_mdeg;
+	u16 phase_val;
+
+	guard(mutex)(&st->lock);
+
+	switch (info) {
+	case IIO_CHAN_INFO_PHASE:
+		phase_val = FIELD_GET(ADF41513_REG2_PHASE_VAL_MSK,
+				      st->regs_hw[ADF41513_REG2]);
+		phase_mdeg = DIV_ROUND_CLOSEST(360 * MILLI * phase_val, BIT(12));
+		*val = phase_mdeg / MILLI;
+		*val2 = (phase_mdeg % MILLI) * 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adf41513_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long info)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+	u32 phase_mdeg;
+	u16 phase_val;
+
+	guard(mutex)(&st->lock);
+
+	switch (info) {
+	case IIO_CHAN_INFO_PHASE:
+		val %= 360;
+		if (val < 0)
+			val += 360;
+		phase_mdeg = val * MILLI + val2 / 1000;
+		phase_val = DIV_ROUND_CLOSEST(phase_mdeg << 12, 360 * MILLI);
+
+		st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK;
+		st->regs[ADF41513_REG2] &= ~ADF41513_REG2_PHASE_VAL_MSK;
+		st->regs[ADF41513_REG2] |= FIELD_PREP(ADF41513_REG2_PHASE_VAL_MSK, phase_val);
+		return adf41513_sync_config(st, ADF41513_SYNC_REG0);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adf41513_reg_access(struct iio_dev *indio_dev,
+			       unsigned int reg,
+			       unsigned int writeval,
+			       unsigned int *readval)
+{
+	struct adf41513_state *st = iio_priv(indio_dev);
+
+	if (reg > ADF41513_REG13)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+
+	if (!readval) {
+		/* direct register access invalidates cached pll settings */
+		st->settings.mode = ADF41513_MODE_INVALID;
+
+		st->regs[reg] = writeval & ~0xF; /* Clear control bits */
+		return adf41513_sync_config(st, BIT(reg));
+	}
+
+	*readval = st->regs_hw[reg];
+	return 0;
+}
+
+static const struct iio_info adf41513_info = {
+	.read_raw = adf41513_read_raw,
+	.write_raw = adf41513_write_raw,
+	.debugfs_reg_access = &adf41513_reg_access,
+};
+
+static int adf41513_parse_fw(struct adf41513_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	int ret;
+	u32 tmp;
+	u32 cp_resistance;
+	u32 cp_current;
+
+	/* power-up frequency */
+	st->data.power_up_frequency_hz = ADF41510_MAX_RF_FREQ;
+	ret = device_property_read_u64(dev, "adi,power-up-frequency",
+				       &st->data.power_up_frequency_hz);
+	if (!ret) {
+		if (st->data.power_up_frequency_hz < ADF41513_MIN_RF_FREQ ||
+		    st->data.power_up_frequency_hz > ADF41513_MAX_RF_FREQ)
+			return dev_err_probe(dev, -ERANGE,
+					     "power-up frequency %llu Hz out of range\n",
+					     st->data.power_up_frequency_hz);
+	}
+
+	/* reference divider factor */
+	st->data.ref_div_factor = ADF41513_MIN_R_CNT;
+	ret = device_property_read_u32(dev, "adi,reference-div-factor", &tmp);
+	if (!ret) {
+		if (tmp < ADF41513_MIN_R_CNT || tmp > ADF41513_MAX_R_CNT)
+			return dev_err_probe(dev, -ERANGE,
+					     "invalid reference div factor %u\n", tmp);
+		st->data.ref_div_factor = tmp;
+	}
+
+	/* reference controls */
+	st->data.ref_doubler_en = device_property_read_bool(dev, "adi,reference-doubler-enable");
+	st->data.ref_div2_en = device_property_read_bool(dev, "adi,reference-div2-enable");
+
+	/* charge pump resistor */
+	cp_resistance = ADF41513_DEFAULT_R_SET;
+	ret = device_property_read_u32(dev, "adi,charge-pump-resistor-ohms", &cp_resistance);
+	if (!ret && (cp_resistance < ADF41513_MIN_R_SET || cp_resistance > ADF41513_MAX_R_SET))
+		return dev_err_probe(dev, -ERANGE, "R_SET %u Ohms out of range\n", cp_resistance);
+
+	/* charge pump current */
+	st->data.charge_pump_voltage_mv = ADF41513_DEFAULT_CP_VOLTAGE_mV;
+	ret = device_property_read_u32(dev, "adi,charge-pump-current-microamp", &cp_current);
+	if (!ret) {
+		tmp = DIV_ROUND_CLOSEST(cp_current * cp_resistance, MILLI); /* convert to mV */
+		if (tmp < ADF41513_MIN_CP_VOLTAGE_mV || tmp > ADF41513_MAX_CP_VOLTAGE_mV)
+			return dev_err_probe(dev, -ERANGE, "I_CP %u uA (%u Ohms) out of range\n",
+					     cp_current, cp_resistance);
+		st->data.charge_pump_voltage_mv = tmp;
+	}
+
+	/* phase detector polarity */
+	st->data.phase_detector_polarity =
+		device_property_read_bool(dev, "adi,phase-detector-polarity-positive-enable");
+
+	/* muxout selection */
+	ret = device_property_match_property_string(dev, "adi,muxout-select",
+						    adf41513_muxout_modes,
+						    ARRAY_SIZE(adf41513_muxout_modes));
+	st->data.muxout_select = ret >= 0 ? ret : ADF41513_MUXOUT_TRISTATE;
+
+	/* muxout logic level: default 3v3 */
+	st->data.muxout_1v8_en = device_property_read_bool(dev, "adi,muxout-level-1v8-enable");
+
+	st->data.lock_detect_count = ADF41513_LD_COUNT_MIN;
+	ret = device_property_read_u32(dev, "adi,lock-detector-count", &tmp);
+	if (!ret) {
+		if (tmp < ADF41513_LD_COUNT_FAST_MIN || tmp > ADF41513_LD_COUNT_MAX ||
+		    !is_power_of_2(tmp))
+			return dev_err_probe(dev, -ERANGE,
+					     "invalid lock detect count: %u\n", tmp);
+		st->data.lock_detect_count = tmp;
+	}
+
+	st->data.freq_resolution_uhz = MICROHZ_PER_HZ;
+
+	return 0;
+}
+
+static int adf41513_setup(struct adf41513_state *st)
+{
+	u32 tmp;
+
+	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
+
+	/* assume DLD pin is used for digital lock detect */
+	st->regs[ADF41513_REG5] = FIELD_PREP(ADF41513_REG5_DLD_MODES_MSK,
+					     ADF41513_DLD_DIG_LD);
+
+	/* configure charge pump current settings */
+	tmp = DIV_ROUND_CLOSEST(st->data.charge_pump_voltage_mv, ADF41513_MIN_CP_VOLTAGE_mV);
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_CP_CURRENT_MSK, tmp - 1);
+
+	/* narrow ABP | loss of lock detect enable | SD reset | LDP from data */
+	st->regs[ADF41513_REG6] = ADF41513_REG6_ABP_MSK |
+				  ADF41513_REG6_LOL_ENABLE_MSK |
+				  ADF41513_REG6_SD_RESET_MSK;
+	if (st->data.phase_detector_polarity)
+		st->regs[ADF41513_REG6] |= ADF41513_REG6_PD_POLARITY_MSK;
+
+	/* PS bias | lock detect count */
+	st->regs[ADF41513_REG7] = FIELD_PREP(ADF41513_REG7_PS_BIAS_MSK, 2);
+	tmp = ilog2(st->data.lock_detect_count);
+	if (st->data.lock_detect_count < ADF41513_LD_COUNT_FAST_LIMIT) {
+		tmp -= const_ilog2(ADF41513_LD_COUNT_FAST_MIN);
+		st->regs[ADF41513_REG7] |= ADF41513_REG7_LD_CLK_SEL_MSK;
+	} else {
+		tmp -= const_ilog2(ADF41513_LD_COUNT_MIN);
+	}
+	st->regs[ADF41513_REG7] |= FIELD_PREP(ADF41513_REG7_LD_COUNT_MSK, tmp);
+
+	/* power down select */
+	st->regs[ADF41513_REG11] = ADF41513_REG11_POWER_DOWN_SEL_MSK;
+
+	/* muxout */
+	st->regs[ADF41513_REG12] = FIELD_PREP(ADF41513_REG12_MUXOUT_MSK,
+					      st->data.muxout_select);
+	st->regs[ADF41513_REG12] |= FIELD_PREP(ADF41513_REG12_LOGIC_LEVEL_MSK,
+					       st->data.muxout_1v8_en ? 0 : 1);
+
+	/* perform initialization sequence with power-up frequency */
+	return adf41513_set_frequency(st, (u64)st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
+				      ADF41513_SYNC_ALL);
+}
+
+static void adf41513_power_down(void *data)
+{
+	struct adf41513_state *st = data;
+
+	adf41513_suspend(st);
+	if (st->chip_enable)
+		gpiod_set_value_cansleep(st->chip_enable, 0);
+}
+
+static int adf41513_pm_suspend(struct device *dev)
+{
+	struct adf41513_state *st = dev_get_drvdata(dev);
+
+	return adf41513_suspend(st);
+}
+
+static int adf41513_pm_resume(struct device *dev)
+{
+	struct adf41513_state *st = dev_get_drvdata(dev);
+
+	return adf41513_resume(st);
+}
+
+static const struct adf41513_chip_info adf41513_chip_info = {
+	.name = "adf41513",
+	.has_prescaler_8_9 = true,
+	.max_rf_freq_hz = ADF41513_MAX_RF_FREQ,
+};
+
+static const struct adf41513_chip_info adf41510_chip_info = {
+	.name = "adf41510",
+	.has_prescaler_8_9 = false,
+	.max_rf_freq_hz = ADF41510_MAX_RF_FREQ,
+};
+
+static int adf41513_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct adf41513_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return -EINVAL;
+
+	spi_set_drvdata(spi, st);
+
+	st->ref_clk = devm_clk_get_enabled(&spi->dev, NULL);
+	if (IS_ERR(st->ref_clk))
+		return PTR_ERR(st->ref_clk);
+
+	st->ref_freq_hz = clk_get_rate(st->ref_clk);
+	if (st->ref_freq_hz < ADF41513_MIN_REF_FREQ || st->ref_freq_hz > ADF41513_MAX_REF_FREQ)
+		return dev_err_probe(&spi->dev, -ERANGE,
+				     "reference frequency %llu Hz out of range\n",
+				     st->ref_freq_hz);
+
+	ret = adf41513_parse_fw(st);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_bulk_get_enable(&spi->dev,
+					     ARRAY_SIZE(adf41513_power_supplies),
+					     adf41513_power_supplies);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to get and enable regulators\n");
+
+	st->chip_enable = devm_gpiod_get_optional(&spi->dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->chip_enable))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->chip_enable),
+				     "fail to request chip enable GPIO\n");
+
+	st->lock_detect = devm_gpiod_get_optional(&spi->dev, "lock-detect", GPIOD_IN);
+	if (IS_ERR(st->lock_detect))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->lock_detect),
+				     "fail to request lock detect GPIO\n");
+
+	ret = devm_mutex_init(&spi->dev, &st->lock);
+	if (ret)
+		return ret;
+
+	indio_dev->name = st->chip_info->name;
+	indio_dev->info = &adf41513_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = &adf41513_chan;
+	indio_dev->num_channels = 1;
+
+	ret = adf41513_setup(st);
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret, "failed to setup device: %d\n", ret);
+
+	ret = devm_add_action_or_reset(&spi->dev, adf41513_power_down, st);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Failed to add power down action: %d\n", ret);
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id adf41513_id[] = {
+	{"adf41510", (kernel_ulong_t)&adf41510_chip_info},
+	{"adf41513", (kernel_ulong_t)&adf41513_chip_info},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, adf41513_id);
+
+static const struct of_device_id adf41513_of_match[] = {
+	{ .compatible = "adi,adf41510", .data = &adf41510_chip_info },
+	{ .compatible = "adi,adf41513", .data = &adf41513_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adf41513_of_match);
+
+static DEFINE_SIMPLE_DEV_PM_OPS(adf41513_pm_ops, adf41513_pm_suspend, adf41513_pm_resume);
+
+static struct spi_driver adf41513_driver = {
+	.driver = {
+		.name = "adf41513",
+		.pm = pm_ptr(&adf41513_pm_ops),
+		.of_match_table = adf41513_of_match,
+	},
+	.probe = adf41513_probe,
+	.id_table = adf41513_id,
+};
+module_spi_driver(adf41513_driver);
+
+MODULE_AUTHOR("Rodrigo Alencar <rodrigo.alencar@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADF41513 PLL Frequency Synthesizer");
+MODULE_LICENSE("GPL");

-- 
2.43.0



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

* [PATCH v2 3/6] iio: frequency: adf41513: handle LE synchronization feature
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 2/6] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 4/6] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

When LE sync is enabled, it is must be set after powering up and must be
disabled when powering down. It is recommended when using the PLL as
a frequency synthesizer, where reference signal will always be present
while the device is being configured.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/frequency/adf41513.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
index a967dc4479e7..ecce4911b296 100644
--- a/drivers/iio/frequency/adf41513.c
+++ b/drivers/iio/frequency/adf41513.c
@@ -220,6 +220,7 @@ struct adf41513_data {
 
 	u8 muxout_select;
 	bool muxout_1v8_en;
+	bool le_sync_en;
 };
 
 struct adf41513_pll_settings {
@@ -731,13 +732,25 @@ static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 s
 static int adf41513_suspend(struct adf41513_state *st)
 {
 	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_POWER_DOWN_MSK, 1);
+	st->regs[ADF41513_REG12] &= ~ADF41513_REG12_LE_SELECT_MSK;
 	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
 }
 
 static int adf41513_resume(struct adf41513_state *st)
 {
+	int ret;
+
 	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_POWER_DOWN_MSK;
-	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	if (ret < 0)
+		return ret;
+
+	if (st->data.le_sync_en) {
+		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
+		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	}
+
+	return ret;
 }
 
 static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
@@ -1061,6 +1074,9 @@ static int adf41513_parse_fw(struct adf41513_state *st)
 		st->data.lock_detect_count = tmp;
 	}
 
+	/* load enable sync */
+	st->data.le_sync_en = device_property_read_bool(dev, "adi,le-sync-enable");
+
 	st->data.freq_resolution_uhz = MICROHZ_PER_HZ;
 
 	return 0;
@@ -1068,6 +1084,7 @@ static int adf41513_parse_fw(struct adf41513_state *st)
 
 static int adf41513_setup(struct adf41513_state *st)
 {
+	int ret;
 	u32 tmp;
 
 	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
@@ -1108,8 +1125,18 @@ static int adf41513_setup(struct adf41513_state *st)
 					       st->data.muxout_1v8_en ? 0 : 1);
 
 	/* perform initialization sequence with power-up frequency */
-	return adf41513_set_frequency(st, (u64)st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
-				      ADF41513_SYNC_ALL);
+	ret = adf41513_set_frequency(st,
+				     (u64)st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
+				     ADF41513_SYNC_ALL);
+	if (ret < 0)
+		return ret;
+
+	if (st->data.le_sync_en) {
+		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
+		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	}
+
+	return ret;
 }
 
 static void adf41513_power_down(void *data)

-- 
2.43.0



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

* [PATCH v2 4/6] iio: frequency: adf41513: features on frequency change
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
                   ` (2 preceding siblings ...)
  2025-12-19 12:34 ` [PATCH v2 3/6] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
  2025-12-19 12:34 ` [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513 Rodrigo Alencar via B4 Relay
  5 siblings, 0 replies; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Set Bleed current when PFD frequency changes (bleed enabled when in
fractional mode). Set lock detector window size, handling bias and
precision. Add phase resync support, setting clock dividers when
PFD frequency changes.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/frequency/adf41513.c | 102 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
index ecce4911b296..91d29baec3e8 100644
--- a/drivers/iio/frequency/adf41513.c
+++ b/drivers/iio/frequency/adf41513.c
@@ -210,6 +210,7 @@ struct adf41513_chip_info {
 struct adf41513_data {
 	u64 power_up_frequency_hz;
 	u64 freq_resolution_uhz;
+	u32 phase_resync_period_ns;
 	u32 charge_pump_voltage_mv;
 	u32 lock_detect_count;
 
@@ -276,6 +277,16 @@ struct adf41513_state {
 	__be32 buf __aligned(IIO_DMA_MINALIGN);
 };
 
+static const u16 adf41513_ld_window_p1ns[] = {
+	9, 12, 16, 17, 21, 28, 29, 35,			/* 0 - 7 */
+	43, 47, 49, 52, 70, 79, 115			/* 8 - 14 */
+};
+
+static const u8 adf41513_ldp_bias[] = {
+	0xC, 0xD, 0xE, 0x8, 0x9, 0x4, 0xA, 0x5,		/* 0 - 7 */
+	0x0, 0x6, 0xB, 0x1, 0x2, 0x7, 0x3		/* 8 - 14 */
+};
+
 static const char * const adf41513_muxout_modes[] = {
 	[ADF41513_MUXOUT_TRISTATE] = "high_z",
 	[ADF41513_MUXOUT_DVDD] = "muxout_high",
@@ -664,9 +675,85 @@ static int adf41513_calc_pll_settings(struct adf41513_state *st,
 	return 0;
 }
 
+static void adf41513_set_bleed_val(struct adf41513_state *st)
+{
+	u32 bleed_value;
+
+	if (st->data.phase_detector_polarity)
+		bleed_value = 90;
+	else
+		bleed_value = 144;
+
+	bleed_value *= 1 + FIELD_GET(ADF41513_REG5_CP_CURRENT_MSK,
+				     st->regs[ADF41513_REG5]);
+	bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value,
+				1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ);
+
+	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_BLEED_CURRENT_MSK;
+	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_BLEED_CURRENT_MSK,
+					      bleed_value);
+}
+
+static void adf41513_set_ld_window(struct adf41513_state *st)
+{
+	/*
+	 * The ideal lock detector window size is halfway between the max
+	 * window, set by the phase comparison period t_PFD = (1 / f_PFD),
+	 * and the minimum is set by (I_BLEED/I_CP) × t_PFD
+	 */
+	u16 ld_window_p1ns = div64_u64(10ULL * NANO * MICROHZ_PER_HZ,
+				       st->settings.pfd_frequency_uhz << 1);
+	u8 ld_idx, ldp, ld_bias;
+
+	if (st->settings.mode != ADF41513_MODE_INTEGER_N) {
+		/* account for bleed current (deduced from eq.6 and eq.7) */
+		if (st->data.phase_detector_polarity)
+			ld_window_p1ns += 4;
+		else
+			ld_window_p1ns += 6;
+	}
+
+	ld_idx = find_closest(ld_window_p1ns, adf41513_ld_window_p1ns,
+			      ARRAY_SIZE(adf41513_ld_window_p1ns));
+	ldp = (adf41513_ldp_bias[ld_idx] >> 2) & 0x3;
+	ld_bias = adf41513_ldp_bias[ld_idx] & 0x3;
+
+	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_LDP_MSK;
+	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_LDP_MSK, ldp);
+	st->regs[ADF41513_REG9] &= ~ADF41513_REG9_LD_BIAS_MSK;
+	st->regs[ADF41513_REG9] |= FIELD_PREP(ADF41513_REG9_LD_BIAS_MSK, ld_bias);
+}
+
+static void adf41513_set_phase_resync(struct adf41513_state *st)
+{
+	u32 total_div, clk1_div, clk2_div;
+
+	if (!st->data.phase_resync_period_ns)
+		return;
+
+	/* Assuming both clock dividers hold similar values */
+	total_div = mul_u64_u64_div_u64(st->settings.pfd_frequency_uhz,
+					st->data.phase_resync_period_ns,
+					1ULL * MICRO * NANO);
+	clk1_div = clamp(int_sqrt(total_div), 1,
+			 ADF41513_MAX_CLK_DIVIDER);
+	clk2_div = clamp(DIV_ROUND_CLOSEST(total_div, clk1_div), 1,
+			 ADF41513_MAX_CLK_DIVIDER);
+
+	st->regs[ADF41513_REG5] &= ~ADF41513_REG5_CLK1_DIV_MSK;
+	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_CLK1_DIV_MSK, clk1_div);
+	st->regs[ADF41513_REG7] &= ~ADF41513_REG7_CLK2_DIV_MSK;
+	st->regs[ADF41513_REG7] |= FIELD_PREP(ADF41513_REG7_CLK2_DIV_MSK, clk2_div);
+
+	/* enable phase resync */
+	st->regs[ADF41513_REG7] |= ADF41513_REG7_CLK_DIV_MODE_MSK;
+}
+
 static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 sync_mask)
 {
 	struct adf41513_pll_settings result;
+	bool pfd_change = false;
+	bool mode_change = false;
 	int ret;
 
 	/* calculate pll settings candidate */
@@ -675,6 +762,8 @@ static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 s
 		return ret;
 
 	/* apply computed results to pll settings */
+	pfd_change = st->settings.pfd_frequency_uhz != result.pfd_frequency_uhz;
+	mode_change = st->settings.mode != result.mode;
 	memcpy(&st->settings, &result, sizeof(struct adf41513_pll_settings));
 
 	dev_dbg(&st->spi->dev,
@@ -726,6 +815,14 @@ static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 s
 		st->regs[ADF41513_REG6] |= ADF41513_REG6_BLEED_ENABLE_MSK;
 	}
 
+	if (pfd_change) {
+		adf41513_set_bleed_val(st);
+		adf41513_set_phase_resync(st);
+	}
+
+	if (pfd_change || mode_change)
+		adf41513_set_ld_window(st);
+
 	return adf41513_sync_config(st, sync_mask | ADF41513_SYNC_REG0);
 }
 
@@ -1061,6 +1158,11 @@ static int adf41513_parse_fw(struct adf41513_state *st)
 						    ARRAY_SIZE(adf41513_muxout_modes));
 	st->data.muxout_select = ret >= 0 ? ret : ADF41513_MUXOUT_TRISTATE;
 
+	st->data.phase_resync_period_ns = 0;
+	ret = device_property_read_u32(dev, "adi,phase-resync-period-ns", &tmp);
+	if (!ret)
+		st->data.phase_resync_period_ns = tmp;
+
 	/* muxout logic level: default 3v3 */
 	st->data.muxout_1v8_en = device_property_read_bool(dev, "adi,muxout-level-1v8-enable");
 

-- 
2.43.0



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

* [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
                   ` (3 preceding siblings ...)
  2025-12-19 12:34 ` [PATCH v2 4/6] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-21 18:00   ` Jonathan Cameron
  2025-12-19 12:34 ` [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513 Rodrigo Alencar via B4 Relay
  5 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

add documentation for ADF41513 driver which describes the device
driver files and shows how userspace may consume the ABI for various
tasks

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 Documentation/iio/adf41513.rst | 255 +++++++++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst    |   1 +
 MAINTAINERS                    |   1 +
 3 files changed, 257 insertions(+)

diff --git a/Documentation/iio/adf41513.rst b/Documentation/iio/adf41513.rst
new file mode 100644
index 000000000000..568e71bc21e4
--- /dev/null
+++ b/Documentation/iio/adf41513.rst
@@ -0,0 +1,255 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============
+ADF41513 driver
+===============
+
+This driver supports Analog Devices' ADF41513 and ADF41510 PLL frequency
+synthesizers on SPI bus.
+
+1. Supported devices
+====================
+
+* `ADF41510 <https://www.analog.com/ADF41510>`_
+* `ADF41513 <https://www.analog.com/ADF41513>`_
+
+The ADF41513 is an ultralow noise frequency synthesizer that can be used to
+implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
+downconversion sections of wireless receivers and transmitters. The ADF41510
+is a similar device that supports frequencies up to 10 GHz.
+
+Both devices support integer-N and fractional-N operation modes, providing
+excellent phase noise performance and flexible frequency generation
+capabilities.
+
+Key Features:
+
+- **ADF41513**: 1 GHz to 26.5 GHz frequency range
+- **ADF41510**: 1 GHz to 10 GHz frequency range
+- Integer-N and fractional-N operation modes
+- Ultra-low phase noise (-235 dBc/Hz integer-N, -231 dBc/Hz fractional-N)
+- High maximum PFD frequency (250 MHz integer-N, 125 MHz fractional-N)
+- 25-bit fixed modulus or 49-bit variable modulus fractional modes
+- Programmable charge pump currents with 16x range
+- Digital lock detect functionality
+- Phase resync capability for consistent output phase
+
+2. Device attributes
+====================
+
+The ADF41513 driver provides the following IIO extended attributes for
+frequency control and monitoring:
+
+Each IIO device has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
+where X is the IIO index of the device. Under these folders reside a set of
+device files that provide access to the synthesizer's functionality.
+
+The following table shows the ADF41513 related device files:
+
++----------------------+-------------------------------------------------------+
+| Device file          | Description                                           |
++======================+=======================================================+
+| frequency            | RF output frequency control and readback (Hz)         |
++----------------------+-------------------------------------------------------+
+| frequency_resolution | Target frequency resolution control (Hz)              |
++----------------------+-------------------------------------------------------+
+| refin_frequency      | Reference input frequency control and readback (Hz)   |
++----------------------+-------------------------------------------------------+
+| powerdown            | Power management control (0=active, 1=power down)     |
++----------------------+-------------------------------------------------------+
+| phase                | RF output phase adjustment and readback (degrees)     |
++----------------------+-------------------------------------------------------+
+
+2.1 Frequency Control
+----------------------
+
+The ``frequency`` attribute controls the RF output frequency with sub-Hz
+precision. The driver automatically selects between integer-N and fractional-N
+modes to achieve the requested frequency with the best possible phase noise
+performance.
+
+**Supported ranges:**
+
+- **ADF41513**: 1,000,000,000 Hz to 26,500,000,000 Hz (1 GHz to 26.5 GHz)
+- **ADF41510**: 1,000,000,000 Hz to 10,000,000,000 Hz (1 GHz to 10 GHz)
+
+The frequency is specified in Hz, for sub-Hz precision use decimal notation.
+For example, 12.102 GHz would be written as "12102000000.000000".
+
+2.2 Frequency Resolution Control
+--------------------------------
+
+The ``frequency_resolution`` attribute controls the target frequency resolution
+that the driver attempts to achieve. This affects the choice between integer-N
+and fractional-N modes, including fixed modulus (25-bit) and variable modulus
+(49-bit) fractional-N modes:
+
+- **Integer-N**: Resolution = f_PFD
+- **Fixed modulus**: Resolution = f_PFD / 2^25 (~3 Hz with 100 MHz PFD)
+- **Variable modulus**: Resolution = f_PFD / 2^49 (µHz resolution possible)
+
+Default resolution is 1 Hz (1,000,000 µHz).
+
+2.3 Reference Input Control
+---------------------------
+
+The ``refin_frequency`` attribute allows control of the reference input
+frequency when using a programmable reference clock. The supported range is
+10 MHz to 800 MHz.
+
+2.4 Power Management
+--------------------
+
+The ``powerdown`` attribute provides software power control:
+
+- **0**: Device active and operational
+- **1**: Device in power-down mode (low power consumption)
+
+2.5 Phase adjustment
+--------------------
+
+The ``phase`` attribute allows adjustment of the output phase in degrees.
+Setting this attribute enables phase adjustment. It can be set from 0 to 360
+degrees. Reading this attribute returns the current phase offset of the output
+signal. To create a consistent phase relationship with the reference signal,
+the phase resync feature needs to be enabled by setting a non-zero value to the
+``adi,phase-resync-period-ns`` device property, which triggers a phase
+resynchronization after locking is achieved.
+
+3. Operating modes
+==================
+
+3.1 Integer-N Mode
+------------------
+
+When the requested frequency can be achieved as an integer multiple of the PFD
+frequency (within the specified resolution tolerance), the driver automatically
+selects integer-N mode for optimal phase noise performance.
+
+In integer-N mode:
+
+- Phase noise: -235 dBc/Hz normalized floor
+- Frequency resolution: f_PFD (same as PFD frequency)
+- Maximum PFD frequency: 250 MHz
+- Bleed current: Disabled for best performance
+
+3.2 Fractional-N Mode
+---------------------
+
+When sub-integer frequency steps are required, the driver automatically selects
+fractional-N mode using either fixed or variable modulus.
+
+**Fixed Modulus (25-bit)**:
+
+- Used when variable modulus is not required
+- Resolution: f_PFD / 2^25
+- Simpler implementation, faster settling
+
+**Variable Modulus (49-bit)**:
+
+- Used for maximum resolution requirements
+- Resolution: f_PFD / 2^49 (theoretical)
+- Exact frequency synthesis capability
+
+In fractional-N mode:
+
+- Phase noise: -231 dBc/Hz normalized floor
+- Maximum PFD frequency: 125 MHz
+- Bleed current: Automatically enabled and optimized
+- Dithering: Enabled to reduce fractional spurs
+
+3.3 Automatic Mode Selection
+----------------------------
+
+The driver automatically selects the optimal operating mode based on:
+
+1. **Frequency accuracy requirements**: Determined by frequency_resolution setting
+2. **Phase noise optimization**: Integer-N preferred when possible
+3. **PFD frequency constraints**: Different limits for integer vs fractional modes
+4. **Prescaler selection**: Automatic 4/5 vs 8/9 prescaler selection based on frequency
+
+4. Usage examples
+=================
+
+4.1 Basic Frequency Setting
+----------------------------
+
+Set output frequency to 12.102 GHz:
+
+.. code-block:: bash
+
+    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
+
+Read current frequency:
+
+.. code-block:: bash
+
+    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
+    12101999999.582767
+
+4.2 High Resolution Frequency Control
+-------------------------------------
+
+Configure for sub-Hz resolution and set a precise frequency:
+
+.. code-block:: bash
+
+    # Set resolution to 0.1 Hz (100,000 µHz)
+    root:/sys/bus/iio/devices/iio:device0> echo 0.1 > out_altvoltage0_frequency_resolution
+
+    # Set frequency to 12.102 GHz (1 µHz precision)
+    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
+    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
+    12101999999.980131
+
+4.3 Reference Frequency Control
+-------------------------------
+
+Change reference input frequency (if using programmable reference):
+
+.. code-block:: bash
+
+    # Set reference to 122.88 MHz
+    root:/sys/bus/iio/devices/iio:device0> echo 122880000 > out_altvoltage0_refin_frequency
+
+    # Verify the change
+    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_refin_frequency
+    122880000
+
+4.4 Power Management
+--------------------
+
+Power down the device:
+
+.. code-block:: bash
+
+    root:/sys/bus/iio/devices/iio:device0> echo 1 > out_altvoltage0_powerdown
+
+    # Power back up
+    root:/sys/bus/iio/devices/iio:device0> echo 0 > out_altvoltage0_powerdown
+
+4.5 PFD Frequency Monitoring
+----------------------------
+
+Read the current PFD frequency:
+
+.. code-block:: bash
+
+    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_pfd_frequency
+    100000000.000000
+
+This shows the PFD is operating at 100 MHz, which means the frequency resolution
+in integer-N mode would be 100 MHz steps.
+
+4.6 Monitor Lock Status
+-----------------------
+
+When lock detect GPIO is configured, check if PLL is locked:
+
+.. code-block:: bash
+
+    # Read frequency - will return error if not locked
+    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
+
+If the PLL is not locked, the frequency read will return ``-EBUSY`` (Device or
+resource busy).
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 315ae37d6fd4..420669af60db 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -29,6 +29,7 @@ Industrial I/O Kernel Drivers
    ad7625
    ad7944
    ade9000
+   adf41513
    adis16475
    adis16480
    adis16550
diff --git a/MAINTAINERS b/MAINTAINERS
index 06ba879a248a..c536c3afc1ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1607,6 +1607,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
+F:	Documentation/iio/adf41513.rst
 F:	drivers/iio/frequency/adf41513.c
 
 ANALOG DEVICES INC ADF4377 DRIVER

-- 
2.43.0



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

* [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513
  2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
                   ` (4 preceding siblings ...)
  2025-12-19 12:34 ` [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
@ 2025-12-19 12:34 ` Rodrigo Alencar via B4 Relay
  2025-12-21 17:52   ` Jonathan Cameron
  5 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2025-12-19 12:34 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add ABI documentation for ADF41513 PLL sysfs interfaces

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../ABI/testing/sysfs-bus-iio-frequency-adf41513   | 27 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 28 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
new file mode 100644
index 000000000000..11ffd248eedb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
@@ -0,0 +1,27 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_resolution
+KernelVersion:	6.20
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Stores channel Y frequency resolution/channel spacing in Hz.
+		The value given directly influences the choice of operation:
+
+		- integer-N: requested frequency is a multiple of the Phase Detector
+		frequency.
+		- fixed modulus: fractional-N mode with fixed modulus.
+		- variable modulus: dual-modulus fractional-N mode with extra variable
+		modulus added on top of the fixed one.
+
+		It is assumed that the algorithm that is used to compute the various
+		dividers, is able to generate proper values for multiples of channel
+		spacing.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_refin_frequency
+KernelVersion:	6.20
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Sets channel Y REFin frequency in Hz. In some clock chained
+		applications, the reference frequency used by the PLL may change during
+		runtime. This attribute allows the user to adjust the reference
+		frequency accordingly.
+		To avoid glitches in the RF output, consider using out_altvoltageY_powerdown
+		to power down the PLL and its RFOut buffers during REFin changes.
diff --git a/MAINTAINERS b/MAINTAINERS
index c536c3afc1ae..48fa1011b797 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1606,6 +1606,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-adf41513
 F:	Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
 F:	Documentation/iio/adf41513.rst
 F:	drivers/iio/frequency/adf41513.c

-- 
2.43.0



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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
@ 2025-12-20  9:21   ` Krzysztof Kozlowski
  2025-12-20 18:05     ` 455.rodrigo.alencar
  2025-12-21 16:59   ` Jonathan Cameron
  1 sibling, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-20  9:21 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:
> dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> Most properties refer to existing PLL driver properties (e.g. ADF4350).

What is "existing PLL driver"? I know about motor drivers, but can you
drive PLL?

And how is ADF4350 related to this binding. I do not see ADF4350
compatible here at all. Describe hardware, a real one.


> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  .../bindings/iio/frequency/adi,adf41513.yaml       | 246 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  2 files changed, 253 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> new file mode 100644
> index 000000000000..01ceb2a7d21b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> @@ -0,0 +1,246 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,adf41513.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADF41513 PLL Frequency Synthesizer
> +
> +maintainers:
> +  - Rodrigo Alencar <rodrigo.alencar@analog.com>
> +
> +description:
> +  The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> +  implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> +  downconversion sections of wireless receivers and transmitters. The ADF41510
> +  supports frequencies up to 10 GHz.
> +
> +  https://www.analog.com/en/products/adf41513.html
> +  https://www.analog.com/en/products/adf41510.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adf41510
> +      - adi,adf41513
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 25000000
> +
> +  clocks:
> +    maxItems: 1
> +    description: Clock that provides the reference input frequency.
> +
> +  avdd1-supply:
> +    description: PFD and Up and Down Digital Driver Power Supply (3.3 V)
> +
> +  avdd2-supply:
> +    description: RF Buffer and Prescaler Power Supply (3.3 V)
> +
> +  avdd3-supply:
> +    description: N Divider Power Supply (3.3 V)
> +
> +  avdd4-supply:
> +    description: R Divider and Lock Detector Power Supply (3.3 V)
> +
> +  avdd5-supply:
> +    description: Sigma-Delta Modulator and SPI Power Supply (3.3 V)
> +
> +  vp-supply:
> +    description: Charge Pump Power Supply (3.3 V)
> +
> +  enable-gpios:
> +    description:
> +      GPIO that controls the chip enable pin. A logic low on this pin
> +      powers down the device and puts the charge pump output into
> +      three-state mode.
> +    maxItems: 1
> +
> +  lock-detect-gpios:
> +    description:
> +      GPIO for lock detect functionality. When configured for digital lock
> +      detect, this pin will output a logic high when the PLL is locked.
> +    maxItems: 1
> +
> +  adi,power-up-frequency:

Nothing improved.

You ignored comments, did not bother to respond to them and then sent
the same.

NAK

Best regards,
Krzysztof


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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-20  9:21   ` Krzysztof Kozlowski
@ 2025-12-20 18:05     ` 455.rodrigo.alencar
  2025-12-21 13:02       ` Krzysztof Kozlowski
  2025-12-21 15:56       ` Jonathan Cameron
  0 siblings, 2 replies; 24+ messages in thread
From: 455.rodrigo.alencar @ 2025-12-20 18:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rodrigo Alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

Hi Krzystof,

thanks for taking a look into this again. It was my first patch it didn't want
to draw more attention or discussion to the V1 patch as it was declared not ready
at its very first review.

On 25/12/20 10:21AM, Krzysztof Kozlowski wrote:
> On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:
> > dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> > can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> > Most properties refer to existing PLL driver properties (e.g. ADF4350).
>
> What is "existing PLL driver"? I know about motor drivers, but can you
> drive PLL?
>
> And how is ADF4350 related to this binding. I do not see ADF4350
> compatible here at all. Describe hardware, a real one.

ADF4350 is an older one, and its bindings can be found at:
Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml
It is a similar part, but yet very different.

>
> Nothing improved.
>
> You ignored comments, did not bother to respond to them and then sent
> the same.

Sorry for not responding on the V1 thread, but the previous patch had to be reviewed internally
first. It is not true that nothing is improved, in fact, it has changed a lot, here are some notes:
* adi,power-up-frequency is not carrying the -hz postfix because it forces to be a uint32 by
the dt-bindings check. For that variable it needs to be uint64 as the part supports up to 26.5 GHz > 2^32
* The properties related to the reference input signal path: reference-div-factor, reference-doubler-enable
reference-div2-enable are declared here because they are constraints for the PFD frequency definition,
which is the frequency that the output signal is updated, important for the loop-filter and VCO design.
* added support for all different power supply regulators.
* adi,lock-detect-precision and adi,lock-detect-bias-microamp: removed, now set
with adf41513_set_ld_window() following datasheet recommendation
* adi,fast-lock-enable: removed, faster lock detect clock is set depending on the lock-detect-count value
* adi,phase-resync-enable, adi,12bit-clk-divider and adi,12bit-clk2-divider: removed, now set with
adf41513_set_phase_resync(), based on the t_sync (from the datasheet: Phase Resync section)
value determined by adi,phase-resync-period-ns, which is also bound to the loop filter design.

kind regards,

Rodrigo Alencar

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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-20 18:05     ` 455.rodrigo.alencar
@ 2025-12-21 13:02       ` Krzysztof Kozlowski
  2025-12-22 10:21         ` Rodrigo Alencar
  2025-12-21 15:56       ` Jonathan Cameron
  1 sibling, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-21 13:02 UTC (permalink / raw)
  To: 455.rodrigo.alencar, Rodrigo Alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On 20/12/2025 19:05, 455.rodrigo.alencar@gmail.com wrote:
> Hi Krzystof,
> 
> thanks for taking a look into this again. It was my first patch it didn't want
> to draw more attention or discussion to the V1 patch as it was declared not ready
> at its very first review.
> 
> On 25/12/20 10:21AM, Krzysztof Kozlowski wrote:
>> On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:
>>> dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
>>> can be used to implement local oscillators (LOs) as high as 26.5 GHz.
>>> Most properties refer to existing PLL driver properties (e.g. ADF4350).
>>
>> What is "existing PLL driver"? I know about motor drivers, but can you
>> drive PLL?
>>
>> And how is ADF4350 related to this binding. I do not see ADF4350
>> compatible here at all. Describe hardware, a real one.
> 
> ADF4350 is an older one, and its bindings can be found at:
> Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml
> It is a similar part, but yet very different.
> 
>>
>> Nothing improved.
>>
>> You ignored comments, did not bother to respond to them and then sent
>> the same.
> 
> Sorry for not responding on the V1 thread, but the previous patch had to be reviewed internally
> first. It is not true that nothing is improved, in fact, it has changed a lot, here are some notes:

Process is not like that. You first review internally, then you send.
After you sent and receive comments, you respond to these comments.

> * adi,power-up-frequency is not carrying the -hz postfix because it forces to be a uint32 by
> the dt-bindings check. For that variable it needs to be uint64 as the part supports up to 26.5 GHz > 2^32

And what granularity do you need? Why mhz does not work?

> * The properties related to the reference input signal path: reference-div-factor, reference-doubler-enable
> reference-div2-enable are declared here because they are constraints for the PFD frequency definition,
> which is the frequency that the output signal is updated, important for the loop-filter and VCO design.
> * added support for all different power supply regulators.

Sorry, but I cannot respond that way. We discuss inline, so I have
entire picture, not some parts of message semi-quoted here. I don't
remember what was there and I am not going to keep looking for that.

You need to adjust to mailing list discussion style, not introduce the
others. I have just way too many other patches to deal with, so
implement the feedback or respond properly.

Best regards,
Krzysztof

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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-20 18:05     ` 455.rodrigo.alencar
  2025-12-21 13:02       ` Krzysztof Kozlowski
@ 2025-12-21 15:56       ` Jonathan Cameron
  2025-12-21 19:56         ` Rodrigo Alencar
  1 sibling, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-21 15:56 UTC (permalink / raw)
  To: 455.rodrigo.alencar
  Cc: Krzysztof Kozlowski, Rodrigo Alencar, linux-kernel, linux-iio,
	devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet

On Sat, 20 Dec 2025 18:05:34 +0000
455.rodrigo.alencar@gmail.com wrote:

> Hi Krzystof,
> 
> thanks for taking a look into this again. It was my first patch it didn't want
> to draw more attention or discussion to the V1 patch as it was declared not ready
> at its very first review.
> 
> On 25/12/20 10:21AM, Krzysztof Kozlowski wrote:
> > On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:  
> > > dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> > > can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> > > Most properties refer to existing PLL driver properties (e.g. ADF4350).  
> >
> > What is "existing PLL driver"? I know about motor drivers, but can you
> > drive PLL?
> >
> > And how is ADF4350 related to this binding. I do not see ADF4350
> > compatible here at all. Describe hardware, a real one.  
> 
> ADF4350 is an older one, and its bindings can be found at:
> Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml
> It is a similar part, but yet very different.
> 
> >
> > Nothing improved.
> >
> > You ignored comments, did not bother to respond to them and then sent
> > the same.  
> 
> Sorry for not responding on the V1 thread, but the previous patch had to be reviewed internally
> first. It is not true that nothing is improved, in fact, it has changed a lot, here are some notes:
> * adi,power-up-frequency is not carrying the -hz postfix because it forces to be a uint32 by
> the dt-bindings check. For that variable it needs to be uint64 as the part supports up to 26.5 GHz > 2^32

What granularity is necessary?  E.g. Could -mhz work here? It's already defined in dts schema for MHz.

> * The properties related to the reference input signal path: reference-div-factor, reference-doubler-enable
> reference-div2-enable are declared here because they are constraints for the PFD frequency definition,
> which is the frequency that the output signal is updated, important for the loop-filter and VCO design.
> * added support for all different power supply regulators.
> * adi,lock-detect-precision and adi,lock-detect-bias-microamp: removed, now set
> with adf41513_set_ld_window() following datasheet recommendation
> * adi,fast-lock-enable: removed, faster lock detect clock is set depending on the lock-detect-count value
> * adi,phase-resync-enable, adi,12bit-clk-divider and adi,12bit-clk2-divider: removed, now set with
> adf41513_set_phase_resync(), based on the t_sync (from the datasheet: Phase Resync section)
> value determined by adi,phase-resync-period-ns, which is also bound to the loop filter design.
> 
> kind regards,
> 
> Rodrigo Alencar


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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
  2025-12-20  9:21   ` Krzysztof Kozlowski
@ 2025-12-21 16:59   ` Jonathan Cameron
  2025-12-21 19:45     ` Rodrigo Alencar
  1 sibling, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-21 16:59 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On Fri, 19 Dec 2025 12:34:48 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> Most properties refer to existing PLL driver properties (e.g. ADF4350).

"Refer" implies a cross reference in this document.   Based upon is probably a better
way to put this.

Otherwise I've mostly commented on properties that to me don't sound like
they belong in the dt-binding as they are policy things that we want
to make runtime configurable.

Thanks,

Jonathan

> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  .../bindings/iio/frequency/adi,adf41513.yaml       | 246 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  2 files changed, 253 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> new file mode 100644
> index 000000000000..01ceb2a7d21b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> @@ -0,0 +1,246 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,adf41513.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADF41513 PLL Frequency Synthesizer
> +
> +maintainers:
> +  - Rodrigo Alencar <rodrigo.alencar@analog.com>
> +
> +description:
> +  The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> +  implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> +  downconversion sections of wireless receivers and transmitters. The ADF41510
> +  supports frequencies up to 10 GHz.
> +
> +  https://www.analog.com/en/products/adf41513.html
> +  https://www.analog.com/en/products/adf41510.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adf41510
> +      - adi,adf41513
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 25000000
> +
> +  clocks:
> +    maxItems: 1
> +    description: Clock that provides the reference input frequency.
> +
> +  avdd1-supply:
> +    description: PFD and Up and Down Digital Driver Power Supply (3.3 V)
> +
> +  avdd2-supply:
> +    description: RF Buffer and Prescaler Power Supply (3.3 V)
> +
> +  avdd3-supply:
> +    description: N Divider Power Supply (3.3 V)
> +
> +  avdd4-supply:
> +    description: R Divider and Lock Detector Power Supply (3.3 V)
> +
> +  avdd5-supply:
> +    description: Sigma-Delta Modulator and SPI Power Supply (3.3 V)
> +
> +  vp-supply:
> +    description: Charge Pump Power Supply (3.3 V)
> +
> +  enable-gpios:
> +    description:
> +      GPIO that controls the chip enable pin. A logic low on this pin
> +      powers down the device and puts the charge pump output into
> +      three-state mode.
> +    maxItems: 1
> +
> +  lock-detect-gpios:
> +    description:
> +      GPIO for lock detect functionality. When configured for digital lock
> +      detect, this pin will output a logic high when the PLL is locked.

This seems to be one potential use of the muxout pin.  So to me feels like
a policy decision that belongs with the driver or userspace, not in dt.
mux-out-gpios:
would make more sense to me.
Some of the potential settings probably don't make sense but then we just
don't support those in the driver if this is connected to a gpio.

> +    maxItems: 1
> +
> +  adi,power-up-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint64
> +    minimum: 1000000000
> +    maximum: 26500000000
> +    default: 10000000000
> +    description:
> +      The PLL tunes to this frequency (in Hz) during the initialization
> +      sequence. This property should be set to a frequency supported by the
> +      loop filter and VCO used in the design. Range is 1 GHz to 26.5 GHz for
> +      ADF41513, and 1 GHz to 10 GHz for ADF41510.

Why is this in DT?  Feels like this should be done by userspace control
prior to setting an enable rather than being in DT.

> +
> +  adi,reference-div-factor:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 32
> +    default: 1
> +    description:
> +      Value for the reference division factor (R Counter). The driver will
> +      increment R Counter as needed to achieve a PFD frequency within the
> +      allowed range. High R counter values will reduce the PFD frequency, which
> +      lowers the frequency resolution, and affects phase noise performance.

Why is this in DT?  Is there not a 'best' choice to be made given a particular
input frequency and desired output frequency?

> +
> +  adi,reference-doubler-enable:
> +    description:
> +      Enables the reference doubler. The maximum reference frequency when
> +      the doubler is enabled is 225 MHz.
As above. 
> +    type: boolean
> +
> +  adi,reference-div2-enable:
> +    description:
> +      Enables the reference divide-by-2 function. This provides a 50%
> +      duty cycle signal to the PFD.
As above.
> +    type: boolean
> +
> +  adi,charge-pump-resistor-ohms:
> +    minimum: 1800
> +    maximum: 10000
> +    default: 2700
> +    description:
> +      External charge pump resistor (R_SET) value in ohms. This sets the maximum
> +      charge pump current along with the charge pump current setting.
> +
> +  adi,charge-pump-current-microamp:
> +    description:
> +      Charge pump current (I_CP) in microamps. The value will be rounded to the
> +      nearest supported value. Range of acceptable values depends on the
> +      charge pump resistor value, such that 810 mV <= I_CP * R_SET <= 12960 mV.
> +      This value depends on the loop filter design.
> +
> +  adi,muxout-select:
> +    description:
> +      On chip multiplexer output selection.
> +      high_z - MUXOUT Pin set to high-Z. (default)
> +      muxout_high - MUXOUT Pin set to high.
> +      muxout_low - MUXOUT Pin set to low.
> +      f_div_rclk - MUXOUT Pin set to R divider output
> +      f_div_nclk - MUXOUT Pin set to N divider output
> +      lock_detect - MUXOUT Pin set to Digital lock detect
> +      serial_data - MUXOUT Pin set to Serial data output
> +      readback - MUXOUT Pin set to Readback mode
> +      f_div_clk1 - MUXOUT Pin set to CLK1 divider output
> +      f_div_rclk_2 - MUXOUT Pin set to R divider/2 output
> +      f_div_nclk_2 - MUXOUT Pin set to N divider/2 output
> +    enum: [high_z, muxout_high, muxout_low, f_div_rclk, f_div_nclk, lock_detect,
> +           serial_data, readback, f_div_clk1, f_div_rclk_2, f_div_nclk_2]

This needs explanation of 'why' it should be in DT?  To me it seems mostly
to be a debug feature that should be controlled perhaps via a debugfs interface.
> +
> +  adi,muxout-level-1v8-enable:
> +    description:
> +      Set MUXOUT and DLD logic levels to 1.8V. Default is 3.3V.
> +    type: boolean
> +
> +  adi,phase-detector-polarity-positive-enable:
> +    description:
> +      Set phase detector polarity to positive. Default is negative.
> +      Use positive polarity with non-inverting loop filter and VCO with
> +      positive tuning slope, or with inverting loop filter and VCO with
> +      negative tuning slope.
> +    type: boolean
> +
> +  adi,lock-detector-count:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 64
> +    description:
> +      Sets the value for Lock Detector count of the PLL, which determines the
> +      number of consecutive phase detector cycles that must be within the lock
> +      detector window before lock is declared. Lower values increase the lock
> +      detection sensitivity.
> +    enum: [2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]

Sounds like policy.  Maybe it is related to the circuit design and there
is a right choice for  particular board? If not we should figure out how to leave
this to userspace control.  Probably as some form of event property.

> +
> +  adi,phase-resync-period-ns:
> +    default: 0
> +    description:
> +      When this value is non-zero, enable phase resync functionality, which
> +      produces a consistent output phase offset with respect to the input
> +      reference. The value specifies the resync period in nanoseconds, used
> +      to configure clock dividers with respect to the PFD frequency. This value
> +      should be set to a value that is at least as long as the worst case lock
> +      time, i.e., it depends mostly on the loop filter design.
> +
> +  adi,le-sync-enable:
> +    description:
> +      Synchronizes Load Enable (LE) transitions with the reference signal to
> +      avoid asynchronous glitches in the output. This is recommended when using
> +      the PLL as a frequency synthesizer, where reference signal will always be
> +      present while the device is being configured. When using the PLL as a
> +      frequency tracker, where the reference signal may be absent for long
> +      periods of time, LE sync should be disabled.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - avdd1-supply
> +  - avdd2-supply
> +  - avdd3-supply
> +  - avdd4-supply
> +  - avdd5-supply
> +  - vp-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pll@0 {
> +            compatible = "adi,adf41513";
> +            reg = <0>;
> +            spi-max-frequency = <10000000>;
> +            clocks = <&ref_clk>;
> +            avdd1-supply = <&vdd_3v3>;
> +            avdd2-supply = <&vdd_3v3>;
> +            avdd3-supply = <&vdd_3v3>;
> +            avdd4-supply = <&vdd_3v3>;
> +            avdd5-supply = <&vdd_3v3>;
> +            vp-supply = <&vdd_3v3>;
> +
> +            adi,power-up-frequency = /bits/ 64 <12000000000>;
> +            adi,charge-pump-current-microamp = <2400>;
> +            adi,phase-detector-polarity-positive-enable;
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pll@0 {
> +            compatible = "adi,adf41513";
> +            reg = <0>;
> +            spi-max-frequency = <25000000>;
> +            clocks = <&ref_clk>;
> +            avdd1-supply = <&avdd1_3v3>;
> +            avdd2-supply = <&avdd2_3v3>;
> +            avdd3-supply = <&avdd3_3v3>;
> +            avdd4-supply = <&avdd4_3v3>;
> +            avdd5-supply = <&avdd5_3v3>;
> +            vp-supply = <&vp_3v3>;
> +            enable-gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
> +            lock-detect-gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
> +
> +            adi,power-up-frequency = /bits/ 64 <15500000000>;
> +            adi,charge-pump-current-microamp = <3600>;
> +            adi,charge-pump-resistor-ohms = <2700>;
> +            adi,reference-doubler-enable;
> +            adi,muxout-select = "lock_detect";
> +            adi,lock-detector-count = <64>;
> +            adi,phase-resync-period-ns = <0>;
> +            adi,phase-detector-polarity-positive-enable;
> +            adi,le-sync-enable;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 29340394ac9d..1c1343f21160 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1601,6 +1601,13 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ada4250.yaml
>  F:	drivers/iio/amplifiers/ada4250.c
>  
> +ANALOG DEVICES INC ADF41513 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,adf41513.yaml
> +
>  ANALOG DEVICES INC ADF4377 DRIVER
>  M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
>  L:	linux-iio@vger.kernel.org
> 


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

* Re: [PATCH v2 2/6] iio: frequency: adf41513: driver implementation
  2025-12-19 12:34 ` [PATCH v2 2/6] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
@ 2025-12-21 17:49   ` Jonathan Cameron
  2025-12-22  9:45     ` Rodrigo Alencar
  0 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-21 17:49 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On Fri, 19 Dec 2025 12:34:49 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> The driver is based on existing PLL drivers in the IIO subsystem and
> implements the following key features:
> 
> - Integer-N and fractional-N (fixed/variable modulus) synthesis modes
> - High-resolution frequency calculations using microhertz (µHz) precision
>   to handle sub-Hz resolution across multi-GHz frequency ranges
> - IIO debugfs interface for direct register access
> - FW property parsing from devicetree including charge pump settings,
>   reference path configuration and muxout options
> - Power management support with suspend/resume callbacks
> - Lock detect GPIO monitoring
> 
> The driver uses 64-bit microhertz values throughout PLL calculations to
> maintain precision when working with frequencies that exceed 32-bit Hz
> representation while requiring fractional Hz resolution.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Hi Rodrigo,

Various comments inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
> new file mode 100644
> index 000000000000..a967dc4479e7
> --- /dev/null
> +++ b/drivers/iio/frequency/adf41513.c


> +
> +static int adf41513_sync_config(struct adf41513_state *st, u16 sync_mask)
> +{
> +	int ret;
> +	int i;
> +
> +	/* write registers in reverse order (R13 to R0)*/
> +	for (i = ADF41513_REG13; i >= ADF41513_REG0; i--) {
> +		if (st->regs_hw[i] != st->regs[i] || sync_mask & BIT(i)) {
For code cases like this where you want to only do something if a condition is matched
it can be neater to invert the condition.  e.g.

		if (st->regs_hw[i] == st->regs[i] && !(sync_mask & BIT(i)))
			continue;

		st->buf = cpu...

> +			st->buf = cpu_to_be32(st->regs[i] | i);
> +			ret = spi_write(st->spi, &st->buf, sizeof(st->buf));
> +			if (ret < 0)
> +				return ret;
> +			st->regs_hw[i] = st->regs[i];
> +			dev_dbg(&st->spi->dev, "REG%d <= 0x%08X\n", i, st->regs[i] | i);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static u64 adf41513_pll_get_rate(struct adf41513_state *st)
> +{
> +	struct adf41513_pll_settings *cfg = &st->settings;
> +
> +	if (cfg->mode != ADF41513_MODE_INVALID)
> +		return cfg->actual_frequency_uhz;
> +
> +	/* get pll settings from regs_hw */
> +	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK,
> +				   st->regs_hw[ADF41513_REG0]);
> +	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK,
> +			       st->regs_hw[ADF41513_REG1]);
> +	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK,
> +			       st->regs_hw[ADF41513_REG3]);
> +	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK,
> +			      st->regs_hw[ADF41513_REG4]);
> +	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK,
> +				   st->regs_hw[ADF41513_REG5]);
> +	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK,
> +				     st->regs_hw[ADF41513_REG5]);
> +	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK,
> +				  st->regs_hw[ADF41513_REG5]);
> +	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,
> +				   st->regs_hw[ADF41513_REG5]);
For cases like this I think keeping to 80 chars is hurting readability
and so it is fine to go a little higher. 
	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK, st->regs_hw[ADF41513_REG0]);
	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK, st->regs_hw[ADF41513_REG1]);
	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK, st->regs_hw[ADF41513_REG3]);
	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK, st->regs_hw[ADF41513_REG4]);
	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK, st->regs_hw[ADF41513_REG5]);
	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK, st->regs_hw[ADF41513_REG5]);
	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK, st->regs_hw[ADF41513_REG5]);
	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,st->regs_hw[ADF41513_REG5]);
Is fine here. I'd also be fine with wrapping the ref_doubler line as it's rather
longer than the others.

> +
> +	/* calculate pfd frequency */
> +	cfg->pfd_frequency_uhz = st->ref_freq_hz * MICROHZ_PER_HZ;
> +	if (cfg->ref_doubler)
> +		cfg->pfd_frequency_uhz <<= 1;
> +	if (cfg->ref_div2)
> +		cfg->pfd_frequency_uhz >>= 1;
> +	cfg->pfd_frequency_uhz = div_u64(cfg->pfd_frequency_uhz,
> +					 cfg->r_counter);
> +	cfg->actual_frequency_uhz = (u64)cfg->int_value * cfg->pfd_frequency_uhz;
> +
> +	/* check if int mode is selected */
> +	if (FIELD_GET(ADF41513_REG6_INT_MODE_MSK, st->regs_hw[ADF41513_REG6])) {
> +		cfg->mode = ADF41513_MODE_INTEGER_N;
> +	} else {
> +		cfg->actual_frequency_uhz += mul_u64_u64_div_u64(cfg->frac1,
> +								 cfg->pfd_frequency_uhz,
> +								 ADF41513_FIXED_MODULUS);
> +
> +		/* check if variable modulus is selected */
> +		if (FIELD_GET(ADF41513_REG0_VAR_MOD_MSK, st->regs_hw[ADF41513_REG0])) {
> +			cfg->actual_frequency_uhz +=
> +				mul_u64_u64_div_u64(cfg->frac2,
> +						    cfg->pfd_frequency_uhz,
> +						    ADF41513_FIXED_MODULUS * cfg->mod2);
> +
> +			cfg->mode = ADF41513_MODE_VARIABLE_MODULUS;
> +		} else {
> +			/* LSB_P1 offset */
> +			if (!FIELD_GET(ADF41513_REG5_LSB_P1_MSK, st->regs_hw[ADF41513_REG5]))
> +				cfg->actual_frequency_uhz +=
> +					div_u64(cfg->pfd_frequency_uhz,
> +						ADF41513_FIXED_MODULUS * 2);
> +			cfg->mode = ADF41513_MODE_FIXED_MODULUS;
> +		}
> +	}
> +
> +	cfg->target_frequency_uhz = cfg->actual_frequency_uhz;
> +
> +	return cfg->actual_frequency_uhz;
> +}


> +static int adf41513_calc_pll_settings(struct adf41513_state *st,
> +				      struct adf41513_pll_settings *result,
> +				      u64 rf_out_uhz)
> +{
> +	u64 max_rf_freq_uhz = st->chip_info->max_rf_freq_hz * MICROHZ_PER_HZ;
> +	u64 min_rf_freq_uhz = ADF41513_MIN_RF_FREQ * MICROHZ_PER_HZ;
> +	u64 pfd_freq_limit_uhz;
> +	int ret;
> +
> +	/* input validation */

That's obvious.  No need to have the comment.

> +	if (rf_out_uhz < min_rf_freq_uhz || rf_out_uhz > max_rf_freq_uhz) {
> +		dev_err(&st->spi->dev, "RF frequency %llu uHz out of range [%llu, %llu] uHz\n",
> +			rf_out_uhz, min_rf_freq_uhz, max_rf_freq_uhz);
> +		return -EINVAL;
> +	}
> +
> +	result->target_frequency_uhz = rf_out_uhz;
> +
> +	/* try integer-N first (best phase noise performance) */
> +	pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_4_5),
> +				 ADF41513_MAX_PFD_FREQ_INT_N_UHZ);
> +	ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = adf41513_calc_integer_n(st, result);
> +	if (ret < 0) {
> +		/* try fractional-N: recompute pfd frequency if necessary */
> +		pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_FRAC_4_5),
> +					 ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ);
> +		if (pfd_freq_limit_uhz < result->pfd_frequency_uhz) {
> +			ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
> +			if (ret < 0)
> +				return ret;
> +		}
> +
> +		/* fixed-modulus attempt */
> +		ret = adf41513_calc_fixed_mod(st, result);
> +		if (ret < 0) {
> +			/* variable-modulus attempt */
> +			ret = adf41513_calc_variable_mod(st, result);
> +			if (ret < 0) {
> +				dev_err(&st->spi->dev,
> +					"no valid PLL configuration found for %llu uHz\n",
> +					rf_out_uhz);
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 sync_mask)
> +{
> +	struct adf41513_pll_settings result;
> +	int ret;
> +
> +	/* calculate pll settings candidate */
> +	ret = adf41513_calc_pll_settings(st, &result, freq_uhz);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* apply computed results to pll settings */
> +	memcpy(&st->settings, &result, sizeof(struct adf41513_pll_settings));

sizeof(st->settings)

> +
> +	dev_dbg(&st->spi->dev,
> +		"%s mode: int=%u, frac1=%u, frac2=%u, mod2=%u, fpdf=%llu Hz, prescaler=%s\n",
> +		(result.mode == ADF41513_MODE_INTEGER_N) ? "integer-n" :
> +		(result.mode == ADF41513_MODE_FIXED_MODULUS) ? "fixed-modulus" : "variable-modulus",
> +		result.int_value, result.frac1, result.frac2, result.mod2,
> +		div64_u64(result.pfd_frequency_uhz, MICROHZ_PER_HZ),
> +		result.prescaler ? "8/9" : "4/5");
> +
> +	/* int */
> +	st->regs[ADF41513_REG0] = FIELD_PREP(ADF41513_REG0_INT_MSK,
> +					     st->settings.int_value);
> +	if (st->settings.mode == ADF41513_MODE_VARIABLE_MODULUS)
> +		st->regs[ADF41513_REG0] |= ADF41513_REG0_VAR_MOD_MSK;
> +	/* frac1 */
> +	st->regs[ADF41513_REG1] = FIELD_PREP(ADF41513_REG1_FRAC1_MSK,
> +					     st->settings.frac1);
> +	if (st->settings.mode != ADF41513_MODE_INTEGER_N)
> +		st->regs[ADF41513_REG1] |= ADF41513_REG1_DITHER2_MSK;
> +
> +	/* frac2 */

Where the field name makes it obvious there is little point in
adding a comment to say the same thing. I'd clear out most if not all
of these. Stick to comments that add significant value.

> +	st->regs[ADF41513_REG3] = FIELD_PREP(ADF41513_REG3_FRAC2_MSK,
> +					     st->settings.frac2);
> +	/* mod2 */
> +	st->regs[ADF41513_REG4] &= ADF41513_REG4_MOD2_MSK;
> +	st->regs[ADF41513_REG4] |= FIELD_PREP(ADF41513_REG4_MOD2_MSK,
> +					      st->settings.mod2);
> +
> +	/* r-cnt | doubler | rdiv2 | prescaler */
> +	st->regs[ADF41513_REG5] &= ~(ADF41513_REG5_R_CNT_MSK |
> +				     ADF41513_REG5_REF_DOUBLER_MSK |
> +				     ADF41513_REG5_RDIV2_MSK |
> +				     ADF41513_REG5_PRESCALER_MSK);
> +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_R_CNT_MSK,
> +					      st->settings.r_counter);
> +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_REF_DOUBLER_MSK,
> +					      st->settings.ref_doubler);
> +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_RDIV2_MSK,
> +					      st->settings.ref_div2);
> +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_PRESCALER_MSK,
> +					      st->settings.prescaler);

Probably better to use FIELD_MODIFY for all of these and let the compiler
figure out it can mask them all in one go.

> +
> +	if (st->settings.mode == ADF41513_MODE_INTEGER_N) {
> +		st->regs[ADF41513_REG6] |= ADF41513_REG6_INT_MODE_MSK;
> +		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_BLEED_ENABLE_MSK;
> +	} else {
> +		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_INT_MODE_MSK;
> +		st->regs[ADF41513_REG6] |= ADF41513_REG6_BLEED_ENABLE_MSK;
> +	}
> +
> +	return adf41513_sync_config(st, sync_mask | ADF41513_SYNC_REG0);
> +}

> +static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
> +				 uintptr_t private,
> +				 const struct iio_chan_spec *chan,
> +				 char *buf)
> +{
> +	struct adf41513_state *st = iio_priv(indio_dev);
> +	u64 freq_uhz;
> +
> +	guard(mutex)(&st->lock);
> +
> +	switch ((u32)private) {
> +	case ADF41513_FREQ:
> +		freq_uhz = adf41513_pll_get_rate(st);
> +		if (st->lock_detect)
> +			if (!gpiod_get_value_cansleep(st->lock_detect)) {
> +				dev_dbg(&st->spi->dev, "PLL un-locked\n");
> +				return -EBUSY;
> +			}
> +		break;
> +	case ADF41513_FREQ_RESOLUTION:
> +		freq_uhz = st->data.freq_resolution_uhz;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return adf41513_uhz_to_str(freq_uhz, buf);
This is a more marginal case than the ones below wrt to a common
function making sense as there is more overlap. I''m not sure it
is worth doing even so (rather than separate callbacks).

> +}
> +
> +static ssize_t adf41513_read(struct iio_dev *indio_dev,
> +			     uintptr_t private,
> +			     const struct iio_chan_spec *chan,
> +			     char *buf)
> +{
> +	struct adf41513_state *st = iio_priv(indio_dev);
> +	u32 val;
> +
> +	guard(mutex)(&st->lock);
> +
> +	switch ((u32)private) {
> +	case ADF41513_FREQ_REFIN:
> +		st->ref_freq_hz = clk_get_rate(st->ref_clk);
> +		return sysfs_emit(buf, "%llu\n", st->ref_freq_hz);

Not much sharing here either (see below). I'd be tempted to just spit this
into specific callbacks.

> +	case ADF41513_POWER_DOWN:
> +		val = FIELD_GET(ADF41513_REG6_POWER_DOWN_MSK,
> +				st->regs_hw[ADF41513_REG6]);
> +		return sysfs_emit(buf, "%u\n", val);
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +static ssize_t adf41513_write(struct iio_dev *indio_dev,
> +			      uintptr_t private,
> +			      const struct iio_chan_spec *chan,
> +			      const char *buf, size_t len)
> +{
> +	struct adf41513_state *st = iio_priv(indio_dev);
> +	unsigned long readin, tmp;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 10, &readin);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	switch ((u32)private) {
> +	case ADF41513_FREQ_REFIN:

There isn't a lot of shared code between different calls of this.
Perhaps just have separate callbacks for each one.

> +		if (readin < ADF41513_MIN_REF_FREQ || readin > ADF41513_MAX_REF_FREQ)
> +			return -EINVAL;
> +
> +		tmp = clk_round_rate(st->ref_clk, readin);
> +		if (tmp != readin)
> +			return -EINVAL;
> +
> +		ret = clk_set_rate(st->ref_clk, tmp);
> +		if (ret < 0)
> +			return ret;
> +
> +		st->ref_freq_hz = readin;
> +		ret = adf41513_set_frequency(st, st->settings.target_frequency_uhz,
> +					     ADF41513_SYNC_DIFF);
> +		break;
> +	case ADF41513_POWER_DOWN:
> +		if (readin)
> +			ret = adf41513_suspend(st);
> +		else
> +			ret = adf41513_resume(st);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret ? ret : len;
> +}
> +
> +#define _ADF41513_EXT_INFO(_name, _ident) { \
> +	.name = _name, \
> +	.read = adf41513_read, \
> +	.write = adf41513_write, \
> +	.private = _ident, \
> +	.shared = IIO_SEPARATE, \
> +}
> +
> +#define _ADF41513_EXT_UHZ_INFO(_name, _ident) { \
> +	.name = _name, \
> +	.read = adf41513_read_uhz, \
> +	.write = adf41513_write_uhz, \
> +	.private = _ident, \
> +	.shared = IIO_SEPARATE, \
> +}
> +
> +static const struct iio_chan_spec_ext_info adf41513_ext_info[] = {
> +	/*
> +	 * Ideally we would use IIO_CHAN_INFO_FREQUENCY, but the device supports
> +	 * frequency values greater 2^32 with sub-Hz resolution, i.e. 64-bit
> +	 * fixed point with 6 decimal places values are used to represent
> +	 * frequencies.
> +	 */
> +	_ADF41513_EXT_UHZ_INFO("frequency", ADF41513_FREQ),
> +	_ADF41513_EXT_UHZ_INFO("frequency_resolution", ADF41513_FREQ_RESOLUTION),
> +	_ADF41513_EXT_INFO("refin_frequency", ADF41513_FREQ_REFIN),
Some of these are not things I recall as being standard ABI.
This one is in one other driver but to make it generic you need to promote
the ABI documentation to a shared file.

> +	_ADF41513_EXT_INFO("powerdown", ADF41513_POWER_DOWN),
> +	{ },

No comma on terminating entries like this.

> +};
> +
> +static const struct iio_chan_spec adf41513_chan = {
> +	.type = IIO_ALTVOLTAGE,
> +	.indexed = 1,
> +	.output = 1,
> +	.channel = 0,
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE),
> +	.ext_info = adf41513_ext_info,
> +};
> +
> +static int adf41513_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long info)
> +{
> +	struct adf41513_state *st = iio_priv(indio_dev);
> +	u32 phase_mdeg;
> +	u16 phase_val;
> +
> +	guard(mutex)(&st->lock);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_PHASE:
> +		phase_val = FIELD_GET(ADF41513_REG2_PHASE_VAL_MSK,
> +				      st->regs_hw[ADF41513_REG2]);
> +		phase_mdeg = DIV_ROUND_CLOSEST(360 * MILLI * phase_val, BIT(12));
> +		*val = phase_mdeg / MILLI;
> +		*val2 = (phase_mdeg % MILLI) * 1000;

This sounds like it is in degrees. Note _phase attributes are in the documented
ABI Documentation/ABI/testing/sysfs-bus-iio as in radians.

> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int adf41513_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long info)
> +{
> +	struct adf41513_state *st = iio_priv(indio_dev);
> +	u32 phase_mdeg;
> +	u16 phase_val;
> +
> +	guard(mutex)(&st->lock);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_PHASE:
> +		val %= 360;
> +		if (val < 0)
> +			val += 360;
> +		phase_mdeg = val * MILLI + val2 / 1000;
> +		phase_val = DIV_ROUND_CLOSEST(phase_mdeg << 12, 360 * MILLI);
> +
> +		st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK;
> +		st->regs[ADF41513_REG2] &= ~ADF41513_REG2_PHASE_VAL_MSK;

FIELD_MODIFY() can save doing the clear and fill as separate calls.

> +		st->regs[ADF41513_REG2] |= FIELD_PREP(ADF41513_REG2_PHASE_VAL_MSK, phase_val);
> +		return adf41513_sync_config(st, ADF41513_SYNC_REG0);
> +	default:
> +		return -EINVAL;
> +	}
> +}


> +static int adf41513_parse_fw(struct adf41513_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	int ret;
> +	u32 tmp;
> +	u32 cp_resistance;
> +	u32 cp_current;
Where you have set of variables of same type and grouping doesn't hurt
readability, declare them all on one line.

	u32 tmp, cp_resistance, cp_current;

I'll not repeat comments I made on the dt-binding in here but I'd expect
this code to change somewhat in response to those.

> +

...


> +static int adf41513_setup(struct adf41513_state *st)
> +{
> +	u32 tmp;
> +
> +	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
> +
> +	/* assume DLD pin is used for digital lock detect */
> +	st->regs[ADF41513_REG5] = FIELD_PREP(ADF41513_REG5_DLD_MODES_MSK,
> +					     ADF41513_DLD_DIG_LD);
> +
> +	/* configure charge pump current settings */
> +	tmp = DIV_ROUND_CLOSEST(st->data.charge_pump_voltage_mv, ADF41513_MIN_CP_VOLTAGE_mV);
> +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_CP_CURRENT_MSK, tmp - 1);
> +
> +	/* narrow ABP | loss of lock detect enable | SD reset | LDP from data */

I'm not sure what LPD from data means. Can't correlate that with the datasheet.
Perhaps add more info or reword.

> +	st->regs[ADF41513_REG6] = ADF41513_REG6_ABP_MSK |
> +				  ADF41513_REG6_LOL_ENABLE_MSK |
> +				  ADF41513_REG6_SD_RESET_MSK;
> +	if (st->data.phase_detector_polarity)
> +		st->regs[ADF41513_REG6] |= ADF41513_REG6_PD_POLARITY_MSK;
> +
> +	/* PS bias | lock detect count */
Confusing comment as covering multiple bits of code. I'd just drop
it on basis the field names in the code are less confusing than the comment.
> +	st->regs[ADF41513_REG7] = FIELD_PREP(ADF41513_REG7_PS_BIAS_MSK, 2);

That magic 2 is interesting as it is truely magic with no explanation on
the datasheet beyond 'program this value'. Even better, on the datasheet I'm
looking at the Prescaler (PS) bias section says set it to 3 and figure 30
say set it to 2.  Maybe add a commeon on this.

> +	tmp = ilog2(st->data.lock_detect_count);
> +	if (st->data.lock_detect_count < ADF41513_LD_COUNT_FAST_LIMIT) {
> +		tmp -= const_ilog2(ADF41513_LD_COUNT_FAST_MIN);
> +		st->regs[ADF41513_REG7] |= ADF41513_REG7_LD_CLK_SEL_MSK;
> +	} else {
> +		tmp -= const_ilog2(ADF41513_LD_COUNT_MIN);
> +	}
> +	st->regs[ADF41513_REG7] |= FIELD_PREP(ADF41513_REG7_LD_COUNT_MSK, tmp);
> +
> +	/* power down select */
> +	st->regs[ADF41513_REG11] = ADF41513_REG11_POWER_DOWN_SEL_MSK;
> +
> +	/* muxout */
> +	st->regs[ADF41513_REG12] = FIELD_PREP(ADF41513_REG12_MUXOUT_MSK,
> +					      st->data.muxout_select);
> +	st->regs[ADF41513_REG12] |= FIELD_PREP(ADF41513_REG12_LOGIC_LEVEL_MSK,
> +					       st->data.muxout_1v8_en ? 0 : 1);
> +
> +	/* perform initialization sequence with power-up frequency */
> +	return adf41513_set_frequency(st, (u64)st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
> +				      ADF41513_SYNC_ALL);
> +}

...

> +
> +static int adf41513_pm_suspend(struct device *dev)
> +{
> +	struct adf41513_state *st = dev_get_drvdata(dev);
> +
> +	return adf41513_suspend(st);

Not at lot in point in the local variable

	return adf41513_suspend(dev_get_drvdata(dev));

> +}
> +
> +static int adf41513_pm_resume(struct device *dev)
> +{
> +	struct adf41513_state *st = dev_get_drvdata(dev);
> +
> +	return adf41513_resume(st);
As above.
> +}
> +
> +static const struct adf41513_chip_info adf41513_chip_info = {
> +	.name = "adf41513",
> +	.has_prescaler_8_9 = true,
> +	.max_rf_freq_hz = ADF41513_MAX_RF_FREQ,
> +};
> +
> +static const struct adf41513_chip_info adf41510_chip_info = {

Just for long term organization when many devices are supported:
keep these structures in alphanumeric order.

> +	.name = "adf41510",
> +	.has_prescaler_8_9 = false,
> +	.max_rf_freq_hz = ADF41510_MAX_RF_FREQ,
> +};
> +
> +static int adf41513_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct adf41513_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));

I'd use a 
	struct device *dev = &spi->dev;
so that you can shorten all the lines where spi->dev is used in here.

> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->chip_info = spi_get_device_match_data(spi);
> +	if (!st->chip_info)
> +		return -EINVAL;
> +
> +	spi_set_drvdata(spi, st);
> +
> +	st->ref_clk = devm_clk_get_enabled(&spi->dev, NULL);
> +	if (IS_ERR(st->ref_clk))
> +		return PTR_ERR(st->ref_clk);
> +
> +	st->ref_freq_hz = clk_get_rate(st->ref_clk);
> +	if (st->ref_freq_hz < ADF41513_MIN_REF_FREQ || st->ref_freq_hz > ADF41513_MAX_REF_FREQ)
> +		return dev_err_probe(&spi->dev, -ERANGE,
> +				     "reference frequency %llu Hz out of range\n",
> +				     st->ref_freq_hz);
> +
> +	ret = adf41513_parse_fw(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_regulator_bulk_get_enable(&spi->dev,
> +					     ARRAY_SIZE(adf41513_power_supplies),
> +					     adf41513_power_supplies);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "failed to get and enable regulators\n");
> +
> +	st->chip_enable = devm_gpiod_get_optional(&spi->dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->chip_enable))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->chip_enable),
> +				     "fail to request chip enable GPIO\n");
> +
> +	st->lock_detect = devm_gpiod_get_optional(&spi->dev, "lock-detect", GPIOD_IN);
> +	if (IS_ERR(st->lock_detect))
> +		return dev_err_probe(&spi->dev, PTR_ERR(st->lock_detect),
> +				     "fail to request lock detect GPIO\n");
> +
> +	ret = devm_mutex_init(&spi->dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->name = st->chip_info->name;
> +	indio_dev->info = &adf41513_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &adf41513_chan;
> +	indio_dev->num_channels = 1;
> +
> +	ret = adf41513_setup(st);
> +	if (ret < 0)
> +		return dev_err_probe(&spi->dev, ret, "failed to setup device: %d\n", ret);
Look at what dev_err_probe() prints.  (short answer, it includes a much nicer print
of the error value than the one you have here).  So dev_err_probe() should never
include the error value itself as that is duplicating the info.
> +
> +	ret = devm_add_action_or_reset(&spi->dev, adf41513_power_down, st);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret, "Failed to add power down action: %d\n", ret);

As above.  No printing ret by hand in dev_err_probe() calls.

> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}

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

* Re: [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513
  2025-12-19 12:34 ` [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513 Rodrigo Alencar via B4 Relay
@ 2025-12-21 17:52   ` Jonathan Cameron
  2025-12-22  9:24     ` Rodrigo Alencar
  0 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-21 17:52 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On Fri, 19 Dec 2025 12:34:53 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Add ABI documentation for ADF41513 PLL sysfs interfaces
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Do this in the patches that add the ABI, or in a patch before them.

Also try building docs.  Unless something changed recently that will
moan about duplication.

When ABI is generalized from one driver to many we have to move
the documentation (and make it generic) such that is is shared by
the relevant drivers. In this case
sysfs-bus-iio-frequency is probably the appropriate file.

> ---
>  .../ABI/testing/sysfs-bus-iio-frequency-adf41513   | 27 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
> new file mode 100644
> index 000000000000..11ffd248eedb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
> @@ -0,0 +1,27 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_resolution
> +KernelVersion:	6.20
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Stores channel Y frequency resolution/channel spacing in Hz.
> +		The value given directly influences the choice of operation:
> +
> +		- integer-N: requested frequency is a multiple of the Phase Detector
> +		frequency.
> +		- fixed modulus: fractional-N mode with fixed modulus.
> +		- variable modulus: dual-modulus fractional-N mode with extra variable
> +		modulus added on top of the fixed one.
> +
> +		It is assumed that the algorithm that is used to compute the various
> +		dividers, is able to generate proper values for multiples of channel
> +		spacing.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_refin_frequency
> +KernelVersion:	6.20
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Sets channel Y REFin frequency in Hz. In some clock chained
> +		applications, the reference frequency used by the PLL may change during
> +		runtime. This attribute allows the user to adjust the reference
> +		frequency accordingly.
> +		To avoid glitches in the RF output, consider using out_altvoltageY_powerdown
> +		to power down the PLL and its RFOut buffers during REFin changes.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c536c3afc1ae..48fa1011b797 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1606,6 +1606,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-adf41513
>  F:	Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
>  F:	Documentation/iio/adf41513.rst
>  F:	drivers/iio/frequency/adf41513.c
> 


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

* Re: [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver
  2025-12-19 12:34 ` [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
@ 2025-12-21 18:00   ` Jonathan Cameron
  2025-12-21 20:20     ` Rodrigo Alencar
  0 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-21 18:00 UTC (permalink / raw)
  To: Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On Fri, 19 Dec 2025 12:34:52 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> add documentation for ADF41513 driver which describes the device
Add

> driver files and shows how userspace may consume the ABI for various
> tasks
.

> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  Documentation/iio/adf41513.rst | 255 +++++++++++++++++++++++++++++++++++++++++
>  Documentation/iio/index.rst    |   1 +
>  MAINTAINERS                    |   1 +
>  3 files changed, 257 insertions(+)
> 
> diff --git a/Documentation/iio/adf41513.rst b/Documentation/iio/adf41513.rst
> new file mode 100644
> index 000000000000..568e71bc21e4
> --- /dev/null
> +++ b/Documentation/iio/adf41513.rst
> @@ -0,0 +1,255 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===============
> +ADF41513 driver
> +===============
> +
> +This driver supports Analog Devices' ADF41513 and ADF41510 PLL frequency
> +synthesizers on SPI bus.
Avoid lists of part numbers inline with text. Those become very noisy
if more parts are added in future. Given next block has bullet point list
of parts no need to mention them here.  Instead use
ADF41513 and similar SPI PLL frequency synthesizers.
> +
> +1. Supported devices
> +====================
> +
> +* `ADF41510 <https://www.analog.com/ADF41510>`_
> +* `ADF41513 <https://www.analog.com/ADF41513>`_
> +
> +The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> +implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> +downconversion sections of wireless receivers and transmitters. The ADF41510
> +is a similar device that supports frequencies up to 10 GHz.
> +
> +Both devices support integer-N and fractional-N operation modes, providing
> +excellent phase noise performance and flexible frequency generation
> +capabilities.
> +
> +Key Features:
> +
> +- **ADF41513**: 1 GHz to 26.5 GHz frequency range
> +- **ADF41510**: 1 GHz to 10 GHz frequency range
Keep them in alphanumeric order. Makes it easier to add more parts by
keeping the placement obvious.

> +- Integer-N and fractional-N operation modes
> +- Ultra-low phase noise (-235 dBc/Hz integer-N, -231 dBc/Hz fractional-N)
> +- High maximum PFD frequency (250 MHz integer-N, 125 MHz fractional-N)
> +- 25-bit fixed modulus or 49-bit variable modulus fractional modes
> +- Programmable charge pump currents with 16x range
> +- Digital lock detect functionality
> +- Phase resync capability for consistent output phase
> +
> +2. Device attributes
> +====================
> +
> +The ADF41513 driver provides the following IIO extended attributes for
> +frequency control and monitoring:
> +
> +Each IIO device has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
> +where X is the IIO index of the device. Under these folders reside a set of
> +device files that provide access to the synthesizer's functionality.
> +
> +The following table shows the ADF41513 related device files:
> +
> ++----------------------+-------------------------------------------------------+
> +| Device file          | Description                                           |
> ++======================+=======================================================+
> +| frequency            | RF output frequency control and readback (Hz)         |
> ++----------------------+-------------------------------------------------------+
> +| frequency_resolution | Target frequency resolution control (Hz)              |
> ++----------------------+-------------------------------------------------------+
> +| refin_frequency      | Reference input frequency control and readback (Hz)   |
> ++----------------------+-------------------------------------------------------+
> +| powerdown            | Power management control (0=active, 1=power down)     |
> ++----------------------+-------------------------------------------------------+
> +| phase                | RF output phase adjustment and readback (degrees)     |
> ++----------------------+-------------------------------------------------------+
> +
> +2.1 Frequency Control
> +----------------------
> +
> +The ``frequency`` attribute controls the RF output frequency with sub-Hz
> +precision. The driver automatically selects between integer-N and fractional-N
> +modes to achieve the requested frequency with the best possible phase noise
> +performance.
> +
> +**Supported ranges:**
> +
> +- **ADF41513**: 1,000,000,000 Hz to 26,500,000,000 Hz (1 GHz to 26.5 GHz)
> +- **ADF41510**: 1,000,000,000 Hz to 10,000,000,000 Hz (1 GHz to 10 GHz)
Alpha numeric order here as well.

> +
> +The frequency is specified in Hz, for sub-Hz precision use decimal notation.
> +For example, 12.102 GHz would be written as "12102000000.000000".
> +
> +2.2 Frequency Resolution Control
> +--------------------------------
> +
> +The ``frequency_resolution`` attribute controls the target frequency resolution
> +that the driver attempts to achieve. This affects the choice between integer-N
> +and fractional-N modes, including fixed modulus (25-bit) and variable modulus
> +(49-bit) fractional-N modes:
> +
> +- **Integer-N**: Resolution = f_PFD
> +- **Fixed modulus**: Resolution = f_PFD / 2^25 (~3 Hz with 100 MHz PFD)
> +- **Variable modulus**: Resolution = f_PFD / 2^49 (µHz resolution possible)
> +
> +Default resolution is 1 Hz (1,000,000 µHz).
> +
> +2.3 Reference Input Control
> +---------------------------
> +
> +The ``refin_frequency`` attribute allows control of the reference input
> +frequency when using a programmable reference clock. The supported range is
> +10 MHz to 800 MHz.

I'm not really sure why need this as opposed to having a standard clock
provide it.  What's the use case?

> +
> +2.4 Power Management
> +--------------------
> +
> +The ``powerdown`` attribute provides software power control:
> +
> +- **0**: Device active and operational
> +- **1**: Device in power-down mode (low power consumption)

This one is fairly standard for DACs etc. I'd not necessarily bother
documenting it specifically here.

> +
> +2.5 Phase adjustment
> +--------------------
> +
> +The ``phase`` attribute allows adjustment of the output phase in degrees.

As per driver feedback, I don't think this is compliant with existing ABI.

> +Setting this attribute enables phase adjustment. It can be set from 0 to 360
> +degrees. Reading this attribute returns the current phase offset of the output
> +signal. To create a consistent phase relationship with the reference signal,
> +the phase resync feature needs to be enabled by setting a non-zero value to the
> +``adi,phase-resync-period-ns`` device property, which triggers a phase
> +resynchronization after locking is achieved.
> +
> +3. Operating modes
> +==================
> +
> +3.1 Integer-N Mode
> +------------------
> +
> +When the requested frequency can be achieved as an integer multiple of the PFD
> +frequency (within the specified resolution tolerance), the driver automatically
> +selects integer-N mode for optimal phase noise performance.
> +
> +In integer-N mode:
> +
> +- Phase noise: -235 dBc/Hz normalized floor
> +- Frequency resolution: f_PFD (same as PFD frequency)
> +- Maximum PFD frequency: 250 MHz
> +- Bleed current: Disabled for best performance
> +
> +3.2 Fractional-N Mode
> +---------------------
> +
> +When sub-integer frequency steps are required, the driver automatically selects
> +fractional-N mode using either fixed or variable modulus.
> +
> +**Fixed Modulus (25-bit)**:
> +
> +- Used when variable modulus is not required
> +- Resolution: f_PFD / 2^25
> +- Simpler implementation, faster settling
> +
> +**Variable Modulus (49-bit)**:
> +
> +- Used for maximum resolution requirements
> +- Resolution: f_PFD / 2^49 (theoretical)
> +- Exact frequency synthesis capability
> +
> +In fractional-N mode:
> +
> +- Phase noise: -231 dBc/Hz normalized floor
> +- Maximum PFD frequency: 125 MHz
> +- Bleed current: Automatically enabled and optimized
> +- Dithering: Enabled to reduce fractional spurs
> +
> +3.3 Automatic Mode Selection
> +----------------------------
> +
> +The driver automatically selects the optimal operating mode based on:
> +
> +1. **Frequency accuracy requirements**: Determined by frequency_resolution setting
> +2. **Phase noise optimization**: Integer-N preferred when possible
> +3. **PFD frequency constraints**: Different limits for integer vs fractional modes
> +4. **Prescaler selection**: Automatic 4/5 vs 8/9 prescaler selection based on frequency
> +
> +4. Usage examples
> +=================
> +
> +4.1 Basic Frequency Setting
> +----------------------------
> +
> +Set output frequency to 12.102 GHz:
> +
> +.. code-block:: bash
> +
> +    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
> +
> +Read current frequency:
> +
> +.. code-block:: bash
> +
> +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> +    12101999999.582767
> +
> +4.2 High Resolution Frequency Control
> +-------------------------------------
> +
> +Configure for sub-Hz resolution and set a precise frequency:
> +
> +.. code-block:: bash
> +
> +    # Set resolution to 0.1 Hz (100,000 µHz)
> +    root:/sys/bus/iio/devices/iio:device0> echo 0.1 > out_altvoltage0_frequency_resolution
> +
> +    # Set frequency to 12.102 GHz (1 µHz precision)
> +    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
> +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> +    12101999999.980131
> +
> +4.3 Reference Frequency Control
> +-------------------------------
> +
> +Change reference input frequency (if using programmable reference):
> +
> +.. code-block:: bash
> +
> +    # Set reference to 122.88 MHz
> +    root:/sys/bus/iio/devices/iio:device0> echo 122880000 > out_altvoltage0_refin_frequency
> +
> +    # Verify the change
> +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_refin_frequency
> +    122880000
> +
> +4.4 Power Management
> +--------------------
> +
> +Power down the device:
> +
> +.. code-block:: bash
> +
> +    root:/sys/bus/iio/devices/iio:device0> echo 1 > out_altvoltage0_powerdown
> +
> +    # Power back up
> +    root:/sys/bus/iio/devices/iio:device0> echo 0 > out_altvoltage0_powerdown

I'd skip this section as being very standard.

> +
> +4.5 PFD Frequency Monitoring
> +----------------------------
> +
> +Read the current PFD frequency:
> +
> +.. code-block:: bash
> +
> +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_pfd_frequency

This one isn't standard ABI or in your ABI doc.
Perhaps drop for now?

> +    100000000.000000
> +
> +This shows the PFD is operating at 100 MHz, which means the frequency resolution
> +in integer-N mode would be 100 MHz steps.
> +
> +4.6 Monitor Lock Status
> +-----------------------
> +
> +When lock detect GPIO is configured, check if PLL is locked:
> +
> +.. code-block:: bash
> +
> +    # Read frequency - will return error if not locked
> +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> +
> +If the PLL is not locked, the frequency read will return ``-EBUSY`` (Device or
> +resource busy).




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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-21 16:59   ` Jonathan Cameron
@ 2025-12-21 19:45     ` Rodrigo Alencar
  2025-12-27 16:51       ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-21 19:45 UTC (permalink / raw)
  To: Jonathan Cameron, Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet


Hi Jonathan, thanks for your time.

On 25/12/21 04:59PM, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 12:34:48 +0000
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> 
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> > can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> > Most properties refer to existing PLL driver properties (e.g. ADF4350).
> 
> "Refer" implies a cross reference in this document.   Based upon is probably a better
> way to put this.
> 
> Otherwise I've mostly commented on properties that to me don't sound like
> they belong in the dt-binding as they are policy things that we want
> to make runtime configurable.
> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > ---
> >  .../bindings/iio/frequency/adi,adf41513.yaml       | 246 +++++++++++++++++++++
> >  MAINTAINERS                                        |   7 +
> >  2 files changed, 253 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> > new file mode 100644
> > index 000000000000..01ceb2a7d21b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> > @@ -0,0 +1,246 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/frequency/adi,adf41513.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADF41513 PLL Frequency Synthesizer
> > +
> > +maintainers:
> > +  - Rodrigo Alencar <rodrigo.alencar@analog.com>
> > +
> > +description:
> > +  The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> > +  implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> > +  downconversion sections of wireless receivers and transmitters. The ADF41510
> > +  supports frequencies up to 10 GHz.
> > +
> > +  https://www.analog.com/en/products/adf41513.html
> > +  https://www.analog.com/en/products/adf41510.html
> > +
> > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,adf41510
> > +      - adi,adf41513
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 25000000
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: Clock that provides the reference input frequency.
> > +
> > +  avdd1-supply:
> > +    description: PFD and Up and Down Digital Driver Power Supply (3.3 V)
> > +
> > +  avdd2-supply:
> > +    description: RF Buffer and Prescaler Power Supply (3.3 V)
> > +
> > +  avdd3-supply:
> > +    description: N Divider Power Supply (3.3 V)
> > +
> > +  avdd4-supply:
> > +    description: R Divider and Lock Detector Power Supply (3.3 V)
> > +
> > +  avdd5-supply:
> > +    description: Sigma-Delta Modulator and SPI Power Supply (3.3 V)
> > +
> > +  vp-supply:
> > +    description: Charge Pump Power Supply (3.3 V)
> > +
> > +  enable-gpios:
> > +    description:
> > +      GPIO that controls the chip enable pin. A logic low on this pin
> > +      powers down the device and puts the charge pump output into
> > +      three-state mode.
> > +    maxItems: 1
> > +
> > +  lock-detect-gpios:
> > +    description:
> > +      GPIO for lock detect functionality. When configured for digital lock
> > +      detect, this pin will output a logic high when the PLL is locked.
> 
> This seems to be one potential use of the muxout pin.  So to me feels like
> a policy decision that belongs with the driver or userspace, not in dt.
> mux-out-gpios:
> would make more sense to me.
> Some of the potential settings probably don't make sense but then we just
> don't support those in the driver if this is connected to a gpio.
 
Actually, there is the DLD pin (separate pin from muxout) dedicated for lock detection.

> > +    maxItems: 1
> > +
> > +  adi,power-up-frequency:
> > +    $ref: /schemas/types.yaml#/definitions/uint64
> > +    minimum: 1000000000
> > +    maximum: 26500000000
> > +    default: 10000000000
> > +    description:
> > +      The PLL tunes to this frequency (in Hz) during the initialization
> > +      sequence. This property should be set to a frequency supported by the
> > +      loop filter and VCO used in the design. Range is 1 GHz to 26.5 GHz for
> > +      ADF41513, and 1 GHz to 10 GHz for ADF41510.
> 
> Why is this in DT?  Feels like this should be done by userspace control
> prior to setting an enable rather than being in DT.

The initialization sequence of the PLL requires the write of all the registers.
This property exist so that meaningful values are written at start up.
Here it is desired to have a frequency value that is within the range supported
by the loop-filter and the external VCO.  
 
> > +
> > +  adi,reference-div-factor:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 32
> > +    default: 1
> > +    description:
> > +      Value for the reference division factor (R Counter). The driver will
> > +      increment R Counter as needed to achieve a PFD frequency within the
> > +      allowed range. High R counter values will reduce the PFD frequency, which
> > +      lowers the frequency resolution, and affects phase noise performance.
> 
> Why is this in DT?  Is there not a 'best' choice to be made given a particular
> input frequency and desired output frequency?

When designing the external circuitry to the PLL, customers often refers to
https://www.analog.com/en/lp/resources/adisimpll.html
which is a piece of software that assists the choice of some parts and
simulates performance metrics. After having the suggested design simulated,
it turns out that the reference signal path defines the output frequency
of the Phase-Frequency Detector (PFD), which is set as a constraint for
the loop-filter design. Therefore, those properties (reference-div-factor,
reference-doubler-enable and reference-div2-enable) are meant to reflect
a hardware constraint for a PFD frequency value expected by the loop-filter/VCO

> > +
> > +  adi,reference-doubler-enable:
> > +    description:
> > +      Enables the reference doubler. The maximum reference frequency when
> > +      the doubler is enabled is 225 MHz.
> As above.

explained above as part of a hardware constraint for external circuitry design

> > +    type: boolean
> > +
> > +  adi,reference-div2-enable:
> > +    description:
> > +      Enables the reference divide-by-2 function. This provides a 50%
> > +      duty cycle signal to the PFD.
> As above.

same

> > +    type: boolean
> > +
> > +  adi,charge-pump-resistor-ohms:
> > +    minimum: 1800
> > +    maximum: 10000
> > +    default: 2700
> > +    description:
> > +      External charge pump resistor (R_SET) value in ohms. This sets the maximum
> > +      charge pump current along with the charge pump current setting.
> > +
> > +  adi,charge-pump-current-microamp:
> > +    description:
> > +      Charge pump current (I_CP) in microamps. The value will be rounded to the
> > +      nearest supported value. Range of acceptable values depends on the
> > +      charge pump resistor value, such that 810 mV <= I_CP * R_SET <= 12960 mV.
> > +      This value depends on the loop filter design.
> > +
> > +  adi,muxout-select:
> > +    description:
> > +      On chip multiplexer output selection.
> > +      high_z - MUXOUT Pin set to high-Z. (default)
> > +      muxout_high - MUXOUT Pin set to high.
> > +      muxout_low - MUXOUT Pin set to low.
> > +      f_div_rclk - MUXOUT Pin set to R divider output
> > +      f_div_nclk - MUXOUT Pin set to N divider output
> > +      lock_detect - MUXOUT Pin set to Digital lock detect
> > +      serial_data - MUXOUT Pin set to Serial data output
> > +      readback - MUXOUT Pin set to Readback mode
> > +      f_div_clk1 - MUXOUT Pin set to CLK1 divider output
> > +      f_div_rclk_2 - MUXOUT Pin set to R divider/2 output
> > +      f_div_nclk_2 - MUXOUT Pin set to N divider/2 output
> > +    enum: [high_z, muxout_high, muxout_low, f_div_rclk, f_div_nclk, lock_detect,
> > +           serial_data, readback, f_div_clk1, f_div_rclk_2, f_div_nclk_2]
> 
> This needs explanation of 'why' it should be in DT?  To me it seems mostly
> to be a debug feature that should be controlled perhaps via a debugfs interface.

There has been a discussion on that, and I understand the concerns.
Altough it looks like a debug pin, I suppose some board designs might
expect the muxout pin to be working as a specific function.
The device driver provides register access through the debugfs interface,
so worst case, the muxout select value could be changed there.

> > +
> > +  adi,muxout-level-1v8-enable:
> > +    description:
> > +      Set MUXOUT and DLD logic levels to 1.8V. Default is 3.3V.
> > +    type: boolean
> > +
> > +  adi,phase-detector-polarity-positive-enable:
> > +    description:
> > +      Set phase detector polarity to positive. Default is negative.
> > +      Use positive polarity with non-inverting loop filter and VCO with
> > +      positive tuning slope, or with inverting loop filter and VCO with
> > +      negative tuning slope.
> > +    type: boolean
> > +
> > +  adi,lock-detector-count:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 64
> > +    description:
> > +      Sets the value for Lock Detector count of the PLL, which determines the
> > +      number of consecutive phase detector cycles that must be within the lock
> > +      detector window before lock is declared. Lower values increase the lock
> > +      detection sensitivity.
> > +    enum: [2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
> 
> Sounds like policy.  Maybe it is related to the circuit design and there
> is a right choice for  particular board? If not we should figure out how to leave
> this to userspace control.  Probably as some form of event property.

There was also a internal discussion around that. I'd say that this might
be related to the application where the design is being used. There should
be value on making it configurable at runtime, mostly for tunning for
specific needs, but this is not a value to be changed at runtime in
field-deployed products.

kind regards,

Rodrigo Alencar

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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-21 15:56       ` Jonathan Cameron
@ 2025-12-21 19:56         ` Rodrigo Alencar
  0 siblings, 0 replies; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-21 19:56 UTC (permalink / raw)
  To: Jonathan Cameron, 455.rodrigo.alencar
  Cc: Krzysztof Kozlowski, Rodrigo Alencar, linux-kernel, linux-iio,
	devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet

On 25/12/21 03:56PM, Jonathan Cameron wrote:
> On Sat, 20 Dec 2025 18:05:34 +0000
> 455.rodrigo.alencar@gmail.com wrote:
> 
> > Hi Krzystof,
> > 
> > thanks for taking a look into this again. It was my first patch it didn't want
> > to draw more attention or discussion to the V1 patch as it was declared not ready
> > at its very first review.
> > 
> > On 25/12/20 10:21AM, Krzysztof Kozlowski wrote:
> > > On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:  
> > > > dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> > > > can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> > > > Most properties refer to existing PLL driver properties (e.g. ADF4350).  
> > >
> > > What is "existing PLL driver"? I know about motor drivers, but can you
> > > drive PLL?
> > >
> > > And how is ADF4350 related to this binding. I do not see ADF4350
> > > compatible here at all. Describe hardware, a real one.  
> > 
> > ADF4350 is an older one, and its bindings can be found at:
> > Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml
> > It is a similar part, but yet very different.
> > 
> > >
> > > Nothing improved.
> > >
> > > You ignored comments, did not bother to respond to them and then sent
> > > the same.  
> > 
> > Sorry for not responding on the V1 thread, but the previous patch had to be reviewed internally
> > first. It is not true that nothing is improved, in fact, it has changed a lot, here are some notes:
> > * adi,power-up-frequency is not carrying the -hz postfix because it forces to be a uint32 by
> > the dt-bindings check. For that variable it needs to be uint64 as the part supports up to 26.5 GHz > 2^32
> 
> What granularity is necessary?  E.g. Could -mhz work here? It's already defined in dts schema for MHz.

That is a good point, granularity is needed when we want to compensate for a
offset flagged during a calibration. That part supports micro Hz resolution
with the dual-modulus support, but that is something I cant have it here.
Since the important thing for this property is to have a frequency in a range
supported by the loop-filter/VCO design, MHz should be fine.

> 
> > * The properties related to the reference input signal path: reference-div-factor, reference-doubler-enable
> > reference-div2-enable are declared here because they are constraints for the PFD frequency definition,
> > which is the frequency that the output signal is updated, important for the loop-filter and VCO design.
> > * added support for all different power supply regulators.
> > * adi,lock-detect-precision and adi,lock-detect-bias-microamp: removed, now set
> > with adf41513_set_ld_window() following datasheet recommendation
> > * adi,fast-lock-enable: removed, faster lock detect clock is set depending on the lock-detect-count value
> > * adi,phase-resync-enable, adi,12bit-clk-divider and adi,12bit-clk2-divider: removed, now set with
> > adf41513_set_phase_resync(), based on the t_sync (from the datasheet: Phase Resync section)
> > value determined by adi,phase-resync-period-ns, which is also bound to the loop filter design.
> > 
kind regards,
 
Rodrigo Alencar

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

* Re: [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver
  2025-12-21 18:00   ` Jonathan Cameron
@ 2025-12-21 20:20     ` Rodrigo Alencar
  2025-12-27 17:06       ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-21 20:20 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On 25/12/21 06:00PM, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 12:34:52 +0000
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
>
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > add documentation for ADF41513 driver which describes the device
> Add

ack

> > driver files and shows how userspace may consume the ABI for various
> > tasks
> .
>
> >
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > ---
> >  Documentation/iio/adf41513.rst | 255 +++++++++++++++++++++++++++++++++++++++++
> >  Documentation/iio/index.rst    |   1 +
> >  MAINTAINERS                    |   1 +
> >  3 files changed, 257 insertions(+)
> >
> > diff --git a/Documentation/iio/adf41513.rst b/Documentation/iio/adf41513.rst
> > new file mode 100644
> > index 000000000000..568e71bc21e4
> > --- /dev/null
> > +++ b/Documentation/iio/adf41513.rst
> > @@ -0,0 +1,255 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +===============
> > +ADF41513 driver
> > +===============
> > +
> > +This driver supports Analog Devices' ADF41513 and ADF41510 PLL frequency
> > +synthesizers on SPI bus.
> Avoid lists of part numbers inline with text. Those become very noisy
> if more parts are added in future. Given next block has bullet point list
> of parts no need to mention them here.  Instead use
> ADF41513 and similar SPI PLL frequency synthesizers.

ack

> > +
> > +1. Supported devices
> > +====================
> > +
> > +* `ADF41510 <https://www.analog.com/ADF41510>`_
> > +* `ADF41513 <https://www.analog.com/ADF41513>`_
> > +
> > +The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> > +implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> > +downconversion sections of wireless receivers and transmitters. The ADF41510
> > +is a similar device that supports frequencies up to 10 GHz.
> > +
> > +Both devices support integer-N and fractional-N operation modes, providing
> > +excellent phase noise performance and flexible frequency generation
> > +capabilities.
> > +
> > +Key Features:
> > +
> > +- **ADF41513**: 1 GHz to 26.5 GHz frequency range
> > +- **ADF41510**: 1 GHz to 10 GHz frequency range
> Keep them in alphanumeric order. Makes it easier to add more parts by
> keeping the placement obvious.

ack

> > +- Integer-N and fractional-N operation modes
> > +- Ultra-low phase noise (-235 dBc/Hz integer-N, -231 dBc/Hz fractional-N)
> > +- High maximum PFD frequency (250 MHz integer-N, 125 MHz fractional-N)
> > +- 25-bit fixed modulus or 49-bit variable modulus fractional modes
> > +- Programmable charge pump currents with 16x range
> > +- Digital lock detect functionality
> > +- Phase resync capability for consistent output phase
> > +
> > +2. Device attributes
> > +====================
> > +
> > +The ADF41513 driver provides the following IIO extended attributes for
> > +frequency control and monitoring:
> > +
> > +Each IIO device has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
> > +where X is the IIO index of the device. Under these folders reside a set of
> > +device files that provide access to the synthesizer's functionality.
> > +
> > +The following table shows the ADF41513 related device files:
> > +
> > ++----------------------+-------------------------------------------------------+
> > +| Device file          | Description                                           |
> > ++======================+=======================================================+
> > +| frequency            | RF output frequency control and readback (Hz)         |
> > ++----------------------+-------------------------------------------------------+
> > +| frequency_resolution | Target frequency resolution control (Hz)              |
> > ++----------------------+-------------------------------------------------------+
> > +| refin_frequency      | Reference input frequency control and readback (Hz)   |
> > ++----------------------+-------------------------------------------------------+
> > +| powerdown            | Power management control (0=active, 1=power down)     |
> > ++----------------------+-------------------------------------------------------+
> > +| phase                | RF output phase adjustment and readback (degrees)     |
> > ++----------------------+-------------------------------------------------------+
> > +
> > +2.1 Frequency Control
> > +----------------------
> > +
> > +The ``frequency`` attribute controls the RF output frequency with sub-Hz
> > +precision. The driver automatically selects between integer-N and fractional-N
> > +modes to achieve the requested frequency with the best possible phase noise
> > +performance.
> > +
> > +**Supported ranges:**
> > +
> > +- **ADF41513**: 1,000,000,000 Hz to 26,500,000,000 Hz (1 GHz to 26.5 GHz)
> > +- **ADF41510**: 1,000,000,000 Hz to 10,000,000,000 Hz (1 GHz to 10 GHz)
> Alpha numeric order here as well.

ack

> > +
> > +The frequency is specified in Hz, for sub-Hz precision use decimal notation.
> > +For example, 12.102 GHz would be written as "12102000000.000000".
> > +
> > +2.2 Frequency Resolution Control
> > +--------------------------------
> > +
> > +The ``frequency_resolution`` attribute controls the target frequency resolution
> > +that the driver attempts to achieve. This affects the choice between integer-N
> > +and fractional-N modes, including fixed modulus (25-bit) and variable modulus
> > +(49-bit) fractional-N modes:
> > +
> > +- **Integer-N**: Resolution = f_PFD
> > +- **Fixed modulus**: Resolution = f_PFD / 2^25 (~3 Hz with 100 MHz PFD)
> > +- **Variable modulus**: Resolution = f_PFD / 2^49 (µHz resolution possible)
> > +
> > +Default resolution is 1 Hz (1,000,000 µHz).
> > +
> > +2.3 Reference Input Control
> > +---------------------------
> > +
> > +The ``refin_frequency`` attribute allows control of the reference input
> > +frequency when using a programmable reference clock. The supported range is
> > +10 MHz to 800 MHz.
>
> I'm not really sure why need this as opposed to having a standard clock
> provide it.  What's the use case?

I was thinking about, and for the application I am currently evaluating the part
the reference signal comes from a DDS, and that signal is not a clock it is a
series of chirp patterns. The important thing about this property is to set
a center frequency for the DDS to work on. In that scenario the PLL will not
really work as a frequency sythentizer, but as a frequency tracker of the
varying reference. Problem is that I've realized that recently after putting
together a device driver for the DDS. Therefore this property is still important,
and I need to make the reference clock input as an optional property.
I thought of making the DDS as a clock provider, but that center frequency
would have a "virtual" meaning, not attached to the hardware configs.

> > +
> > +2.4 Power Management
> > +--------------------
> > +
> > +The ``powerdown`` attribute provides software power control:
> > +
> > +- **0**: Device active and operational
> > +- **1**: Device in power-down mode (low power consumption)
>
> This one is fairly standard for DACs etc. I'd not necessarily bother
> documenting it specifically here.

ack

> > +
> > +2.5 Phase adjustment
> > +--------------------
> > +
> > +The ``phase`` attribute allows adjustment of the output phase in degrees.
>
> As per driver feedback, I don't think this is compliant with existing ABI.

ABI is not in degrees? the attribute is named out_altvoltage0_phase

> > +Setting this attribute enables phase adjustment. It can be set from 0 to 360
> > +degrees. Reading this attribute returns the current phase offset of the output
> > +signal. To create a consistent phase relationship with the reference signal,
> > +the phase resync feature needs to be enabled by setting a non-zero value to the
> > +``adi,phase-resync-period-ns`` device property, which triggers a phase
> > +resynchronization after locking is achieved.
> > +
> > +3. Operating modes
> > +==================
> > +
> > +3.1 Integer-N Mode
> > +------------------
> > +
> > +When the requested frequency can be achieved as an integer multiple of the PFD
> > +frequency (within the specified resolution tolerance), the driver automatically
> > +selects integer-N mode for optimal phase noise performance.
> > +
> > +In integer-N mode:
> > +
> > +- Phase noise: -235 dBc/Hz normalized floor
> > +- Frequency resolution: f_PFD (same as PFD frequency)
> > +- Maximum PFD frequency: 250 MHz
> > +- Bleed current: Disabled for best performance
> > +
> > +3.2 Fractional-N Mode
> > +---------------------
> > +
> > +When sub-integer frequency steps are required, the driver automatically selects
> > +fractional-N mode using either fixed or variable modulus.
> > +
> > +**Fixed Modulus (25-bit)**:
> > +
> > +- Used when variable modulus is not required
> > +- Resolution: f_PFD / 2^25
> > +- Simpler implementation, faster settling
> > +
> > +**Variable Modulus (49-bit)**:
> > +
> > +- Used for maximum resolution requirements
> > +- Resolution: f_PFD / 2^49 (theoretical)
> > +- Exact frequency synthesis capability
> > +
> > +In fractional-N mode:
> > +
> > +- Phase noise: -231 dBc/Hz normalized floor
> > +- Maximum PFD frequency: 125 MHz
> > +- Bleed current: Automatically enabled and optimized
> > +- Dithering: Enabled to reduce fractional spurs
> > +
> > +3.3 Automatic Mode Selection
> > +----------------------------
> > +
> > +The driver automatically selects the optimal operating mode based on:
> > +
> > +1. **Frequency accuracy requirements**: Determined by frequency_resolution setting
> > +2. **Phase noise optimization**: Integer-N preferred when possible
> > +3. **PFD frequency constraints**: Different limits for integer vs fractional modes
> > +4. **Prescaler selection**: Automatic 4/5 vs 8/9 prescaler selection based on frequency
> > +
> > +4. Usage examples
> > +=================
> > +
> > +4.1 Basic Frequency Setting
> > +----------------------------
> > +
> > +Set output frequency to 12.102 GHz:
> > +
> > +.. code-block:: bash
> > +
> > +    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
> > +
> > +Read current frequency:
> > +
> > +.. code-block:: bash
> > +
> > +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> > +    12101999999.582767
> > +
> > +4.2 High Resolution Frequency Control
> > +-------------------------------------
> > +
> > +Configure for sub-Hz resolution and set a precise frequency:
> > +
> > +.. code-block:: bash
> > +
> > +    # Set resolution to 0.1 Hz (100,000 µHz)
> > +    root:/sys/bus/iio/devices/iio:device0> echo 0.1 > out_altvoltage0_frequency_resolution
> > +
> > +    # Set frequency to 12.102 GHz (1 µHz precision)
> > +    root:/sys/bus/iio/devices/iio:device0> echo 12102000000 > out_altvoltage0_frequency
> > +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> > +    12101999999.980131
> > +
> > +4.3 Reference Frequency Control
> > +-------------------------------
> > +
> > +Change reference input frequency (if using programmable reference):
> > +
> > +.. code-block:: bash
> > +
> > +    # Set reference to 122.88 MHz
> > +    root:/sys/bus/iio/devices/iio:device0> echo 122880000 > out_altvoltage0_refin_frequency
> > +
> > +    # Verify the change
> > +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_refin_frequency
> > +    122880000
> > +
> > +4.4 Power Management
> > +--------------------
> > +
> > +Power down the device:
> > +
> > +.. code-block:: bash
> > +
> > +    root:/sys/bus/iio/devices/iio:device0> echo 1 > out_altvoltage0_powerdown
> > +
> > +    # Power back up
> > +    root:/sys/bus/iio/devices/iio:device0> echo 0 > out_altvoltage0_powerdown
>
> I'd skip this section as being very standard.

ack

> > +
> > +4.5 PFD Frequency Monitoring
> > +----------------------------
> > +
> > +Read the current PFD frequency:
> > +
> > +.. code-block:: bash
> > +
> > +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_pfd_frequency
>
> This one isn't standard ABI or in your ABI doc.
> Perhaps drop for now?

yes, should have dropped this earlier, thanks.

> > +    100000000.000000
> > +
> > +This shows the PFD is operating at 100 MHz, which means the frequency resolution
> > +in integer-N mode would be 100 MHz steps.
> > +
> > +4.6 Monitor Lock Status
> > +-----------------------
> > +
> > +When lock detect GPIO is configured, check if PLL is locked:
> > +
> > +.. code-block:: bash
> > +
> > +    # Read frequency - will return error if not locked
> > +    root:/sys/bus/iio/devices/iio:device0> cat out_altvoltage0_frequency
> > +
> > +If the PLL is not locked, the frequency read will return ``-EBUSY`` (Device or
> > +resource busy).

kind regards,

Rodrigo

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

* Re: [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513
  2025-12-21 17:52   ` Jonathan Cameron
@ 2025-12-22  9:24     ` Rodrigo Alencar
  0 siblings, 0 replies; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-22  9:24 UTC (permalink / raw)
  To: Jonathan Cameron, Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On 25/12/21 05:52PM, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 12:34:53 +0000
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> 
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > Add ABI documentation for ADF41513 PLL sysfs interfaces
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> Do this in the patches that add the ABI, or in a patch before them.
> 
> Also try building docs.  Unless something changed recently that will
> moan about duplication.
> 
> When ABI is generalized from one driver to many we have to move
> the documentation (and make it generic) such that is is shared by
> the relevant drivers. In this case
> sysfs-bus-iio-frequency is probably the appropriate file.

that makes sense, will do. thanks.
 
> > ---
> >  .../ABI/testing/sysfs-bus-iio-frequency-adf41513   | 27 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
> > new file mode 100644
> > index 000000000000..11ffd248eedb
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-adf41513
> > @@ -0,0 +1,27 @@
> > +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency_resolution
> > +KernelVersion:	6.20
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Stores channel Y frequency resolution/channel spacing in Hz.
> > +		The value given directly influences the choice of operation:
> > +
> > +		- integer-N: requested frequency is a multiple of the Phase Detector
> > +		frequency.
> > +		- fixed modulus: fractional-N mode with fixed modulus.
> > +		- variable modulus: dual-modulus fractional-N mode with extra variable
> > +		modulus added on top of the fixed one.
> > +
> > +		It is assumed that the algorithm that is used to compute the various
> > +		dividers, is able to generate proper values for multiples of channel
> > +		spacing.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_refin_frequency
> > +KernelVersion:	6.20
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Sets channel Y REFin frequency in Hz. In some clock chained
> > +		applications, the reference frequency used by the PLL may change during
> > +		runtime. This attribute allows the user to adjust the reference
> > +		frequency accordingly.
> > +		To avoid glitches in the RF output, consider using out_altvoltageY_powerdown
> > +		to power down the PLL and its RFOut buffers during REFin changes.
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c536c3afc1ae..48fa1011b797 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1606,6 +1606,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-adf41513
> >  F:	Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> >  F:	Documentation/iio/adf41513.rst
> >  F:	drivers/iio/frequency/adf41513.c
> > 
> 

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

* Re: [PATCH v2 2/6] iio: frequency: adf41513: driver implementation
  2025-12-21 17:49   ` Jonathan Cameron
@ 2025-12-22  9:45     ` Rodrigo Alencar
  2025-12-27 16:56       ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-22  9:45 UTC (permalink / raw)
  To: Jonathan Cameron, Rodrigo Alencar via B4 Relay
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On 25/12/21 05:49PM, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 12:34:49 +0000
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> 
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > The driver is based on existing PLL drivers in the IIO subsystem and
> > implements the following key features:
> > 
> > - Integer-N and fractional-N (fixed/variable modulus) synthesis modes
> > - High-resolution frequency calculations using microhertz (µHz) precision
> >   to handle sub-Hz resolution across multi-GHz frequency ranges
> > - IIO debugfs interface for direct register access
> > - FW property parsing from devicetree including charge pump settings,
> >   reference path configuration and muxout options
> > - Power management support with suspend/resume callbacks
> > - Lock detect GPIO monitoring
> > 
> > The driver uses 64-bit microhertz values throughout PLL calculations to
> > maintain precision when working with frequencies that exceed 32-bit Hz
> > representation while requiring fractional Hz resolution.
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> Hi Rodrigo,
> 
> Various comments inline.
> 
> Thanks,
> 
> Jonathan

Hi Jonathan,

valuable review, thanks!

kind regards,

Rodrigo Alencar
 
> > diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
> > new file mode 100644
> > index 000000000000..a967dc4479e7
> > --- /dev/null
> > +++ b/drivers/iio/frequency/adf41513.c
> 
> 
> > +
> > +static int adf41513_sync_config(struct adf41513_state *st, u16 sync_mask)
> > +{
> > +	int ret;
> > +	int i;
> > +
> > +	/* write registers in reverse order (R13 to R0)*/
> > +	for (i = ADF41513_REG13; i >= ADF41513_REG0; i--) {
> > +		if (st->regs_hw[i] != st->regs[i] || sync_mask & BIT(i)) {
> For code cases like this where you want to only do something if a condition is matched
> it can be neater to invert the condition.  e.g.
> 
> 		if (st->regs_hw[i] == st->regs[i] && !(sync_mask & BIT(i)))
> 			continue;
> 
> 		st->buf = cpu...
> 
> > +			st->buf = cpu_to_be32(st->regs[i] | i);
> > +			ret = spi_write(st->spi, &st->buf, sizeof(st->buf));
> > +			if (ret < 0)
> > +				return ret;
> > +			st->regs_hw[i] = st->regs[i];
> > +			dev_dbg(&st->spi->dev, "REG%d <= 0x%08X\n", i, st->regs[i] | i);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static u64 adf41513_pll_get_rate(struct adf41513_state *st)
> > +{
> > +	struct adf41513_pll_settings *cfg = &st->settings;
> > +
> > +	if (cfg->mode != ADF41513_MODE_INVALID)
> > +		return cfg->actual_frequency_uhz;
> > +
> > +	/* get pll settings from regs_hw */
> > +	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK,
> > +				   st->regs_hw[ADF41513_REG0]);
> > +	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK,
> > +			       st->regs_hw[ADF41513_REG1]);
> > +	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK,
> > +			       st->regs_hw[ADF41513_REG3]);
> > +	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK,
> > +			      st->regs_hw[ADF41513_REG4]);
> > +	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK,
> > +				   st->regs_hw[ADF41513_REG5]);
> > +	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK,
> > +				     st->regs_hw[ADF41513_REG5]);
> > +	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK,
> > +				  st->regs_hw[ADF41513_REG5]);
> > +	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,
> > +				   st->regs_hw[ADF41513_REG5]);
> For cases like this I think keeping to 80 chars is hurting readability
> and so it is fine to go a little higher. 
> 	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK, st->regs_hw[ADF41513_REG0]);
> 	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK, st->regs_hw[ADF41513_REG1]);
> 	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK, st->regs_hw[ADF41513_REG3]);
> 	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK, st->regs_hw[ADF41513_REG4]);
> 	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK, st->regs_hw[ADF41513_REG5]);
> 	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK, st->regs_hw[ADF41513_REG5]);
> 	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK, st->regs_hw[ADF41513_REG5]);
> 	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,st->regs_hw[ADF41513_REG5]);
> Is fine here. I'd also be fine with wrapping the ref_doubler line as it's rather
> longer than the others.

ack
 
> > +
> > +	/* calculate pfd frequency */
> > +	cfg->pfd_frequency_uhz = st->ref_freq_hz * MICROHZ_PER_HZ;
> > +	if (cfg->ref_doubler)
> > +		cfg->pfd_frequency_uhz <<= 1;
> > +	if (cfg->ref_div2)
> > +		cfg->pfd_frequency_uhz >>= 1;
> > +	cfg->pfd_frequency_uhz = div_u64(cfg->pfd_frequency_uhz,
> > +					 cfg->r_counter);
> > +	cfg->actual_frequency_uhz = (u64)cfg->int_value * cfg->pfd_frequency_uhz;
> > +
> > +	/* check if int mode is selected */
> > +	if (FIELD_GET(ADF41513_REG6_INT_MODE_MSK, st->regs_hw[ADF41513_REG6])) {
> > +		cfg->mode = ADF41513_MODE_INTEGER_N;
> > +	} else {
> > +		cfg->actual_frequency_uhz += mul_u64_u64_div_u64(cfg->frac1,
> > +								 cfg->pfd_frequency_uhz,
> > +								 ADF41513_FIXED_MODULUS);
> > +
> > +		/* check if variable modulus is selected */
> > +		if (FIELD_GET(ADF41513_REG0_VAR_MOD_MSK, st->regs_hw[ADF41513_REG0])) {
> > +			cfg->actual_frequency_uhz +=
> > +				mul_u64_u64_div_u64(cfg->frac2,
> > +						    cfg->pfd_frequency_uhz,
> > +						    ADF41513_FIXED_MODULUS * cfg->mod2);
> > +
> > +			cfg->mode = ADF41513_MODE_VARIABLE_MODULUS;
> > +		} else {
> > +			/* LSB_P1 offset */
> > +			if (!FIELD_GET(ADF41513_REG5_LSB_P1_MSK, st->regs_hw[ADF41513_REG5]))
> > +				cfg->actual_frequency_uhz +=
> > +					div_u64(cfg->pfd_frequency_uhz,
> > +						ADF41513_FIXED_MODULUS * 2);
> > +			cfg->mode = ADF41513_MODE_FIXED_MODULUS;
> > +		}
> > +	}
> > +
> > +	cfg->target_frequency_uhz = cfg->actual_frequency_uhz;
> > +
> > +	return cfg->actual_frequency_uhz;
> > +}
> 
> 
> > +static int adf41513_calc_pll_settings(struct adf41513_state *st,
> > +				      struct adf41513_pll_settings *result,
> > +				      u64 rf_out_uhz)
> > +{
> > +	u64 max_rf_freq_uhz = st->chip_info->max_rf_freq_hz * MICROHZ_PER_HZ;
> > +	u64 min_rf_freq_uhz = ADF41513_MIN_RF_FREQ * MICROHZ_PER_HZ;
> > +	u64 pfd_freq_limit_uhz;
> > +	int ret;
> > +
> > +	/* input validation */
> 
> That's obvious.  No need to have the comment.

ack

> > +	if (rf_out_uhz < min_rf_freq_uhz || rf_out_uhz > max_rf_freq_uhz) {
> > +		dev_err(&st->spi->dev, "RF frequency %llu uHz out of range [%llu, %llu] uHz\n",
> > +			rf_out_uhz, min_rf_freq_uhz, max_rf_freq_uhz);
> > +		return -EINVAL;
> > +	}
> > +
> > +	result->target_frequency_uhz = rf_out_uhz;
> > +
> > +	/* try integer-N first (best phase noise performance) */
> > +	pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_4_5),
> > +				 ADF41513_MAX_PFD_FREQ_INT_N_UHZ);
> > +	ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = adf41513_calc_integer_n(st, result);
> > +	if (ret < 0) {
> > +		/* try fractional-N: recompute pfd frequency if necessary */
> > +		pfd_freq_limit_uhz = min(div_u64(rf_out_uhz, ADF41513_MIN_INT_FRAC_4_5),
> > +					 ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ);
> > +		if (pfd_freq_limit_uhz < result->pfd_frequency_uhz) {
> > +			ret = adf41513_calc_pfd_frequency(st, result, pfd_freq_limit_uhz);
> > +			if (ret < 0)
> > +				return ret;
> > +		}
> > +
> > +		/* fixed-modulus attempt */
> > +		ret = adf41513_calc_fixed_mod(st, result);
> > +		if (ret < 0) {
> > +			/* variable-modulus attempt */
> > +			ret = adf41513_calc_variable_mod(st, result);
> > +			if (ret < 0) {
> > +				dev_err(&st->spi->dev,
> > +					"no valid PLL configuration found for %llu uHz\n",
> > +					rf_out_uhz);
> > +				return -EINVAL;
> > +			}
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 sync_mask)
> > +{
> > +	struct adf41513_pll_settings result;
> > +	int ret;
> > +
> > +	/* calculate pll settings candidate */
> > +	ret = adf41513_calc_pll_settings(st, &result, freq_uhz);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* apply computed results to pll settings */
> > +	memcpy(&st->settings, &result, sizeof(struct adf41513_pll_settings));
> 
> sizeof(st->settings)
> 
> > +
> > +	dev_dbg(&st->spi->dev,
> > +		"%s mode: int=%u, frac1=%u, frac2=%u, mod2=%u, fpdf=%llu Hz, prescaler=%s\n",
> > +		(result.mode == ADF41513_MODE_INTEGER_N) ? "integer-n" :
> > +		(result.mode == ADF41513_MODE_FIXED_MODULUS) ? "fixed-modulus" : "variable-modulus",
> > +		result.int_value, result.frac1, result.frac2, result.mod2,
> > +		div64_u64(result.pfd_frequency_uhz, MICROHZ_PER_HZ),
> > +		result.prescaler ? "8/9" : "4/5");
> > +
> > +	/* int */
> > +	st->regs[ADF41513_REG0] = FIELD_PREP(ADF41513_REG0_INT_MSK,
> > +					     st->settings.int_value);
> > +	if (st->settings.mode == ADF41513_MODE_VARIABLE_MODULUS)
> > +		st->regs[ADF41513_REG0] |= ADF41513_REG0_VAR_MOD_MSK;
> > +	/* frac1 */
> > +	st->regs[ADF41513_REG1] = FIELD_PREP(ADF41513_REG1_FRAC1_MSK,
> > +					     st->settings.frac1);
> > +	if (st->settings.mode != ADF41513_MODE_INTEGER_N)
> > +		st->regs[ADF41513_REG1] |= ADF41513_REG1_DITHER2_MSK;
> > +
> > +	/* frac2 */
> 
> Where the field name makes it obvious there is little point in
> adding a comment to say the same thing. I'd clear out most if not all
> of these. Stick to comments that add significant value.

ack

> > +	st->regs[ADF41513_REG3] = FIELD_PREP(ADF41513_REG3_FRAC2_MSK,
> > +					     st->settings.frac2);
> > +	/* mod2 */
> > +	st->regs[ADF41513_REG4] &= ADF41513_REG4_MOD2_MSK;
> > +	st->regs[ADF41513_REG4] |= FIELD_PREP(ADF41513_REG4_MOD2_MSK,
> > +					      st->settings.mod2);
> > +
> > +	/* r-cnt | doubler | rdiv2 | prescaler */
> > +	st->regs[ADF41513_REG5] &= ~(ADF41513_REG5_R_CNT_MSK |
> > +				     ADF41513_REG5_REF_DOUBLER_MSK |
> > +				     ADF41513_REG5_RDIV2_MSK |
> > +				     ADF41513_REG5_PRESCALER_MSK);
> > +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_R_CNT_MSK,
> > +					      st->settings.r_counter);
> > +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_REF_DOUBLER_MSK,
> > +					      st->settings.ref_doubler);
> > +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_RDIV2_MSK,
> > +					      st->settings.ref_div2);
> > +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_PRESCALER_MSK,
> > +					      st->settings.prescaler);
> 
> Probably better to use FIELD_MODIFY for all of these and let the compiler
> figure out it can mask them all in one go.

ack

> > +
> > +	if (st->settings.mode == ADF41513_MODE_INTEGER_N) {
> > +		st->regs[ADF41513_REG6] |= ADF41513_REG6_INT_MODE_MSK;
> > +		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_BLEED_ENABLE_MSK;
> > +	} else {
> > +		st->regs[ADF41513_REG6] &= ~ADF41513_REG6_INT_MODE_MSK;
> > +		st->regs[ADF41513_REG6] |= ADF41513_REG6_BLEED_ENABLE_MSK;
> > +	}
> > +
> > +	return adf41513_sync_config(st, sync_mask | ADF41513_SYNC_REG0);
> > +}
> 
> > +static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
> > +				 uintptr_t private,
> > +				 const struct iio_chan_spec *chan,
> > +				 char *buf)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	u64 freq_uhz;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch ((u32)private) {
> > +	case ADF41513_FREQ:
> > +		freq_uhz = adf41513_pll_get_rate(st);
> > +		if (st->lock_detect)
> > +			if (!gpiod_get_value_cansleep(st->lock_detect)) {
> > +				dev_dbg(&st->spi->dev, "PLL un-locked\n");
> > +				return -EBUSY;
> > +			}
> > +		break;
> > +	case ADF41513_FREQ_RESOLUTION:
> > +		freq_uhz = st->data.freq_resolution_uhz;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return adf41513_uhz_to_str(freq_uhz, buf);
> This is a more marginal case than the ones below wrt to a common
> function making sense as there is more overlap. I''m not sure it
> is worth doing even so (rather than separate callbacks).

ack. will split them.

> > +}
> > +
> > +static ssize_t adf41513_read(struct iio_dev *indio_dev,
> > +			     uintptr_t private,
> > +			     const struct iio_chan_spec *chan,
> > +			     char *buf)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	u32 val;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch ((u32)private) {
> > +	case ADF41513_FREQ_REFIN:
> > +		st->ref_freq_hz = clk_get_rate(st->ref_clk);
> > +		return sysfs_emit(buf, "%llu\n", st->ref_freq_hz);
> 
> Not much sharing here either (see below). I'd be tempted to just spit this
> into specific callbacks.

ack.

> > +	case ADF41513_POWER_DOWN:
> > +		val = FIELD_GET(ADF41513_REG6_POWER_DOWN_MSK,
> > +				st->regs_hw[ADF41513_REG6]);
> > +		return sysfs_emit(buf, "%u\n", val);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> 
> > +
> > +static ssize_t adf41513_write(struct iio_dev *indio_dev,
> > +			      uintptr_t private,
> > +			      const struct iio_chan_spec *chan,
> > +			      const char *buf, size_t len)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	unsigned long readin, tmp;
> > +	int ret;
> > +
> > +	ret = kstrtoul(buf, 10, &readin);
> > +	if (ret)
> > +		return ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch ((u32)private) {
> > +	case ADF41513_FREQ_REFIN:
> 
> There isn't a lot of shared code between different calls of this.
> Perhaps just have separate callbacks for each one.

ack.

> > +		if (readin < ADF41513_MIN_REF_FREQ || readin > ADF41513_MAX_REF_FREQ)
> > +			return -EINVAL;
> > +
> > +		tmp = clk_round_rate(st->ref_clk, readin);
> > +		if (tmp != readin)
> > +			return -EINVAL;
> > +
> > +		ret = clk_set_rate(st->ref_clk, tmp);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		st->ref_freq_hz = readin;
> > +		ret = adf41513_set_frequency(st, st->settings.target_frequency_uhz,
> > +					     ADF41513_SYNC_DIFF);
> > +		break;
> > +	case ADF41513_POWER_DOWN:
> > +		if (readin)
> > +			ret = adf41513_suspend(st);
> > +		else
> > +			ret = adf41513_resume(st);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return ret ? ret : len;
> > +}
> > +
> > +#define _ADF41513_EXT_INFO(_name, _ident) { \
> > +	.name = _name, \
> > +	.read = adf41513_read, \
> > +	.write = adf41513_write, \
> > +	.private = _ident, \
> > +	.shared = IIO_SEPARATE, \
> > +}
> > +
> > +#define _ADF41513_EXT_UHZ_INFO(_name, _ident) { \
> > +	.name = _name, \
> > +	.read = adf41513_read_uhz, \
> > +	.write = adf41513_write_uhz, \
> > +	.private = _ident, \
> > +	.shared = IIO_SEPARATE, \
> > +}
> > +
> > +static const struct iio_chan_spec_ext_info adf41513_ext_info[] = {
> > +	/*
> > +	 * Ideally we would use IIO_CHAN_INFO_FREQUENCY, but the device supports
> > +	 * frequency values greater 2^32 with sub-Hz resolution, i.e. 64-bit
> > +	 * fixed point with 6 decimal places values are used to represent
> > +	 * frequencies.
> > +	 */
> > +	_ADF41513_EXT_UHZ_INFO("frequency", ADF41513_FREQ),
> > +	_ADF41513_EXT_UHZ_INFO("frequency_resolution", ADF41513_FREQ_RESOLUTION),
> > +	_ADF41513_EXT_INFO("refin_frequency", ADF41513_FREQ_REFIN),
> Some of these are not things I recall as being standard ABI.
> This one is in one other driver but to make it generic you need to promote
> the ABI documentation to a shared file.

ack. will create the shared ABI file.

> > +	_ADF41513_EXT_INFO("powerdown", ADF41513_POWER_DOWN),
> > +	{ },
> 
> No comma on terminating entries like this.

ack

> > +};
> > +
> > +static const struct iio_chan_spec adf41513_chan = {
> > +	.type = IIO_ALTVOLTAGE,
> > +	.indexed = 1,
> > +	.output = 1,
> > +	.channel = 0,
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_PHASE),
> > +	.ext_info = adf41513_ext_info,
> > +};
> > +
> > +static int adf41513_read_raw(struct iio_dev *indio_dev,
> > +			     struct iio_chan_spec const *chan,
> > +			     int *val, int *val2, long info)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	u32 phase_mdeg;
> > +	u16 phase_val;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch (info) {
> > +	case IIO_CHAN_INFO_PHASE:
> > +		phase_val = FIELD_GET(ADF41513_REG2_PHASE_VAL_MSK,
> > +				      st->regs_hw[ADF41513_REG2]);
> > +		phase_mdeg = DIV_ROUND_CLOSEST(360 * MILLI * phase_val, BIT(12));
> > +		*val = phase_mdeg / MILLI;
> > +		*val2 = (phase_mdeg % MILLI) * 1000;
> 
> This sounds like it is in degrees. Note _phase attributes are in the documented
> ABI Documentation/ABI/testing/sysfs-bus-iio as in radians.

ack. will use radians conversion.

> > +		return IIO_VAL_INT_PLUS_MICRO;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int adf41513_write_raw(struct iio_dev *indio_dev,
> > +			      struct iio_chan_spec const *chan,
> > +			      int val, int val2, long info)
> > +{
> > +	struct adf41513_state *st = iio_priv(indio_dev);
> > +	u32 phase_mdeg;
> > +	u16 phase_val;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	switch (info) {
> > +	case IIO_CHAN_INFO_PHASE:
> > +		val %= 360;
> > +		if (val < 0)
> > +			val += 360;
> > +		phase_mdeg = val * MILLI + val2 / 1000;
> > +		phase_val = DIV_ROUND_CLOSEST(phase_mdeg << 12, 360 * MILLI);
> > +
> > +		st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK;
> > +		st->regs[ADF41513_REG2] &= ~ADF41513_REG2_PHASE_VAL_MSK;
> 
> FIELD_MODIFY() can save doing the clear and fill as separate calls.

ack.

> > +		st->regs[ADF41513_REG2] |= FIELD_PREP(ADF41513_REG2_PHASE_VAL_MSK, phase_val);
> > +		return adf41513_sync_config(st, ADF41513_SYNC_REG0);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> 
> 
> > +static int adf41513_parse_fw(struct adf41513_state *st)
> > +{
> > +	struct device *dev = &st->spi->dev;
> > +	int ret;
> > +	u32 tmp;
> > +	u32 cp_resistance;
> > +	u32 cp_current;
> Where you have set of variables of same type and grouping doesn't hurt
> readability, declare them all on one line.
> 
> 	u32 tmp, cp_resistance, cp_current;

ack

> I'll not repeat comments I made on the dt-binding in here but I'd expect
> this code to change somewhat in response to those.

understood. for now, will use -mhz for the power-up frequency, but I will
see how the discussion follows.

> > +
> 
> ...
> 
> 
> > +static int adf41513_setup(struct adf41513_state *st)
> > +{
> > +	u32 tmp;
> > +
> > +	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
> > +
> > +	/* assume DLD pin is used for digital lock detect */
> > +	st->regs[ADF41513_REG5] = FIELD_PREP(ADF41513_REG5_DLD_MODES_MSK,
> > +					     ADF41513_DLD_DIG_LD);
> > +
> > +	/* configure charge pump current settings */
> > +	tmp = DIV_ROUND_CLOSEST(st->data.charge_pump_voltage_mv, ADF41513_MIN_CP_VOLTAGE_mV);
> > +	st->regs[ADF41513_REG5] |= FIELD_PREP(ADF41513_REG5_CP_CURRENT_MSK, tmp - 1);
> > +
> > +	/* narrow ABP | loss of lock detect enable | SD reset | LDP from data */
> 
> I'm not sure what LPD from data means. Can't correlate that with the datasheet.
> Perhaps add more info or reword.

will remove, this is not applicable anymore.

> > +	st->regs[ADF41513_REG6] = ADF41513_REG6_ABP_MSK |
> > +				  ADF41513_REG6_LOL_ENABLE_MSK |
> > +				  ADF41513_REG6_SD_RESET_MSK;
> > +	if (st->data.phase_detector_polarity)
> > +		st->regs[ADF41513_REG6] |= ADF41513_REG6_PD_POLARITY_MSK;
> > +
> > +	/* PS bias | lock detect count */
> Confusing comment as covering multiple bits of code. I'd just drop
> it on basis the field names in the code are less confusing than the comment.

ack. will drop.

> > +	st->regs[ADF41513_REG7] = FIELD_PREP(ADF41513_REG7_PS_BIAS_MSK, 2);
> 
> That magic 2 is interesting as it is truely magic with no explanation on
> the datasheet beyond 'program this value'. Even better, on the datasheet I'm
> looking at the Prescaler (PS) bias section says set it to 3 and figure 30
> say set it to 2.  Maybe add a commeon on this.

understood. 2 should be the correct vallue, but I will ask around.

> > +	tmp = ilog2(st->data.lock_detect_count);
> > +	if (st->data.lock_detect_count < ADF41513_LD_COUNT_FAST_LIMIT) {
> > +		tmp -= const_ilog2(ADF41513_LD_COUNT_FAST_MIN);
> > +		st->regs[ADF41513_REG7] |= ADF41513_REG7_LD_CLK_SEL_MSK;
> > +	} else {
> > +		tmp -= const_ilog2(ADF41513_LD_COUNT_MIN);
> > +	}
> > +	st->regs[ADF41513_REG7] |= FIELD_PREP(ADF41513_REG7_LD_COUNT_MSK, tmp);
> > +
> > +	/* power down select */
> > +	st->regs[ADF41513_REG11] = ADF41513_REG11_POWER_DOWN_SEL_MSK;
> > +
> > +	/* muxout */
> > +	st->regs[ADF41513_REG12] = FIELD_PREP(ADF41513_REG12_MUXOUT_MSK,
> > +					      st->data.muxout_select);
> > +	st->regs[ADF41513_REG12] |= FIELD_PREP(ADF41513_REG12_LOGIC_LEVEL_MSK,
> > +					       st->data.muxout_1v8_en ? 0 : 1);
> > +
> > +	/* perform initialization sequence with power-up frequency */
> > +	return adf41513_set_frequency(st, (u64)st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
> > +				      ADF41513_SYNC_ALL);
> > +}
> 
> ...
> 
> > +
> > +static int adf41513_pm_suspend(struct device *dev)
> > +{
> > +	struct adf41513_state *st = dev_get_drvdata(dev);
> > +
> > +	return adf41513_suspend(st);
> 
> Not at lot in point in the local variable
> 
> 	return adf41513_suspend(dev_get_drvdata(dev));

ack

> > +}
> > +
> > +static int adf41513_pm_resume(struct device *dev)
> > +{
> > +	struct adf41513_state *st = dev_get_drvdata(dev);
> > +
> > +	return adf41513_resume(st);
> As above.
> > +}
> > +
> > +static const struct adf41513_chip_info adf41513_chip_info = {
> > +	.name = "adf41513",
> > +	.has_prescaler_8_9 = true,
> > +	.max_rf_freq_hz = ADF41513_MAX_RF_FREQ,
> > +};
> > +
> > +static const struct adf41513_chip_info adf41510_chip_info = {
> 
> Just for long term organization when many devices are supported:
> keep these structures in alphanumeric order.

ack

> > +	.name = "adf41510",
> > +	.has_prescaler_8_9 = false,
> > +	.max_rf_freq_hz = ADF41510_MAX_RF_FREQ,
> > +};
> > +
> > +static int adf41513_probe(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct adf41513_state *st;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> 
> I'd use a 
> 	struct device *dev = &spi->dev;
> so that you can shorten all the lines where spi->dev is used in here.

ack

> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	st = iio_priv(indio_dev);
> > +	st->spi = spi;
> > +	st->chip_info = spi_get_device_match_data(spi);
> > +	if (!st->chip_info)
> > +		return -EINVAL;
> > +
> > +	spi_set_drvdata(spi, st);
> > +
> > +	st->ref_clk = devm_clk_get_enabled(&spi->dev, NULL);
> > +	if (IS_ERR(st->ref_clk))
> > +		return PTR_ERR(st->ref_clk);
> > +
> > +	st->ref_freq_hz = clk_get_rate(st->ref_clk);
> > +	if (st->ref_freq_hz < ADF41513_MIN_REF_FREQ || st->ref_freq_hz > ADF41513_MAX_REF_FREQ)
> > +		return dev_err_probe(&spi->dev, -ERANGE,
> > +				     "reference frequency %llu Hz out of range\n",
> > +				     st->ref_freq_hz);
> > +
> > +	ret = adf41513_parse_fw(st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_regulator_bulk_get_enable(&spi->dev,
> > +					     ARRAY_SIZE(adf41513_power_supplies),
> > +					     adf41513_power_supplies);
> > +	if (ret)
> > +		return dev_err_probe(&spi->dev, ret,
> > +				     "failed to get and enable regulators\n");
> > +
> > +	st->chip_enable = devm_gpiod_get_optional(&spi->dev, "enable", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(st->chip_enable))
> > +		return dev_err_probe(&spi->dev, PTR_ERR(st->chip_enable),
> > +				     "fail to request chip enable GPIO\n");
> > +
> > +	st->lock_detect = devm_gpiod_get_optional(&spi->dev, "lock-detect", GPIOD_IN);
> > +	if (IS_ERR(st->lock_detect))
> > +		return dev_err_probe(&spi->dev, PTR_ERR(st->lock_detect),
> > +				     "fail to request lock detect GPIO\n");
> > +
> > +	ret = devm_mutex_init(&spi->dev, &st->lock);
> > +	if (ret)
> > +		return ret;
> > +
> > +	indio_dev->name = st->chip_info->name;
> > +	indio_dev->info = &adf41513_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->channels = &adf41513_chan;
> > +	indio_dev->num_channels = 1;
> > +
> > +	ret = adf41513_setup(st);
> > +	if (ret < 0)
> > +		return dev_err_probe(&spi->dev, ret, "failed to setup device: %d\n", ret);
> Look at what dev_err_probe() prints.  (short answer, it includes a much nicer print
> of the error value than the one you have here).  So dev_err_probe() should never
> include the error value itself as that is duplicating the info.

yes, that looks bad. will adjust.

> > +
> > +	ret = devm_add_action_or_reset(&spi->dev, adf41513_power_down, st);
> > +	if (ret)
> > +		return dev_err_probe(&spi->dev, ret, "Failed to add power down action: %d\n", ret);
> 
> As above.  No printing ret by hand in dev_err_probe() calls.

ack

> > +
> > +	return devm_iio_device_register(&spi->dev, indio_dev);
> > +}

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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-21 13:02       ` Krzysztof Kozlowski
@ 2025-12-22 10:21         ` Rodrigo Alencar
  0 siblings, 0 replies; 24+ messages in thread
From: Rodrigo Alencar @ 2025-12-22 10:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rodrigo Alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet

On 25/12/21 02:02PM, Krzysztof Kozlowski wrote:
> On 20/12/2025 19:05, 455.rodrigo.alencar@gmail.com wrote:
> > Hi Krzystof,
> > 
> > thanks for taking a look into this again. It was my first patch it didn't want
> > to draw more attention or discussion to the V1 patch as it was declared not ready
> > at its very first review.
> > 
> > On 25/12/20 10:21AM, Krzysztof Kozlowski wrote:
> >> On Fri, Dec 19, 2025 at 12:34:48PM +0000, Rodrigo Alencar wrote:
> >>> dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> >>> can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> >>> Most properties refer to existing PLL driver properties (e.g. ADF4350).
> >>
> >> What is "existing PLL driver"? I know about motor drivers, but can you
> >> drive PLL?
> >>
> >> And how is ADF4350 related to this binding. I do not see ADF4350
> >> compatible here at all. Describe hardware, a real one.
> > 
> > ADF4350 is an older one, and its bindings can be found at:
> > Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml
> > It is a similar part, but yet very different.
> > 
> >>
> >> Nothing improved.
> >>
> >> You ignored comments, did not bother to respond to them and then sent
> >> the same.
> > 
> > Sorry for not responding on the V1 thread, but the previous patch had to be reviewed internally
> > first. It is not true that nothing is improved, in fact, it has changed a lot, here are some notes:
> 
> Process is not like that. You first review internally, then you send.
> After you sent and receive comments, you respond to these comments.

ack.
 
> > * adi,power-up-frequency is not carrying the -hz postfix because it forces to be a uint32 by
> > the dt-bindings check. For that variable it needs to be uint64 as the part supports up to 26.5 GHz > 2^32
> 
> And what granularity do you need? Why mhz does not work?

~Hz granularity is needed to adjust frequency offsets spotted by calibration,
but I suppose that is a whim that can be dropped indeed, as the important
thing about this property is to populate frequency configs for the initialization
sequence, which requires all registers to be written.

> > * The properties related to the reference input signal path: reference-div-factor, reference-doubler-enable
> > reference-div2-enable are declared here because they are constraints for the PFD frequency definition,
> > which is the frequency that the output signal is updated, important for the loop-filter and VCO design.
> > * added support for all different power supply regulators.
> 
> Sorry, but I cannot respond that way. We discuss inline, so I have
> entire picture, not some parts of message semi-quoted here. I don't
> remember what was there and I am not going to keep looking for that.
> 
> You need to adjust to mailing list discussion style, not introduce the
> others. I have just way too many other patches to deal with, so
> implement the feedback or respond properly.

ack. Will adjust to the requested style. Already deviating from it again,
but those were the comments from the V1 review:
(https://lore.kernel.org/all/20251111-feathered-winged-bloodhound-b7e1a3@kuoka/)

> 
> Please organize the patch documenting compatible (DT bindings) before their user.
> See also: https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46

done.

> > +
> > +  chip-enable-gpios:
> 
> enable-gpios

done.

> > +  adi,power-up-frequency:
> > +    $ref: /schemas/types.yaml#/definitions/uint64
> 
> Use standard unit suffixes. Frequency is in Hz for example.

sorry, this was the problem, will use -mhz as suggested.

> > +  adi,reference-div-factor:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 32
> > +    description:
> > +      Reference division factor (R Counter). If not specified, the driver
> > +      will calculate the optimal value automatically.
> 
> Then why do you need this property? If driver calculates the optimal,
> why anyone would put wrong or sub-optimal value to DT?
> 
> Drop.

The description was bad so I rewrote this. The value is hardware constraint
for the output frequency of the Phase-Frequency Detector (PFD),
which is important for the external loop-filter/VCO design. The driver may
only change the R counter if the PFD frequency goes off limits. In that case,
some designs can acomodate different PFD frequencies, but that is not usual
and likely not recommended.

> > +
> > +        /* Example with advanced features enabled */
> > +        pll_advanced@0 {
> 
> pll@

done

thanks for the patience.

kind regards,

Rodrigo Alencar

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

* Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
  2025-12-21 19:45     ` Rodrigo Alencar
@ 2025-12-27 16:51       ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-27 16:51 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-kernel,
	linux-iio, devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet

On Sun, 21 Dec 2025 19:45:11 +0000
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:

> Hi Jonathan, thanks for your time.
> 
> On 25/12/21 04:59PM, Jonathan Cameron wrote:
> > On Fri, 19 Dec 2025 12:34:48 +0000
> > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> >   
> > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > 
> > > dt-bindings for ADF41513, an ultralow noise PLL frequency synthesizer that
> > > can be used to implement local oscillators (LOs) as high as 26.5 GHz.
> > > Most properties refer to existing PLL driver properties (e.g. ADF4350).  
> > 
> > "Refer" implies a cross reference in this document.   Based upon is probably a better
> > way to put this.
> > 
> > Otherwise I've mostly commented on properties that to me don't sound like
> > they belong in the dt-binding as they are policy things that we want
> > to make runtime configurable.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > 
> > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > ---
> > >  .../bindings/iio/frequency/adi,adf41513.yaml       | 246 +++++++++++++++++++++
> > >  MAINTAINERS                                        |   7 +
> > >  2 files changed, 253 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> > > new file mode 100644
> > > index 000000000000..01ceb2a7d21b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml
> > > @@ -0,0 +1,246 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/frequency/adi,adf41513.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices ADF41513 PLL Frequency Synthesizer
> > > +
> > > +maintainers:
> > > +  - Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > +
> > > +description:
> > > +  The ADF41513 is an ultralow noise frequency synthesizer that can be used to
> > > +  implement local oscillators (LOs) as high as 26.5 GHz in the upconversion and
> > > +  downconversion sections of wireless receivers and transmitters. The ADF41510
> > > +  supports frequencies up to 10 GHz.
> > > +
> > > +  https://www.analog.com/en/products/adf41513.html
> > > +  https://www.analog.com/en/products/adf41510.html
> > > +
> > > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,adf41510
> > > +      - adi,adf41513
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  spi-max-frequency:
> > > +    maximum: 25000000
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +    description: Clock that provides the reference input frequency.
> > > +
> > > +  avdd1-supply:
> > > +    description: PFD and Up and Down Digital Driver Power Supply (3.3 V)
> > > +
> > > +  avdd2-supply:
> > > +    description: RF Buffer and Prescaler Power Supply (3.3 V)
> > > +
> > > +  avdd3-supply:
> > > +    description: N Divider Power Supply (3.3 V)
> > > +
> > > +  avdd4-supply:
> > > +    description: R Divider and Lock Detector Power Supply (3.3 V)
> > > +
> > > +  avdd5-supply:
> > > +    description: Sigma-Delta Modulator and SPI Power Supply (3.3 V)
> > > +
> > > +  vp-supply:
> > > +    description: Charge Pump Power Supply (3.3 V)
> > > +
> > > +  enable-gpios:
> > > +    description:
> > > +      GPIO that controls the chip enable pin. A logic low on this pin
> > > +      powers down the device and puts the charge pump output into
> > > +      three-state mode.
> > > +    maxItems: 1
> > > +
> > > +  lock-detect-gpios:
> > > +    description:
> > > +      GPIO for lock detect functionality. When configured for digital lock
> > > +      detect, this pin will output a logic high when the PLL is locked.  
> > 
> > This seems to be one potential use of the muxout pin.  So to me feels like
> > a policy decision that belongs with the driver or userspace, not in dt.
> > mux-out-gpios:
> > would make more sense to me.
> > Some of the potential settings probably don't make sense but then we just
> > don't support those in the driver if this is connected to a gpio.  
>  
> Actually, there is the DLD pin (separate pin from muxout) dedicated for lock detection.

Ah. I'd missed that.  I can't figure out why it is also routed
to the muxout and whether that signal is the same.  Ah well, doesn't
matter with this one being the DLD pin.

> 
> > > +    maxItems: 1
> > > +
> > > +  adi,power-up-frequency:
> > > +    $ref: /schemas/types.yaml#/definitions/uint64
> > > +    minimum: 1000000000
> > > +    maximum: 26500000000
> > > +    default: 10000000000
> > > +    description:
> > > +      The PLL tunes to this frequency (in Hz) during the initialization
> > > +      sequence. This property should be set to a frequency supported by the
> > > +      loop filter and VCO used in the design. Range is 1 GHz to 26.5 GHz for
> > > +      ADF41513, and 1 GHz to 10 GHz for ADF41510.  
> > 
> > Why is this in DT?  Feels like this should be done by userspace control
> > prior to setting an enable rather than being in DT.  
> 
> The initialization sequence of the PLL requires the write of all the registers.
> This property exist so that meaningful values are written at start up.
> Here it is desired to have a frequency value that is within the range supported
> by the loop-filter and the external VCO.  
>  
> > > +
> > > +  adi,reference-div-factor:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    minimum: 1
> > > +    maximum: 32
> > > +    default: 1
> > > +    description:
> > > +      Value for the reference division factor (R Counter). The driver will
> > > +      increment R Counter as needed to achieve a PFD frequency within the
> > > +      allowed range. High R counter values will reduce the PFD frequency, which
> > > +      lowers the frequency resolution, and affects phase noise performance.  
> > 
> > Why is this in DT?  Is there not a 'best' choice to be made given a particular
> > input frequency and desired output frequency?  
> 
> When designing the external circuitry to the PLL, customers often refers to
> https://www.analog.com/en/lp/resources/adisimpll.html
> which is a piece of software that assists the choice of some parts and
> simulates performance metrics. After having the suggested design simulated,
> it turns out that the reference signal path defines the output frequency
> of the Phase-Frequency Detector (PFD), which is set as a constraint for
> the loop-filter design. Therefore, those properties (reference-div-factor,
> reference-doubler-enable and reference-div2-enable) are meant to reflect
> a hardware constraint for a PFD frequency value expected by the loop-filter/VCO

Makes sense.  Thanks for the details.  Perhaps add a little more on this
to the binding description to make it clear these are design time thing.

> 
> > > +
> > > +  adi,reference-doubler-enable:
> > > +    description:
> > > +      Enables the reference doubler. The maximum reference frequency when
> > > +      the doubler is enabled is 225 MHz.  
> > As above.  
> 
> explained above as part of a hardware constraint for external circuitry design
> 
> > > +    type: boolean
> > > +
> > > +  adi,reference-div2-enable:
> > > +    description:
> > > +      Enables the reference divide-by-2 function. This provides a 50%
> > > +      duty cycle signal to the PFD.  
> > As above.  
> 
> same
> 
> > > +    type: boolean
> > > +
> > > +  adi,charge-pump-resistor-ohms:
> > > +    minimum: 1800
> > > +    maximum: 10000
> > > +    default: 2700
> > > +    description:
> > > +      External charge pump resistor (R_SET) value in ohms. This sets the maximum
> > > +      charge pump current along with the charge pump current setting.
> > > +
> > > +  adi,charge-pump-current-microamp:
> > > +    description:
> > > +      Charge pump current (I_CP) in microamps. The value will be rounded to the
> > > +      nearest supported value. Range of acceptable values depends on the
> > > +      charge pump resistor value, such that 810 mV <= I_CP * R_SET <= 12960 mV.
> > > +      This value depends on the loop filter design.
> > > +
> > > +  adi,muxout-select:
> > > +    description:
> > > +      On chip multiplexer output selection.
> > > +      high_z - MUXOUT Pin set to high-Z. (default)
> > > +      muxout_high - MUXOUT Pin set to high.
> > > +      muxout_low - MUXOUT Pin set to low.
> > > +      f_div_rclk - MUXOUT Pin set to R divider output
> > > +      f_div_nclk - MUXOUT Pin set to N divider output
> > > +      lock_detect - MUXOUT Pin set to Digital lock detect
> > > +      serial_data - MUXOUT Pin set to Serial data output
> > > +      readback - MUXOUT Pin set to Readback mode
> > > +      f_div_clk1 - MUXOUT Pin set to CLK1 divider output
> > > +      f_div_rclk_2 - MUXOUT Pin set to R divider/2 output
> > > +      f_div_nclk_2 - MUXOUT Pin set to N divider/2 output
> > > +    enum: [high_z, muxout_high, muxout_low, f_div_rclk, f_div_nclk, lock_detect,
> > > +           serial_data, readback, f_div_clk1, f_div_rclk_2, f_div_nclk_2]  
> > 
> > This needs explanation of 'why' it should be in DT?  To me it seems mostly
> > to be a debug feature that should be controlled perhaps via a debugfs interface.  
> 
> There has been a discussion on that, and I understand the concerns.
> Altough it looks like a debug pin, I suppose some board designs might
> expect the muxout pin to be working as a specific function.
> The device driver provides register access through the debugfs interface,
> so worst case, the muxout select value could be changed there.

I'd like to know if there are known to be such designs in the wild before
supporting this in DT.  It feels very speculative.  The nature of the many
way mux makes me think it's pretty unlikely that the intent is to use
this as an input to external circuitry.

> 
> > > +
> > > +  adi,muxout-level-1v8-enable:
> > > +    description:
> > > +      Set MUXOUT and DLD logic levels to 1.8V. Default is 3.3V.
> > > +    type: boolean
> > > +
> > > +  adi,phase-detector-polarity-positive-enable:
> > > +    description:
> > > +      Set phase detector polarity to positive. Default is negative.
> > > +      Use positive polarity with non-inverting loop filter and VCO with
> > > +      positive tuning slope, or with inverting loop filter and VCO with
> > > +      negative tuning slope.
> > > +    type: boolean
> > > +
> > > +  adi,lock-detector-count:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    default: 64
> > > +    description:
> > > +      Sets the value for Lock Detector count of the PLL, which determines the
> > > +      number of consecutive phase detector cycles that must be within the lock
> > > +      detector window before lock is declared. Lower values increase the lock
> > > +      detection sensitivity.
> > > +    enum: [2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]  
> > 
> > Sounds like policy.  Maybe it is related to the circuit design and there
> > is a right choice for  particular board? If not we should figure out how to leave
> > this to userspace control.  Probably as some form of event property.  
> 
> There was also a internal discussion around that. I'd say that this might
> be related to the application where the design is being used. There should
> be value on making it configurable at runtime, mostly for tunning for
> specific needs, but this is not a value to be changed at runtime in
> field-deployed products.

Add something about detection of lock being something that is dependent on
external circuitry and so there is an optimal value to put here that is
expected to be fixed.

Thanks,

Jonathan

> 
> kind regards,
> 
> Rodrigo Alencar
> 


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

* Re: [PATCH v2 2/6] iio: frequency: adf41513: driver implementation
  2025-12-22  9:45     ` Rodrigo Alencar
@ 2025-12-27 16:56       ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-27 16:56 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-kernel,
	linux-iio, devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet


...

> > > +	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK,
> > > +				  st->regs_hw[ADF41513_REG5]);
> > > +	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,
> > > +				   st->regs_hw[ADF41513_REG5]);  
> > For cases like this I think keeping to 80 chars is hurting readability
> > and so it is fine to go a little higher. 
> > 	cfg->int_value = FIELD_GET(ADF41513_REG0_INT_MSK, st->regs_hw[ADF41513_REG0]);
> > 	cfg->frac1 = FIELD_GET(ADF41513_REG1_FRAC1_MSK, st->regs_hw[ADF41513_REG1]);
> > 	cfg->frac2 = FIELD_GET(ADF41513_REG3_FRAC2_MSK, st->regs_hw[ADF41513_REG3]);
> > 	cfg->mod2 = FIELD_GET(ADF41513_REG4_MOD2_MSK, st->regs_hw[ADF41513_REG4]);
> > 	cfg->r_counter = FIELD_GET(ADF41513_REG5_R_CNT_MSK, st->regs_hw[ADF41513_REG5]);
> > 	cfg->ref_doubler = FIELD_GET(ADF41513_REG5_REF_DOUBLER_MSK, st->regs_hw[ADF41513_REG5]);
> > 	cfg->ref_div2 = FIELD_GET(ADF41513_REG5_RDIV2_MSK, st->regs_hw[ADF41513_REG5]);
> > 	cfg->prescaler = FIELD_GET(ADF41513_REG5_PRESCALER_MSK,st->regs_hw[ADF41513_REG5]);
> > Is fine here. I'd also be fine with wrapping the ref_doubler line as it's rather
> > longer than the others.  
> 
> ack

Small kernel development process thing.  For efficiency general rule is
don't bother replying at all to suggestions you accept. It just adds noise.
Much better to just crop that chunk of the reply out so we can
rapidly see where more discussion is needed.

>  
> > > +
> > > +	/* calculate pfd frequency */

...

> > > +static int adf41513_parse_fw(struct adf41513_state *st)
> > > +{
> > > +	struct device *dev = &st->spi->dev;
> > > +	int ret;
> > > +	u32 tmp;
> > > +	u32 cp_resistance;
> > > +	u32 cp_current;  
> > Where you have set of variables of same type and grouping doesn't hurt
> > readability, declare them all on one line.
> > 
> > 	u32 tmp, cp_resistance, cp_current;  
> 
> ack
> 
> > I'll not repeat comments I made on the dt-binding in here but I'd expect
> > this code to change somewhat in response to those.  
> 
> understood. for now, will use -mhz for the power-up frequency, but I will
> see how the discussion follows.

That one is indeed non obvious and so (assuming I didn't miss anything) this
is the one block where there was a non trivial comment in your reply so
ideally would have been only bit there.

I get grumpy about this every now and then and this time you get to be
the target!

Jonathan

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

* Re: [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver
  2025-12-21 20:20     ` Rodrigo Alencar
@ 2025-12-27 17:06       ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-12-27 17:06 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet



> > > +2.3 Reference Input Control
> > > +---------------------------
> > > +
> > > +The ``refin_frequency`` attribute allows control of the reference input
> > > +frequency when using a programmable reference clock. The supported range is
> > > +10 MHz to 800 MHz.  
> >
> > I'm not really sure why need this as opposed to having a standard clock
> > provide it.  What's the use case?  
> 
> I was thinking about, and for the application I am currently evaluating the part
> the reference signal comes from a DDS, and that signal is not a clock it is a
> series of chirp patterns. The important thing about this property is to set
> a center frequency for the DDS to work on. In that scenario the PLL will not
> really work as a frequency sythentizer, but as a frequency tracker of the
> varying reference. Problem is that I've realized that recently after putting
> together a device driver for the DDS. Therefore this property is still important,
> and I need to make the reference clock input as an optional property.
> I thought of making the DDS as a clock provider, but that center frequency
> would have a "virtual" meaning, not attached to the hardware configs.

Interesting use case!  I'm not really sure how we support it. I can think of a
a few approaches that might work. In some way it's a bit similar to an amplifier
in front of an ADC or other analog front end, just working in the frequency domain.
I've never been entirely happy with how we support those either though!

> 

> > > +2.5 Phase adjustment
> > > +--------------------
> > > +
> > > +The ``phase`` attribute allows adjustment of the output phase in degrees.  
> >
> > As per driver feedback, I don't think this is compliant with existing ABI.  
> 
> ABI is not in degrees? the attribute is named out_altvoltage0_phase

All angle things (including phase) in IIO are in Radians (we try to stick to SI
units). Check these in
Documentation/ABI/testing/sysfs-bus-iio*

Note that is not always as complete as it should be so if anything is missing
and in use we should add it.  We don't blanket add stuff the ABI construction
code allows as some cases will never occur in reality.

Jonathan


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

end of thread, other threads:[~2025-12-27 17:06 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19 12:34 [PATCH v2 0/6] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
2025-12-19 12:34 ` [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
2025-12-20  9:21   ` Krzysztof Kozlowski
2025-12-20 18:05     ` 455.rodrigo.alencar
2025-12-21 13:02       ` Krzysztof Kozlowski
2025-12-22 10:21         ` Rodrigo Alencar
2025-12-21 15:56       ` Jonathan Cameron
2025-12-21 19:56         ` Rodrigo Alencar
2025-12-21 16:59   ` Jonathan Cameron
2025-12-21 19:45     ` Rodrigo Alencar
2025-12-27 16:51       ` Jonathan Cameron
2025-12-19 12:34 ` [PATCH v2 2/6] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
2025-12-21 17:49   ` Jonathan Cameron
2025-12-22  9:45     ` Rodrigo Alencar
2025-12-27 16:56       ` Jonathan Cameron
2025-12-19 12:34 ` [PATCH v2 3/6] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay
2025-12-19 12:34 ` [PATCH v2 4/6] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
2025-12-19 12:34 ` [PATCH v2 5/6] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
2025-12-21 18:00   ` Jonathan Cameron
2025-12-21 20:20     ` Rodrigo Alencar
2025-12-27 17:06       ` Jonathan Cameron
2025-12-19 12:34 ` [PATCH v2 6/6] Documentation: ABI: testing: add support for ADF41513 Rodrigo Alencar via B4 Relay
2025-12-21 17:52   ` Jonathan Cameron
2025-12-22  9:24     ` Rodrigo Alencar

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