* [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers
@ 2026-01-16 14:32 Rodrigo Alencar via B4 Relay
2026-01-16 14:32 ` [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
` (6 more replies)
0 siblings, 7 replies; 28+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 v4:
- Proper usage of units.h macros
- Simplifications to DT property parsing
- Adjustments to return value handling
- Drop of simple DT property node example
- Link to v3: https://lore.kernel.org/r/20260108-adf41513-iio-driver-v3-0-23d1371aef48@analog.com
Changes in v3:
- Use FIELD_MODIFY macro in driver implementation
- Drop refin_frequency iio attribute
- Drop muxout-select property from dt-bindings (and rename logic-level property)
- Use -mhz suffix in power-up frequency property
- Address documentation issues
- Link to v2: https://lore.kernel.org/r/20251219-adf41513-iio-driver-v2-0-be29a83d5793@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 (7):
dt-bindings: iio: frequency: add adf41513
units: Add HZ_PER_GHZ definition
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 common ABI file for iio/frequency
Documentation/ABI/testing/sysfs-bus-iio-frequency | 11 +
.../bindings/iio/frequency/adi,adf41513.yaml | 212 ++++
Documentation/iio/adf41513.rst | 199 +++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 10 +
drivers/iio/frequency/Kconfig | 10 +
drivers/iio/frequency/Makefile | 1 +
drivers/iio/frequency/adf41513.c | 1295 ++++++++++++++++++++
include/linux/units.h | 1 +
9 files changed, 1740 insertions(+)
---
base-commit: b82f3047dae4aba38cb26c55c28444db4d77f521
change-id: 20251110-adf41513-iio-driver-aaca8a7f808e
Best regards,
--
Rodrigo Alencar <rodrigo.alencar@analog.com>
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 14:35 ` Krzysztof Kozlowski 2026-01-16 14:32 ` [PATCH v4 2/7] units: Add HZ_PER_GHZ definition Rodrigo Alencar via B4 Relay ` (5 subsequent siblings) 6 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 are based upon an existing PLL device properties (e.g. ADF4350). Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> --- .../bindings/iio/frequency/adi,adf41513.yaml | 212 +++++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 219 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..a797b70dc9bc --- /dev/null +++ b/Documentation/devicetree/bindings/iio/frequency/adi,adf41513.yaml @@ -0,0 +1,212 @@ +# 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/adf41510.html + https://www.analog.com/en/products/adf41513.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-mhz: + minimum: 1000 + maximum: 26500 + default: 10000 + description: + The PLL tunes to this frequency 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. + As it affects the PFD frequency, this value depends on the loop filter + design. + + adi,reference-doubler-enable: + description: + Enables the reference doubler when deriving the PFD frequency. + The maximum reference frequency when the doubler is enabled is 225 MHz. + As it affects the PFD frequency, this value depends on the loop filter + design. + type: boolean + + adi,reference-div2-enable: + description: + Enables the reference divide-by-2 function when deriving the PFD + frequency. As it affects the PFD frequency, this value depends on the + loop filter design. + 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 and the VCO design. + + adi,logic-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, while higher values provides a more stable lock + detection. Applications that consume the lock detect signal may require + different settings based on system requirements. + 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 the 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, LE sync + should be left disabled. + type: boolean + +required: + - compatible + - reg + - clocks + - avdd1-supply + - avdd2-supply + - avdd3-supply + - avdd4-supply + - avdd5-supply + - vp-supply + +unevaluatedProperties: false + +examples: + - | + #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-mhz = <15500>; + adi,charge-pump-current-microamp = <3600>; + adi,charge-pump-resistor-ohms = <2700>; + adi,reference-doubler-enable; + 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 4dd9f758a871..095af34e234e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1618,6 +1618,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] 28+ messages in thread
* Re: [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 2026-01-16 14:32 ` [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay @ 2026-01-16 14:35 ` Krzysztof Kozlowski 0 siblings, 0 replies; 28+ messages in thread From: Krzysztof Kozlowski @ 2026-01-16 14:35 UTC (permalink / raw) To: rodrigo.alencar, 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 On 16/01/2026 15:32, Rodrigo Alencar via B4 Relay 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 are based upon an existing PLL device properties > (e.g. ADF4350). > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> I will not do the work twice without you providing the reason for that. <form letter> This is a friendly reminder during the review process. It looks like you received a tag and forgot to add it. If you do not know the process, here is a short explanation: Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions of patchset, under or above your Signed-off-by tag, unless patch changed significantly (e.g. new properties added to the DT bindings). Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply. Please read: https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577 If a tag was not added on purpose, please state why and what changed. </form letter> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 2/7] units: Add HZ_PER_GHZ definition 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 15:26 ` Andy Shevchenko 2026-01-16 14:32 ` [PATCH v4 3/7] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay ` (4 subsequent siblings) 6 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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> New PLL IIO driver supports output frequency of several GHz. The new define helps to constraint DT properties and frequency calculation parameters. Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> --- include/linux/units.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/units.h b/include/linux/units.h index 00e15de33eca..06870e9e90b8 100644 --- a/include/linux/units.h +++ b/include/linux/units.h @@ -27,6 +27,7 @@ #define HZ_PER_KHZ 1000UL #define HZ_PER_MHZ 1000000UL +#define HZ_PER_GHZ 1000000000UL #define KHZ_PER_MHZ 1000UL #define KHZ_PER_GHZ 1000000UL -- 2.43.0 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] units: Add HZ_PER_GHZ definition 2026-01-16 14:32 ` [PATCH v4 2/7] units: Add HZ_PER_GHZ definition Rodrigo Alencar via B4 Relay @ 2026-01-16 15:26 ` Andy Shevchenko 2026-01-16 19:01 ` Jonathan Cameron 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-16 15:26 UTC (permalink / raw) To: rodrigo.alencar, Andi Shyti 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 +Cc: Andi On Fri, Jan 16, 2026 at 02:32:21PM +0000, Rodrigo Alencar via B4 Relay wrote: > > New PLL IIO driver supports output frequency of several GHz. > The new define helps to constraint DT properties and frequency > calculation parameters. There is already pending patch to add this in I²C host controller subsystem. On one hand the conflict, if any, is easy to resolve (the other patch adds a couple of comments). On the other we are at almost rc6 and it seems DT people are not happy about something, which would mean that the series most likely will miss next merge window. So, the solutions are: - do nothing and resolve conflicts, if any - define this constant locally in the respective IIO driver and drop it after merge - postpone this series to the next cycle (effectively drop this patch) - ask Andi to provide an immutable branch / tag with I²C host patches - ask Andi to provide the only that patch in immutable tag / branch, but it will require him to rebase his tree I'm skeptical about last two options on the grounds on how the IIO works and possible rebase requirement (which is not good to have). I leave it to you and the respective maintainers to make a final decision. > diff --git a/include/linux/units.h b/include/linux/units.h > index 00e15de33eca..06870e9e90b8 100644 > --- a/include/linux/units.h > +++ b/include/linux/units.h > @@ -27,6 +27,7 @@ > > #define HZ_PER_KHZ 1000UL > #define HZ_PER_MHZ 1000000UL > +#define HZ_PER_GHZ 1000000000UL > > #define KHZ_PER_MHZ 1000UL > #define KHZ_PER_GHZ 1000000UL -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 2/7] units: Add HZ_PER_GHZ definition 2026-01-16 15:26 ` Andy Shevchenko @ 2026-01-16 19:01 ` Jonathan Cameron 0 siblings, 0 replies; 28+ messages in thread From: Jonathan Cameron @ 2026-01-16 19:01 UTC (permalink / raw) To: Andy Shevchenko Cc: rodrigo.alencar, Andi Shyti, 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, 16 Jan 2026 17:26:56 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > +Cc: Andi > > On Fri, Jan 16, 2026 at 02:32:21PM +0000, Rodrigo Alencar via B4 Relay wrote: > > > > New PLL IIO driver supports output frequency of several GHz. > > The new define helps to constraint DT properties and frequency > > calculation parameters. > > There is already pending patch to add this in I²C host controller subsystem. > On one hand the conflict, if any, is easy to resolve (the other patch adds > a couple of comments). On the other we are at almost rc6 and it seems DT people > are not happy about something, which would mean that the series most likely > will miss next merge window. Linus is planning an rc8 so this 'might' merge this cycle. https://lore.kernel.org/all/CAHk-=wib+MG0grZgub=SkCpPnNXPFE1nHsDpFQz1sBwOsrV_VQ@mail.gmail.com/ > > So, the solutions are: > - do nothing and resolve conflicts, if any > - define this constant locally in the respective IIO driver and drop it after merge Do that and shout about it in the patch description. If I merge this next cycle I can clean up. > - postpone this series to the next cycle (effectively drop this patch) > - ask Andi to provide an immutable branch / tag with I²C host patches > - ask Andi to provide the only that patch in immutable tag / branch, but it > will require him to rebase his tree > > I'm skeptical about last two options on the grounds on how the IIO works and > possible rebase requirement (which is not good to have). Not worth it for a single define. I do have a request out for an i3c fix that effectively the same request, but that's for breakage that otherwise requires ifdef magic to work around (which I'll still do if no progress in a few days). Thanks for highlighting this Andy. Whilst I saw the i2c series, I've slept since then so might well have forgotten that! Jonathan > > I leave it to you and the respective maintainers to make a final decision. > > > diff --git a/include/linux/units.h b/include/linux/units.h > > index 00e15de33eca..06870e9e90b8 100644 > > --- a/include/linux/units.h > > +++ b/include/linux/units.h > > @@ -27,6 +27,7 @@ > > > > #define HZ_PER_KHZ 1000UL > > #define HZ_PER_MHZ 1000000UL > > +#define HZ_PER_GHZ 1000000000UL > > > > #define KHZ_PER_MHZ 1000UL > > #define KHZ_PER_GHZ 1000000UL > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 2/7] units: Add HZ_PER_GHZ definition Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 19:29 ` Jonathan Cameron 2026-01-19 7:31 ` Andy Shevchenko 2026-01-16 14:32 ` [PATCH v4 4/7] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay ` (3 subsequent siblings) 6 siblings, 2 replies; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 | 1166 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 1178 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 095af34e234e..1edfa6a1c641 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1624,6 +1624,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..9068c427d8e9 --- /dev/null +++ b/drivers/iio/frequency/adf41513.c @@ -0,0 +1,1166 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ADF41513 SPI PLL Frequency Synthesizer driver + * + * Copyright 2026 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 ADF41510_MAX_RF_FREQ_HZ (10ULL * HZ_PER_GHZ) +#define ADF41513_MIN_RF_FREQ_HZ (1ULL * HZ_PER_GHZ) +#define ADF41513_MAX_RF_FREQ_HZ (26500ULL * HZ_PER_MHZ) + +#define ADF41513_MIN_REF_FREQ_HZ (10 * HZ_PER_MHZ) +#define ADF41513_MAX_REF_FREQ_HZ (800 * HZ_PER_MHZ) +#define ADF41513_MAX_REF_FREQ_DOUBLER_HZ (225 * HZ_PER_MHZ) + +#define ADF41513_MAX_PFD_FREQ_INT_N_UHZ (250ULL * MEGA * MICROHZ_PER_HZ) +#define ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ (125ULL * MEGA * MICROHZ_PER_HZ) +#define ADF41513_MAX_FREQ_RESOLUTION_UHZ (100ULL * KILO * 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_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_MAX_PHASE_VAL (BIT(12) - 1) +#define ADF41513_MAX_CLK_DIVIDER (BIT(12) - 1) + +#define ADF41513_HZ_DECIMAL_PRECISION 6 +#define ADF41513_PS_BIAS_INIT 0x2 +#define ADF41513_MAX_PHASE_MICRORAD ((2 * 314159265UL) / 100) + +enum { + ADF41513_FREQ, + ADF41513_POWER_DOWN, + ADF41513_FREQ_RESOLUTION, +}; + +enum adf41513_pll_mode { + ADF41513_MODE_INVALID, + ADF41513_MODE_INTEGER_N, + ADF41513_MODE_FIXED_MODULUS, + ADF41513_MODE_VARIABLE_MODULUS, +}; + +struct adf41513_chip_info { + 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; + + bool logic_lvl_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; + u32 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_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, MICRO, &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))) + continue; + + 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 = (u64)st->ref_freq_hz * MICRO; + 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_HZ) { + 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 = (u64)st->ref_freq_hz * MICRO; + 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 * MICRO; + u64 min_rf_freq_uhz = ADF41513_MIN_RF_FREQ_HZ * MICRO; + u64 pfd_freq_limit_uhz; + int ret; + + 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; + + 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(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, MICRO), + result.prescaler ? "8/9" : "4/5"); + + 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; + + 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; + + st->regs[ADF41513_REG3] = FIELD_PREP(ADF41513_REG3_FRAC2_MSK, + st->settings.frac2); + FIELD_MODIFY(ADF41513_REG4_MOD2_MSK, &st->regs[ADF41513_REG4], + st->settings.mod2); + FIELD_MODIFY(ADF41513_REG5_R_CNT_MSK, &st->regs[ADF41513_REG5], + st->settings.r_counter); + FIELD_MODIFY(ADF41513_REG5_REF_DOUBLER_MSK, &st->regs[ADF41513_REG5], + st->settings.ref_doubler); + FIELD_MODIFY(ADF41513_REG5_RDIV2_MSK, &st->regs[ADF41513_REG5], + st->settings.ref_div2); + FIELD_MODIFY(ADF41513_REG5_PRESCALER_MSK, &st->regs[ADF41513_REG5], + 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 && + !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_powerdown(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_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_powerdown(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; + int ret; + + ret = kstrtoul(buf, 10, &readin); + if (ret) + return ret; + + guard(mutex)(&st->lock); + + switch ((u32)private) { + 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_PD_INFO(_name, _ident) { \ + .name = _name, \ + .read = adf41513_read_powerdown, \ + .write = adf41513_write_powerdown, \ + .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_PD_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); + u64 phase_urad; + 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_urad = (u64)phase_val * ADF41513_MAX_PHASE_MICRORAD; + phase_urad >>= 12; + *val = (u32)phase_urad / MICRO; + *val2 = (u32)phase_urad % MICRO; + 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); + u64 phase_urad; + u16 phase_val; + + guard(mutex)(&st->lock); + + switch (info) { + case IIO_CHAN_INFO_PHASE: + phase_urad = (u64)val * MICRO + val2; + if (val < 0 || val2 < 0 || phase_urad >= ADF41513_MAX_PHASE_MICRORAD) + return -EINVAL; + + phase_val = DIV_U64_ROUND_CLOSEST(phase_urad << 12, + ADF41513_MAX_PHASE_MICRORAD); + phase_val = min(phase_val, ADF41513_MAX_PHASE_VAL); + st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK; + FIELD_MODIFY(ADF41513_REG2_PHASE_VAL_MSK, + &st->regs[ADF41513_REG2], 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) { + if (reg <= ADF41513_REG6) + 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, cp_resistance, cp_current; + + /* power-up frequency */ + st->data.power_up_frequency_hz = ADF41510_MAX_RF_FREQ_HZ; + ret = device_property_read_u32(dev, "adi,power-up-frequency-mhz", &tmp); + if (!ret) { + st->data.power_up_frequency_hz = (u64)tmp * HZ_PER_MHZ; + if (st->data.power_up_frequency_hz < ADF41513_MIN_RF_FREQ_HZ || + st->data.power_up_frequency_hz > ADF41513_MAX_RF_FREQ_HZ) + return dev_err_probe(dev, -ERANGE, + "power-up frequency %llu Hz out of range\n", + st->data.power_up_frequency_hz); + } + + tmp = ADF41513_MIN_R_CNT; + device_property_read_u32(dev, "adi,reference-div-factor", &tmp); + 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; + + 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"); + + cp_resistance = ADF41513_DEFAULT_R_SET; + device_property_read_u32(dev, "adi,charge-pump-resistor-ohms", &cp_resistance); + if (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); + + 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; + } + + st->data.phase_detector_polarity = + device_property_read_bool(dev, "adi,phase-detector-polarity-positive-enable"); + + st->data.logic_lvl_1v8_en = device_property_read_bool(dev, "adi,logic-level-1v8-enable"); + + tmp = ADF41513_LD_COUNT_MIN; + device_property_read_u32(dev, "adi,lock-detector-count", &tmp); + 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)); + + /* assuming DLD pin is used for lock detection */ + st->regs[ADF41513_REG5] = FIELD_PREP(ADF41513_REG5_DLD_MODES_MSK, + ADF41513_DLD_DIG_LD); + + 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); + + 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; + + st->regs[ADF41513_REG7] = FIELD_PREP(ADF41513_REG7_PS_BIAS_MSK, + ADF41513_PS_BIAS_INIT); + 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); + + st->regs[ADF41513_REG11] = ADF41513_REG11_POWER_DOWN_SEL_MSK; + st->regs[ADF41513_REG12] = FIELD_PREP(ADF41513_REG12_LOGIC_LEVEL_MSK, + st->data.logic_lvl_1v8_en ? 0 : 1); + + /* perform initialization sequence with power-up frequency */ + return adf41513_set_frequency(st, st->data.power_up_frequency_hz * MICRO, + 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) +{ + return adf41513_suspend(dev_get_drvdata(dev)); +} + +static int adf41513_pm_resume(struct device *dev) +{ + return adf41513_resume(dev_get_drvdata(dev)); +} + +static const struct adf41513_chip_info adf41510_chip_info = { + .has_prescaler_8_9 = false, + .max_rf_freq_hz = ADF41510_MAX_RF_FREQ_HZ, +}; + +static const struct adf41513_chip_info adf41513_chip_info = { + .has_prescaler_8_9 = true, + .max_rf_freq_hz = ADF41513_MAX_RF_FREQ_HZ, +}; + +static int adf41513_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct adf41513_state *st; + struct device *dev = &spi->dev; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + st->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(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_HZ || + st->ref_freq_hz > ADF41513_MAX_REF_FREQ_HZ) + return dev_err_probe(dev, -ERANGE, + "reference frequency %u Hz out of range\n", + st->ref_freq_hz); + + ret = adf41513_parse_fw(st); + if (ret) + return ret; + + ret = devm_regulator_bulk_get_enable(dev, + ARRAY_SIZE(adf41513_power_supplies), + adf41513_power_supplies); + if (ret) + return dev_err_probe(dev, ret, + "failed to get and enable regulators\n"); + + st->chip_enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH); + if (IS_ERR(st->chip_enable)) + return dev_err_probe(dev, PTR_ERR(st->chip_enable), + "fail to request chip enable GPIO\n"); + + st->lock_detect = devm_gpiod_get_optional(dev, "lock-detect", GPIOD_IN); + if (IS_ERR(st->lock_detect)) + return dev_err_probe(dev, PTR_ERR(st->lock_detect), + "fail to request lock detect GPIO\n"); + + ret = devm_mutex_init(dev, &st->lock); + if (ret) + return ret; + + indio_dev->name = spi_get_device_id(spi)->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(dev, ret, "failed to setup device\n"); + + ret = devm_add_action_or_reset(dev, adf41513_power_down, st); + if (ret) + return dev_err_probe(dev, ret, "Failed to add power down action\n"); + + return devm_iio_device_register(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] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-16 14:32 ` [PATCH v4 3/7] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay @ 2026-01-16 19:29 ` Jonathan Cameron 2026-01-19 13:10 ` Rodrigo Alencar 2026-01-19 7:31 ` Andy Shevchenko 1 sibling, 1 reply; 28+ messages in thread From: Jonathan Cameron @ 2026-01-16 19:29 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, 16 Jan 2026 14:32:22 +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, A couple of additional comments for this version. Thanks, Jonathan > diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c > new file mode 100644 > index 000000000000..9068c427d8e9 > --- /dev/null > +++ b/drivers/iio/frequency/adf41513.c > + > +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); > + u64 phase_urad; > + u16 phase_val; > + > + guard(mutex)(&st->lock); > + > + switch (info) { > + case IIO_CHAN_INFO_PHASE: > + phase_urad = (u64)val * MICRO + val2; > + if (val < 0 || val2 < 0 || phase_urad >= ADF41513_MAX_PHASE_MICRORAD) Check val and val2 before setting phase_urad. Whilst it's not a bug it is a lot less readable to perform checks on inputs after you've already used them to compute something. if (val < 0 || val2 < 0) return -EINVAL; phase_urad = ... if (phase_urad >= ...) return -EINVAL; > + return -EINVAL; > + > + phase_val = DIV_U64_ROUND_CLOSEST(phase_urad << 12, > + ADF41513_MAX_PHASE_MICRORAD); > + phase_val = min(phase_val, ADF41513_MAX_PHASE_VAL); > + st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK; > + FIELD_MODIFY(ADF41513_REG2_PHASE_VAL_MSK, > + &st->regs[ADF41513_REG2], 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, cp_resistance, cp_current; > + > + /* power-up frequency */ > + st->data.power_up_frequency_hz = ADF41510_MAX_RF_FREQ_HZ; > + ret = device_property_read_u32(dev, "adi,power-up-frequency-mhz", &tmp); > + if (!ret) { Can easily do the same as below here No precision issue given definition of the _MAX_RF_FREQ_HZ value includes a larger power 10 multiplier. tmp = ADF41510_MAX_RF_FREQ_HZ / HZ_PER_MHZ; device_property_read_u32(dev, "adi,power-up-frequency-mhz", &tmp; st->data.power_up_frequency_hz = (u64)tmp * HZ_PER_MHZ; if (st... or use a local u64 for a temporary you only assign to power_up_frequency_hz after checks. That would be more consistent with the code that follows. > + st->data.power_up_frequency_hz = (u64)tmp * HZ_PER_MHZ; > + if (st->data.power_up_frequency_hz < ADF41513_MIN_RF_FREQ_HZ || > + st->data.power_up_frequency_hz > ADF41513_MAX_RF_FREQ_HZ) > + return dev_err_probe(dev, -ERANGE, > + "power-up frequency %llu Hz out of range\n", > + st->data.power_up_frequency_hz); > + } > + > + tmp = ADF41513_MIN_R_CNT; > + device_property_read_u32(dev, "adi,reference-div-factor", &tmp); > + 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; > + > + 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"); > + > + cp_resistance = ADF41513_DEFAULT_R_SET; > + device_property_read_u32(dev, "adi,charge-pump-resistor-ohms", &cp_resistance); > + if (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); > + > + st->data.charge_pump_voltage_mv = ADF41513_DEFAULT_CP_VOLTAGE_mV; This leaves some odd corner cases. If DT defines cp_resistance but not cp_current then we ignore the cp_resitance. If you want to insist it is either both or nothing, that needs enforcing in the dt-binding. I think I slightly prefer this option.. Alternative is define a default current such that the maths works to give the DEFAULT_CP_VOLTAGE_mV if both properties use defaults and use that here + document in the dt-binding as the default for this property. That may mean if only one property is set we reject the pair and fail to probe. You have comment about valid combinations in the dt-binding so that's fine. > + 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; > + } > + > + st->data.phase_detector_polarity = > + device_property_read_bool(dev, "adi,phase-detector-polarity-positive-enable"); > + > + st->data.logic_lvl_1v8_en = device_property_read_bool(dev, "adi,logic-level-1v8-enable"); > + > + tmp = ADF41513_LD_COUNT_MIN; > + device_property_read_u32(dev, "adi,lock-detector-count", &tmp); > + 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; > +} ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-16 19:29 ` Jonathan Cameron @ 2026-01-19 13:10 ` Rodrigo Alencar 0 siblings, 0 replies; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-19 13:10 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 26/01/16 07:29PM, Jonathan Cameron wrote: > On Fri, 16 Jan 2026 14:32:22 +0000 > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > ... > > + > > + cp_resistance = ADF41513_DEFAULT_R_SET; > > + device_property_read_u32(dev, "adi,charge-pump-resistor-ohms", &cp_resistance); > > + if (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); > > + > > + st->data.charge_pump_voltage_mv = ADF41513_DEFAULT_CP_VOLTAGE_mV; > > This leaves some odd corner cases. > If DT defines cp_resistance but not cp_current then we ignore the cp_resitance. > If you want to insist it is either both or nothing, that needs enforcing in the dt-binding. > I think I slightly prefer this option.. > > Alternative is define a default current such that the maths works to give the DEFAULT_CP_VOLTAGE_mV > if both properties use defaults and use that here + document in the dt-binding as the default > for this property. That may mean if only one property is set we reject the pair and fail > to probe. You have comment about valid combinations in the dt-binding so that's fine. Understood. I suppose the following in the dt-binding would be enough: dependencies: adi,charge-pump-resistor-ohms: ["adi,charge-pump-current-microamp"] as current can be defined alone (it would use the default resistor value). Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-16 14:32 ` [PATCH v4 3/7] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay 2026-01-16 19:29 ` Jonathan Cameron @ 2026-01-19 7:31 ` Andy Shevchenko 2026-01-19 11:21 ` Rodrigo Alencar 1 sibling, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-19 7:31 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, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: > 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. ... > +#define ADF41513_MAX_PFD_FREQ_INT_N_UHZ (250ULL * MEGA * MICROHZ_PER_HZ) > +#define ADF41513_MAX_PFD_FREQ_FRAC_N_UHZ (125ULL * MEGA * MICROHZ_PER_HZ) > +#define ADF41513_MAX_FREQ_RESOLUTION_UHZ (100ULL * KILO * MICROHZ_PER_HZ) Yep, thanks, looks much better now! ... > +struct adf41513_chip_info { > + bool has_prescaler_8_9; > + u64 max_rf_freq_hz; If you swap them, it might be a 4 bytes gain in some cases. Have you run `pahole`? > +}; ... > +struct adf41513_pll_settings { > + enum adf41513_pll_mode mode; Sounds to me like a room to improve the layout here, > + /* 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; > +}; ... > + * See iio_write_channel_info and __iio_str_to_fixpoint in Refer to function as func(), i.o.w. mind the parentheses. > + * drivers/iio/industrialio-core.c Missing period at the end. ... > +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) { This can be found by strchr() / strrchr() (depending on the expectations of the input). > + frac_part = true; > + } else { > + return -EINVAL; > + } > + str++; > + } With the above the rest becomes just a couple of simple_strtoull() calls with a couple of int_pow(10) calls (and some validation on top). > + for (; f_count > 0; f_count--) > + uhz *= 10; This is int_pow(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, MICRO, &frac_part); Perhaps MICROHZ_PER_HZ? This will be consistent with the int_value in _calc_*() below. > + return sysfs_emit(buf, "%llu.%06u\n", int_part, frac_part); > +} ... > + cfg->pfd_frequency_uhz = div_u64(cfg->pfd_frequency_uhz, > + cfg->r_counter); Here 81 characters... > + cfg->actual_frequency_uhz = (u64)cfg->int_value * cfg->pfd_frequency_uhz; ...and here, but in one case the line is wrapped. I would unwrap the first one. ... > + result->ref_div2 = st->data.ref_div2_en ? 1 : 0; > + result->ref_doubler = st->data.ref_doubler_en ? 1 : 0; Seems like "? 1 : 0" just redundant. ... > +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) ? Redundant parentheses. > + 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; > + } This and below the part for frac check seems very similar, I would consider adding a helper. > + 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_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); This also seems familiar with the above mentioned check (for 50% tolerance). > + /* 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, I'm wondering why mod2 can't be defined as u64. This will allow to drop castings. > + 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; > +} ... > + /* apply computed results to pll settings */ > + memcpy(&st->settings, &result, sizeof(st->settings)); Can't we simply use st->settings = result; ? ... > +static ssize_t adf41513_read_powerdown(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) { Why casting? Ditto for similar cases. > + 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; It can be Elvis operator: return ret ?: len; Ditto for similar cases. > +} ... > +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); > + u64 phase_urad; > + 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_urad = (u64)phase_val * ADF41513_MAX_PHASE_MICRORAD; > + phase_urad >>= 12; > + *val = (u32)phase_urad / MICRO; > + *val2 = (u32)phase_urad % MICRO; This needs a short comment explaining why castings are okay (i.o.w. why the 64-bit variable will contain 32-bit value). > + 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); > + u64 phase_urad; > + u16 phase_val; > + > + guard(mutex)(&st->lock); > + > + switch (info) { > + case IIO_CHAN_INFO_PHASE: > + phase_urad = (u64)val * MICRO + val2; > + if (val < 0 || val2 < 0 || phase_urad >= ADF41513_MAX_PHASE_MICRORAD) > + return -EINVAL; It's better to check val* before use them. > + phase_val = DIV_U64_ROUND_CLOSEST(phase_urad << 12, > + ADF41513_MAX_PHASE_MICRORAD); > + phase_val = min(phase_val, ADF41513_MAX_PHASE_VAL); > + st->regs[ADF41513_REG2] |= ADF41513_REG2_PHASE_ADJ_MSK; > + FIELD_MODIFY(ADF41513_REG2_PHASE_VAL_MSK, > + &st->regs[ADF41513_REG2], phase_val); > + return adf41513_sync_config(st, ADF41513_SYNC_REG0); > + default: > + return -EINVAL; > + } > +} ... > +static void adf41513_power_down(void *data) > +{ > + struct adf41513_state *st = data; > + > + adf41513_suspend(st); > + if (st->chip_enable) Remove this dup check. > + gpiod_set_value_cansleep(st->chip_enable, 0); > +} ... > +static int adf41513_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct adf41513_state *st; > + struct device *dev = &spi->dev; > + int ret; Please, use reversed xmas tree ordering. > + indio_dev = devm_iio_device_alloc(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(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_HZ || > + st->ref_freq_hz > ADF41513_MAX_REF_FREQ_HZ) > + return dev_err_probe(dev, -ERANGE, > + "reference frequency %u Hz out of range\n", > + st->ref_freq_hz); > + > + ret = adf41513_parse_fw(st); > + if (ret) > + return ret; > + > + ret = devm_regulator_bulk_get_enable(dev, > + ARRAY_SIZE(adf41513_power_supplies), > + adf41513_power_supplies); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to get and enable regulators\n"); > + > + st->chip_enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH); > + if (IS_ERR(st->chip_enable)) > + return dev_err_probe(dev, PTR_ERR(st->chip_enable), > + "fail to request chip enable GPIO\n"); > + > + st->lock_detect = devm_gpiod_get_optional(dev, "lock-detect", GPIOD_IN); > + if (IS_ERR(st->lock_detect)) > + return dev_err_probe(dev, PTR_ERR(st->lock_detect), > + "fail to request lock detect GPIO\n"); > + > + ret = devm_mutex_init(dev, &st->lock); > + if (ret) > + return ret; > + > + indio_dev->name = spi_get_device_id(spi)->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(dev, ret, "failed to setup device\n"); > + > + ret = devm_add_action_or_reset(dev, adf41513_power_down, st); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to add power down action\n"); > + > + return devm_iio_device_register(dev, indio_dev); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-19 7:31 ` Andy Shevchenko @ 2026-01-19 11:21 ` Rodrigo Alencar 2026-01-19 13:42 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-19 11:21 UTC (permalink / raw) To: Andy Shevchenko, 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 26/01/19 09:31AM, Andy Shevchenko wrote: > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > +struct adf41513_pll_settings { > > + enum adf41513_pll_mode mode; > > Sounds to me like a room to improve the layout here, > I am targeting a 32-bit cpu, just moved in_value down: would this be fine? (pahole output): struct adf41513_pll_settings { enum adf41513_pll_mode mode; /* 0 4 */ u8 r_counter; /* 4 1 */ u8 ref_doubler; /* 5 1 */ u8 ref_div2; /* 6 1 */ u8 prescaler; /* 7 1 */ u64 target_frequency_uhz; /* 8 8 */ u64 actual_frequency_uhz; /* 16 8 */ u64 pfd_frequency_uhz; /* 24 8 */ u32 frac1; /* 32 4 */ u32 frac2; /* 36 4 */ u32 mod2; /* 40 4 */ u16 int_value; /* 44 2 */ /* size: 48, cachelines: 1, members: 12 */ /* padding: 2 */ /* last cacheline: 48 bytes */ }; > > + /* 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; > > +}; > ... > > +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) { > > This can be found by strchr() / strrchr() (depending on the expectations of > the input). > > > + frac_part = true; > > + } else { > > + return -EINVAL; > > + } > > + str++; > > + } > > With the above the rest becomes just a couple of simple_strtoull() calls with > a couple of int_pow(10) calls (and some validation on top). > > > + for (; f_count > 0; f_count--) > > + uhz *= 10; > > This is int_pow(10). > > > + *freq_uhz = uhz; > > + > > + return 0; > > +} The current implementation is kind of a stripped version of __iio_str_to_fixpoint(). Would you prefer something like this, then?: static int adf41513_parse_uhz(const char *str, u64 *freq_uhz) { u64 integer_part = 0, fractional_part = 0; const char *decimal_point; char *endptr; int frac_digits; if (str[0] == '+') str++; /* Find decimal point */ decimal_point = strchr(str, '.'); if (decimal_point) { /* Parse integer part (if exists before decimal point) */ if (decimal_point > str) { integer_part = simple_strtoull(str, &endptr, 10); if (endptr != decimal_point) return -EINVAL; } /* Parse fractional part */ fractional_part = simple_strtoull(decimal_point + 1, &endptr, 10); if (*endptr != '\0' && *endptr != '\n') return -EINVAL; /* Adjust for desired precision */ frac_digits = strcspn(decimal_point + 1, "\n"); if (frac_digits > ADF41513_HZ_DECIMAL_PRECISION) fractional_part /= int_pow(10, frac_digits - ADF41513_HZ_DECIMAL_PRECISION); else fractional_part *= int_pow(10, ADF41513_HZ_DECIMAL_PRECISION - frac_digits); } else { /* No decimal point - just parse the integer */ ret = kstrtoull(str, 10, &integer_part); if (ret) return ret; } /* Combine integer and fractional parts */ *freq_uhz = integer_part * int_pow(10, ADF41513_HZ_DECIMAL_PRECISION) + fractional_part; return 0; } > ... > > > +static int adf41513_uhz_to_str(u64 freq_uhz, char *buf) > > +{ > > + u32 frac_part; > > + u64 int_part = div_u64_rem(freq_uhz, MICRO, &frac_part); > > Perhaps MICROHZ_PER_HZ? This will be consistent with the int_value in > _calc_*() below. Here, the meaning is different. int_part is in Hz and frac_part in uHz. Will add the suffixes to the variables. > > + return sysfs_emit(buf, "%llu.%06u\n", int_part, frac_part); > > +} > ... > > + 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; > > + } > > This and below the part for frac check seems very similar, I would consider > adding a helper. > It is similar, but in each case different variables are being handled. This one we can only deal with INT, the next one FRAC1 and the last MOD2. I am not sure how a single helper can deal with all of them separately. > > + if (freq_error_uhz > st->data.freq_resolution_uhz) > > + 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); > > This also seems familiar with the above mentioned check (for 50% tolerance). Here, there is no check, MOD2 has plenty of range to deal with the requested frequency resolution. Also this is the last attempt, after integer mode and fixed-modulus failed to achieve the requested frequency value. Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-19 11:21 ` Rodrigo Alencar @ 2026-01-19 13:42 ` Andy Shevchenko 2026-01-19 16:37 ` Rodrigo Alencar 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-19 13:42 UTC (permalink / raw) To: Rodrigo Alencar Cc: rodrigo.alencar, 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 Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > +struct adf41513_pll_settings { > > > + enum adf41513_pll_mode mode; > > > > Sounds to me like a room to improve the layout here, > > I am targeting a 32-bit cpu, just moved in_value down: > would this be fine? (pahole output): Likely. > struct adf41513_pll_settings { > enum adf41513_pll_mode mode; /* 0 4 */ Wondering if this can be shorter if moved down... > u8 r_counter; /* 4 1 */ > u8 ref_doubler; /* 5 1 */ > u8 ref_div2; /* 6 1 */ > u8 prescaler; /* 7 1 */ > u64 target_frequency_uhz; /* 8 8 */ > u64 actual_frequency_uhz; /* 16 8 */ > u64 pfd_frequency_uhz; /* 24 8 */ > u32 frac1; /* 32 4 */ > u32 frac2; /* 36 4 */ > u32 mod2; /* 40 4 */ > u16 int_value; /* 44 2 */ > > /* size: 48, cachelines: 1, members: 12 */ > /* padding: 2 */ > /* last cacheline: 48 bytes */ > }; ...at least I have had in mind that "mode" should be moved to be near to "int_value". But I think it will take 4 bytes still as we don't use short enums compile wise. > > > + /* 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; > > > +}; ... > > > +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) { > > > > This can be found by strchr() / strrchr() (depending on the expectations of > > the input). > > > > > + frac_part = true; > > > + } else { > > > + return -EINVAL; > > > + } > > > + str++; > > > + } > > > > With the above the rest becomes just a couple of simple_strtoull() calls with > > a couple of int_pow(10) calls (and some validation on top). > > > > > + for (; f_count > 0; f_count--) > > > + uhz *= 10; > > > > This is int_pow(10). > > > > > + *freq_uhz = uhz; > > > + > > > + return 0; > > > +} > > The current implementation is kind of a stripped version of > __iio_str_to_fixpoint(). Would you prefer something like this, then?: Do they have most of the parts in common? If so, why can't we use __iio_str_to_fixpoint() directly? Or why can't we slightly refactor that to give us the results we need here? > static int adf41513_parse_uhz(const char *str, u64 *freq_uhz) > { > u64 integer_part = 0, fractional_part = 0; > const char *decimal_point; > char *endptr; > int frac_digits; > > if (str[0] == '+') > str++; > > /* Find decimal point */ > decimal_point = strchr(str, '.'); > if (decimal_point) { > /* Parse integer part (if exists before decimal point) */ > if (decimal_point > str) { I don't think you need this check, simple_strtoull() should return 0. Also check the ranges, perhaps you want in some cases simple_strtoul(). > integer_part = simple_strtoull(str, &endptr, 10); > if (endptr != decimal_point) > return -EINVAL; > } > > /* Parse fractional part */ > fractional_part = simple_strtoull(decimal_point + 1, &endptr, 10); The idea of using the simple strtoull() (second "l") is to check for overflows, so if the number is > UINT_MAX we probably should return -ERANGE. We have somewhere already such a code in the kernel, maybe it's a time to have advanced version of simple_strtouint(). drivers/crypto/intel/qat/qat_common/qat_uclo.c:206: ae = simple_strtoull(str, &end, 10); drivers/crypto/intel/qat/qat_common/qat_uclo.c-207- if (ae > UINT_MAX || str == end || (end - str) > 19) drivers/crypto/intel/qat/qat_common/qat_uclo.c-208- return -EINVAL; > if (*endptr != '\0' && *endptr != '\n') > return -EINVAL; > > /* Adjust for desired precision */ > frac_digits = strcspn(decimal_point + 1, "\n"); This is already precalculated: endptr - decimal_point (+ 1 ?). > if (frac_digits > ADF41513_HZ_DECIMAL_PRECISION) > fractional_part /= int_pow(10, frac_digits - ADF41513_HZ_DECIMAL_PRECISION); > else > fractional_part *= int_pow(10, ADF41513_HZ_DECIMAL_PRECISION - frac_digits); > } else { > /* No decimal point - just parse the integer */ > ret = kstrtoull(str, 10, &integer_part); > if (ret) > return ret; > } > > /* Combine integer and fractional parts */ > *freq_uhz = integer_part * int_pow(10, ADF41513_HZ_DECIMAL_PRECISION) + fractional_part; > > return 0; > } ... > > > +static int adf41513_uhz_to_str(u64 freq_uhz, char *buf) > > > +{ > > > + u32 frac_part; > > > + u64 int_part = div_u64_rem(freq_uhz, MICRO, &frac_part); > > > > Perhaps MICROHZ_PER_HZ? This will be consistent with the int_value in > > _calc_*() below. > > Here, the meaning is different. int_part is in Hz and frac_part in uHz. > Will add the suffixes to the variables. Yes, but here it's a constant divisor, and not a multiplier. Meaning that one divides µHz by µHz. > > > + return sysfs_emit(buf, "%llu.%06u\n", int_part, frac_part); > > > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-19 13:42 ` Andy Shevchenko @ 2026-01-19 16:37 ` Rodrigo Alencar 2026-01-19 17:07 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-19 16:37 UTC (permalink / raw) To: Andy Shevchenko, Rodrigo Alencar Cc: rodrigo.alencar, 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 26/01/19 03:42PM, Andy Shevchenko wrote: > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > +struct adf41513_pll_settings { > > > > + enum adf41513_pll_mode mode; > > > > > > Sounds to me like a room to improve the layout here, > > > > I am targeting a 32-bit cpu, just moved in_value down: > > would this be fine? (pahole output): > > Likely. > > > struct adf41513_pll_settings { > > enum adf41513_pll_mode mode; /* 0 4 */ > > Wondering if this can be shorter if moved down... > > > u8 r_counter; /* 4 1 */ > > u8 ref_doubler; /* 5 1 */ > > u8 ref_div2; /* 6 1 */ > > u8 prescaler; /* 7 1 */ > > u64 target_frequency_uhz; /* 8 8 */ > > u64 actual_frequency_uhz; /* 16 8 */ > > u64 pfd_frequency_uhz; /* 24 8 */ > > u32 frac1; /* 32 4 */ > > u32 frac2; /* 36 4 */ > > u32 mod2; /* 40 4 */ > > u16 int_value; /* 44 2 */ > > > > /* size: 48, cachelines: 1, members: 12 */ > > /* padding: 2 */ > > /* last cacheline: 48 bytes */ > > }; > > ...at least I have had in mind that "mode" should be moved to be near > to "int_value". But I think it will take 4 bytes still as we don't use > short enums compile wise. > As you mentioned without short-enums it does not make any difference. > > > > +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) { > > > > > > This can be found by strchr() / strrchr() (depending on the expectations of > > > the input). > > > > > > > + frac_part = true; > > > > + } else { > > > > + return -EINVAL; > > > > + } > > > > + str++; > > > > + } > > > > > > With the above the rest becomes just a couple of simple_strtoull() calls with > > > a couple of int_pow(10) calls (and some validation on top). > > > > > > > + for (; f_count > 0; f_count--) > > > > + uhz *= 10; > > > > > > This is int_pow(10). > > > > > > > + *freq_uhz = uhz; > > > > + > > > > + return 0; > > > > +} > > > > The current implementation is kind of a stripped version of > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > Do they have most of the parts in common? If so, why can't we use > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > that to give us the results we need here? __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz was modified to accomodate the u64 parsing removing unnecessary stuff. I am preparing V5 to use simple_strtoull. Thanks for early review and suggestions. Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-19 16:37 ` Rodrigo Alencar @ 2026-01-19 17:07 ` Andy Shevchenko 2026-01-20 10:43 ` Rodrigo Alencar 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-19 17:07 UTC (permalink / raw) To: Rodrigo Alencar Cc: rodrigo.alencar, 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 Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > +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) { > > > > > > > > This can be found by strchr() / strrchr() (depending on the expectations of > > > > the input). > > > > > > > > > + frac_part = true; > > > > > + } else { > > > > > + return -EINVAL; > > > > > + } > > > > > + str++; > > > > > + } > > > > > > > > With the above the rest becomes just a couple of simple_strtoull() calls with > > > > a couple of int_pow(10) calls (and some validation on top). > > > > > > > > > + for (; f_count > 0; f_count--) > > > > > + uhz *= 10; > > > > > > > > This is int_pow(10). > > > > > > > > > + *freq_uhz = uhz; > > > > > + > > > > > + return 0; > > > > > +} > > > > > > The current implementation is kind of a stripped version of > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > Do they have most of the parts in common? If so, why can't we use > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > that to give us the results we need here? > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > was modified to accomodate the u64 parsing removing unnecessary stuff. But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). The integer can be bigger than 10⁹? > I am preparing V5 to use simple_strtoull. Thanks for early review > and suggestions. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-19 17:07 ` Andy Shevchenko @ 2026-01-20 10:43 ` Rodrigo Alencar 2026-01-20 11:24 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-20 10:43 UTC (permalink / raw) To: Andy Shevchenko, Rodrigo Alencar Cc: rodrigo.alencar, 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 26/01/19 07:07PM, Andy Shevchenko wrote: > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > +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) { > > > > > > > > > > This can be found by strchr() / strrchr() (depending on the expectations of > > > > > the input). > > > > > > > > > > > + frac_part = true; > > > > > > + } else { > > > > > > + return -EINVAL; > > > > > > + } > > > > > > + str++; > > > > > > + } > > > > > > > > > > With the above the rest becomes just a couple of simple_strtoull() calls with > > > > > a couple of int_pow(10) calls (and some validation on top). > > > > > > > > > > > + for (; f_count > 0; f_count--) > > > > > > + uhz *= 10; > > > > > > > > > > This is int_pow(10). > > > > > > > > > > > + *freq_uhz = uhz; > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > > > The current implementation is kind of a stripped version of > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > Do they have most of the parts in common? If so, why can't we use > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > that to give us the results we need here? > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > The integer can be bigger than 10⁹? > Correct, integer part of the frequency value goes up to 26.5 GHz (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can achieve micro Hz resolution. Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-20 10:43 ` Rodrigo Alencar @ 2026-01-20 11:24 ` Andy Shevchenko 2026-01-20 13:07 ` Rodrigo Alencar 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-20 11:24 UTC (permalink / raw) To: Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote: > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > The current implementation is kind of a stripped version of > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > that to give us the results we need here? > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > The integer can be bigger than 10⁹? > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > achieve micro Hz resolution. µHz is not a problem since it's up to nHz. So, the difference so far is the integer part that can be 64-bit. Again, can we factor out something to be used for this and for the __iio_str_to_fixpoint() cases? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-20 11:24 ` Andy Shevchenko @ 2026-01-20 13:07 ` Rodrigo Alencar 2026-01-20 13:38 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-20 13:07 UTC (permalink / raw) To: Andy Shevchenko, Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 26/01/20 01:24PM, Andy Shevchenko wrote: > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > <455.rodrigo.alencar@gmail.com> wrote: > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > The current implementation is kind of a stripped version of > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > that to give us the results we need here? > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > The integer can be bigger than 10⁹? > > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > achieve micro Hz resolution. > > µHz is not a problem since it's up to nHz. > So, the difference so far is the integer part that can be 64-bit. > Again, can we factor out something to be used for this and for the > __iio_str_to_fixpoint() cases? I am not sure what you are suggesting, but I am avoiding changes to iio core at this point. If any other user needs similar behavior, I'd say we would need to have __iio_str_to_fixpoint() implementation modified, so to create a version of iio_str_to_fixpoint() that handles long long variables. Possibly consuming simple_strtoull instead of doing the manual parsing. Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-20 13:07 ` Rodrigo Alencar @ 2026-01-20 13:38 ` Andy Shevchenko 2026-01-21 9:41 ` Rodrigo Alencar 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-20 13:38 UTC (permalink / raw) To: Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 Tue, Jan 20, 2026 at 01:07:49PM +0000, Rodrigo Alencar wrote: > On 26/01/20 01:24PM, Andy Shevchenko wrote: > > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > > <455.rodrigo.alencar@gmail.com> wrote: > > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > > The current implementation is kind of a stripped version of > > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > > that to give us the results we need here? > > > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > > The integer can be bigger than 10⁹? > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > > achieve micro Hz resolution. > > > > µHz is not a problem since it's up to nHz. > > So, the difference so far is the integer part that can be 64-bit. > > Again, can we factor out something to be used for this and for the > > __iio_str_to_fixpoint() cases? > > I am not sure what you are suggesting, To make changes to reuse the code. > but I am avoiding changes to iio core at this point. Why? > If any other user needs similar behavior, > I'd say we would need to have __iio_str_to_fixpoint() implementation > modified, so to create a version of iio_str_to_fixpoint() that handles > long long variables. Possibly consuming simple_strtoull instead of > doing the manual parsing. That's the problem here. With Yet Another Cool Parser this all becomes unmaintainable very soon (basically as you said when new comer needs a third variant of it). This is not good. Instead better to create (amend, expand) existing test cases, split out a foundation API that parses 64-bit parts (maybe even for fractional as well, dunno) and evolve a needed (sub)API from it. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-20 13:38 ` Andy Shevchenko @ 2026-01-21 9:41 ` Rodrigo Alencar 2026-01-21 9:52 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar @ 2026-01-21 9:41 UTC (permalink / raw) To: Andy Shevchenko, Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 26/01/20 03:38PM, Andy Shevchenko wrote: > On Tue, Jan 20, 2026 at 01:07:49PM +0000, Rodrigo Alencar wrote: > > On 26/01/20 01:24PM, Andy Shevchenko wrote: > > > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > > > <455.rodrigo.alencar@gmail.com> wrote: > > > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > > > The current implementation is kind of a stripped version of > > > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > > > that to give us the results we need here? > > > > > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > > > The integer can be bigger than 10⁹? > > > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > > > achieve micro Hz resolution. > > > > > > µHz is not a problem since it's up to nHz. > > > So, the difference so far is the integer part that can be 64-bit. > > > Again, can we factor out something to be used for this and for the > > > __iio_str_to_fixpoint() cases? > > > > I am not sure what you are suggesting, > > To make changes to reuse the code. > > > but I am avoiding changes to iio core at this point. > > Why? I understood that core changes would require more than one user supporting the change. > > If any other user needs similar behavior, > > I'd say we would need to have __iio_str_to_fixpoint() implementation > > modified, so to create a version of iio_str_to_fixpoint() that handles > > long long variables. Possibly consuming simple_strtoull instead of > > doing the manual parsing. > > That's the problem here. With Yet Another Cool Parser this all becomes > unmaintainable very soon Considering that the need for a new parser for 64-bit parts is only driven by this specific PLL driver, I wonder how things become that unmaintainable. > (basically as you said when new comer needs a third > variant of it). This is not good. Instead better to create (amend, expand) > existing test cases, split out a foundation API that parses 64-bit parts > (maybe even for fractional as well, dunno) and evolve a needed (sub)API > from it. I don't disagree with you though, I suppose I will need a green light to move on with this? Kind regards, Rodrigo Alencar ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-21 9:41 ` Rodrigo Alencar @ 2026-01-21 9:52 ` Andy Shevchenko 2026-01-21 14:21 ` Nuno Sá 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2026-01-21 9:52 UTC (permalink / raw) To: Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 Wed, Jan 21, 2026 at 09:41:25AM +0000, Rodrigo Alencar wrote: > On 26/01/20 03:38PM, Andy Shevchenko wrote: > > On Tue, Jan 20, 2026 at 01:07:49PM +0000, Rodrigo Alencar wrote: > > > On 26/01/20 01:24PM, Andy Shevchenko wrote: > > > > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > > > > <455.rodrigo.alencar@gmail.com> wrote: > > > > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > > > > The current implementation is kind of a stripped version of > > > > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > > > > that to give us the results we need here? > > > > > > > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > > > > The integer can be bigger than 10⁹? > > > > > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > > > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > > > > achieve micro Hz resolution. > > > > > > > > µHz is not a problem since it's up to nHz. > > > > So, the difference so far is the integer part that can be 64-bit. > > > > Again, can we factor out something to be used for this and for the > > > > __iio_str_to_fixpoint() cases? > > > > > > I am not sure what you are suggesting, > > > > To make changes to reuse the code. > > > > > but I am avoiding changes to iio core at this point. > > > > Why? > > I understood that core changes would require more than one user > supporting the change. At least one. And we have tons of them as the callers of __iio_str_to_fixpoint() are not going to disappear. Basically it's a surgery in the middle of the existing chain of APIs. To me one user is enough justification for such a surgery. For the newly introduced API (imagine __iio_str_to_fixpoint() as an example) it's indeed one user not enough. > > > If any other user needs similar behavior, > > > I'd say we would need to have __iio_str_to_fixpoint() implementation > > > modified, so to create a version of iio_str_to_fixpoint() that handles > > > long long variables. Possibly consuming simple_strtoull instead of > > > doing the manual parsing. > > > > That's the problem here. With Yet Another Cool Parser this all becomes > > unmaintainable very soon > > Considering that the need for a new parser for 64-bit parts is only driven > by this specific PLL driver, I wonder how things become that unmaintainable. Because there is a duplication of the code (to some extent) and if we found a bug in the one implementation it will be hard to fix (or even remeber) about the other. > > (basically as you said when new comer needs a third > > variant of it). This is not good. Instead better to create (amend, expand) > > existing test cases, split out a foundation API that parses 64-bit parts > > (maybe even for fractional as well, dunno) and evolve a needed (sub)API > > from it. > > I don't disagree with you though, I suppose I will need a green light to > move on with this? Fine with me, let's gather opinions of David, Nuno, Jonathan, and others. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-21 9:52 ` Andy Shevchenko @ 2026-01-21 14:21 ` Nuno Sá 2026-01-22 19:33 ` Jonathan Cameron 0 siblings, 1 reply; 28+ messages in thread From: Nuno Sá @ 2026-01-21 14:21 UTC (permalink / raw) To: Andy Shevchenko, Rodrigo Alencar Cc: Andy Shevchenko, rodrigo.alencar, 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 Wed, 2026-01-21 at 11:52 +0200, Andy Shevchenko wrote: > On Wed, Jan 21, 2026 at 09:41:25AM +0000, Rodrigo Alencar wrote: > > On 26/01/20 03:38PM, Andy Shevchenko wrote: > > > On Tue, Jan 20, 2026 at 01:07:49PM +0000, Rodrigo Alencar wrote: > > > > On 26/01/20 01:24PM, Andy Shevchenko wrote: > > > > > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > > > > > <455.rodrigo.alencar@gmail.com> wrote: > > > > > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > > > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: > > ... > > > > > > > > > > > The current implementation is kind of a stripped version of > > > > > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > > > > > that to give us the results we need here? > > > > > > > > > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > > > > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > > > > > The integer can be bigger than 10⁹? > > > > > > > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > > > > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > > > > > achieve micro Hz resolution. > > > > > > > > > > µHz is not a problem since it's up to nHz. > > > > > So, the difference so far is the integer part that can be 64-bit. > > > > > Again, can we factor out something to be used for this and for the > > > > > __iio_str_to_fixpoint() cases? > > > > > > > > I am not sure what you are suggesting, > > > > > > To make changes to reuse the code. > > > > > > > but I am avoiding changes to iio core at this point. > > > > > > Why? > > > > I understood that core changes would require more than one user > > supporting the change. > > At least one. And we have tons of them as the callers of > __iio_str_to_fixpoint() are not going to disappear. Basically it's a surgery in > the middle of the existing chain of APIs. To me one user is enough justification > for such a surgery. For the newly introduced API (imagine __iio_str_to_fixpoint() > as an example) it's indeed one user not enough. > > > > > If any other user needs similar behavior, > > > > I'd say we would need to have __iio_str_to_fixpoint() implementation > > > > modified, so to create a version of iio_str_to_fixpoint() that handles > > > > long long variables. Possibly consuming simple_strtoull instead of > > > > doing the manual parsing. > > > > > > That's the problem here. With Yet Another Cool Parser this all becomes > > > unmaintainable very soon > > > > Considering that the need for a new parser for 64-bit parts is only driven > > by this specific PLL driver, I wonder how things become that unmaintainable. > > Because there is a duplication of the code (to some extent) and if we found > a bug in the one implementation it will be hard to fix (or even remeber) about > the other. Agreed! > > > > (basically as you said when new comer needs a third > > > variant of it). This is not good. Instead better to create (amend, expand) > > > existing test cases, split out a foundation API that parses 64-bit parts > > > (maybe even for fractional as well, dunno) and evolve a needed (sub)API > > > from it. > > > > I don't disagree with you though, I suppose I will need a green light to > > move on with this? > > Fine with me, let's gather opinions of David, Nuno, Jonathan, and others. Fine with me. - Nuno Sá ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 3/7] iio: frequency: adf41513: driver implementation 2026-01-21 14:21 ` Nuno Sá @ 2026-01-22 19:33 ` Jonathan Cameron 0 siblings, 0 replies; 28+ messages in thread From: Jonathan Cameron @ 2026-01-22 19:33 UTC (permalink / raw) To: Nuno Sá Cc: Andy Shevchenko, Rodrigo Alencar, Andy Shevchenko, 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 Wed, 21 Jan 2026 14:21:00 +0000 Nuno Sá <noname.nuno@gmail.com> wrote: > On Wed, 2026-01-21 at 11:52 +0200, Andy Shevchenko wrote: > > On Wed, Jan 21, 2026 at 09:41:25AM +0000, Rodrigo Alencar wrote: > > > On 26/01/20 03:38PM, Andy Shevchenko wrote: > > > > On Tue, Jan 20, 2026 at 01:07:49PM +0000, Rodrigo Alencar wrote: > > > > > On 26/01/20 01:24PM, Andy Shevchenko wrote: > > > > > > On Tue, Jan 20, 2026 at 12:43 PM Rodrigo Alencar > > > > > > <455.rodrigo.alencar@gmail.com> wrote: > > > > > > > On 26/01/19 07:07PM, Andy Shevchenko wrote: > > > > > > > > On Mon, Jan 19, 2026 at 04:37:09PM +0000, Rodrigo Alencar wrote: > > > > > > > > > On 26/01/19 03:42PM, Andy Shevchenko wrote: > > > > > > > > > > On Mon, Jan 19, 2026 at 11:21:59AM +0000, Rodrigo Alencar wrote: > > > > > > > > > > > On 26/01/19 09:31AM, Andy Shevchenko wrote: > > > > > > > > > > > > On Fri, Jan 16, 2026 at 02:32:22PM +0000, Rodrigo Alencar via B4 Relay wrote: > > > > ... > > > > > > > > > > > > > The current implementation is kind of a stripped version of > > > > > > > > > > > __iio_str_to_fixpoint(). Would you prefer something like this, then?: > > > > > > > > > > > > > > > > > > > > Do they have most of the parts in common? If so, why can't we use > > > > > > > > > > __iio_str_to_fixpoint() directly? Or why can't we slightly refactor > > > > > > > > > > that to give us the results we need here? > > > > > > > > > > > > > > > > > > __iio_str_to_fixpoint() only parses "int" chunks, adf41513_parse_uhz > > > > > > > > > was modified to accomodate the u64 parsing removing unnecessary stuff. > > > > > > > > > > > > > > > > But why? The fractional part most likely will be kept int (it's up to 10⁻⁹). > > > > > > > > The integer can be bigger than 10⁹? > > > > > > > > > > > > > > Correct, integer part of the frequency value goes up to 26.5 GHz > > > > > > > (uint_max is approx 4.3 GHz). Also, with the dual modulus, the PLL can > > > > > > > achieve micro Hz resolution. > > > > > > > > > > > > µHz is not a problem since it's up to nHz. > > > > > > So, the difference so far is the integer part that can be 64-bit. > > > > > > Again, can we factor out something to be used for this and for the > > > > > > __iio_str_to_fixpoint() cases? > > > > > > > > > > I am not sure what you are suggesting, > > > > > > > > To make changes to reuse the code. > > > > > > > > > but I am avoiding changes to iio core at this point. > > > > > > > > Why? > > > > > > I understood that core changes would require more than one user > > > supporting the change. > > > > At least one. And we have tons of them as the callers of > > __iio_str_to_fixpoint() are not going to disappear. Basically it's a surgery in > > the middle of the existing chain of APIs. To me one user is enough justification > > for such a surgery. For the newly introduced API (imagine __iio_str_to_fixpoint() > > as an example) it's indeed one user not enough. > > > > > > > If any other user needs similar behavior, > > > > > I'd say we would need to have __iio_str_to_fixpoint() implementation > > > > > modified, so to create a version of iio_str_to_fixpoint() that handles > > > > > long long variables. Possibly consuming simple_strtoull instead of > > > > > doing the manual parsing. > > > > > > > > That's the problem here. With Yet Another Cool Parser this all becomes > > > > unmaintainable very soon > > > > > > Considering that the need for a new parser for 64-bit parts is only driven > > > by this specific PLL driver, I wonder how things become that unmaintainable. > > > > Because there is a duplication of the code (to some extent) and if we found > > a bug in the one implementation it will be hard to fix (or even remeber) about > > the other. > > Agreed! > > > > > > > (basically as you said when new comer needs a third > > > > variant of it). This is not good. Instead better to create (amend, expand) > > > > existing test cases, split out a foundation API that parses 64-bit parts > > > > (maybe even for fractional as well, dunno) and evolve a needed (sub)API > > > > from it. > > > > > > I don't disagree with you though, I suppose I will need a green light to > > > move on with this? > > > > Fine with me, let's gather opinions of David, Nuno, Jonathan, and others. > > > Fine with me. Likewise. A common / foundational function with specialized wrappers would be good. > > - Nuno Sá > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 4/7] iio: frequency: adf41513: handle LE synchronization feature 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay ` (2 preceding siblings ...) 2026-01-16 14:32 ` [PATCH v4 3/7] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay ` (2 subsequent siblings) 6 siblings, 0 replies; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c index 9068c427d8e9..dfbe55bb6973 100644 --- a/drivers/iio/frequency/adf41513.c +++ b/drivers/iio/frequency/adf41513.c @@ -220,6 +220,7 @@ struct adf41513_data { bool phase_detector_polarity; bool logic_lvl_1v8_en; + bool le_sync_en; }; struct adf41513_pll_settings { @@ -698,13 +699,27 @@ 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) + 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); + if (ret) + return ret; + } + + return ret; } static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev, @@ -989,6 +1004,8 @@ static int adf41513_parse_fw(struct adf41513_state *st) "invalid lock detect count: %u\n", tmp); 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; @@ -996,6 +1013,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)); @@ -1029,8 +1047,19 @@ static int adf41513_setup(struct adf41513_state *st) st->data.logic_lvl_1v8_en ? 0 : 1); /* perform initialization sequence with power-up frequency */ - return adf41513_set_frequency(st, st->data.power_up_frequency_hz * MICRO, - ADF41513_SYNC_ALL); + ret = adf41513_set_frequency(st, st->data.power_up_frequency_hz * MICRO, + ADF41513_SYNC_ALL); + if (ret) + 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); + if (ret) + return ret; + } + + return 0; } static void adf41513_power_down(void *data) -- 2.43.0 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay ` (3 preceding siblings ...) 2026-01-16 14:32 ` [PATCH v4 4/7] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 15:36 ` Andy Shevchenko 2026-01-16 14:32 ` [PATCH v4 6/7] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay 6 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 | 100 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c index dfbe55bb6973..2a983b9b8fab 100644 --- a/drivers/iio/frequency/adf41513.c +++ b/drivers/iio/frequency/adf41513.c @@ -20,6 +20,7 @@ #include <linux/property.h> #include <linux/regulator/consumer.h> #include <linux/spi/spi.h> +#include <linux/time64.h> #include <linux/types.h> #include <linux/units.h> @@ -211,6 +212,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; @@ -275,6 +277,16 @@ struct adf41513_state { __be32 buf __aligned(IIO_DMA_MINALIGN); }; +static const u16 adf41513_ld_window_x10_ns[] = { + 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_power_supplies[] = { "avdd1", "avdd2", "avdd3", "avdd4", "avdd5", "vp", }; @@ -642,9 +654,82 @@ 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 * MEGA * MICROHZ_PER_HZ); + + FIELD_MODIFY(ADF41513_REG6_BLEED_CURRENT_MSK, &st->regs[ADF41513_REG6], + 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_10x_ns = div64_u64(10ULL * NSEC_PER_SEC * 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_10x_ns += 4; + else + ld_window_10x_ns += 6; + } + + ld_idx = find_closest(ld_window_10x_ns, adf41513_ld_window_x10_ns, + ARRAY_SIZE(adf41513_ld_window_x10_ns)); + ldp = (adf41513_ldp_bias[ld_idx] >> 2) & 0x3; + ld_bias = adf41513_ldp_bias[ld_idx] & 0x3; + + FIELD_MODIFY(ADF41513_REG6_LDP_MSK, &st->regs[ADF41513_REG6], ldp); + FIELD_MODIFY(ADF41513_REG9_LD_BIAS_MSK, &st->regs[ADF41513_REG9], 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); + + FIELD_MODIFY(ADF41513_REG5_CLK1_DIV_MSK, &st->regs[ADF41513_REG5], + clk1_div); + FIELD_MODIFY(ADF41513_REG7_CLK2_DIV_MSK, &st->regs[ADF41513_REG7], + 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; ret = adf41513_calc_pll_settings(st, &result, freq_uhz); @@ -652,6 +737,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(st->settings)); dev_dbg(&st->spi->dev, @@ -693,6 +780,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); } @@ -994,6 +1089,11 @@ static int adf41513_parse_fw(struct adf41513_state *st) st->data.phase_detector_polarity = device_property_read_bool(dev, "adi,phase-detector-polarity-positive-enable"); + 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; + st->data.logic_lvl_1v8_en = device_property_read_bool(dev, "adi,logic-level-1v8-enable"); tmp = ADF41513_LD_COUNT_MIN; -- 2.43.0 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change 2026-01-16 14:32 ` [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay @ 2026-01-16 15:36 ` Andy Shevchenko 0 siblings, 0 replies; 28+ messages in thread From: Andy Shevchenko @ 2026-01-16 15:36 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, Jan 16, 2026 at 02:32:24PM +0000, Rodrigo Alencar via B4 Relay wrote: > 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. ... > +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]); I think it's better to have yet another temporary variable for this multiplier... > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, ...and here three operands instead. bleed_value = div64_u64(st->settings.pfd_frequency_uhz * curr * bleed_value, > + 1600ULL * MEGA * MICROHZ_PER_HZ); > + > + FIELD_MODIFY(ADF41513_REG6_BLEED_CURRENT_MSK, &st->regs[ADF41513_REG6], > + 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_10x_ns = div64_u64(10ULL * NSEC_PER_SEC * MICROHZ_PER_HZ, > + st->settings.pfd_frequency_uhz << 1); Okay, if we go this direction... > + 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_10x_ns += 4; > + else > + ld_window_10x_ns += 6; > + } > + > + ld_idx = find_closest(ld_window_10x_ns, adf41513_ld_window_x10_ns, > + ARRAY_SIZE(adf41513_ld_window_x10_ns)); > + ldp = (adf41513_ldp_bias[ld_idx] >> 2) & 0x3; > + ld_bias = adf41513_ldp_bias[ld_idx] & 0x3; > + > + FIELD_MODIFY(ADF41513_REG6_LDP_MSK, &st->regs[ADF41513_REG6], ldp); > + FIELD_MODIFY(ADF41513_REG9_LD_BIAS_MSK, &st->regs[ADF41513_REG9], 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); ...for the consistency we may also use the same approach here 1ULL * MICROHZ_PER_HZ * NSEC_PER_SEC); At least it will be consistent with the annihilated units. > + 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); > + > + FIELD_MODIFY(ADF41513_REG5_CLK1_DIV_MSK, &st->regs[ADF41513_REG5], > + clk1_div); > + FIELD_MODIFY(ADF41513_REG7_CLK2_DIV_MSK, &st->regs[ADF41513_REG7], > + clk2_div); > + > + /* enable phase resync */ > + st->regs[ADF41513_REG7] |= ADF41513_REG7_CLK_DIV_MODE_MSK; > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 6/7] docs: iio: add documentation for adf41513 driver 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay ` (4 preceding siblings ...) 2026-01-16 14:32 ` [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay 6 siblings, 0 replies; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 | 199 +++++++++++++++++++++++++++++++++++++++++ Documentation/iio/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 201 insertions(+) diff --git a/Documentation/iio/adf41513.rst b/Documentation/iio/adf41513.rst new file mode 100644 index 000000000000..4193c825b532 --- /dev/null +++ b/Documentation/iio/adf41513.rst @@ -0,0 +1,199 @@ +.. SPDX-License-Identifier: GPL-2.0 + +=============== +ADF41513 driver +=============== + +This driver supports Analog Devices' 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: + +- **ADF41510**: 1 GHz to 10 GHz frequency range +- **ADF41513**: 1 GHz to 26.5 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) | ++----------------------+-------------------------------------------------------+ +| powerdown | Power management control (0=active, 1=power down) | ++----------------------+-------------------------------------------------------+ +| phase | RF output phase adjustment and readback (radians) | ++----------------------+-------------------------------------------------------+ + +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:** + +- **ADF41510**: 1,000,000,000 Hz to 10,000,000,000 Hz (1 GHz to 10 GHz) +- **ADF41513**: 1,000,000,000 Hz to 26,500,000,000 Hz (1 GHz to 26.5 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 Phase adjustment +-------------------- + +The ``phase`` attribute allows adjustment of the output phase in radians. +Setting this attribute enables phase adjustment. It can be set from 0 to 2*pi +radians. 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 + +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 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 ba3e609c6a13..605871765c78 100644 --- a/Documentation/iio/index.rst +++ b/Documentation/iio/index.rst @@ -30,6 +30,7 @@ Industrial I/O Kernel Drivers ad7625 ad7944 ade9000 + adf41513 adis16475 adis16480 adis16550 diff --git a/MAINTAINERS b/MAINTAINERS index 1edfa6a1c641..e61619af9248 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1624,6 +1624,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] 28+ messages in thread
* [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay ` (5 preceding siblings ...) 2026-01-16 14:32 ` [PATCH v4 6/7] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 ` Rodrigo Alencar via B4 Relay 2026-01-16 15:28 ` Andy Shevchenko 6 siblings, 1 reply; 28+ messages in thread From: Rodrigo Alencar via B4 Relay @ 2026-01-16 14:32 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 file for PLL/DDS devices with frequency_resolution sysfs entry attribute used by ADF4350 and ADF41513 Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> --- Documentation/ABI/testing/sysfs-bus-iio-frequency | 11 +++++++++++ MAINTAINERS | 1 + 2 files changed, 12 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency b/Documentation/ABI/testing/sysfs-bus-iio-frequency new file mode 100644 index 000000000000..1ce8ae578fd6 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency @@ -0,0 +1,11 @@ +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 for PLL + devices. The given value directly influences the operating mode when + fractional-N synthesis is required, as it derives values for + configurable modulus parameters used in the calculation of the output + frequency. 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. diff --git a/MAINTAINERS b/MAINTAINERS index e61619af9248..46a88498cc58 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1623,6 +1623,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 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] 28+ messages in thread
* Re: [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency 2026-01-16 14:32 ` [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay @ 2026-01-16 15:28 ` Andy Shevchenko 0 siblings, 0 replies; 28+ messages in thread From: Andy Shevchenko @ 2026-01-16 15:28 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, Jan 16, 2026 at 02:32:26PM +0000, Rodrigo Alencar via B4 Relay wrote: > Add ABI documentation file for PLL/DDS devices with frequency_resolution > sysfs entry attribute used by ADF4350 and ADF41513 In this and previous patch the commit messages miss the last period in them. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2026-01-22 19:33 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-16 14:32 [PATCH v4 0/7] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 1/7] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay 2026-01-16 14:35 ` Krzysztof Kozlowski 2026-01-16 14:32 ` [PATCH v4 2/7] units: Add HZ_PER_GHZ definition Rodrigo Alencar via B4 Relay 2026-01-16 15:26 ` Andy Shevchenko 2026-01-16 19:01 ` Jonathan Cameron 2026-01-16 14:32 ` [PATCH v4 3/7] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay 2026-01-16 19:29 ` Jonathan Cameron 2026-01-19 13:10 ` Rodrigo Alencar 2026-01-19 7:31 ` Andy Shevchenko 2026-01-19 11:21 ` Rodrigo Alencar 2026-01-19 13:42 ` Andy Shevchenko 2026-01-19 16:37 ` Rodrigo Alencar 2026-01-19 17:07 ` Andy Shevchenko 2026-01-20 10:43 ` Rodrigo Alencar 2026-01-20 11:24 ` Andy Shevchenko 2026-01-20 13:07 ` Rodrigo Alencar 2026-01-20 13:38 ` Andy Shevchenko 2026-01-21 9:41 ` Rodrigo Alencar 2026-01-21 9:52 ` Andy Shevchenko 2026-01-21 14:21 ` Nuno Sá 2026-01-22 19:33 ` Jonathan Cameron 2026-01-16 14:32 ` [PATCH v4 4/7] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 5/7] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay 2026-01-16 15:36 ` Andy Shevchenko 2026-01-16 14:32 ` [PATCH v4 6/7] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay 2026-01-16 14:32 ` [PATCH v4 7/7] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay 2026-01-16 15:28 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox