[PATCH 0/7] Implement CPU frequency scaling for TH1520

[PATCH 0/7] Implement CPU frequency scaling for TH1520

[Yao Zi]

On TH1520 SoC, c910_clk feeds the CPU cluster. It could be glitchlessly
reparented to one of the two PLLs: either to cpu_pll0 indirectly through
c910_i0_clk, or to cpu_pll1 directly. This series fixes a bug in PLL
enabling code, supports rate change for PLL, and finally implements
frequency scaling support for c910_clk.

However, to achieve reliable frequency scaling, CPU voltage must be
adjusted together with frequency, and AON-firmware-based PMIC support
for TH1520 SoC is still missing in mainline. Thus PATCH 7 that fills OPP
table for TH1520 CPU and enables CPUfreq is only for testing purpose,
not intended for upstream (yet).

Testing is done on Lichee Pi 4A board, only operating points safe
to be used with the the default PMIC configuration are enabled in
devicetree. I've confirmed there's a performance gain when running
coremark and some building work compared to the case without cpufreq.

This series is based on next-20251120, thanks for your time and review.

Yao Zi (7):
  dt-bindings: clock: thead,th1520-clk-ap: Add ID for C910 bus clock
  clk: thead: th1520-ap: Poll for PLL lock and wait for stability
  clk: thead: th1520-ap: Add C910 bus clock
  clk: thead: th1520-ap: Support setting PLL rates
  clk: thead: th1520-ap: Add macro to define multiplexers with flags
  clk: thead: th1520-ap: Support CPU frequency scaling
  [Not For Upstream] riscv: dts: thead: Add CPU clock and OPP table for
    TH1520

 arch/riscv/boot/dts/thead/th1520.dtsi         |  35 ++
 drivers/clk/thead/clk-th1520-ap.c             | 350 +++++++++++++++++-
 .../dt-bindings/clock/thead,th1520-clk-ap.h   |   1 +
 3 files changed, 379 insertions(+), 7 deletions(-)

-- 
2.51.2

[PATCH 1/7] dt-bindings: clock: thead,th1520-clk-ap: Add ID for C910 bus clock

[Yao Zi]

Add binding ID for C910 bus clock, which takes CLK_C910 as parent and is
essential for C910 cluster's operation.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 include/dt-bindings/clock/thead,th1520-clk-ap.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/thead,th1520-clk-ap.h b/include/dt-bindings/clock/thead,th1520-clk-ap.h
index 09a9aa7b3ab1..68b35cc61204 100644
--- a/include/dt-bindings/clock/thead,th1520-clk-ap.h
+++ b/include/dt-bindings/clock/thead,th1520-clk-ap.h
@@ -93,6 +93,7 @@
 #define CLK_SRAM3		83
 #define CLK_PLL_GMAC_100M	84
 #define CLK_UART_SCLK		85
+#define CLK_C910_BUS		86
 
 /* VO clocks */
 #define CLK_AXI4_VO_ACLK		0
-- 
2.51.2

[PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Yao Zi]

All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
lock status is indicated by register PLL_STS (offset 0x80 inside AP
clock controller). We should poll the register to ensure the PLL
actually locks after enabling it.

Furthermore, a 30us delay is added after enabling the PLL, after which
the PLL could be considered stable as stated by vendor clock code.

Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 71ad03a998e8..d870f0c665f8 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -8,11 +8,14 @@
 #include <dt-bindings/clock/thead,th1520-clk-ap.h>
 #include <linux/bitfield.h>
 #include <linux/clk-provider.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
+#define TH1520_PLL_STS		0x80
+
 #define TH1520_PLL_POSTDIV2	GENMASK(26, 24)
 #define TH1520_PLL_POSTDIV1	GENMASK(22, 20)
 #define TH1520_PLL_FBDIV	GENMASK(19, 8)
@@ -23,6 +26,13 @@
 #define TH1520_PLL_FRAC		GENMASK(23, 0)
 #define TH1520_PLL_FRAC_BITS    24
 
+/*
+ * All PLLs in TH1520 take 21250ns at maximum to lock, let's take its double
+ * for safety.
+ */
+#define TH1520_PLL_LOCK_TIMEOUT_US	44
+#define TH1520_PLL_STABLE_DELAY_US	30
+
 struct ccu_internal {
 	u8	shift;
 	u8	width;
@@ -64,6 +74,7 @@ struct ccu_div {
 
 struct ccu_pll {
 	struct ccu_common	common;
+	u32			lock_sts_mask;
 };
 
 #define TH_CCU_ARG(_shift, _width)					\
@@ -299,9 +310,21 @@ static void ccu_pll_disable(struct clk_hw *hw)
 static int ccu_pll_enable(struct clk_hw *hw)
 {
 	struct ccu_pll *pll = hw_to_ccu_pll(hw);
+	u32 reg;
+	int ret;
 
-	return regmap_clear_bits(pll->common.map, pll->common.cfg1,
-				 TH1520_PLL_VCO_RST);
+	regmap_clear_bits(pll->common.map, pll->common.cfg1,
+			  TH1520_PLL_VCO_RST);
+
+	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
+					      reg, reg & pll->lock_sts_mask,
+					      5, TH1520_PLL_LOCK_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	udelay(TH1520_PLL_STABLE_DELAY_US);
+
+	return 0;
 }
 
 static int ccu_pll_is_enabled(struct clk_hw *hw)
@@ -389,6 +412,7 @@ static struct ccu_pll cpu_pll0_clk = {
 					      &clk_pll_ops,
 					      CLK_IS_CRITICAL),
 	},
+	.lock_sts_mask		= BIT(1),
 };
 
 static struct ccu_pll cpu_pll1_clk = {
@@ -401,6 +425,7 @@ static struct ccu_pll cpu_pll1_clk = {
 					      &clk_pll_ops,
 					      CLK_IS_CRITICAL),
 	},
+	.lock_sts_mask		= BIT(4),
 };
 
 static struct ccu_pll gmac_pll_clk = {
@@ -413,6 +438,7 @@ static struct ccu_pll gmac_pll_clk = {
 					      &clk_pll_ops,
 					      CLK_IS_CRITICAL),
 	},
+	.lock_sts_mask		= BIT(3),
 };
 
 static const struct clk_hw *gmac_pll_clk_parent[] = {
@@ -433,6 +459,7 @@ static struct ccu_pll video_pll_clk = {
 					      &clk_pll_ops,
 					      CLK_IS_CRITICAL),
 	},
+	.lock_sts_mask		= BIT(7),
 };
 
 static const struct clk_hw *video_pll_clk_parent[] = {
@@ -453,6 +480,7 @@ static struct ccu_pll dpu0_pll_clk = {
 					      &clk_pll_ops,
 					      0),
 	},
+	.lock_sts_mask		= BIT(8),
 };
 
 static const struct clk_hw *dpu0_pll_clk_parent[] = {
@@ -469,6 +497,7 @@ static struct ccu_pll dpu1_pll_clk = {
 					      &clk_pll_ops,
 					      0),
 	},
+	.lock_sts_mask		= BIT(9),
 };
 
 static const struct clk_hw *dpu1_pll_clk_parent[] = {
@@ -485,6 +514,7 @@ static struct ccu_pll tee_pll_clk = {
 					      &clk_pll_ops,
 					      CLK_IS_CRITICAL),
 	},
+	.lock_sts_mask		= BIT(10),
 };
 
 static const struct clk_parent_data c910_i0_parents[] = {
-- 
2.51.2

[PATCH 3/7] clk: thead: th1520-ap: Add C910 bus clock

[Yao Zi]

This divider takes c910_clk as parent and is essential for the C910
cluster to operate, thus is marked as CLK_IS_CRITICAL.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/clk/thead/clk-th1520-ap.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index d870f0c665f8..b820d47387bb 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -539,6 +539,20 @@ static struct ccu_mux c910_clk = {
 	.mux	= TH_CCU_MUX("c910", c910_parents, 0, 1),
 };
 
+static struct ccu_div c910_bus_clk = {
+	.enable		= BIT(7),
+	.div_en		= BIT(11),
+	.div		= TH_CCU_DIV_FLAGS(8, 3, 0),
+	.common		= {
+		.clkid		= CLK_C910_BUS,
+		.cfg0		= 0x100,
+		.hw.init	= CLK_HW_INIT_HW("c910-bus",
+						 &c910_clk.mux.hw,
+						 &ccu_div_ops,
+						 CLK_IS_CRITICAL),
+	},
+};
+
 static const struct clk_parent_data ahb2_cpusys_parents[] = {
 	{ .hw = &gmac_pll_clk.common.hw },
 	{ .index = 0 }
@@ -1051,6 +1065,7 @@ static struct ccu_common *th1520_pll_clks[] = {
 };
 
 static struct ccu_common *th1520_div_clks[] = {
+	&c910_bus_clk.common,
 	&ahb2_cpusys_hclk.common,
 	&apb3_cpusys_pclk.common,
 	&axi4_cpusys2_aclk.common,
@@ -1194,7 +1209,7 @@ static const struct th1520_plat_data th1520_ap_platdata = {
 	.th1520_mux_clks = th1520_mux_clks,
 	.th1520_gate_clks = th1520_gate_clks,
 
-	.nr_clks = CLK_UART_SCLK + 1,
+	.nr_clks = CLK_C910_BUS + 1,
 
 	.nr_pll_clks = ARRAY_SIZE(th1520_pll_clks),
 	.nr_div_clks = ARRAY_SIZE(th1520_div_clks),
-- 
2.51.2

[PATCH 4/7] clk: thead: th1520-ap: Support setting PLL rates

[Yao Zi]

TH1520 ships several PLLs that could operate in either integer or
fractional mode. However, the TRM only lists a few configuration whose
stability is considered guaranteed.

Add a table-lookup rate determination logic to support PLL rate setting,
and fill up frequency-configuration tables for AP-subsystem PLLs.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/clk/thead/clk-th1520-ap.c | 142 ++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index b820d47387bb..bf8e80c39a9e 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -22,6 +22,7 @@
 #define TH1520_PLL_REFDIV	GENMASK(5, 0)
 #define TH1520_PLL_BYPASS	BIT(30)
 #define TH1520_PLL_VCO_RST	BIT(29)
+#define TH1520_PLL_DACPD	BIT(25)
 #define TH1520_PLL_DSMPD	BIT(24)
 #define TH1520_PLL_FRAC		GENMASK(23, 0)
 #define TH1520_PLL_FRAC_BITS    24
@@ -72,9 +73,19 @@ struct ccu_div {
 	struct ccu_common	common;
 };
 
+struct ccu_pll_cfg {
+	unsigned long		freq;
+	u32			fbdiv;
+	u32			frac;
+	u32			postdiv1;
+	u32			postdiv2;
+};
+
 struct ccu_pll {
 	struct ccu_common	common;
 	u32			lock_sts_mask;
+	int			cfgnum;
+	const struct ccu_pll_cfg *cfgs;
 };
 
 #define TH_CCU_ARG(_shift, _width)					\
@@ -391,17 +402,102 @@ static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
+static const struct ccu_pll_cfg *ccu_pll_lookup_best_cfg(struct ccu_pll *pll,
+							 unsigned long rate)
+{
+	unsigned long best_delta = ULONG_MAX;
+	const struct ccu_pll_cfg *best_cfg;
+	int i;
+
+	for (i = 0; i < pll->cfgnum; i++) {
+		const struct ccu_pll_cfg *cfg = &pll->cfgs[i];
+		unsigned long delta;
+
+		delta = abs_diff(cfg->freq, rate);
+		if (delta < best_delta) {
+			best_delta	= delta;
+			best_cfg	= cfg;
+		}
+	}
+
+	return best_cfg;
+}
+
+static int ccu_pll_determine_rate(struct clk_hw *hw,
+				  struct clk_rate_request *req)
+{
+	struct ccu_pll *pll = hw_to_ccu_pll(hw);
+
+	req->rate = ccu_pll_lookup_best_cfg(pll, req->rate)->freq;
+
+	return 0;
+}
+
+static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+			    unsigned long parent_rate)
+{
+	struct ccu_pll *pll = hw_to_ccu_pll(hw);
+	const struct ccu_pll_cfg *cfg;
+
+	cfg = ccu_pll_lookup_best_cfg(pll, rate);
+
+	ccu_pll_disable(hw);
+
+	regmap_write(pll->common.map, pll->common.cfg0,
+		     FIELD_PREP(TH1520_PLL_REFDIV,	1)		|
+		     FIELD_PREP(TH1520_PLL_FBDIV,	cfg->fbdiv)	|
+		     FIELD_PREP(TH1520_PLL_POSTDIV1,	cfg->postdiv1)	|
+		     FIELD_PREP(TH1520_PLL_POSTDIV2,	cfg->postdiv2));
+
+	regmap_update_bits(pll->common.map, pll->common.cfg1,
+			   TH1520_PLL_DACPD | TH1520_PLL_DSMPD |
+			   TH1520_PLL_FRAC,
+			   cfg->frac ? cfg->frac :
+				TH1520_PLL_DACPD | TH1520_PLL_DSMPD);
+
+	return ccu_pll_enable(hw);
+}
+
 static const struct clk_ops clk_pll_ops = {
 	.disable	= ccu_pll_disable,
 	.enable		= ccu_pll_enable,
 	.is_enabled	= ccu_pll_is_enabled,
 	.recalc_rate	= ccu_pll_recalc_rate,
+	.determine_rate	= ccu_pll_determine_rate,
+	.set_rate	= ccu_pll_set_rate,
 };
 
 static const struct clk_parent_data osc_24m_clk[] = {
 	{ .index = 0 }
 };
 
+static const struct ccu_pll_cfg cpu_pll_cfgs[] = {
+	{ 125000000,	125,	0, 6, 4 },
+	{ 200000000,	125,	0, 5, 3 },
+	{ 300000000,	125,	0, 5, 2 },
+	{ 400000000,	100,	0, 3, 2 },
+	{ 500000000,	125,	0, 6, 1 },
+	{ 600000000,	125,	0, 5, 1 },
+	{ 702000000,	117,	0, 4, 1 },
+	{ 800000000,	100,	0, 3, 1 },
+	{ 900000000,	75,	0, 2, 1 },
+	{ 1000000000,	125,	0, 3, 1 },
+	{ 1104000000,	92,	0, 2, 1 },
+	{ 1200000000,	100,	0, 2, 1 },
+	{ 1296000000,	108,	0, 2, 1 },
+	{ 1404000000,	117,	0, 2, 1 },
+	{ 1500000000,	125,	0, 2, 1 },
+	{ 1608000000,	67,	0, 1, 1 },
+	{ 1704000000,	71,	0, 1, 1 },
+	{ 1800000000,	75,	0, 1, 1 },
+	{ 1896000000,	79,	0, 1, 1 },
+	{ 1992000000,	83,	0, 1, 1 },
+	{ 2112000000,	88,	0, 1, 1 },
+	{ 2208000000,	92,	0, 1, 1 },
+	{ 2304000000,	96,	0, 1, 1 },
+	{ 2400000000,	100,	0, 1, 1 },
+};
+
 static struct ccu_pll cpu_pll0_clk = {
 	.common		= {
 		.clkid		= CLK_CPU_PLL0,
@@ -413,6 +509,8 @@ static struct ccu_pll cpu_pll0_clk = {
 					      CLK_IS_CRITICAL),
 	},
 	.lock_sts_mask		= BIT(1),
+	.cfgnum			= ARRAY_SIZE(cpu_pll_cfgs),
+	.cfgs			= cpu_pll_cfgs,
 };
 
 static struct ccu_pll cpu_pll1_clk = {
@@ -426,6 +524,16 @@ static struct ccu_pll cpu_pll1_clk = {
 					      CLK_IS_CRITICAL),
 	},
 	.lock_sts_mask		= BIT(4),
+	.cfgnum			= ARRAY_SIZE(cpu_pll_cfgs),
+	.cfgs			= cpu_pll_cfgs,
+};
+
+static const struct ccu_pll_cfg gmac_pll_cfg = {
+	.freq		= 1000000000,
+	.fbdiv		= 125,
+	.frac		= 0,
+	.postdiv1	= 3,
+	.postdiv2	= 1,
 };
 
 static struct ccu_pll gmac_pll_clk = {
@@ -439,6 +547,8 @@ static struct ccu_pll gmac_pll_clk = {
 					      CLK_IS_CRITICAL),
 	},
 	.lock_sts_mask		= BIT(3),
+	.cfgnum			= 1,
+	.cfgs			= &gmac_pll_cfg,
 };
 
 static const struct clk_hw *gmac_pll_clk_parent[] = {
@@ -449,6 +559,14 @@ static const struct clk_parent_data gmac_pll_clk_pd[] = {
 	{ .hw = &gmac_pll_clk.common.hw }
 };
 
+static const struct ccu_pll_cfg video_pll_cfg = {
+	.freq		= 792000000,
+	.fbdiv		= 99,
+	.frac		= 0,
+	.postdiv1	= 3,
+	.postdiv2	= 1,
+};
+
 static struct ccu_pll video_pll_clk = {
 	.common		= {
 		.clkid		= CLK_VIDEO_PLL,
@@ -460,6 +578,8 @@ static struct ccu_pll video_pll_clk = {
 					      CLK_IS_CRITICAL),
 	},
 	.lock_sts_mask		= BIT(7),
+	.cfgnum			= 1,
+	.cfgs			= &video_pll_cfg,
 };
 
 static const struct clk_hw *video_pll_clk_parent[] = {
@@ -470,6 +590,14 @@ static const struct clk_parent_data video_pll_clk_pd[] = {
 	{ .hw = &video_pll_clk.common.hw }
 };
 
+static const struct ccu_pll_cfg dpu_pll_cfg = {
+	.freq		= 1188000000,
+	.fbdiv		= 99,
+	.frac		= 0,
+	.postdiv1	= 2,
+	.postdiv2	= 1,
+};
+
 static struct ccu_pll dpu0_pll_clk = {
 	.common		= {
 		.clkid		= CLK_DPU0_PLL,
@@ -481,6 +609,8 @@ static struct ccu_pll dpu0_pll_clk = {
 					      0),
 	},
 	.lock_sts_mask		= BIT(8),
+	.cfgnum			= 1,
+	.cfgs			= &dpu_pll_cfg,
 };
 
 static const struct clk_hw *dpu0_pll_clk_parent[] = {
@@ -498,12 +628,22 @@ static struct ccu_pll dpu1_pll_clk = {
 					      0),
 	},
 	.lock_sts_mask		= BIT(9),
+	.cfgnum			= 1,
+	.cfgs			= &dpu_pll_cfg,
 };
 
 static const struct clk_hw *dpu1_pll_clk_parent[] = {
 	&dpu1_pll_clk.common.hw
 };
 
+static const struct ccu_pll_cfg tee_pll_cfg = {
+	.freq		= 792000000,
+	.fbdiv		= 99,
+	.frac		= 0,
+	.postdiv1	= 3,
+	.postdiv2	= 1,
+};
+
 static struct ccu_pll tee_pll_clk = {
 	.common		= {
 		.clkid		= CLK_TEE_PLL,
@@ -515,6 +655,8 @@ static struct ccu_pll tee_pll_clk = {
 					      CLK_IS_CRITICAL),
 	},
 	.lock_sts_mask		= BIT(10),
+	.cfgnum			= 1,
+	.cfgs			= &tee_pll_cfg,
 };
 
 static const struct clk_parent_data c910_i0_parents[] = {
-- 
2.51.2

[PATCH 5/7] clk: thead: th1520-ap: Add macro to define multiplexers with flags

[Yao Zi]

The new macro, TH_CCU_MUX_FLAGS, extends TH_CCU_MUX macro by adding two
parameters to specify clock flags and multiplexer flags.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/clk/thead/clk-th1520-ap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index bf8e80c39a9e..79f001a047b2 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -101,17 +101,22 @@ struct ccu_pll {
 		.flags	= _flags,					\
 	}
 
-#define TH_CCU_MUX(_name, _parents, _shift, _width)			\
+#define TH_CCU_MUX_FLAGS(_name, _parents, _shift, _width, _flags,	\
+			 _mux_flags)					\
 	{								\
 		.mask		= GENMASK(_width - 1, 0),		\
 		.shift		= _shift,				\
+		.flags		= _mux_flags,				\
 		.hw.init	= CLK_HW_INIT_PARENTS_DATA(		\
 					_name,				\
 					_parents,			\
 					&clk_mux_ops,			\
-					0),				\
+					_flags),			\
 	}
 
+#define TH_CCU_MUX(_name, _parents, _shift, _width)			\
+	TH_CCU_MUX_FLAGS(_name, _parents, _shift, _width, 0, 0)
+
 #define CCU_GATE(_clkid, _struct, _name, _parent, _reg, _bit, _flags)	\
 	struct ccu_gate _struct = {					\
 		.clkid	= _clkid,					\
-- 
2.51.2

[PATCH 6/7] clk: thead: th1520-ap: Support CPU frequency scaling

[Yao Zi]

On TH1520 SoC, c910_clk feeds the CPU cluster. It could be glitchlessly
reparented to one of the two PLLs: either to cpu_pll0 indirectly through
c910_i0_clk, or to cpu_pll1 directly.

To achieve glitchless rate change, customized clock operations are
implemented for c910_clk: on rate change, the PLL not currently in use
is configured to the requested rate first, then c910_clk reparents to
it.

Additionally, c910_bus_clk, which in turn takes c910_clk as parent,
has a frequency limit of 750MHz. A clock notifier is registered on
c910_clk to adjust c910_bus_clk on c910_clk rate change.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/clk/thead/clk-th1520-ap.c | 148 +++++++++++++++++++++++++++++-
 1 file changed, 146 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 79f001a047b2..cd3b396c9a3d 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -7,9 +7,11 @@
 
 #include <dt-bindings/clock/thead,th1520-clk-ap.h>
 #include <linux/bitfield.h>
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -34,6 +36,9 @@
 #define TH1520_PLL_LOCK_TIMEOUT_US	44
 #define TH1520_PLL_STABLE_DELAY_US	30
 
+/* c910_bus_clk must be kept below 750MHz for stability */
+#define TH1520_C910_BUS_MAX_RATE	(750 * 1000 * 1000)
+
 struct ccu_internal {
 	u8	shift;
 	u8	width;
@@ -472,6 +477,72 @@ static const struct clk_ops clk_pll_ops = {
 	.set_rate	= ccu_pll_set_rate,
 };
 
+/*
+ * c910_clk could be reparented glitchlessly for DVFS. There are two parents,
+ *  - c910_i0_clk, dervided from cpu_pll0_clk or osc_24m.
+ *  - cpu_pll1_clk, which provides the exact same set of rates as cpu_pll0_clk.
+ *
+ * During rate setting, always forward the request to the unused parent, and
+ * then switch c910_clk to it to avoid glitch.
+ */
+static u8 c910_clk_get_parent(struct clk_hw *hw)
+{
+	return clk_mux_ops.get_parent(hw);
+}
+
+static int c910_clk_set_parent(struct clk_hw *hw, u8 index)
+{
+	return clk_mux_ops.set_parent(hw, index);
+}
+
+static unsigned long c910_clk_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	return parent_rate;
+}
+
+static int c910_clk_determine_rate(struct clk_hw *hw,
+				   struct clk_rate_request *req)
+{
+	u8 alt_parent_index = !c910_clk_get_parent(hw);
+	struct clk_hw *alt_parent;
+
+	alt_parent = clk_hw_get_parent_by_index(hw, alt_parent_index);
+
+	req->rate		= clk_hw_round_rate(alt_parent, req->rate);
+	req->best_parent_hw	= alt_parent;
+	req->best_parent_rate	= req->rate;
+
+	return 0;
+}
+
+static int c910_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	return -EOPNOTSUPP;
+}
+
+static int c910_clk_set_rate_and_parent(struct clk_hw *hw, unsigned long rate,
+					unsigned long parent_rate, u8 index)
+{
+	struct clk_hw *parent = clk_hw_get_parent_by_index(hw, index);
+
+	clk_set_rate(parent->clk, parent_rate);
+
+	c910_clk_set_parent(hw, index);
+
+	return 0;
+}
+
+static const struct clk_ops c910_clk_ops = {
+	.get_parent		= c910_clk_get_parent,
+	.set_parent		= c910_clk_set_parent,
+	.recalc_rate		= c910_clk_recalc_rate,
+	.determine_rate		= c910_clk_determine_rate,
+	.set_rate		= c910_clk_set_rate,
+	.set_rate_and_parent	= c910_clk_set_rate_and_parent,
+};
+
 static const struct clk_parent_data osc_24m_clk[] = {
 	{ .index = 0 }
 };
@@ -672,7 +743,8 @@ static const struct clk_parent_data c910_i0_parents[] = {
 static struct ccu_mux c910_i0_clk = {
 	.clkid	= CLK_C910_I0,
 	.reg	= 0x100,
-	.mux	= TH_CCU_MUX("c910-i0", c910_i0_parents, 1, 1),
+	.mux	= TH_CCU_MUX_FLAGS("c910-i0", c910_i0_parents, 1, 1,
+				   CLK_SET_RATE_PARENT, CLK_MUX_ROUND_CLOSEST),
 };
 
 static const struct clk_parent_data c910_parents[] = {
@@ -683,7 +755,14 @@ static const struct clk_parent_data c910_parents[] = {
 static struct ccu_mux c910_clk = {
 	.clkid	= CLK_C910,
 	.reg	= 0x100,
-	.mux	= TH_CCU_MUX("c910", c910_parents, 0, 1),
+	.mux	= {
+		.mask		= BIT(0),
+		.shift		= 0,
+		.hw.init	= CLK_HW_INIT_PARENTS_DATA("c910",
+							   c910_parents,
+							   &c910_clk_ops,
+							   CLK_SET_RATE_PARENT),
+	},
 };
 
 static struct ccu_div c910_bus_clk = {
@@ -1372,11 +1451,69 @@ static const struct th1520_plat_data th1520_vo_platdata = {
 	.nr_gate_clks = ARRAY_SIZE(th1520_vo_gate_clks),
 };
 
+/*
+ * Maintain clock rate of c910_bus_clk below TH1520_C910_BUS_MAX_RATE (750MHz)
+ * when its parent, c910_clk, changes the rate.
+ *
+ * Additionally, TRM is unclear about c910_bus_clk behavior with divisor set to
+ * 2, thus we should ensure the new divisor stays in (2, MAXDIVISOR).
+ */
+static unsigned long c910_bus_clk_divisor(struct ccu_div *cd,
+					  unsigned long parent_rate)
+{
+	return clamp(DIV_ROUND_UP(parent_rate, TH1520_C910_BUS_MAX_RATE),
+		     2U, 1U << cd->div.width);
+}
+
+static int c910_clk_notifier_cb(struct notifier_block *nb,
+				unsigned long action, void *data)
+{
+	struct clk_notifier_data *cnd = data;
+	unsigned long new_divisor, ref_rate;
+
+	if (action != PRE_RATE_CHANGE && action != POST_RATE_CHANGE)
+		return NOTIFY_DONE;
+
+	new_divisor	= c910_bus_clk_divisor(&c910_bus_clk, cnd->new_rate);
+
+	if (cnd->new_rate > cnd->old_rate) {
+		/*
+		 * Scaling up. Adjust c910_bus_clk divisor
+		 * - before c910_clk rate change to ensure the constraints
+		 *   aren't broken after scaling to higher rates,
+		 * - after c910_clk rate change to keep c910_bus_clk as high as
+		 *   possible
+		 */
+		ref_rate = action == PRE_RATE_CHANGE ?
+				cnd->old_rate : cnd->new_rate;
+		clk_set_rate(c910_bus_clk.common.hw.clk,
+			     ref_rate / new_divisor);
+	} else if (cnd->new_rate < cnd->old_rate &&
+		    action == POST_RATE_CHANGE) {
+		/*
+		 * Scaling down. Adjust c910_bus_clk divisor only after
+		 * c910_clk rate change to keep c910_bus_clk as high as
+		 * possible, Scaling down never breaks the constraints.
+		 */
+		clk_set_rate(c910_bus_clk.common.hw.clk,
+			     cnd->new_rate / new_divisor);
+	} else {
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block c910_clk_notifier = {
+	.notifier_call	= c910_clk_notifier_cb,
+};
+
 static int th1520_clk_probe(struct platform_device *pdev)
 {
 	const struct th1520_plat_data *plat_data;
 	struct device *dev = &pdev->dev;
 	struct clk_hw_onecell_data *priv;
+	struct clk *notifier_clk;
 
 	struct regmap *map;
 	void __iomem *base;
@@ -1463,6 +1600,13 @@ static int th1520_clk_probe(struct platform_device *pdev)
 		ret = devm_clk_hw_register(dev, &emmc_sdio_ref_clk.hw);
 		if (ret)
 			return ret;
+
+		notifier_clk = devm_clk_hw_get_clk(dev, &c910_clk.mux.hw,
+						   "dvfs");
+		ret = devm_clk_notifier_register(dev, notifier_clk,
+						 &c910_clk_notifier);
+		if (ret)
+			return ret;
 	}
 
 	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, priv);
-- 
2.51.2

[PATCH 7/7] [Not For Upstream] riscv: dts: thead: Add CPU clock and OPP table for TH1520

[Yao Zi]

Add operating point table for CPU cores, and wire up clocks for CPU
nodes.

This patch isn't intended for upstreaming but only for testing purpose,
since the PMIC driver for scaling CPU voltage isn't ready yet. Only
operating points whose voltage is satisified by Lichee Module 4A's PMIC
default, i.e. <= 1.5GHz, are enabled.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 35 +++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index bd5d33840884..6020d568ad7c 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -38,6 +38,8 @@ c910_0: cpu@0 {
 			d-cache-sets = <512>;
 			next-level-cache = <&l2_cache>;
 			mmu-type = "riscv,sv39";
+			operating-points-v2 = <&cpu_opp>;
+			clocks = <&clk CLK_C910>;
 
 			cpu0_intc: interrupt-controller {
 				compatible = "riscv,cpu-intc";
@@ -65,6 +67,8 @@ c910_1: cpu@1 {
 			d-cache-sets = <512>;
 			next-level-cache = <&l2_cache>;
 			mmu-type = "riscv,sv39";
+			operating-points-v2 = <&cpu_opp>;
+			clocks = <&clk CLK_C910>;
 
 			cpu1_intc: interrupt-controller {
 				compatible = "riscv,cpu-intc";
@@ -92,6 +96,8 @@ c910_2: cpu@2 {
 			d-cache-sets = <512>;
 			next-level-cache = <&l2_cache>;
 			mmu-type = "riscv,sv39";
+			operating-points-v2 = <&cpu_opp>;
+			clocks = <&clk CLK_C910>;
 
 			cpu2_intc: interrupt-controller {
 				compatible = "riscv,cpu-intc";
@@ -119,6 +125,8 @@ c910_3: cpu@3 {
 			d-cache-sets = <512>;
 			next-level-cache = <&l2_cache>;
 			mmu-type = "riscv,sv39";
+			operating-points-v2 = <&cpu_opp>;
+			clocks = <&clk CLK_C910>;
 
 			cpu3_intc: interrupt-controller {
 				compatible = "riscv,cpu-intc";
@@ -137,6 +145,33 @@ l2_cache: l2-cache {
 		};
 	};
 
+	cpu_opp: opp-table-cpu {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <600000>;
+		};
+
+		opp-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <700000>;
+		};
+
+		opp-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+			opp-microvolt = <800000>;
+		};
+
+/*
+		opp-1848000000 {
+			opp-hz = /bits/ 64 <1848000000>;
+			opp-microvolt = <1000000>;
+		};
+ */
+	};
+
 	pmu {
 		compatible = "riscv,pmu";
 		riscv,event-to-mhpmcounters =
-- 
2.51.2

Re: [PATCH 1/7] dt-bindings: clock: thead,th1520-clk-ap: Add ID for C910 bus clock

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On Thu, Nov 20, 2025 at 01:14:10PM +0000, Yao Zi wrote:
> Add binding ID for C910 bus clock, which takes CLK_C910 as parent and is
> essential for C910 cluster's operation.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  include/dt-bindings/clock/thead,th1520-clk-ap.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/clock/thead,th1520-clk-ap.h b/include/dt-bindings/clock/thead,th1520-clk-ap.h
> index 09a9aa7b3ab1..68b35cc61204 100644
> --- a/include/dt-bindings/clock/thead,th1520-clk-ap.h
> +++ b/include/dt-bindings/clock/thead,th1520-clk-ap.h
> @@ -93,6 +93,7 @@
>  #define CLK_SRAM3		83
>  #define CLK_PLL_GMAC_100M	84
>  #define CLK_UART_SCLK		85
> +#define CLK_C910_BUS		86
>  
>  /* VO clocks */
>  #define CLK_AXI4_VO_ACLK		0
> -- 
> 2.51.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> lock status is indicated by register PLL_STS (offset 0x80 inside AP
> clock controller). We should poll the register to ensure the PLL
> actually locks after enabling it.
> 
> Furthermore, a 30us delay is added after enabling the PLL, after which
> the PLL could be considered stable as stated by vendor clock code.
> 
> Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)

Thanks for working on this patch series.

[...]
> @@ -299,9 +310,21 @@ static void ccu_pll_disable(struct clk_hw *hw)
>  static int ccu_pll_enable(struct clk_hw *hw)
>  {
>  	struct ccu_pll *pll = hw_to_ccu_pll(hw);
> +	u32 reg;
> +	int ret;
>  
> -	return regmap_clear_bits(pll->common.map, pll->common.cfg1,
> -				 TH1520_PLL_VCO_RST);
> +	regmap_clear_bits(pll->common.map, pll->common.cfg1,
> +			  TH1520_PLL_VCO_RST);
> +
> +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> +					      reg, reg & pll->lock_sts_mask,
> +					      5, TH1520_PLL_LOCK_TIMEOUT_US);

Is there a reason for the specific value of 5 uS polling delay?

> +	if (ret)
> +		return ret;
> +
> +	udelay(TH1520_PLL_STABLE_DELAY_US);

Is it the case that the 30 uS delay after the lock bit is set is just so
that it has the same behavior as the vendor's code? Or did you notice
stability problems without this?

Thanks,
Drew

Re: [PATCH 5/7] clk: thead: th1520-ap: Add macro to define multiplexers with flags

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:14PM +0000, Yao Zi wrote:
> The new macro, TH_CCU_MUX_FLAGS, extends TH_CCU_MUX macro by adding two
> parameters to specify clock flags and multiplexer flags.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> index bf8e80c39a9e..79f001a047b2 100644
> --- a/drivers/clk/thead/clk-th1520-ap.c
> +++ b/drivers/clk/thead/clk-th1520-ap.c
> @@ -101,17 +101,22 @@ struct ccu_pll {
>  		.flags	= _flags,					\
>  	}
>  
> -#define TH_CCU_MUX(_name, _parents, _shift, _width)			\
> +#define TH_CCU_MUX_FLAGS(_name, _parents, _shift, _width, _flags,	\
> +			 _mux_flags)					\
>  	{								\
>  		.mask		= GENMASK(_width - 1, 0),		\

checkpatch warns [1] about this line:

CHECK: Macro argument '_width' may be better as '(_width)' to avoid precedence issues

I noticed it in the patchwork CI results [2] but I think we can ignore
that as this patch is not actually changing that line.

Thanks,
Drew

[1] https://gist.github.com/linux-riscv-bot/a335020c99ef628bb38e0a4ea85e0c45
[2] https://patchwork.kernel.org/project/linux-riscv/patch/20251120131416.26236-6-ziyao@disroot.org/

Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Yao Zi]

On Mon, Nov 24, 2025 at 02:08:00PM -0800, Drew Fustini wrote:
> On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> > All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> > lock status is indicated by register PLL_STS (offset 0x80 inside AP
> > clock controller). We should poll the register to ensure the PLL
> > actually locks after enabling it.
> > 
> > Furthermore, a 30us delay is added after enabling the PLL, after which
> > the PLL could be considered stable as stated by vendor clock code.
> > 
> > Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> Thanks for working on this patch series.
> 
> [...]
> > @@ -299,9 +310,21 @@ static void ccu_pll_disable(struct clk_hw *hw)
> >  static int ccu_pll_enable(struct clk_hw *hw)
> >  {
> >  	struct ccu_pll *pll = hw_to_ccu_pll(hw);
> > +	u32 reg;
> > +	int ret;
> >  
> > -	return regmap_clear_bits(pll->common.map, pll->common.cfg1,
> > -				 TH1520_PLL_VCO_RST);
> > +	regmap_clear_bits(pll->common.map, pll->common.cfg1,
> > +			  TH1520_PLL_VCO_RST);
> > +
> > +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> > +					      reg, reg & pll->lock_sts_mask,
> > +					      5, TH1520_PLL_LOCK_TIMEOUT_US);
> 
> Is there a reason for the specific value of 5 uS polling delay?

No, it was picked randomly. A smaller value would reduce latency of
PLL enabling, and I could tune it more carefully by some testing. But
it's hard to predict how much improvement it will bring.

> > +	if (ret)
> > +		return ret;
> > +
> > +	udelay(TH1520_PLL_STABLE_DELAY_US);
> 
> Is it the case that the 30 uS delay after the lock bit is set is just so
> that it has the same behavior as the vendor's code? Or did you notice
> stability problems without this?

This aligns with the vendor code, and I haven't yet observed stability
issues without the delay. But I think it's more safe to keep the
behavior similar since it's hard to test all working conditions.

> Thanks,
> Drew

Best regards,
Yao Zi

Re: [PATCH 5/7] clk: thead: th1520-ap: Add macro to define multiplexers with flags

[Yao Zi]

On Mon, Nov 24, 2025 at 02:14:15PM -0800, Drew Fustini wrote:
> On Thu, Nov 20, 2025 at 01:14:14PM +0000, Yao Zi wrote:
> > The new macro, TH_CCU_MUX_FLAGS, extends TH_CCU_MUX macro by adding two
> > parameters to specify clock flags and multiplexer flags.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  drivers/clk/thead/clk-th1520-ap.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> > index bf8e80c39a9e..79f001a047b2 100644
> > --- a/drivers/clk/thead/clk-th1520-ap.c
> > +++ b/drivers/clk/thead/clk-th1520-ap.c
> > @@ -101,17 +101,22 @@ struct ccu_pll {
> >  		.flags	= _flags,					\
> >  	}
> >  
> > -#define TH_CCU_MUX(_name, _parents, _shift, _width)			\
> > +#define TH_CCU_MUX_FLAGS(_name, _parents, _shift, _width, _flags,	\
> > +			 _mux_flags)					\
> >  	{								\
> >  		.mask		= GENMASK(_width - 1, 0),		\
> 
> checkpatch warns [1] about this line:
> 
> CHECK: Macro argument '_width' may be better as '(_width)' to avoid precedence issues
> 
> I noticed it in the patchwork CI results [2] but I think we can ignore
> that as this patch is not actually changing that line.

Agree. This is only a "CHECK"-level warning, and for these clock
definition macros, I don't think there's a chance that precedence
problem would happen since only literals are feed into _width.

Enclosing the parameter in parentheses also makes the code a little
messy. so I'd prefer to keep it as-is, too.

> Thanks,
> Drew

Regards,
Yao Zi

> [1] https://gist.github.com/linux-riscv-bot/a335020c99ef628bb38e0a4ea85e0c45
> [2] https://patchwork.kernel.org/project/linux-riscv/patch/20251120131416.26236-6-ziyao@disroot.org/
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Drew Fustini]

On Tue, Nov 25, 2025 at 03:19:30AM +0000, Yao Zi wrote:
> On Mon, Nov 24, 2025 at 02:08:00PM -0800, Drew Fustini wrote:
> > On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> > > All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> > > lock status is indicated by register PLL_STS (offset 0x80 inside AP
> > > clock controller). We should poll the register to ensure the PLL
> > > actually locks after enabling it.
> > > 
> > > Furthermore, a 30us delay is added after enabling the PLL, after which
> > > the PLL could be considered stable as stated by vendor clock code.
> > > 
> > > Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> > > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > > ---
> > >  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
> > >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > Thanks for working on this patch series.
> > 
> > [...]
> > > @@ -299,9 +310,21 @@ static void ccu_pll_disable(struct clk_hw *hw)
> > >  static int ccu_pll_enable(struct clk_hw *hw)
> > >  {
> > >  	struct ccu_pll *pll = hw_to_ccu_pll(hw);
> > > +	u32 reg;
> > > +	int ret;
> > >  
> > > -	return regmap_clear_bits(pll->common.map, pll->common.cfg1,
> > > -				 TH1520_PLL_VCO_RST);
> > > +	regmap_clear_bits(pll->common.map, pll->common.cfg1,
> > > +			  TH1520_PLL_VCO_RST);
> > > +
> > > +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> > > +					      reg, reg & pll->lock_sts_mask,
> > > +					      5, TH1520_PLL_LOCK_TIMEOUT_US);
> > 
> > Is there a reason for the specific value of 5 uS polling delay?
> 
> No, it was picked randomly. A smaller value would reduce latency of
> PLL enabling, and I could tune it more carefully by some testing. But
> it's hard to predict how much improvement it will bring.

Okay, I was just curious. I think it is okay to stick with that current
value if it is working correctly.

> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	udelay(TH1520_PLL_STABLE_DELAY_US);
> > 
> > Is it the case that the 30 uS delay after the lock bit is set is just so
> > that it has the same behavior as the vendor's code? Or did you notice
> > stability problems without this?
> 
> This aligns with the vendor code, and I haven't yet observed stability
> issues without the delay. But I think it's more safe to keep the
> behavior similar since it's hard to test all working conditions.

Okay, that seems reasonable to play it safe.

Thanks,
Drew

Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> lock status is indicated by register PLL_STS (offset 0x80 inside AP
> clock controller). We should poll the register to ensure the PLL
> actually locks after enabling it.
> 
> Furthermore, a 30us delay is added after enabling the PLL, after which
> the PLL could be considered stable as stated by vendor clock code.
> 
> Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
[...]
> +/*
> + * All PLLs in TH1520 take 21250ns at maximum to lock, let's take its double
> + * for safety.
> + */
> +#define TH1520_PLL_LOCK_TIMEOUT_US	44
> +#define TH1520_PLL_STABLE_DELAY_US	30

I'm taking a second look at this and I think it might be best to add a
define for the polling loop delay of 5. It could be helpful when other
people read the code later.

[...]
> +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> +					      reg, reg & pll->lock_sts_mask,
> +					      5, TH1520_PLL_LOCK_TIMEOUT_US);

The loop delay is only used here but I think using a #define would make
it more readable.

Other than that:
Reviewed-by: Drew Fustini <fustini@kernel.org>

If no other changes are needed I could fix this up on apply. Let's see
what other comments there may be. It's too late for me to send a 6.19
clk pull request so this will have to target the next merge window. I
can put it into linux-next once 6.19-rc1 is released.

Thanks,
Drew

Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Yao Zi]

On Wed, Nov 26, 2025 at 08:52:00AM -0600, Drew Fustini wrote:
> On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> > All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> > lock status is indicated by register PLL_STS (offset 0x80 inside AP
> > clock controller). We should poll the register to ensure the PLL
> > actually locks after enabling it.
> > 
> > Furthermore, a 30us delay is added after enabling the PLL, after which
> > the PLL could be considered stable as stated by vendor clock code.
> > 
> > Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> [...]
> > +/*
> > + * All PLLs in TH1520 take 21250ns at maximum to lock, let's take its double
> > + * for safety.
> > + */
> > +#define TH1520_PLL_LOCK_TIMEOUT_US	44
> > +#define TH1520_PLL_STABLE_DELAY_US	30
> 
> I'm taking a second look at this and I think it might be best to add a
> define for the polling loop delay of 5. It could be helpful when other
> people read the code later.
> 
> [...]
> > +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> > +					      reg, reg & pll->lock_sts_mask,
> > +					      5, TH1520_PLL_LOCK_TIMEOUT_US);
> 
> The loop delay is only used here but I think using a #define would make
> it more readable.

There are TH1520_PLL_LOCK_TIMEOUT_US and TH1520_PLL_STABLE_DELAY_US
defined because they're meaningful constants, either specified by TRM or
implied by vendor code, however the 5us delay is only a randomly-picked
value, as what I've mentioned before.

Anyway, I'm fine with a separate definition. So please go ahead if it
looks better to you.

> Other than that:
> Reviewed-by: Drew Fustini <fustini@kernel.org>
> 
> If no other changes are needed I could fix this up on apply. Let's see
> what other comments there may be. It's too late for me to send a 6.19
> clk pull request so this will have to target the next merge window. I
> can put it into linux-next once 6.19-rc1 is released.

Many thanks for it.

> Thanks,
> Drew

Best regards,
Yao Zi

Re: [PATCH 3/7] clk: thead: th1520-ap: Add C910 bus clock

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:12PM +0000, Yao Zi wrote:
> This divider takes c910_clk as parent and is essential for the C910
> cluster to operate, thus is marked as CLK_IS_CRITICAL.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

Reviewed-by: Drew Fustini <fustini@kernel.org>

Re: [PATCH 4/7] clk: thead: th1520-ap: Support setting PLL rates

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:13PM +0000, Yao Zi wrote:
> TH1520 ships several PLLs that could operate in either integer or
> fractional mode. However, the TRM only lists a few configuration whose
> stability is considered guaranteed.
> 
> Add a table-lookup rate determination logic to support PLL rate setting,
> and fill up frequency-configuration tables for AP-subsystem PLLs.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 142 ++++++++++++++++++++++++++++++
>  1 file changed, 142 insertions(+)

Reviewed-by: Drew Fustini <fustini@kernel.org>

Re: [PATCH 5/7] clk: thead: th1520-ap: Add macro to define multiplexers with flags

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:14PM +0000, Yao Zi wrote:
> The new macro, TH_CCU_MUX_FLAGS, extends TH_CCU_MUX macro by adding two
> parameters to specify clock flags and multiplexer flags.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Drew Fustini <fustini@kernel.org>

Re: [PATCH 6/7] clk: thead: th1520-ap: Support CPU frequency scaling

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:15PM +0000, Yao Zi wrote:
> On TH1520 SoC, c910_clk feeds the CPU cluster. It could be glitchlessly
> reparented to one of the two PLLs: either to cpu_pll0 indirectly through
> c910_i0_clk, or to cpu_pll1 directly.
> 
> To achieve glitchless rate change, customized clock operations are
> implemented for c910_clk: on rate change, the PLL not currently in use
> is configured to the requested rate first, then c910_clk reparents to
> it.
> 
> Additionally, c910_bus_clk, which in turn takes c910_clk as parent,
> has a frequency limit of 750MHz. A clock notifier is registered on
> c910_clk to adjust c910_bus_clk on c910_clk rate change.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 148 +++++++++++++++++++++++++++++-
>  1 file changed, 146 insertions(+), 2 deletions(-)
[...] 
> +/*
> + * c910_clk could be reparented glitchlessly for DVFS. There are two parents,
> + *  - c910_i0_clk, dervided from cpu_pll0_clk or osc_24m.

Typo: 'derived' instead of 'dervided'.

[...]

Unless there are other comments that require changes, I can fix up the
typo when applied.

Reviewed-by: Drew Fustini <fustini@kernel.org>

Re: [PATCH 0/7] Implement CPU frequency scaling for TH1520

[Drew Fustini]

On Thu, Nov 20, 2025 at 01:14:09PM +0000, Yao Zi wrote:
> On TH1520 SoC, c910_clk feeds the CPU cluster. It could be glitchlessly
> reparented to one of the two PLLs: either to cpu_pll0 indirectly through
> c910_i0_clk, or to cpu_pll1 directly. This series fixes a bug in PLL
> enabling code, supports rate change for PLL, and finally implements
> frequency scaling support for c910_clk.
> 
> However, to achieve reliable frequency scaling, CPU voltage must be
> adjusted together with frequency, and AON-firmware-based PMIC support
> for TH1520 SoC is still missing in mainline. Thus PATCH 7 that fills OPP
> table for TH1520 CPU and enables CPUfreq is only for testing purpose,
> not intended for upstream (yet).
> 
> Testing is done on Lichee Pi 4A board, only operating points safe
> to be used with the the default PMIC configuration are enabled in
> devicetree. I've confirmed there's a performance gain when running
> coremark and some building work compared to the case without cpufreq.
> 
> This series is based on next-20251120, thanks for your time and review.
> 
> Yao Zi (7):
>   dt-bindings: clock: thead,th1520-clk-ap: Add ID for C910 bus clock
>   clk: thead: th1520-ap: Poll for PLL lock and wait for stability
>   clk: thead: th1520-ap: Add C910 bus clock
>   clk: thead: th1520-ap: Support setting PLL rates
>   clk: thead: th1520-ap: Add macro to define multiplexers with flags
>   clk: thead: th1520-ap: Support CPU frequency scaling
>   [Not For Upstream] riscv: dts: thead: Add CPU clock and OPP table for
>     TH1520
> 
>  arch/riscv/boot/dts/thead/th1520.dtsi         |  35 ++
>  drivers/clk/thead/clk-th1520-ap.c             | 350 +++++++++++++++++-
>  .../dt-bindings/clock/thead,th1520-clk-ap.h   |   1 +
>  3 files changed, 379 insertions(+), 7 deletions(-)
> 
> -- 
> 2.51.2
> 

Applied to thead-clk-for-next, thanks!

[1/7] dt-bindings: clock: thead,th1520-clk-ap: Add ID for C910 bus clock
      https://git.kernel.org/fustini/c/5f352125f8a0
[1/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability
      https://git.kernel.org/fustini/c/892abfbed71e
[2/7] clk: thead: th1520-ap: Add C910 bus clock
      https://git.kernel.org/fustini/c/b436f8a82aaa
[3/7] clk: thead: th1520-ap: Support setting PLL rates
      https://git.kernel.org/fustini/c/238cc6316a88
[5/7] clk: thead: th1520-ap: Add macro to define multiplexers with flags
      https://git.kernel.org/fustini/c/5dbee3503771
[6/7] clk: thead: th1520-ap: Support CPU frequency scaling
      https://git.kernel.org/fustini/c/30441a56b1d1

-Drew