Re: [PATCH RFC 0/3] clk: meson: Refactor PLL pre-divider as a divider clock
From: Jian Hu
Date: Fri Oct 09 2026 - 02:37:51 EST
Hi Jerome,
Thanks for your review.
On 9/24/2026 5:35 PM, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On mer. 23 sept. 2026 at 19:14, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@xxxxxxxxxx> wrote:
This series refactors the Meson PLL framework to remove the dedicatedSo if I summarize this RFC, you have simply taken the divider out of the
PLL pre-divider (N) parameter from the PLL implementation and model it
as a separate divider clock.
Currently, the Meson PLL framework models the PLL pre-divider using a
dedicated n field in struct meson_clk_pll_data. This makes the
pre-divider part of the PLL-specific implementation, although the
Common Clock Framework already provides a generic divider clock.
This series separates the pre-divider from the PLL and makes the PLL
DCO take the pre-divider clock as its parent. This allows the
pre-divider to be modeled using the standard CCF divider implementation
and simplifies the PLL framework.
The series currently covers T7 as an RFC to get feedback on the
framework design before applying the same approach to other SoCs.
Series:
clk: meson: pll: Remove the dedicated n parameter
dt-bindings: clock: amlogic: Add T7 pre-divider clock IDs
clk: meson: t7: Model PLL pre-divider as a divider clock
The other Meson SoCs will be converted separately after the T7 PLL
framework refactoring has been reviewed and the overall approach is
agreed upon.
Any feedback on the proposed clock hierarchy and the separation of the
PLL pre-divider from the PLL itself would be appreciated.
PLL, no futher addaptation. right ?
I'm happy with it on the general principle and fine with the change as
long as you test it on as much platform as you can, clearly flagging
those you have just compiled tested.
A change like this would likely need to land early in the cycle give as
much time as possible for testing.
However there a couple of thing I'm concerned about:
* You've drop the table support: are you sure this is not needed anymore
? don't you want to be able to restrict mutlipliers to specific values
sometimes ? If not, then OK.
* the determine_rate() make no call to round the parent rate: Since the
parent will be the divier, how do you progate the rate change so N
moves and the best parent rate is found ? For sure this fractional
multiplier clock will need CLK_SET_RATE_PARENT to adjust the
pre-divider.
* Goes with the point above, but I'm not seeing anything that favors
lower N for lower jitter, Or mention of a minimum input rate (which
could be a property) ?
Those are constraints I think I have understood from your explanation
here [1] but maybe you've got new information to share ?
This is overall going in the right direction but determine_rate() and
constraints need work.
Note: you are more likely to get test feedback if you add g12 (sm1) as an
example. Those are still the most widely used amlogic platforms with
mainline.
[1]: https://lore.kernel.org/linux-clk/c9c4945f-cdfc-4382-b8ca-71b69d91deb4@xxxxxxxxxxx/
Yes, your summary is correct: the RFC simply takes the pre-divider out of the PLL.
1) Keeping the table support
Agreed, removing it was premature. Some tables cannot be expressed as
a multiplier range:
- pinned (m, n) pairs, e.g. axg PCIe GP0 (m=200, n=3) and meson8m2
GP0 (m=182, n=3)
- a pinned m, e.g. g12a PCIe PLL (m=150)
- sparse tables, e.g. meson8b HDMI PLL
The per-platform conversions will turn the tables with contiguous m
and n = 1 into range, and keep the tables for the rest, so
the framework will support both.
2) N is fixed per PLL
The main new information: the pre-divider is not meant to be selected
dynamically. Per the hardware design, each PLL has a single fixed N
value, defined together with the rest of the PLL: N = 1 for most PLLs,
and N = 3 for a few special cases on older SoCs (the axg PCIe PLL and
the meson8m2 GP0 PLL). The PLL input frequency constraints are
respected by this fixed value.
Keeping N at 1 minimizes PLL jitter and yields the best performance.
So this is less about dropping the constraints than about the fact
that there is nothing to choose at runtime: no N search in
determine_rate(), and no PFD input frequency constraint to enforce,
since the PLL input frequency is a constant for each PLL.
3) No CLK_SET_RATE_PARENT on the DCO
With N fixed, the pre-divider rate never changes, so the DCO does not
set CLK_SET_RATE_PARENT on purpose: the flag would claim that the
parent rate may change to satisfy the child, which is not the case
here. determine_rate() never touches best_parent_rate so the flag
would be a no operation today.
4) Implementation methods for pre-divider clock
While testing the RFC I found that with the single-entry pre-divider
table ({val = 1, div = 1}), the pre-divider register can never actually
be programmed.
The pre-divider field resets to 0, which is not a valid setting.
During registration, recalc_rate() reports the parent rate for the
zero register value. With N = 1 the reported rate is the parent rate,
and the single-entry table also rounds every rate request back to the
parent rate, so clk_set_rate() always bails out early (rounded rate ==
current rate) and clk_regmap_div_set_rate() is never called. The
register keeps its invalid reset value.
v2 programs the fixed N at registration time, with a new
init_val field in clk_regmap_div_data, applied from .init() once the
regmap is available.
Or do you have any other good ideas?
The patch is available[1], Please help to review it.
5) Testing and rollout
So far this has been boot tested on T7. For v2 I will
convert the SoCs one by one, starting with g12a and sm1 which
have the most mainline users, then the remaining platforms.
[1]
--- a/drivers/clk/meson/clk-regmap.c
+++ b/drivers/clk/meson/clk-regmap.c
@@ -163,10 +163,28 @@ static int clk_regmap_div_set_rate(struct clk_hw *hw, unsigned long rate,
clk_div_mask(div->width) << div->shift, val);
};
+static int clk_regmap_div_init(struct clk_hw *hw)
+{
+ int ret;
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
+
+ ret = clk_regmap_init(hw);
+ if (ret)
+ return ret;
+
+ if (div->init_val)
+ regmap_update_bits(clk->map, div->offset,
+ clk_div_mask(div->width) << div->shift,
+ div->init_val << div->shift);
+
+ return 0;
+}
+
/* Would prefer clk_regmap_div_ro_ops but clashes with qcom */
const struct clk_ops clk_regmap_divider_ops = {
- .init = clk_regmap_init,
+ .init = clk_regmap_div_init,
Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>--
---
Jian Hu (3):
clk: meson: pll: Remove the dedicated n parameter
dt-bindings: clock: amlogic: Add T7 pre-divider clock IDs
clk: meson: t7: Model PLL pre-divider as a divider clock
drivers/clk/meson/clk-pll.c | 178 +++++------------------
drivers/clk/meson/clk-pll.h | 13 --
drivers/clk/meson/t7-pll.c | 183 ++++++++++++++++++------
include/dt-bindings/clock/amlogic,t7-pll-clkc.h | 6 +
4 files changed, 181 insertions(+), 199 deletions(-)
---
base-commit: 43e1705ecab981c66baee89041e6f728c0436f19
change-id: 20260923-meson_refactor_n-e7f25904e536
Best regards,
--
Jian Hu <jian.hu@xxxxxxxxxxx>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-amlogic
Jerome
--
Jian