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 | 99 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
index 0cdf24989c93..c838e219ca22 100644
--- a/drivers/iio/frequency/adf41513.c
+++ b/drivers/iio/frequency/adf41513.c
@@ -211,6 +211,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 +276,16 @@ struct adf41513_state {
__be32 buf __aligned(IIO_DMA_MINALIGN);
};
+static const u16 adf41513_ld_window_p1ns[] = {
+ 9, 12, 16, 17, 21, 28, 29, 35, /* 0 - 7 */
+ 43, 47, 49, 52, 70, 79, 115 /* 8 - 14 */
+};
+
+static const u8 adf41513_ldp_bias[] = {
+ 0xC, 0xD, 0xE, 0x8, 0x9, 0x4, 0xA, 0x5, /* 0 - 7 */
+ 0x0, 0x6, 0xB, 0x1, 0x2, 0x7, 0x3 /* 8 - 14 */
+};
+
static const char * const adf41513_power_supplies[] = {
"avdd1", "avdd2", "avdd3", "avdd4", "avdd5", "vp"
};
@@ -641,9 +652,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 * HZ_PER_MHZ * 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_p1ns = div64_u64(10ULL * NANO * MICROHZ_PER_HZ,
+ st->settings.pfd_frequency_uhz << 1);
+ u8 ld_idx, ldp, ld_bias;
+
+ if (st->settings.mode != ADF41513_MODE_INTEGER_N) {
+ /* account for bleed current (deduced from eq.6 and eq.7) */
+ if (st->data.phase_detector_polarity)
+ ld_window_p1ns += 4;
+ else
+ ld_window_p1ns += 6;
+ }
+
+ ld_idx = find_closest(ld_window_p1ns, adf41513_ld_window_p1ns,
+ ARRAY_SIZE(adf41513_ld_window_p1ns));
+ ldp = (adf41513_ldp_bias[ld_idx] >> 2) & 0x3;
+ ld_bias = adf41513_ldp_bias[ld_idx] & 0x3;
+
+ 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);
@@ -651,6 +735,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,
@@ -692,6 +778,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);
}
@@ -995,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");
st->data.lock_detect_count = ADF41513_LD_COUNT_MIN;
--
2.43.0
On Thu, Jan 08, 2026 at 12:14:53PM +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 const u16 adf41513_ld_window_p1ns[] = {
> + 9, 12, 16, 17, 21, 28, 29, 35, /* 0 - 7 */
> + 43, 47, 49, 52, 70, 79, 115 /* 8 - 14 */
Leave trailing comma.
> +};
> +
> +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 */
Ditto.
> +};
> +
> static const char * const adf41513_power_supplies[] = {
> "avdd1", "avdd2", "avdd3", "avdd4", "avdd5", "vp"
Ditto.
> };
...
> + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value,
> + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ);
> + u16 ld_window_p1ns = div64_u64(10ULL * NANO * MICROHZ_PER_HZ,
> + st->settings.pfd_frequency_uhz << 1);
These multiplications (here and elsewhere) are (very) confusing.
I believe you want to have a frequency in Hz in µHz resolution. The second one
can be close to this if used GIGA instead of NANO. But I think the better way
to have something like the first one but with MICRO instead of MICROHZ_PER_HZ.
Please, put an order in these.
...
> + /* 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);
This sounds good as we multiply Hz by ns.
...
> + st->data.phase_resync_period_ns = 0;
Do we even need this?
> + ret = device_property_read_u32(dev, "adi,phase-resync-period-ns", &tmp);
> + if (!ret)
> + st->data.phase_resync_period_ns = tmp;
Is the _period_ns of type u32? Then simply
device_property_read_u32(dev, "adi,phase-resync-period-ns", &st->data.phase_resync_period_ns);
--
With Best Regards,
Andy Shevchenko
On 26/01/09 09:07PM, Andy Shevchenko wrote:
> On Thu, Jan 08, 2026 at 12:14:53PM +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 const u16 adf41513_ld_window_p1ns[] = {
> > + 9, 12, 16, 17, 21, 28, 29, 35, /* 0 - 7 */
> > + 43, 47, 49, 52, 70, 79, 115 /* 8 - 14 */
>
> Leave trailing comma.
>
> > +};
> > +
> > +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 */
>
> Ditto.
>
> > +};
> > +
> > static const char * const adf41513_power_supplies[] = {
> > "avdd1", "avdd2", "avdd3", "avdd4", "avdd5", "vp"
>
> Ditto.
>
> > };
>
> ...
>
> > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value,
> > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ);
>
> > + u16 ld_window_p1ns = div64_u64(10ULL * NANO * MICROHZ_PER_HZ,
> > + st->settings.pfd_frequency_uhz << 1);
>
> These multiplications (here and elsewhere) are (very) confusing.
>
> I believe you want to have a frequency in Hz in µHz resolution. The second one
> can be close to this if used GIGA instead of NANO. But I think the better way
> to have something like the first one but with MICRO instead of MICROHZ_PER_HZ.
>
> Please, put an order in these.
The first one: the numerator is in µHz, so the denominator is also in µHz so to
cancel the units.
The second one: window size is nanoseconds with 0.1 precision in the datasheet.
The numerator contains MICROHZ_PER_HZ to convert µHz -> Hz = 1/s, and then
10ULL * NANO to convert 1/s into 0.1 ns.
How is that confusing? I am not sure GIGA is the right choice, as NANO shows
that I am targeting nanoseconds, no?
> ...
>
> > + /* 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);
>
> This sounds good as we multiply Hz by ns.
>
the numerator has a time in nanoseconds, so NANO 'cancels' that, as MICRO 'cancels'
the micro under µHz.
> ...
>
> > + st->data.phase_resync_period_ns = 0;
>
> Do we even need this?
>
true, will adjust.
> > + ret = device_property_read_u32(dev, "adi,phase-resync-period-ns", &tmp);
> > + if (!ret)
> > + st->data.phase_resync_period_ns = tmp;
>
> Is the _period_ns of type u32? Then simply
>
> device_property_read_u32(dev, "adi,phase-resync-period-ns", &st->data.phase_resync_period_ns);
>
> --
> With Best Regards,
> Andy Shevchenko
>
kind regards,
Rodrigo Alencar
On Mon, Jan 12, 2026 at 09:45:49AM +0000, Rodrigo Alencar wrote: > On 26/01/09 09:07PM, Andy Shevchenko wrote: > > On Thu, Jan 08, 2026 at 12:14:53PM +0000, Rodrigo Alencar via B4 Relay wrote: First of all, remove the things you are agree with. ... A side note: based on this discussion one may want to add a clarification on how to use the unit-based multipliers to the documentation (top comment on units.h also will work). ... > > > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, > > > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ); You multiply Hz * Hz. One of them should be simply SI multiplier. To me it sounds like one of 1600ULL * MEGA * MICROHZ_PER_HZ); 1600ULL * HZ_PER_MHZ * MICRO); will be the correct one (and I lean towards the first one as you want units to match). The same is done in the definitions above somewhere. > > > + u16 ld_window_p1ns = div64_u64(10ULL * NANO * MICROHZ_PER_HZ, > > > + st->settings.pfd_frequency_uhz << 1); > > > > These multiplications (here and elsewhere) are (very) confusing. > > > > I believe you want to have a frequency in Hz in µHz resolution. The second one > > can be close to this if used GIGA instead of NANO. But I think the better way > > to have something like the first one but with MICRO instead of MICROHZ_PER_HZ. > > > > Please, put an order in these. > > The first one: the numerator is in µHz, so the denominator is also in µHz so to > cancel the units. > > The second one: window size is nanoseconds with 0.1 precision in the datasheet. > The numerator contains MICROHZ_PER_HZ to convert µHz -> Hz = 1/s, and then > 10ULL * NANO to convert 1/s into 0.1 ns. > > How is that confusing? I am not sure GIGA is the right choice, as NANO shows > that I am targeting nanoseconds, no? So, You wanted then one of u16 ld_window_p1ns = div64_u64(10ULL * NSEC_PER_SEC * MICROHZ_PER_HZ, u16 ld_window_p1ns = div64_u64(10ULL * NANO * MICRO, (and I lean towards the first one as it may hint about the scale and resulting units). Also make units in the name to be delimited with _. u16 ld_window_p1_ns = ... ... > > > + /* 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); > > > > This sounds good as we multiply Hz by ns. > > the numerator has a time in nanoseconds, so NANO 'cancels' that, as MICRO 'cancels' > the micro under µHz. Exactly, that's why I replied it sounds good. -- With Best Regards, Andy Shevchenko
On Mon, 12 Jan 2026 12:54:50 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Mon, Jan 12, 2026 at 09:45:49AM +0000, Rodrigo Alencar wrote: > > On 26/01/09 09:07PM, Andy Shevchenko wrote: > > > On Thu, Jan 08, 2026 at 12:14:53PM +0000, Rodrigo Alencar via B4 Relay wrote: > > First of all, remove the things you are agree with. > > ... > > A side note: based on this discussion one may want to add a clarification > on how to use the unit-based multipliers to the documentation (top comment > on units.h also will work). > > ... > > > > > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, > > > > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ); > > You multiply Hz * Hz. One of them should be simply SI multiplier. > To me it sounds like one of > > 1600ULL * MEGA * MICROHZ_PER_HZ); > 1600ULL * HZ_PER_MHZ * MICRO); > > will be the correct one (and I lean towards the first one as you want units > to match). I don't really care, but... They are Hz * Hz / Hz * Hz / Hz = HZ if we assume the first number is in Hz. The others are all ratios. So original is fine as far as I can tell. Jonathan
On Fri, Jan 16, 2026 at 05:57:43PM +0000, Jonathan Cameron wrote: > On Mon, 12 Jan 2026 12:54:50 +0200 > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Mon, Jan 12, 2026 at 09:45:49AM +0000, Rodrigo Alencar wrote: > > > On 26/01/09 09:07PM, Andy Shevchenko wrote: > > > > On Thu, Jan 08, 2026 at 12:14:53PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, > > > > > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ); > > > > You multiply Hz * Hz. One of them should be simply SI multiplier. > > To me it sounds like one of > > > > 1600ULL * MEGA * MICROHZ_PER_HZ); > > 1600ULL * HZ_PER_MHZ * MICRO); > > > > will be the correct one (and I lean towards the first one as you want units > > to match). > > I don't really care, but... They are Hz * Hz / Hz * Hz / Hz = HZ > if we assume the first number is in Hz. The others are all ratios. > > So original is fine as far as I can tell. I don't see it like this. I consider that we should have only one meaningful units as the rest is just a value. What you wrote above has a little sense to me, sorry. -- With Best Regards, Andy Shevchenko
On Mon, 19 Jan 2026 09:38:09 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Fri, Jan 16, 2026 at 05:57:43PM +0000, Jonathan Cameron wrote: > > On Mon, 12 Jan 2026 12:54:50 +0200 > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > On Mon, Jan 12, 2026 at 09:45:49AM +0000, Rodrigo Alencar wrote: > > > > On 26/01/09 09:07PM, Andy Shevchenko wrote: > > > > > On Thu, Jan 08, 2026 at 12:14:53PM +0000, Rodrigo Alencar via B4 Relay wrote: > > ... > > > > > > > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, > > > > > > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ); > > > > > > You multiply Hz * Hz. One of them should be simply SI multiplier. > > > To me it sounds like one of > > > > > > 1600ULL * MEGA * MICROHZ_PER_HZ); > > > 1600ULL * HZ_PER_MHZ * MICRO); > > > > > > will be the correct one (and I lean towards the first one as you want units > > > to match). > > > > I don't really care, but... They are Hz * Hz / Hz * Hz / Hz = HZ > > if we assume the first number is in Hz. The others are all ratios. > > > > So original is fine as far as I can tell. > > I don't see it like this. I consider that we should have only one meaningful > units as the rest is just a value. What you wrote above has a little sense > to me, sorry. > I agree, but none of those XHZ PER HZ is mathematically valid way of applying a unit. This is because the per means divide so the units cancel out. Literally it's (0.0000001Hz / 1Hz) So using them to assign a unit is meaningless. All they are doing is hinting that we are manipulating values already in some scaling of Hz. Personally I'm not sure there is value in the unit specific defines given this. They kind of hint we are dealing with frequencies, but that's it. Jonathan
On Mon, Jan 19, 2026 at 10:41:59AM +0000, Jonathan Cameron wrote: > On Mon, 19 Jan 2026 09:38:09 +0200 > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Fri, Jan 16, 2026 at 05:57:43PM +0000, Jonathan Cameron wrote: > > > On Mon, 12 Jan 2026 12:54:50 +0200 > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > On Mon, Jan 12, 2026 at 09:45:49AM +0000, Rodrigo Alencar wrote: > > > > > On 26/01/09 09:07PM, Andy Shevchenko wrote: > > > > > > On Thu, Jan 08, 2026 at 12:14:53PM +0000, Rodrigo Alencar via B4 Relay wrote: ... > > > > > > > + bleed_value = div64_u64(st->settings.pfd_frequency_uhz * bleed_value, > > > > > > > + 1600ULL * HZ_PER_MHZ * MICROHZ_PER_HZ); > > > > > > > > You multiply Hz * Hz. One of them should be simply SI multiplier. > > > > To me it sounds like one of > > > > > > > > 1600ULL * MEGA * MICROHZ_PER_HZ); > > > > 1600ULL * HZ_PER_MHZ * MICRO); > > > > > > > > will be the correct one (and I lean towards the first one as you want units > > > > to match). > > > > > > I don't really care, but... They are Hz * Hz / Hz * Hz / Hz = HZ > > > if we assume the first number is in Hz. The others are all ratios. > > > > > > So original is fine as far as I can tell. > > > > I don't see it like this. I consider that we should have only one meaningful > > units as the rest is just a value. What you wrote above has a little sense > > to me, sorry. > > > > I agree, but none of those XHZ PER HZ is mathematically valid way of applying a unit. > This is because the per means divide so the units cancel out. > Literally it's (0.0000001Hz / 1Hz) > So using them to assign a unit is meaningless. All they are doing is hinting > that we are manipulating values already in some scaling of Hz. My understanding is that they give a hint about units and used scale (which is also provided by the unit suffix in the respective variable name in this case). In some cases the variables do not have suffixes and having a named multiplier helps that. > Personally I'm not sure there is value in the unit specific defines given > this. They kind of hint we are dealing with frequencies, but that's it. -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.