[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251221165928.324db536@jic23-huawei>
Date: Sun, 21 Dec 2025 16:59:28 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Rodrigo Alencar via B4 Relay
<devnull+rodrigo.alencar.analog.com@...nel.org>
Cc: rodrigo.alencar@...log.com, linux-kernel@...r.kernel.org,
linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-doc@...r.kernel.org, David Lechner <dlechner@...libre.com>, Andy
Shevchenko <andy@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Michael
Hennerich <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v2 1/6] dt-bindings: iio: frequency: add adf41513
On Fri, 19 Dec 2025 12:34:48 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@...nel.org> wrote:
> From: Rodrigo Alencar <rodrigo.alencar@...log.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@...log.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@...log.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@...log.com>
> +L: linux-iio@...r.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@...log.com>
> L: linux-iio@...r.kernel.org
>
Powered by blists - more mailing lists