Re: [PATCH v2 1/3] clk: meson: Support PLL with fixed fractional denominators
From: Chuan Liu
Date: Sun Sep 08 2024 - 21:56:08 EST
Hi, Jerome, Thank you for your quick reply! I didn't get back to you
because of the weekend.
On 2024/9/6 19:22, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Fri 06 Sep 2024 at 18:34, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@xxxxxxxxxx> wrote:
From: Chuan Liu <chuan.liu@xxxxxxxxxxx>
Some PLLS with fractional multipliers have fractional denominators with
fixed values, instead of the previous "(1 << pll-> frc.width)".
Signed-off-by: Chuan Liu <chuan.liu@xxxxxxxxxxx>
---
drivers/clk/meson/clk-pll.c | 8 +++++---
drivers/clk/meson/clk-pll.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index bc570a2ff3a3..a141ab450009 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -57,12 +57,13 @@ static unsigned long __pll_params_to_rate(unsigned long parent_rate,
struct meson_clk_pll_data *pll)
{
u64 rate = (u64)parent_rate * m;
+ unsigned int frac_max = unlikely(pll->frac_max) ? pll->frac_max
:
^ Please don't play with this unless
you've got justification a for it.
Sorry, I don't quite understand this one. Is it because you suggest keeping
"(1 << pll->frac_max)" here, followed by "if" to determine whether to assign
"pll->frac_max"?
"unlikely" is used here. My idea is that it will be possible to
determine the value
of "frac_max" at compile time, which will result in one less "if"
judgment and
slightly improve drive performance.
By justification, I mean actual numbers showing the difference it makes
and not just for the platforms listed in this series, but all the
platforms supported by this driver.
You're right, In this way, even if the latter value changes, our driver
will be
compatible.
+ (1 << pll->frac.width);
if (frac && MESON_PARM_APPLICABLE(&pll->frac)) {
u64 frac_rate = (u64)parent_rate * frac;
- rate += DIV_ROUND_UP_ULL(frac_rate,
- (1 << pll->frac.width));
+ rate += DIV_ROUND_UP_ULL(frac_rate, frac_max);
}
return DIV_ROUND_UP_ULL(rate, n);
@@ -100,7 +101,8 @@ static unsigned int __pll_params_with_frac(unsigned long rate,
unsigned int n,
struct meson_clk_pll_data *pll)
{
- unsigned int frac_max = (1 << pll->frac.width);
+ unsigned int frac_max = unlikely(pll->frac_max) ? pll->frac_max :
+ (1 << pll->frac.width);
u64 val = (u64)rate * n;
/* Bail out if we are already over the requested rate */
diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
index 7b6b87274073..949157fb7bf5 100644
--- a/drivers/clk/meson/clk-pll.h
+++ b/drivers/clk/meson/clk-pll.h
@@ -43,6 +43,7 @@ struct meson_clk_pll_data {
unsigned int init_count;
const struct pll_params_table *table;
const struct pll_mult_range *range;
+ unsigned int frac_max;
u8 flags;
};
--
Jerome