[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20240830061639.2316-1-ganboing@gmail.com>
Date: Thu, 29 Aug 2024 23:16:39 -0700
From: Bo Gan <ganboing@...il.com>
To: linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org,
sboyd@...nel.org
Cc: samuel.holland@...ive.com,
emil.renner.berthing@...onical.com,
mturquette@...libre.com,
paul.walmsley@...ive.com
Subject: [PATCH v2] clk: analogbits: Fix incorrect calculation of vco rate delta
In function `wrpll_configure_for_rate`, we try to determine the best PLL
configuration for a target rate. However, in the loop where we try values
of R, we should compare the derived `vco` with `target_vco_rate`. However,
we were in fact comparing it with `target_rate`, which is actually after
Q shift. This is incorrect, and sometimes can result in suboptimal clock
rates. This patch fixes it.
Fixes: 7b9487a9a5c4 ("clk: analogbits: add Wide-Range PLL library")
Signed-off-by: Bo Gan <ganboing@...il.com>
---
v1 -> v2: Add Fixes tag
drivers/clk/analogbits/wrpll-cln28hpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c b/drivers/clk/analogbits/wrpll-cln28hpc.c
index 65d422a588e1..9d178afc73bd 100644
--- a/drivers/clk/analogbits/wrpll-cln28hpc.c
+++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
@@ -255,81 +255,81 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 target_rate,
}
c->flags &= ~WRPLL_FLAGS_BYPASS_MASK;
/* Calculate the Q shift and target VCO rate */
divq = __wrpll_calc_divq(target_rate, &target_vco_rate);
if (!divq)
return -1;
c->divq = divq;
/* Precalculate the pre-Q divider target ratio */
ratio = div64_u64((target_vco_rate << ROUND_SHIFT), parent_rate);
fbdiv = __wrpll_calc_fbdiv(c);
best_r = 0;
best_f = 0;
best_delta = MAX_VCO_FREQ;
/*
* Consider all values for R which land within
* [MIN_POST_DIVR_FREQ, MAX_POST_DIVR_FREQ]; prefer smaller R
*/
for (r = c->init_r; r <= c->max_r; ++r) {
f_pre_div = ratio * r;
f = (f_pre_div + (1 << ROUND_SHIFT)) >> ROUND_SHIFT;
f >>= (fbdiv - 1);
post_divr_freq = div_u64(parent_rate, r);
vco_pre = fbdiv * post_divr_freq;
vco = vco_pre * f;
/* Ensure rounding didn't take us out of range */
if (vco > target_vco_rate) {
--f;
vco = vco_pre * f;
} else if (vco < MIN_VCO_FREQ) {
++f;
vco = vco_pre * f;
}
- delta = abs(target_rate - vco);
+ delta = abs(target_vco_rate - vco);
if (delta < best_delta) {
best_delta = delta;
best_r = r;
best_f = f;
}
}
c->divr = best_r - 1;
c->divf = best_f - 1;
post_divr_freq = div_u64(parent_rate, best_r);
/* Pick the best PLL jitter filter */
range = __wrpll_calc_filter_range(post_divr_freq);
if (range < 0)
return range;
c->range = range;
return 0;
}
EXPORT_SYMBOL_GPL(wrpll_configure_for_rate);
/**
* wrpll_calc_output_rate() - calculate the PLL's target output rate
* @c: ptr to a struct wrpll_cfg record to read from
* @parent_rate: PLL refclk rate
*
* Given a pointer to the PLL's current input configuration @c and the
* PLL's input reference clock rate @parent_rate (before the R
* pre-divider), calculate the PLL's output clock rate (after the Q
* post-divider).
*
* Context: Any context. Caller must protect the memory pointed to by @c
* from simultaneous modification.
*
* Return: the PLL's output clock rate, in Hz. The return value from
* this function is intended to be convenient to pass directly
* to the Linux clock framework; thus there is no explicit
* error return value.
*/
--
2.34.1
Powered by blists - more mailing lists