* [PATCH 0/5] clk: spacemit: preserve and safely synchronize firmware PLLs
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware may leave PLLs running with valid settings absent from Linux's
rate tables. Replacing those settings during clock registration can
stop CPUs that depend on them.
This series removes the init fallback to the first rate-table entry,
decodes hardware rates directly and preserves unlisted rates. Opt-in
synchronization applies only to differing parameters for the same
decoded rate. It temporarily moves mapped CPU consumers to validated
PLL1 branches and restores their parents after the reprogrammed PLL
locks. PLL1 itself is not synchronized, and enabled outputs outside the
managed CPU paths prevent synchronization.
The series also corrects CPU divider and shared-parent modeling and
makes MIX rate selection agree with programming. Runtime PLL rate
changes still require a stopped PLL; automatic DVFS switching and new
rate-table entries are outside this series.
---
Troy Mitchell (5):
clk: spacemit: derive PLL rates from hardware
clk: spacemit: make MIX rate selection consistent
clk: spacemit: describe CPU clock dividers and shared PLL muxes
clk: spacemit: reject rate changes to running firmware PLLs
clk: spacemit: safely synchronize PLL parameters during init
drivers/clk/spacemit/ccu-k1.c | 60 +++--
drivers/clk/spacemit/ccu-k3.c | 137 +++++++---
drivers/clk/spacemit/ccu_common.c | 12 +
drivers/clk/spacemit/ccu_common.h | 2 +
drivers/clk/spacemit/ccu_mix.c | 35 ++-
drivers/clk/spacemit/ccu_mix.h | 18 ++
drivers/clk/spacemit/ccu_pll.c | 535 +++++++++++++++++++++++++++++++-------
drivers/clk/spacemit/ccu_pll.h | 43 ++-
8 files changed, 689 insertions(+), 153 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260907-spacemit-pll-init-c942bb249244
Best regards,
--
Troy Mitchell <troy.mitchell@linux.spacemit.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 0/5] clk: spacemit: preserve and safely synchronize firmware PLLs
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware may leave PLLs running with valid settings absent from Linux's
rate tables. Replacing those settings during clock registration can
stop CPUs that depend on them.
This series removes the init fallback to the first rate-table entry,
decodes hardware rates directly and preserves unlisted rates. Opt-in
synchronization applies only to differing parameters for the same
decoded rate. It temporarily moves mapped CPU consumers to validated
PLL1 branches and restores their parents after the reprogrammed PLL
locks. PLL1 itself is not synchronized, and enabled outputs outside the
managed CPU paths prevent synchronization.
The series also corrects CPU divider and shared-parent modeling and
makes MIX rate selection agree with programming. Runtime PLL rate
changes still require a stopped PLL; automatic DVFS switching and new
rate-table entries are outside this series.
---
Troy Mitchell (5):
clk: spacemit: derive PLL rates from hardware
clk: spacemit: make MIX rate selection consistent
clk: spacemit: describe CPU clock dividers and shared PLL muxes
clk: spacemit: reject rate changes to running firmware PLLs
clk: spacemit: safely synchronize PLL parameters during init
drivers/clk/spacemit/ccu-k1.c | 60 +++--
drivers/clk/spacemit/ccu-k3.c | 137 +++++++---
drivers/clk/spacemit/ccu_common.c | 12 +
drivers/clk/spacemit/ccu_common.h | 2 +
drivers/clk/spacemit/ccu_mix.c | 35 ++-
drivers/clk/spacemit/ccu_mix.h | 18 ++
drivers/clk/spacemit/ccu_pll.c | 535 +++++++++++++++++++++++++++++++-------
drivers/clk/spacemit/ccu_pll.h | 43 ++-
8 files changed, 689 insertions(+), 153 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260907-spacemit-pll-init-c942bb249244
Best regards,
--
Troy Mitchell <troy.mitchell@linux.spacemit.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/5] clk: spacemit: derive PLL rates from hardware
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-09 14:07 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware can program valid PLL settings that have no exact register
match in the rate table, even when they produce a listed frequency.
An exact table lookup in recalc_rate() therefore returns zero instead of
the hardware rate.
Replace the register-table lookup with calculations from the K1 PLL and
K3 PLLA register fields, accounting for signed and unsigned fractional
feedback respectively. Return zero for unsupported modes or register
read failures. Rate selection and programming remain table-based.
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_pll.c | 100 +++++++++++++++++++++++++++++++++++------
1 file changed, 86 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index d4066a0ed4526..c9852fed31017 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -4,8 +4,10 @@
* Copyright (c) 2024-2025 Haylen Chu <heylenay@4d2.org>
*/
+#include <linux/bitfield.h>
#include <linux/clk-provider.h>
#include <linux/math.h>
+#include <linux/math64.h>
#include <linux/regmap.h>
#include "ccu_common.h"
@@ -14,12 +16,24 @@
#define PLL_TIMEOUT_US 3000
#define PLL_DELAY_US 5
+#define PLL_SWCR1_PREDIV GENMASK(13, 12)
+#define PLL_SWCR1_INTERNAL BIT(29)
+#define PLL_SWCR3_INT GENMASK(30, 24)
+#define PLL_SWCR3_FRAC GENMASK(23, 0)
+
#define PLL_SWCR3_EN ((u32)BIT(31))
#define PLL_SWCR3_MASK GENMASK(30, 0)
#define PLLA_SWCR2_EN ((u32)BIT(16))
#define PLLA_SWCR2_MASK GENMASK(15, 8)
+#define PLLA_SWCR1_USER_MODE BIT(25)
+#define PLLA_SWCR1_INT GENMASK(22, 16)
+#define PLLA_SWCR1_REFSEL GENMASK(15, 14)
+#define PLLA_SWCR1_FRAC GENMASK(13, 0)
+#define PLLA_SWCR3_PREDIV GENMASK(21, 20)
+#define PLL_FRAC_BITS 22
+
static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *pll,
unsigned long rate)
{
@@ -115,17 +129,78 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
-static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static int ccu_pll_get_params(struct ccu_pll *pll,
+ struct ccu_pll_rate_tbl *params, bool plla)
{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
- const struct ccu_pll_rate_tbl *entry;
+ struct ccu_common *common = &pll->common;
+ int ret;
+
+ ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
+ if (ret)
+ return ret;
+ params->swcr2 = 0;
+ if (plla) {
+ ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
+ if (ret)
+ return ret;
+ }
+ return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
+}
+
+static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr3 = params->swcr3, prediv;
+ s64 divider;
+ u64 rate;
+
+ /* The programmed divider is not used in internal configuration mode. */
+ if (swcr1 & PLL_SWCR1_INTERNAL)
+ return 0;
+
+ prediv = FIELD_GET(PLL_SWCR1_PREDIV, swcr1) + 1;
+ divider = (s64)FIELD_GET(PLL_SWCR3_INT, swcr3) << PLL_FRAC_BITS;
+ /* The 24-bit fractional code is signed, with an LSB of 2^-22. */
+ divider += sign_extend32(FIELD_GET(PLL_SWCR3_FRAC, swcr3), 23);
+ if (divider <= 0)
+ return 0;
+
+ /* Fvco = Fref * Npre * (Nint + Nfrac). */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
+
+static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
+ u32 swcr3 = params->swcr3, prediv, frac;
+ u64 divider, rate;
+
+ /* Decode the software-controlled mode described by the PLL calculator. */
+ if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
+ (swcr1 & PLLA_SWCR1_REFSEL))
+ return 0;
+
+ prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
+ frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
+ frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
+ divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
+ divider += frac;
- entry = ccu_pll_lookup_matched_entry(pll);
+ /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
- WARN_ON_ONCE(!entry);
+static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ccu_pll_rate_tbl params;
- return entry ? entry->rate : 0;
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
+ return 0;
+ return ccu_pll_calc_rate(¶ms, parent_rate);
}
static int ccu_pll_determine_rate(struct clk_hw *hw,
@@ -232,14 +307,11 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
- const struct ccu_pll_rate_tbl *entry;
-
- entry = ccu_plla_lookup_matched_entry(pll);
+ struct ccu_pll_rate_tbl params;
- WARN_ON_ONCE(!entry);
-
- return entry ? entry->rate : 0;
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, true))
+ return 0;
+ return ccu_plla_calc_rate(¶ms, parent_rate);
}
static int ccu_plla_init(struct clk_hw *hw)
--
2.55.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 1/5] clk: spacemit: derive PLL rates from hardware
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware can program valid PLL settings that have no exact register
match in the rate table, even when they produce a listed frequency.
An exact table lookup in recalc_rate() therefore returns zero instead of
the hardware rate.
Replace the register-table lookup with calculations from the K1 PLL and
K3 PLLA register fields, accounting for signed and unsigned fractional
feedback respectively. Return zero for unsupported modes or register
read failures. Rate selection and programming remain table-based.
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_pll.c | 100 +++++++++++++++++++++++++++++++++++------
1 file changed, 86 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index d4066a0ed4526..c9852fed31017 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -4,8 +4,10 @@
* Copyright (c) 2024-2025 Haylen Chu <heylenay@4d2.org>
*/
+#include <linux/bitfield.h>
#include <linux/clk-provider.h>
#include <linux/math.h>
+#include <linux/math64.h>
#include <linux/regmap.h>
#include "ccu_common.h"
@@ -14,12 +16,24 @@
#define PLL_TIMEOUT_US 3000
#define PLL_DELAY_US 5
+#define PLL_SWCR1_PREDIV GENMASK(13, 12)
+#define PLL_SWCR1_INTERNAL BIT(29)
+#define PLL_SWCR3_INT GENMASK(30, 24)
+#define PLL_SWCR3_FRAC GENMASK(23, 0)
+
#define PLL_SWCR3_EN ((u32)BIT(31))
#define PLL_SWCR3_MASK GENMASK(30, 0)
#define PLLA_SWCR2_EN ((u32)BIT(16))
#define PLLA_SWCR2_MASK GENMASK(15, 8)
+#define PLLA_SWCR1_USER_MODE BIT(25)
+#define PLLA_SWCR1_INT GENMASK(22, 16)
+#define PLLA_SWCR1_REFSEL GENMASK(15, 14)
+#define PLLA_SWCR1_FRAC GENMASK(13, 0)
+#define PLLA_SWCR3_PREDIV GENMASK(21, 20)
+#define PLL_FRAC_BITS 22
+
static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *pll,
unsigned long rate)
{
@@ -115,17 +129,78 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
-static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
+static int ccu_pll_get_params(struct ccu_pll *pll,
+ struct ccu_pll_rate_tbl *params, bool plla)
{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
- const struct ccu_pll_rate_tbl *entry;
+ struct ccu_common *common = &pll->common;
+ int ret;
+
+ ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
+ if (ret)
+ return ret;
+ params->swcr2 = 0;
+ if (plla) {
+ ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
+ if (ret)
+ return ret;
+ }
+ return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
+}
+
+static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr3 = params->swcr3, prediv;
+ s64 divider;
+ u64 rate;
+
+ /* The programmed divider is not used in internal configuration mode. */
+ if (swcr1 & PLL_SWCR1_INTERNAL)
+ return 0;
+
+ prediv = FIELD_GET(PLL_SWCR1_PREDIV, swcr1) + 1;
+ divider = (s64)FIELD_GET(PLL_SWCR3_INT, swcr3) << PLL_FRAC_BITS;
+ /* The 24-bit fractional code is signed, with an LSB of 2^-22. */
+ divider += sign_extend32(FIELD_GET(PLL_SWCR3_FRAC, swcr3), 23);
+ if (divider <= 0)
+ return 0;
+
+ /* Fvco = Fref * Npre * (Nint + Nfrac). */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
+
+static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
+ u32 swcr3 = params->swcr3, prediv, frac;
+ u64 divider, rate;
+
+ /* Decode the software-controlled mode described by the PLL calculator. */
+ if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
+ (swcr1 & PLLA_SWCR1_REFSEL))
+ return 0;
+
+ prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
+ frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
+ frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
+ divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
+ divider += frac;
- entry = ccu_pll_lookup_matched_entry(pll);
+ /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
- WARN_ON_ONCE(!entry);
+static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ccu_pll_rate_tbl params;
- return entry ? entry->rate : 0;
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
+ return 0;
+ return ccu_pll_calc_rate(¶ms, parent_rate);
}
static int ccu_pll_determine_rate(struct clk_hw *hw,
@@ -232,14 +307,11 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
- const struct ccu_pll_rate_tbl *entry;
-
- entry = ccu_plla_lookup_matched_entry(pll);
+ struct ccu_pll_rate_tbl params;
- WARN_ON_ONCE(!entry);
-
- return entry ? entry->rate : 0;
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, true))
+ return 0;
+ return ccu_plla_calc_rate(¶ms, parent_rate);
}
static int ccu_plla_init(struct clk_hw *hw)
--
2.55.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-09 14:07 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
CCF passes the selected parent's rate to set_rate(). Searching other
parents at that point can produce a divider for a different source,
making the programmed rate disagree with CCF's selection.
Restrict divider selection to the supplied parent rate and use the same
rounding as divider_recalc_rate(). Track the best error separately so
low-rate requests do not leave the initial zero-Hz candidate selected.
Skip zero-rate parents and have determine_rate() reject requests when no
usable parent exists.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_mix.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
index a8b407049bf4d..da3c5685d4f65 100644
--- a/drivers/clk/spacemit/ccu_mix.c
+++ b/drivers/clk/spacemit/ccu_mix.c
@@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
struct ccu_mix *mix = hw_to_ccu_mix(hw);
unsigned int parent_num = clk_hw_get_num_parents(hw);
struct ccu_div_config *div = &mix->div;
- u32 div_max = 1 << div->width;
unsigned long best_rate = 0;
+ unsigned long best_delta = ULONG_MAX;
for (int i = 0; i < parent_num; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
unsigned long parent_rate;
+ u32 div_max = 1 << div->width;
if (!parent)
continue;
parent_rate = clk_hw_get_rate(parent);
+ if (!parent_rate)
+ continue;
for (int j = 1; j <= div_max; j++) {
- unsigned long tmp = DIV_ROUND_CLOSEST_ULL(parent_rate, j);
+ unsigned long tmp = DIV_ROUND_UP_ULL(parent_rate, j);
+ unsigned long delta = abs_diff(tmp, rate);
- if (abs(tmp - rate) < abs(best_rate - rate)) {
+ if (delta < best_delta) {
+ best_delta = delta;
best_rate = tmp;
if (div_val)
@@ -146,7 +151,7 @@ static int ccu_mix_determine_rate(struct clk_hw *hw,
&req->best_parent_hw,
&req->best_parent_rate,
NULL);
- return 0;
+ return req->rate ? 0 : -EINVAL;
}
static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -155,9 +160,19 @@ static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
struct ccu_mix *mix = hw_to_ccu_mix(hw);
struct ccu_common *common = &mix->common;
struct ccu_div_config *div = &mix->div;
- u32 current_div, target_div, mask;
+ u32 current_div, target_div = 0, mask;
+ unsigned long best_delta = ULONG_MAX;
+
+ /* set_rate must use the parent selected by CCF, not search other parents. */
+ for (u32 i = 1; i <= BIT(div->width); i++) {
+ unsigned long divided = DIV_ROUND_UP_ULL(parent_rate, i);
+ unsigned long delta = abs_diff(divided, rate);
- ccu_mix_calc_best_rate(hw, rate, NULL, NULL, &target_div);
+ if (delta < best_delta) {
+ best_delta = delta;
+ target_div = i - 1;
+ }
+ }
current_div = ccu_read(common, ctrl) >> div->shift;
current_div &= (1 << div->width) - 1;
--
2.55.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
CCF passes the selected parent's rate to set_rate(). Searching other
parents at that point can produce a divider for a different source,
making the programmed rate disagree with CCF's selection.
Restrict divider selection to the supplied parent rate and use the same
rounding as divider_recalc_rate(). Track the best error separately so
low-rate requests do not leave the initial zero-Hz candidate selected.
Skip zero-rate parents and have determine_rate() reject requests when no
usable parent exists.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_mix.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
index a8b407049bf4d..da3c5685d4f65 100644
--- a/drivers/clk/spacemit/ccu_mix.c
+++ b/drivers/clk/spacemit/ccu_mix.c
@@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
struct ccu_mix *mix = hw_to_ccu_mix(hw);
unsigned int parent_num = clk_hw_get_num_parents(hw);
struct ccu_div_config *div = &mix->div;
- u32 div_max = 1 << div->width;
unsigned long best_rate = 0;
+ unsigned long best_delta = ULONG_MAX;
for (int i = 0; i < parent_num; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
unsigned long parent_rate;
+ u32 div_max = 1 << div->width;
if (!parent)
continue;
parent_rate = clk_hw_get_rate(parent);
+ if (!parent_rate)
+ continue;
for (int j = 1; j <= div_max; j++) {
- unsigned long tmp = DIV_ROUND_CLOSEST_ULL(parent_rate, j);
+ unsigned long tmp = DIV_ROUND_UP_ULL(parent_rate, j);
+ unsigned long delta = abs_diff(tmp, rate);
- if (abs(tmp - rate) < abs(best_rate - rate)) {
+ if (delta < best_delta) {
+ best_delta = delta;
best_rate = tmp;
if (div_val)
@@ -146,7 +151,7 @@ static int ccu_mix_determine_rate(struct clk_hw *hw,
&req->best_parent_hw,
&req->best_parent_rate,
NULL);
- return 0;
+ return req->rate ? 0 : -EINVAL;
}
static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -155,9 +160,19 @@ static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
struct ccu_mix *mix = hw_to_ccu_mix(hw);
struct ccu_common *common = &mix->common;
struct ccu_div_config *div = &mix->div;
- u32 current_div, target_div, mask;
+ u32 current_div, target_div = 0, mask;
+ unsigned long best_delta = ULONG_MAX;
+
+ /* set_rate must use the parent selected by CCF, not search other parents. */
+ for (u32 i = 1; i <= BIT(div->width); i++) {
+ unsigned long divided = DIV_ROUND_UP_ULL(parent_rate, i);
+ unsigned long delta = abs_diff(divided, rate);
- ccu_mix_calc_best_rate(hw, rate, NULL, NULL, &target_div);
+ if (delta < best_delta) {
+ best_delta = delta;
+ target_div = i - 1;
+ }
+ }
current_div = ccu_read(common, ctrl) >> div->shift;
current_div &= (1 << div->width) - 1;
--
2.55.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-09 14:07 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
The K1 CPU clocks have a core divider, while K3 CPU clocks bypass theirs
for some sources. K3 also allows C1/C3 to share the C0/C2 PLLs. Omitting
these details from the clock model can give CCF an incorrect rate or
parent.
Model the dividers, K3 bypass conditions and shared parents. Register
the sharing muxes as internal clocks without adding DT clock IDs.
Exclude K3 selector 4 from the modeled parents to match BSP policy.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: e371a77255b8 ("clk: spacemit: k3: add the clock tree")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu-k1.c | 8 +++----
drivers/clk/spacemit/ccu-k3.c | 47 +++++++++++++++++++++++++++------------
drivers/clk/spacemit/ccu_common.c | 12 ++++++++++
drivers/clk/spacemit/ccu_common.h | 2 ++
drivers/clk/spacemit/ccu_mix.c | 10 ++++++++-
drivers/clk/spacemit/ccu_mix.h | 18 +++++++++++++++
6 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
index dee14d25f75d4..40cc96c4ee876 100644
--- a/drivers/clk/spacemit/ccu-k1.c
+++ b/drivers/clk/spacemit/ccu-k1.c
@@ -391,8 +391,8 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
CCU_PARENT_HW(pll2_d3),
CCU_PARENT_HW(cpu_c0_hi_clk),
};
-CCU_MUX_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL, BIT(12), 0, 3,
- CLK_IS_CRITICAL);
+CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c0_ace_clk, CCU_PARENT_HW(cpu_c0_core_clk), APMU_CPU_C0_CLK_CTRL, 6, 3,
CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c0_tcm_clk, CCU_PARENT_HW(cpu_c0_core_clk), APMU_CPU_C0_CLK_CTRL, 9, 3,
@@ -413,8 +413,8 @@ static const struct clk_parent_data cpu_c1_clk_parents[] = {
CCU_PARENT_HW(pll2_d3),
CCU_PARENT_HW(cpu_c1_hi_clk),
};
-CCU_MUX_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL, BIT(12), 0, 3,
- CLK_IS_CRITICAL);
+CCU_MUX_DIV_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c1_ace_clk, CCU_PARENT_HW(cpu_c1_core_clk), APMU_CPU_C1_CLK_CTRL, 6, 3,
CLK_IS_CRITICAL);
diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
index 92b930d5ff305..2f0bd74f7da4d 100644
--- a/drivers/clk/spacemit/ccu-k3.c
+++ b/drivers/clk/spacemit/ccu-k3.c
@@ -587,52 +587,64 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll3_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
CCU_PARENT_HW(pll3_d1),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
+
+static const struct clk_parent_data cpu_c1_pll_src_parents[] = {
+ CCU_PARENT_HW(pll4_d1),
+ CCU_PARENT_HW(pll3_d1),
+};
+CCU_MUX_DEFINE(cpu_c1_pll_src, cpu_c1_pll_src_parents, APMU_CPU_C1_CLK_CTRL, 13, 1, 0);
static const struct clk_parent_data cpu_c1_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll4_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
- CCU_PARENT_HW(pll4_d1),
+ CCU_PARENT_HW(cpu_c1_pll_src),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
static const struct clk_parent_data cpu_c2_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll5_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
CCU_PARENT_HW(pll5_d1),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c2_core_clk, cpu_c2_clk_parents, APMU_CPU_C2_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c2_core_clk, cpu_c2_clk_parents, APMU_CPU_C2_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
+
+static const struct clk_parent_data cpu_c3_pll_src_parents[] = {
+ CCU_PARENT_HW(pll8_d1),
+ CCU_PARENT_HW(pll5_d1),
+};
+CCU_MUX_DEFINE(cpu_c3_pll_src, cpu_c3_pll_src_parents, APMU_CPU_C3_CLK_CTRL, 13, 1, 0);
static const struct clk_parent_data cpu_c3_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll8_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
- CCU_PARENT_HW(pll8_d1),
+ CCU_PARENT_HW(cpu_c3_pll_src),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c3_core_clk, cpu_c3_clk_parents, APMU_CPU_C3_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c3_core_clk, cpu_c3_clk_parents, APMU_CPU_C3_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
static const struct clk_parent_data ccic2phy_parents[] = {
CCU_PARENT_HW(pll1_d24_102p4),
@@ -1447,10 +1459,17 @@ static struct clk_hw *k3_ccu_apmu_hws[] = {
[CLK_APMU_ISIM_VCLK3] = &isim_vclk_out3.common.hw,
};
+static struct clk_hw *k3_ccu_apmu_internal_hws[] = {
+ &cpu_c1_pll_src.common.hw,
+ &cpu_c3_pll_src.common.hw,
+};
+
static const struct spacemit_ccu_data k3_ccu_apmu_data = {
.reset_name = "k3-apmu-reset",
.hws = k3_ccu_apmu_hws,
.num = ARRAY_SIZE(k3_ccu_apmu_hws),
+ .internal_hws = k3_ccu_apmu_internal_hws,
+ .num_internal = ARRAY_SIZE(k3_ccu_apmu_internal_hws),
};
static struct clk_hw *k3_ccu_dciu_hws[] = {
diff --git a/drivers/clk/spacemit/ccu_common.c b/drivers/clk/spacemit/ccu_common.c
index 8696bb9cba2d8..759da6988ca78 100644
--- a/drivers/clk/spacemit/ccu_common.c
+++ b/drivers/clk/spacemit/ccu_common.c
@@ -30,6 +30,18 @@ static int spacemit_ccu_register(struct device *dev,
clk_data->num = data->num;
+ /* Internal muxes have no binding IDs but must precede their children. */
+ for (i = 0; i < data->num_internal; i++) {
+ struct clk_hw *hw = data->internal_hws[i];
+ struct ccu_common *common = hw_to_ccu_common(hw);
+
+ common->regmap = regmap;
+ common->lock_regmap = lock_regmap;
+ ret = devm_clk_hw_register(dev, hw);
+ if (ret)
+ return ret;
+ }
+
for (i = 0; i < data->num; i++) {
struct clk_hw *hw = data->hws[i];
struct ccu_common *common;
diff --git a/drivers/clk/spacemit/ccu_common.h b/drivers/clk/spacemit/ccu_common.h
index 8691698e007d9..659dc68ab902a 100644
--- a/drivers/clk/spacemit/ccu_common.h
+++ b/drivers/clk/spacemit/ccu_common.h
@@ -43,6 +43,8 @@ struct spacemit_ccu_data {
const char *reset_name;
struct clk_hw **hws;
size_t num;
+ struct clk_hw **internal_hws;
+ size_t num_internal;
};
#define ccu_read(c, reg) \
diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
index da3c5685d4f65..68596b9fa5b8f 100644
--- a/drivers/clk/spacemit/ccu_mix.c
+++ b/drivers/clk/spacemit/ccu_mix.c
@@ -13,6 +13,8 @@
#define MIX_FC_TIMEOUT_US 10000
#define MIX_FC_DELAY_US 5
+static u8 ccu_mux_get_parent(struct clk_hw *hw);
+
static void ccu_gate_disable(struct clk_hw *hw)
{
struct ccu_mix *mix = hw_to_ccu_mix(hw);
@@ -57,6 +59,9 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
struct ccu_div_config *div = &mix->div;
unsigned long val;
+ if (div->bypass & BIT(ccu_mux_get_parent(hw)))
+ return parent_rate;
+
val = ccu_read(&mix->common, ctrl) >> div->shift;
val &= (1 << div->width) - 1;
@@ -113,7 +118,7 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
for (int i = 0; i < parent_num; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
unsigned long parent_rate;
- u32 div_max = 1 << div->width;
+ u32 div_max = div->bypass & BIT(i) ? 1 : 1 << div->width;
if (!parent)
continue;
@@ -163,6 +168,9 @@ static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
u32 current_div, target_div = 0, mask;
unsigned long best_delta = ULONG_MAX;
+ if (div->bypass & BIT(ccu_mux_get_parent(hw)))
+ return rate == parent_rate ? 0 : -EINVAL;
+
/* set_rate must use the parent selected by CCF, not search other parents. */
for (u32 i = 1; i <= BIT(div->width); i++) {
unsigned long divided = DIV_ROUND_UP_ULL(parent_rate, i);
diff --git a/drivers/clk/spacemit/ccu_mix.h b/drivers/clk/spacemit/ccu_mix.h
index dbba9bf49b3bd..ac88c282d660a 100644
--- a/drivers/clk/spacemit/ccu_mix.h
+++ b/drivers/clk/spacemit/ccu_mix.h
@@ -36,6 +36,8 @@ struct ccu_mux_config {
struct ccu_div_config {
u8 shift;
u8 width;
+ /* Parent indices which bypass the divider; zero means no bypass. */
+ u32 bypass;
};
struct ccu_mix {
@@ -203,6 +205,22 @@ static struct ccu_mix _name = { \
}, \
}
+#define CCU_MUX_DIV_BYPASS_FC_DEFINE(_name, _parents, _reg_ctrl, \
+ _mshift, _mwidth, _mask_fc, \
+ _muxshift, _muxwidth, _flags, \
+ _bypass) \
+static struct ccu_mix _name = { \
+ .div = { .shift = _mshift, .width = _mwidth, .bypass = _bypass }, \
+ .mux = CCU_MUX_INIT(_muxshift, _muxwidth), \
+ .common = { \
+ .reg_ctrl = _reg_ctrl, \
+ .reg_fc = _reg_ctrl, \
+ .mask_fc = _mask_fc, \
+ CCU_MIX_INITHW_PARENTS(_name, _parents, \
+ spacemit_ccu_mux_div_ops, _flags), \
+ }, \
+}
+
#define CCU_MUX_FC_DEFINE(_name, _parents, _reg_ctrl, _mask_fc, _muxshift, \
_muxwidth, _flags) \
static struct ccu_mix _name = { \
--
2.55.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
The K1 CPU clocks have a core divider, while K3 CPU clocks bypass theirs
for some sources. K3 also allows C1/C3 to share the C0/C2 PLLs. Omitting
these details from the clock model can give CCF an incorrect rate or
parent.
Model the dividers, K3 bypass conditions and shared parents. Register
the sharing muxes as internal clocks without adding DT clock IDs.
Exclude K3 selector 4 from the modeled parents to match BSP policy.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: e371a77255b8 ("clk: spacemit: k3: add the clock tree")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu-k1.c | 8 +++----
drivers/clk/spacemit/ccu-k3.c | 47 +++++++++++++++++++++++++++------------
drivers/clk/spacemit/ccu_common.c | 12 ++++++++++
drivers/clk/spacemit/ccu_common.h | 2 ++
drivers/clk/spacemit/ccu_mix.c | 10 ++++++++-
drivers/clk/spacemit/ccu_mix.h | 18 +++++++++++++++
6 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
index dee14d25f75d4..40cc96c4ee876 100644
--- a/drivers/clk/spacemit/ccu-k1.c
+++ b/drivers/clk/spacemit/ccu-k1.c
@@ -391,8 +391,8 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
CCU_PARENT_HW(pll2_d3),
CCU_PARENT_HW(cpu_c0_hi_clk),
};
-CCU_MUX_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL, BIT(12), 0, 3,
- CLK_IS_CRITICAL);
+CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c0_ace_clk, CCU_PARENT_HW(cpu_c0_core_clk), APMU_CPU_C0_CLK_CTRL, 6, 3,
CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c0_tcm_clk, CCU_PARENT_HW(cpu_c0_core_clk), APMU_CPU_C0_CLK_CTRL, 9, 3,
@@ -413,8 +413,8 @@ static const struct clk_parent_data cpu_c1_clk_parents[] = {
CCU_PARENT_HW(pll2_d3),
CCU_PARENT_HW(cpu_c1_hi_clk),
};
-CCU_MUX_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL, BIT(12), 0, 3,
- CLK_IS_CRITICAL);
+CCU_MUX_DIV_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
CCU_DIV_DEFINE(cpu_c1_ace_clk, CCU_PARENT_HW(cpu_c1_core_clk), APMU_CPU_C1_CLK_CTRL, 6, 3,
CLK_IS_CRITICAL);
diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
index 92b930d5ff305..2f0bd74f7da4d 100644
--- a/drivers/clk/spacemit/ccu-k3.c
+++ b/drivers/clk/spacemit/ccu-k3.c
@@ -587,52 +587,64 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll3_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
CCU_PARENT_HW(pll3_d1),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
+
+static const struct clk_parent_data cpu_c1_pll_src_parents[] = {
+ CCU_PARENT_HW(pll4_d1),
+ CCU_PARENT_HW(pll3_d1),
+};
+CCU_MUX_DEFINE(cpu_c1_pll_src, cpu_c1_pll_src_parents, APMU_CPU_C1_CLK_CTRL, 13, 1, 0);
static const struct clk_parent_data cpu_c1_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll4_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
- CCU_PARENT_HW(pll4_d1),
+ CCU_PARENT_HW(cpu_c1_pll_src),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c1_core_clk, cpu_c1_clk_parents, APMU_CPU_C1_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
static const struct clk_parent_data cpu_c2_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll5_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
CCU_PARENT_HW(pll5_d1),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c2_core_clk, cpu_c2_clk_parents, APMU_CPU_C2_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c2_core_clk, cpu_c2_clk_parents, APMU_CPU_C2_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
+
+static const struct clk_parent_data cpu_c3_pll_src_parents[] = {
+ CCU_PARENT_HW(pll8_d1),
+ CCU_PARENT_HW(pll5_d1),
+};
+CCU_MUX_DEFINE(cpu_c3_pll_src, cpu_c3_pll_src_parents, APMU_CPU_C3_CLK_CTRL, 13, 1, 0);
static const struct clk_parent_data cpu_c3_clk_parents[] = {
CCU_PARENT_HW(pll1_d3_819p2),
CCU_PARENT_HW(pll1_d5_491p52),
CCU_PARENT_HW(pll1_d4_614p4),
CCU_PARENT_HW(pll2_d3),
- CCU_PARENT_HW(pll8_d2),
+ { .index = -1 }, /* Leave selector 4 unmodeled, as in the BSP. */
CCU_PARENT_HW(pll1_d2_1228p8),
CCU_PARENT_HW(pll2_d2),
- CCU_PARENT_HW(pll8_d1),
+ CCU_PARENT_HW(cpu_c3_pll_src),
};
-CCU_MUX_DIV_FC_DEFINE(cpu_c3_core_clk, cpu_c3_clk_parents, APMU_CPU_C3_CLK_CTRL,
- 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
+CCU_MUX_DIV_BYPASS_FC_DEFINE(cpu_c3_core_clk, cpu_c3_clk_parents, APMU_CPU_C3_CLK_CTRL,
+ 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL, GENMASK(7, 4));
static const struct clk_parent_data ccic2phy_parents[] = {
CCU_PARENT_HW(pll1_d24_102p4),
@@ -1447,10 +1459,17 @@ static struct clk_hw *k3_ccu_apmu_hws[] = {
[CLK_APMU_ISIM_VCLK3] = &isim_vclk_out3.common.hw,
};
+static struct clk_hw *k3_ccu_apmu_internal_hws[] = {
+ &cpu_c1_pll_src.common.hw,
+ &cpu_c3_pll_src.common.hw,
+};
+
static const struct spacemit_ccu_data k3_ccu_apmu_data = {
.reset_name = "k3-apmu-reset",
.hws = k3_ccu_apmu_hws,
.num = ARRAY_SIZE(k3_ccu_apmu_hws),
+ .internal_hws = k3_ccu_apmu_internal_hws,
+ .num_internal = ARRAY_SIZE(k3_ccu_apmu_internal_hws),
};
static struct clk_hw *k3_ccu_dciu_hws[] = {
diff --git a/drivers/clk/spacemit/ccu_common.c b/drivers/clk/spacemit/ccu_common.c
index 8696bb9cba2d8..759da6988ca78 100644
--- a/drivers/clk/spacemit/ccu_common.c
+++ b/drivers/clk/spacemit/ccu_common.c
@@ -30,6 +30,18 @@ static int spacemit_ccu_register(struct device *dev,
clk_data->num = data->num;
+ /* Internal muxes have no binding IDs but must precede their children. */
+ for (i = 0; i < data->num_internal; i++) {
+ struct clk_hw *hw = data->internal_hws[i];
+ struct ccu_common *common = hw_to_ccu_common(hw);
+
+ common->regmap = regmap;
+ common->lock_regmap = lock_regmap;
+ ret = devm_clk_hw_register(dev, hw);
+ if (ret)
+ return ret;
+ }
+
for (i = 0; i < data->num; i++) {
struct clk_hw *hw = data->hws[i];
struct ccu_common *common;
diff --git a/drivers/clk/spacemit/ccu_common.h b/drivers/clk/spacemit/ccu_common.h
index 8691698e007d9..659dc68ab902a 100644
--- a/drivers/clk/spacemit/ccu_common.h
+++ b/drivers/clk/spacemit/ccu_common.h
@@ -43,6 +43,8 @@ struct spacemit_ccu_data {
const char *reset_name;
struct clk_hw **hws;
size_t num;
+ struct clk_hw **internal_hws;
+ size_t num_internal;
};
#define ccu_read(c, reg) \
diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
index da3c5685d4f65..68596b9fa5b8f 100644
--- a/drivers/clk/spacemit/ccu_mix.c
+++ b/drivers/clk/spacemit/ccu_mix.c
@@ -13,6 +13,8 @@
#define MIX_FC_TIMEOUT_US 10000
#define MIX_FC_DELAY_US 5
+static u8 ccu_mux_get_parent(struct clk_hw *hw);
+
static void ccu_gate_disable(struct clk_hw *hw)
{
struct ccu_mix *mix = hw_to_ccu_mix(hw);
@@ -57,6 +59,9 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
struct ccu_div_config *div = &mix->div;
unsigned long val;
+ if (div->bypass & BIT(ccu_mux_get_parent(hw)))
+ return parent_rate;
+
val = ccu_read(&mix->common, ctrl) >> div->shift;
val &= (1 << div->width) - 1;
@@ -113,7 +118,7 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
for (int i = 0; i < parent_num; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
unsigned long parent_rate;
- u32 div_max = 1 << div->width;
+ u32 div_max = div->bypass & BIT(i) ? 1 : 1 << div->width;
if (!parent)
continue;
@@ -163,6 +168,9 @@ static int ccu_mix_set_rate(struct clk_hw *hw, unsigned long rate,
u32 current_div, target_div = 0, mask;
unsigned long best_delta = ULONG_MAX;
+ if (div->bypass & BIT(ccu_mux_get_parent(hw)))
+ return rate == parent_rate ? 0 : -EINVAL;
+
/* set_rate must use the parent selected by CCF, not search other parents. */
for (u32 i = 1; i <= BIT(div->width); i++) {
unsigned long divided = DIV_ROUND_UP_ULL(parent_rate, i);
diff --git a/drivers/clk/spacemit/ccu_mix.h b/drivers/clk/spacemit/ccu_mix.h
index dbba9bf49b3bd..ac88c282d660a 100644
--- a/drivers/clk/spacemit/ccu_mix.h
+++ b/drivers/clk/spacemit/ccu_mix.h
@@ -36,6 +36,8 @@ struct ccu_mux_config {
struct ccu_div_config {
u8 shift;
u8 width;
+ /* Parent indices which bypass the divider; zero means no bypass. */
+ u32 bypass;
};
struct ccu_mix {
@@ -203,6 +205,22 @@ static struct ccu_mix _name = { \
}, \
}
+#define CCU_MUX_DIV_BYPASS_FC_DEFINE(_name, _parents, _reg_ctrl, \
+ _mshift, _mwidth, _mask_fc, \
+ _muxshift, _muxwidth, _flags, \
+ _bypass) \
+static struct ccu_mix _name = { \
+ .div = { .shift = _mshift, .width = _mwidth, .bypass = _bypass }, \
+ .mux = CCU_MUX_INIT(_muxshift, _muxwidth), \
+ .common = { \
+ .reg_ctrl = _reg_ctrl, \
+ .reg_fc = _reg_ctrl, \
+ .mask_fc = _mask_fc, \
+ CCU_MIX_INITHW_PARENTS(_name, _parents, \
+ spacemit_ccu_mux_div_ops, _flags), \
+ }, \
+}
+
#define CCU_MUX_FC_DEFINE(_name, _parents, _reg_ctrl, _mask_fc, _muxshift, \
_muxwidth, _flags) \
static struct ccu_mix _name = { \
--
2.55.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-09 14:07 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
running by firmware can have a zero prepare count, so this flag alone
cannot prevent set_rate() from reprogramming a live PLL.
Check the hardware state and reject set_rate() while either the enable
or lock bit is set on K1 PLLs and K3 PLLAs. Propagate register read/write
failures and reject rate requests with no table candidate. Callers must
still move consumers away and stop the PLL before changing its rate.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_pll.c | 92 +++++++++++++++++++++++++++++++++---------
1 file changed, 72 insertions(+), 20 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index c9852fed31017..2573b8396cefa 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -38,9 +38,9 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *p
unsigned long rate)
{
struct ccu_pll_config *config = &pll->config;
- const struct ccu_pll_rate_tbl *best_entry;
+ const struct ccu_pll_rate_tbl *best_entry = NULL;
unsigned long best_delta = ULONG_MAX;
- int i;
+ u32 i;
for (i = 0; i < config->tbl_num; i++) {
const struct ccu_pll_rate_tbl *entry = &config->rate_tbl[i];
@@ -75,19 +75,25 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_matched_entry(struct ccu_pl
return NULL;
}
-static void ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
+static int ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
+ int ret;
- regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
- ccu_update(common, swcr3, PLL_SWCR3_MASK, entry->swcr3);
+ ret = regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
+ if (ret)
+ return ret;
+ return ccu_update(common, swcr3, PLL_SWCR3_MASK, entry->swcr3);
}
static int ccu_pll_is_enabled(struct clk_hw *hw)
{
struct ccu_common *common = hw_to_ccu_common(hw);
+ u32 val;
+ int ret;
- return ccu_read(common, swcr3) & PLL_SWCR3_EN;
+ ret = regmap_read(common->regmap, common->reg_swcr3, &val);
+ return ret ? ret : !!(val & PLL_SWCR3_EN);
}
static int ccu_pll_enable(struct clk_hw *hw)
@@ -95,8 +101,11 @@ static int ccu_pll_enable(struct clk_hw *hw)
struct ccu_pll *pll = hw_to_ccu_pll(hw);
struct ccu_common *common = &pll->common;
unsigned int tmp;
+ int ret;
- ccu_update(common, swcr3, PLL_SWCR3_EN, PLL_SWCR3_EN);
+ ret = ccu_update(common, swcr3, PLL_SWCR3_EN, PLL_SWCR3_EN);
+ if (ret)
+ return ret;
/* check lock status */
return regmap_read_poll_timeout_atomic(common->lock_regmap,
@@ -113,6 +122,17 @@ static void ccu_pll_disable(struct clk_hw *hw)
ccu_update(common, swcr3, PLL_SWCR3_EN, 0);
}
+static int ccu_pll_check_stopped(struct ccu_pll *pll)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_read(pll->common.lock_regmap, pll->config.reg_lock, &val);
+ if (ret)
+ return ret;
+ return val & pll->config.mask_lock ? -EBUSY : 0;
+}
+
/*
* PLLs must be gated before changing rate, which is ensured by
* flag CLK_SET_RATE_GATE.
@@ -122,11 +142,20 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
const struct ccu_pll_rate_tbl *entry;
+ int ret;
- entry = ccu_pll_lookup_best_rate(pll, rate);
- ccu_pll_update_param(pll, entry);
+ /* CLK_SET_RATE_GATE does not account for firmware-only users. */
+ ret = ccu_pll_is_enabled(hw);
+ if (ret)
+ return ret < 0 ? ret : -EBUSY;
+ ret = ccu_pll_check_stopped(pll);
+ if (ret)
+ return ret;
- return 0;
+ entry = ccu_pll_lookup_best_rate(pll, rate);
+ if (!entry)
+ return -EINVAL;
+ return ccu_pll_update_param(pll, entry);
}
static int ccu_pll_get_params(struct ccu_pll *pll,
@@ -207,8 +236,12 @@ static int ccu_pll_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
+ const struct ccu_pll_rate_tbl *entry;
- req->rate = ccu_pll_lookup_best_rate(pll, req->rate)->rate;
+ entry = ccu_pll_lookup_best_rate(pll, req->rate);
+ if (!entry)
+ return -EINVAL;
+ req->rate = entry->rate;
return 0;
}
@@ -249,20 +282,28 @@ static const struct ccu_pll_rate_tbl *ccu_plla_lookup_matched_entry(struct ccu_p
return NULL;
}
-static void ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
+static int ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
+ int ret;
- regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
- regmap_write(common->regmap, common->reg_swcr3, entry->swcr3);
- ccu_update(common, swcr2, PLLA_SWCR2_MASK, entry->swcr2);
+ ret = regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
+ if (ret)
+ return ret;
+ ret = regmap_write(common->regmap, common->reg_swcr3, entry->swcr3);
+ if (ret)
+ return ret;
+ return ccu_update(common, swcr2, PLLA_SWCR2_MASK, entry->swcr2);
}
static int ccu_plla_is_enabled(struct clk_hw *hw)
{
struct ccu_common *common = hw_to_ccu_common(hw);
+ u32 val;
+ int ret;
- return ccu_read(common, swcr2) & PLLA_SWCR2_EN;
+ ret = regmap_read(common->regmap, common->reg_swcr2, &val);
+ return ret ? ret : !!(val & PLLA_SWCR2_EN);
}
static int ccu_plla_enable(struct clk_hw *hw)
@@ -270,8 +311,11 @@ static int ccu_plla_enable(struct clk_hw *hw)
struct ccu_pll *pll = hw_to_ccu_pll(hw);
struct ccu_common *common = &pll->common;
unsigned int tmp;
+ int ret;
- ccu_update(common, swcr2, PLLA_SWCR2_EN, PLLA_SWCR2_EN);
+ ret = ccu_update(common, swcr2, PLLA_SWCR2_EN, PLLA_SWCR2_EN);
+ if (ret)
+ return ret;
/* check lock status */
return regmap_read_poll_timeout_atomic(common->lock_regmap,
@@ -297,11 +341,19 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
const struct ccu_pll_rate_tbl *entry;
+ int ret;
- entry = ccu_pll_lookup_best_rate(pll, rate);
- ccu_plla_update_param(pll, entry);
+ ret = ccu_plla_is_enabled(hw);
+ if (ret)
+ return ret < 0 ? ret : -EBUSY;
+ ret = ccu_pll_check_stopped(pll);
+ if (ret)
+ return ret;
- return 0;
+ entry = ccu_pll_lookup_best_rate(pll, rate);
+ if (!entry)
+ return -EINVAL;
+ return ccu_plla_update_param(pll, entry);
}
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
--
2.55.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
running by firmware can have a zero prepare count, so this flag alone
cannot prevent set_rate() from reprogramming a live PLL.
Check the hardware state and reject set_rate() while either the enable
or lock bit is set on K1 PLLs and K3 PLLAs. Propagate register read/write
failures and reject rate requests with no table candidate. Callers must
still move consumers away and stop the PLL before changing its rate.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu_pll.c | 92 +++++++++++++++++++++++++++++++++---------
1 file changed, 72 insertions(+), 20 deletions(-)
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index c9852fed31017..2573b8396cefa 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -38,9 +38,9 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *p
unsigned long rate)
{
struct ccu_pll_config *config = &pll->config;
- const struct ccu_pll_rate_tbl *best_entry;
+ const struct ccu_pll_rate_tbl *best_entry = NULL;
unsigned long best_delta = ULONG_MAX;
- int i;
+ u32 i;
for (i = 0; i < config->tbl_num; i++) {
const struct ccu_pll_rate_tbl *entry = &config->rate_tbl[i];
@@ -75,19 +75,25 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_matched_entry(struct ccu_pl
return NULL;
}
-static void ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
+static int ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
+ int ret;
- regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
- ccu_update(common, swcr3, PLL_SWCR3_MASK, entry->swcr3);
+ ret = regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
+ if (ret)
+ return ret;
+ return ccu_update(common, swcr3, PLL_SWCR3_MASK, entry->swcr3);
}
static int ccu_pll_is_enabled(struct clk_hw *hw)
{
struct ccu_common *common = hw_to_ccu_common(hw);
+ u32 val;
+ int ret;
- return ccu_read(common, swcr3) & PLL_SWCR3_EN;
+ ret = regmap_read(common->regmap, common->reg_swcr3, &val);
+ return ret ? ret : !!(val & PLL_SWCR3_EN);
}
static int ccu_pll_enable(struct clk_hw *hw)
@@ -95,8 +101,11 @@ static int ccu_pll_enable(struct clk_hw *hw)
struct ccu_pll *pll = hw_to_ccu_pll(hw);
struct ccu_common *common = &pll->common;
unsigned int tmp;
+ int ret;
- ccu_update(common, swcr3, PLL_SWCR3_EN, PLL_SWCR3_EN);
+ ret = ccu_update(common, swcr3, PLL_SWCR3_EN, PLL_SWCR3_EN);
+ if (ret)
+ return ret;
/* check lock status */
return regmap_read_poll_timeout_atomic(common->lock_regmap,
@@ -113,6 +122,17 @@ static void ccu_pll_disable(struct clk_hw *hw)
ccu_update(common, swcr3, PLL_SWCR3_EN, 0);
}
+static int ccu_pll_check_stopped(struct ccu_pll *pll)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_read(pll->common.lock_regmap, pll->config.reg_lock, &val);
+ if (ret)
+ return ret;
+ return val & pll->config.mask_lock ? -EBUSY : 0;
+}
+
/*
* PLLs must be gated before changing rate, which is ensured by
* flag CLK_SET_RATE_GATE.
@@ -122,11 +142,20 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
const struct ccu_pll_rate_tbl *entry;
+ int ret;
- entry = ccu_pll_lookup_best_rate(pll, rate);
- ccu_pll_update_param(pll, entry);
+ /* CLK_SET_RATE_GATE does not account for firmware-only users. */
+ ret = ccu_pll_is_enabled(hw);
+ if (ret)
+ return ret < 0 ? ret : -EBUSY;
+ ret = ccu_pll_check_stopped(pll);
+ if (ret)
+ return ret;
- return 0;
+ entry = ccu_pll_lookup_best_rate(pll, rate);
+ if (!entry)
+ return -EINVAL;
+ return ccu_pll_update_param(pll, entry);
}
static int ccu_pll_get_params(struct ccu_pll *pll,
@@ -207,8 +236,12 @@ static int ccu_pll_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
+ const struct ccu_pll_rate_tbl *entry;
- req->rate = ccu_pll_lookup_best_rate(pll, req->rate)->rate;
+ entry = ccu_pll_lookup_best_rate(pll, req->rate);
+ if (!entry)
+ return -EINVAL;
+ req->rate = entry->rate;
return 0;
}
@@ -249,20 +282,28 @@ static const struct ccu_pll_rate_tbl *ccu_plla_lookup_matched_entry(struct ccu_p
return NULL;
}
-static void ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
+static int ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
+ int ret;
- regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
- regmap_write(common->regmap, common->reg_swcr3, entry->swcr3);
- ccu_update(common, swcr2, PLLA_SWCR2_MASK, entry->swcr2);
+ ret = regmap_write(common->regmap, common->reg_swcr1, entry->swcr1);
+ if (ret)
+ return ret;
+ ret = regmap_write(common->regmap, common->reg_swcr3, entry->swcr3);
+ if (ret)
+ return ret;
+ return ccu_update(common, swcr2, PLLA_SWCR2_MASK, entry->swcr2);
}
static int ccu_plla_is_enabled(struct clk_hw *hw)
{
struct ccu_common *common = hw_to_ccu_common(hw);
+ u32 val;
+ int ret;
- return ccu_read(common, swcr2) & PLLA_SWCR2_EN;
+ ret = regmap_read(common->regmap, common->reg_swcr2, &val);
+ return ret ? ret : !!(val & PLLA_SWCR2_EN);
}
static int ccu_plla_enable(struct clk_hw *hw)
@@ -270,8 +311,11 @@ static int ccu_plla_enable(struct clk_hw *hw)
struct ccu_pll *pll = hw_to_ccu_pll(hw);
struct ccu_common *common = &pll->common;
unsigned int tmp;
+ int ret;
- ccu_update(common, swcr2, PLLA_SWCR2_EN, PLLA_SWCR2_EN);
+ ret = ccu_update(common, swcr2, PLLA_SWCR2_EN, PLLA_SWCR2_EN);
+ if (ret)
+ return ret;
/* check lock status */
return regmap_read_poll_timeout_atomic(common->lock_regmap,
@@ -297,11 +341,19 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
const struct ccu_pll_rate_tbl *entry;
+ int ret;
- entry = ccu_pll_lookup_best_rate(pll, rate);
- ccu_plla_update_param(pll, entry);
+ ret = ccu_plla_is_enabled(hw);
+ if (ret)
+ return ret < 0 ? ret : -EBUSY;
+ ret = ccu_pll_check_stopped(pll);
+ if (ret)
+ return ret;
- return 0;
+ entry = ccu_pll_lookup_best_rate(pll, rate);
+ if (!entry)
+ return -EINVAL;
+ return ccu_plla_update_param(pll, entry);
}
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
--
2.55.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/5] clk: spacemit: safely synchronize PLL parameters during init
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-09 14:07 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware PLL settings can be valid without matching a rate-table entry.
Falling back to the first entry during init can halt CPUs by disabling
their clock source.
Replace that fallback with opt-in parameter synchronization. Preserve
unlisted rates and PLLs without a sync descriptor, including PLL1. Leave
already matching parameters unchanged. For differing parameters, require
an entry that decodes to the same rate and no enabled outputs outside
the described CPU paths.
Park mapped CPU consumers on validated PLL1 branches no faster than
their current clock before stopping their PLL and programming the
matching entry. Re-enable previously enabled PLLs and wait for lock
before restoring CPU parents; leave initially disabled PLLs disabled.
Handle K1 PLL3 and K3 PLL3/4/5/8, parking sharing clusters first and
restoring them last. K3 CPU parking still depends on firmware cluster
initialization for FC completion.
After a programming or relock failure, attempt to restore the old PLL
parameters. Leave parked CPUs on PLL1 if PLL recovery fails, and keep
the fallback gates critical in case an FC handshake does not complete.
Keep synchronization in init, before CCF links the PLL to its children.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu-k1.c | 52 ++++-
drivers/clk/spacemit/ccu-k3.c | 90 +++++++--
drivers/clk/spacemit/ccu_pll.c | 447 +++++++++++++++++++++++++++++++----------
drivers/clk/spacemit/ccu_pll.h | 43 +++-
4 files changed, 485 insertions(+), 147 deletions(-)
diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
index 40cc96c4ee876..0085179f8d7a7 100644
--- a/drivers/clk/spacemit/ccu-k1.c
+++ b/drivers/clk/spacemit/ccu-k1.c
@@ -41,17 +41,47 @@ static const struct ccu_pll_rate_tbl pll3_rate_tbl[] = {
CCU_PLL_RATE(3200000000UL, 0x0050dd67, 0x43eaaaab),
};
-CCU_PLL_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR3, MPMU_POSR, POSR_PLL1_LOCK,
- CLK_SET_RATE_GATE);
-CCU_PLL_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
- CLK_SET_RATE_GATE);
-CCU_PLL_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
- CLK_SET_RATE_GATE);
-
-CCU_FACTOR_GATE_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1);
+/* PLL1 supplies the fallback and must retain its firmware configuration. */
+CCU_PLL_SYNC_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR2,
+ APBS_PLL1_SWCR3, MPMU_POSR, POSR_PLL1_LOCK,
+ CLK_SET_RATE_GATE, NULL);
+
+static const struct ccu_pll_sync pll2_sync = {
+ .safe_pll = &pll1,
+};
+
+static const struct ccu_pll_cpu_mux pll3_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, GENMASK(2, 0), 5 },
+ { APMU_CPU_C1_CLK_CTRL, GENMASK(2, 0), 7 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 5 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_sync pll3_sync = {
+ .safe_pll = &pll1,
+ .apmu_compatible = "spacemit,k1-syscon-apmu",
+ .muxes = pll3_cpu_muxes,
+ .num_muxes = ARRAY_SIZE(pll3_cpu_muxes),
+ .cpu_outputs = GENMASK(2, 0),
+ .safe_sel = 4,
+ .slow_sel = 3,
+ .reg_safe_gate = MPMU_ACGR,
+};
+
+CCU_PLL_SYNC_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2,
+ APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
+ CLK_SET_RATE_GATE, &pll2_sync);
+CCU_PLL_SYNC_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2,
+ APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
+ CLK_SET_RATE_GATE, &pll3_sync);
+
+/* A failed CPU FC handshake must not lose either fallback clock. */
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d3, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(2), 3, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d4, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(3), 4, 1);
-CCU_FACTOR_GATE_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1);
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d6, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(5), 6, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d7, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(6), 7, 1);
CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d8, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(7), 8, 1,
@@ -111,7 +141,7 @@ CCU_FACTOR_DEFINE(pll1_d3072_0p8, CCU_PARENT_HW(pll1_d384_6p4), 8, 1);
CCU_GATE_DEFINE(pll1_d6_409p6, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(0), 0);
CCU_FACTOR_GATE_DEFINE(pll1_d12_204p8, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(5), 2, 1);
-CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), 0);
+CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d10_245p76, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(18), 2, 1);
CCU_GATE_DEFINE(pll1_d4_614p4, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(15), 0);
@@ -120,7 +150,7 @@ CCU_FACTOR_GATE_DEFINE(pll1_d78_31p5, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(6),
CCU_GATE_DEFINE(pll1_d3_819p2, CCU_PARENT_HW(pll1_d3), MPMU_ACGR, BIT(14), 0);
-CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), 0);
+CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), CLK_IS_CRITICAL);
CCU_GATE_DEFINE(slow_uart, CCU_PARENT_NAME(osc), MPMU_ACGR, BIT(1), CLK_IGNORE_UNUSED);
CCU_DDN_DEFINE(slow_uart1_14p74, pll1_d16_153p6, MPMU_SUCCR, 16, 13, 0, 13, 2, 0);
diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
index 2f0bd74f7da4d..b01f2ce5d394f 100644
--- a/drivers/clk/spacemit/ccu-k3.c
+++ b/drivers/clk/spacemit/ccu-k3.c
@@ -55,25 +55,77 @@ static const struct ccu_pll_rate_tbl pll8_rate_tbl[] = {
CCU_PLLA_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR2, APBS_PLL1_SWCR3,
MPMU_POSR, POSR_PLL1_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2, APBS_PLL2_SWCR3,
- MPMU_POSR, POSR_PLL2_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2, APBS_PLL3_SWCR3,
- MPMU_POSR, POSR_PLL3_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll4, pll4_rate_tbl, APBS_PLL4_SWCR1, APBS_PLL4_SWCR2, APBS_PLL4_SWCR3,
- MPMU_POSR, POSR_PLL4_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll5, pll5_rate_tbl, APBS_PLL5_SWCR1, APBS_PLL5_SWCR2, APBS_PLL5_SWCR3,
- MPMU_POSR, POSR_PLL5_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll6, pll6_rate_tbl, APBS_PLL6_SWCR1, APBS_PLL6_SWCR2, APBS_PLL6_SWCR3,
- MPMU_POSR, POSR_PLL6_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll7, pll7_rate_tbl, APBS_PLL7_SWCR1, APBS_PLL7_SWCR2, APBS_PLL7_SWCR3,
- MPMU_POSR, POSR_PLL7_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll8, pll8_rate_tbl, APBS_PLL8_SWCR1, APBS_PLL8_SWCR2, APBS_PLL8_SWCR3,
- MPMU_POSR, POSR_PLL8_LOCK, CLK_SET_RATE_GATE);
-
-CCU_FACTOR_GATE_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1);
+
+/* PLLs without a managed CPU path require all output gates to be off. */
+static const struct ccu_pll_sync idle_pll_sync = {
+ .safe_pll = &pll1,
+};
+
+/* Park the secondary cluster first when it shares the primary cluster PLL. */
+static const struct ccu_pll_cpu_mux pll3_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, BIT(13) | GENMASK(2, 0), BIT(13) | 7 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll4_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, BIT(13) | GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll5_cpu_muxes[] = {
+ { APMU_CPU_C3_CLK_CTRL, BIT(13) | GENMASK(2, 0), BIT(13) | 7 },
+ { APMU_CPU_C2_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll8_cpu_muxes[] = {
+ { APMU_CPU_C3_CLK_CTRL, BIT(13) | GENMASK(2, 0), 7 },
+};
+
+#define K3_CPU_PLL_SYNC(_pll) \
+static const struct ccu_pll_sync _pll##_sync = { \
+ .safe_pll = &pll1, \
+ .apmu_compatible = "spacemit,k3-syscon-apmu", \
+ .muxes = _pll##_cpu_muxes, \
+ .num_muxes = ARRAY_SIZE(_pll##_cpu_muxes), \
+ .cpu_outputs = BIT(0), \
+ .safe_sel = 5, \
+ .slow_sel = 1, \
+ .reg_safe_gate = MPMU_ACGR, \
+}
+
+K3_CPU_PLL_SYNC(pll3);
+K3_CPU_PLL_SYNC(pll4);
+K3_CPU_PLL_SYNC(pll5);
+K3_CPU_PLL_SYNC(pll8);
+
+CCU_PLLA_SYNC_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2,
+ APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2,
+ APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
+ CLK_SET_RATE_GATE, &pll3_sync);
+CCU_PLLA_SYNC_DEFINE(pll4, pll4_rate_tbl, APBS_PLL4_SWCR1, APBS_PLL4_SWCR2,
+ APBS_PLL4_SWCR3, MPMU_POSR, POSR_PLL4_LOCK,
+ CLK_SET_RATE_GATE, &pll4_sync);
+CCU_PLLA_SYNC_DEFINE(pll5, pll5_rate_tbl, APBS_PLL5_SWCR1, APBS_PLL5_SWCR2,
+ APBS_PLL5_SWCR3, MPMU_POSR, POSR_PLL5_LOCK,
+ CLK_SET_RATE_GATE, &pll5_sync);
+CCU_PLLA_SYNC_DEFINE(pll6, pll6_rate_tbl, APBS_PLL6_SWCR1, APBS_PLL6_SWCR2,
+ APBS_PLL6_SWCR3, MPMU_POSR, POSR_PLL6_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll7, pll7_rate_tbl, APBS_PLL7_SWCR1, APBS_PLL7_SWCR2,
+ APBS_PLL7_SWCR3, MPMU_POSR, POSR_PLL7_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll8, pll8_rate_tbl, APBS_PLL8_SWCR1, APBS_PLL8_SWCR2,
+ APBS_PLL8_SWCR3, MPMU_POSR, POSR_PLL8_LOCK,
+ CLK_SET_RATE_GATE, &pll8_sync);
+
+/* A failed CPU FC handshake must not lose either fallback clock. */
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d3, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(2), 3, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d4, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(3), 4, 1);
-CCU_FACTOR_GATE_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1);
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d6, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(5), 6, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d7, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(6), 7, 1);
CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d8, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(7), 8, 1,
@@ -178,7 +230,7 @@ CCU_FACTOR_DEFINE(pll1_d3072_0p8, CCU_PARENT_HW(pll1_d384_6p4), 8, 1);
CCU_GATE_DEFINE(pll1_d6_409p6, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(0), 0);
CCU_FACTOR_GATE_DEFINE(pll1_d12_204p8, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(5), 2, 1);
-CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), 0);
+CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d10_245p76, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(18), 2, 1);
CCU_GATE_DEFINE(pll1_d4_614p4, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(15), 0);
@@ -187,7 +239,7 @@ CCU_FACTOR_GATE_DEFINE(pll1_d78_31p5, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(6),
CCU_GATE_DEFINE(pll1_d3_819p2, CCU_PARENT_HW(pll1_d3), MPMU_ACGR, BIT(14), 0);
-CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), 0);
+CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), CLK_IS_CRITICAL);
static const struct clk_parent_data apb_parents[] = {
CCU_PARENT_HW(pll1_d96_25p6),
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index 2573b8396cefa..3d8624c4ce5d1 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -8,6 +8,8 @@
#include <linux/clk-provider.h>
#include <linux/math.h>
#include <linux/math64.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
#include <linux/regmap.h>
#include "ccu_common.h"
@@ -34,6 +36,15 @@
#define PLLA_SWCR3_PREDIV GENMASK(21, 20)
#define PLL_FRAC_BITS 22
+#define PLL_OUTPUT_GATES GENMASK(7, 0)
+#define PLL_POWERDOWN_BYPASS BIT(23)
+#define PLL_SAFE_OUTPUT_GATE (BIT(1) | BIT(4))
+#define PLL_SAFE_MPMU_GATE (BIT(16) | BIT(21))
+#define PLL_CPU_SEL GENMASK(2, 0)
+#define PLL_CPU_FC BIT(12)
+#define PLL_CPU_TIMEOUT_US 10000
+#define PLL_MAX_CPU_MUXES 4
+
static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *pll,
unsigned long rate)
{
@@ -55,26 +66,6 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *p
return best_entry;
}
-static const struct ccu_pll_rate_tbl *ccu_pll_lookup_matched_entry(struct ccu_pll *pll)
-{
- struct ccu_pll_config *config = &pll->config;
- u32 swcr1, swcr3;
- int i;
-
- swcr1 = ccu_read(&pll->common, swcr1);
- swcr3 = ccu_read(&pll->common, swcr3);
- swcr3 &= PLL_SWCR3_MASK;
-
- for (i = 0; i < config->tbl_num; i++) {
- const struct ccu_pll_rate_tbl *entry = &config->rate_tbl[i];
-
- if (swcr1 == entry->swcr1 && swcr3 == entry->swcr3)
- return entry;
- }
-
- return NULL;
-}
-
static int ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
@@ -158,24 +149,6 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
return ccu_pll_update_param(pll, entry);
}
-static int ccu_pll_get_params(struct ccu_pll *pll,
- struct ccu_pll_rate_tbl *params, bool plla)
-{
- struct ccu_common *common = &pll->common;
- int ret;
-
- ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
- if (ret)
- return ret;
- params->swcr2 = 0;
- if (plla) {
- ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
- if (ret)
- return ret;
- }
- return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
-}
-
static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
unsigned long parent_rate)
{
@@ -199,39 +172,6 @@ static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
}
-static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
- unsigned long parent_rate)
-{
- u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
- u32 swcr3 = params->swcr3, prediv, frac;
- u64 divider, rate;
-
- /* Decode the software-controlled mode described by the PLL calculator. */
- if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
- (swcr1 & PLLA_SWCR1_REFSEL))
- return 0;
-
- prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
- frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
- frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
- divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
- divider += frac;
-
- /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
- rate = (u64)parent_rate * prediv * divider;
- return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
-}
-
-static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- struct ccu_pll_rate_tbl params;
-
- if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
- return 0;
- return ccu_pll_calc_rate(¶ms, parent_rate);
-}
-
static int ccu_pll_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
@@ -246,42 +186,6 @@ static int ccu_pll_determine_rate(struct clk_hw *hw,
return 0;
}
-static int ccu_pll_init(struct clk_hw *hw)
-{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
-
- if (ccu_pll_lookup_matched_entry(pll))
- return 0;
-
- ccu_pll_disable(hw);
- ccu_pll_update_param(pll, &pll->config.rate_tbl[0]);
-
- return 0;
-}
-
-static const struct ccu_pll_rate_tbl *ccu_plla_lookup_matched_entry(struct ccu_pll *pll)
-{
- struct ccu_pll_config *config = &pll->config;
- const struct ccu_pll_rate_tbl *entry;
- u32 i, swcr1, swcr2, swcr3;
-
- swcr1 = ccu_read(&pll->common, swcr1);
- swcr2 = ccu_read(&pll->common, swcr2);
- swcr2 &= PLLA_SWCR2_MASK;
- swcr3 = ccu_read(&pll->common, swcr3);
-
- for (i = 0; i < config->tbl_num; i++) {
- entry = &config->rate_tbl[i];
-
- if (swcr1 == entry->swcr1 &&
- swcr2 == entry->swcr2 &&
- swcr3 == entry->swcr3)
- return entry;
- }
-
- return NULL;
-}
-
static int ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
@@ -356,6 +260,68 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
return ccu_plla_update_param(pll, entry);
}
+static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
+ u32 swcr3 = params->swcr3, prediv, frac;
+ u64 divider, rate;
+
+ /* Decode the software-controlled mode described by the PLL calculator. */
+ if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
+ (swcr1 & PLLA_SWCR1_REFSEL))
+ return 0;
+
+ prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
+ frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
+ frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
+ divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
+ divider += frac;
+
+ /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
+
+static int ccu_pll_get_params(struct ccu_pll *pll,
+ struct ccu_pll_rate_tbl *params, bool plla)
+{
+ struct ccu_common *common = &pll->common;
+ int ret;
+
+ ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
+ if (ret)
+ return ret;
+ params->swcr2 = 0;
+ if (plla) {
+ ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
+ if (ret)
+ return ret;
+ }
+ return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
+}
+
+static bool ccu_pll_params_equal(const struct ccu_pll_rate_tbl *a,
+ const struct ccu_pll_rate_tbl *b, bool plla)
+{
+ if (a->swcr1 != b->swcr1)
+ return false;
+ if (plla)
+ return a->swcr3 == b->swcr3 &&
+ !((a->swcr2 ^ b->swcr2) & PLLA_SWCR2_MASK);
+ return !((a->swcr3 ^ b->swcr3) & PLL_SWCR3_MASK);
+}
+
+static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ccu_pll_rate_tbl params;
+
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
+ return 0;
+ return ccu_pll_calc_rate(¶ms, parent_rate);
+}
+
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -366,19 +332,278 @@ static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
return ccu_plla_calc_rate(¶ms, parent_rate);
}
-static int ccu_plla_init(struct clk_hw *hw)
+struct ccu_pll_park {
+ struct regmap *apmu;
+ u32 saved[PLL_MAX_CPU_MUXES];
+ u32 selected[PLL_MAX_CPU_MUXES];
+ unsigned long parked;
+};
+
+static int ccu_pll_select_cpu(struct regmap *regmap, u32 reg, u32 sel)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_update_bits(regmap, reg, PLL_CPU_SEL, sel);
+ if (ret)
+ return ret;
+ ret = regmap_update_bits(regmap, reg, PLL_CPU_FC, PLL_CPU_FC);
+ if (ret)
+ return ret;
+ ret = regmap_read_poll_timeout_atomic(regmap, reg, val,
+ !(val & PLL_CPU_FC), PLL_DELAY_US,
+ PLL_CPU_TIMEOUT_US);
+ if (ret)
+ return ret;
+ return (val & PLL_CPU_SEL) == sel ? 0 : -EIO;
+}
+
+static void ccu_pll_unpark(const struct ccu_pll_sync *sync,
+ struct ccu_pll_park *park)
+{
+ int i;
+
+ /* Restore sharing-capable secondary clusters last. */
+ for (i = sync->num_muxes - 1; i >= 0; i--) {
+ if (!(park->parked & BIT(i)))
+ continue;
+ if (!ccu_pll_select_cpu(park->apmu, sync->muxes[i].reg,
+ park->saved[i] & PLL_CPU_SEL))
+ park->parked &= ~BIT(i);
+ }
+
+ /*
+ * The fallback gates remain critical: after an FC timeout the selector
+ * register alone cannot prove that the CPU has left the temporary path.
+ */
+}
+
+static int ccu_pll_park_cpus(const struct ccu_pll_sync *sync,
+ struct ccu_pll_park *park, bool plla,
+ unsigned long rate, unsigned long parent_rate)
+{
+ struct ccu_pll *pll1 = sync->safe_pll;
+ struct ccu_common *safe = &pll1->common;
+ struct ccu_pll_rate_tbl params;
+ struct device_node *np;
+ unsigned long safe_rate;
+ u32 val;
+ u32 i;
+ int ret;
+ bool needed = false;
+
+ if (!sync->num_muxes)
+ return 0;
+ if (sync->num_muxes > PLL_MAX_CPU_MUXES)
+ return -EINVAL;
+ /* Like CCU probe, create the regmap before its clocks are registered. */
+ np = of_find_compatible_node(NULL, NULL, sync->apmu_compatible);
+ if (!np)
+ return -ENODEV;
+ park->apmu = device_node_to_regmap(np);
+ of_node_put(np);
+ if (IS_ERR(park->apmu))
+ return PTR_ERR(park->apmu);
+
+ for (i = 0; i < sync->num_muxes; i++) {
+ const struct ccu_pll_cpu_mux *mux = &sync->muxes[i];
+
+ ret = regmap_read(park->apmu, mux->reg, &park->saved[i]);
+ if (ret)
+ return ret;
+ if (park->saved[i] & PLL_CPU_FC)
+ return -EBUSY;
+ /* K3 selector 4 is unmodeled; do not infer a live parent. */
+ if (plla && (park->saved[i] & PLL_CPU_SEL) == 4)
+ return -EINVAL;
+ needed |= (park->saved[i] & mux->mask) == mux->value;
+ }
+ if (!needed)
+ return 0;
+
+ /* PLL1 is never repaired here, nor used as a fallback if unrecognized. */
+ if (!safe->regmap || !safe->lock_regmap)
+ return -ENODEV;
+ if (!pll1->config.tbl_num)
+ return -EINVAL;
+ ret = ccu_pll_get_params(pll1, ¶ms, plla);
+ if (ret)
+ return ret;
+ if (!ccu_pll_params_equal(¶ms, &pll1->config.rate_tbl[0], plla))
+ return -EINVAL;
+ ret = regmap_read(safe->lock_regmap, pll1->config.reg_lock, &val);
+ if (ret)
+ return ret;
+ if (!(val & pll1->config.mask_lock))
+ return -EBUSY;
+
+ safe_rate = plla ? ccu_plla_calc_rate(¶ms, parent_rate) :
+ ccu_pll_calc_rate(¶ms, parent_rate);
+ for (i = 0; i < sync->num_muxes; i++) {
+ u32 old = park->saved[i], sel = old & PLL_CPU_SEL;
+ u32 div = ((old >> 3) & 7) + 1;
+ unsigned long cpu_rate, fast_rate, slow_rate;
+
+ if ((old & sync->muxes[i].mask) != sync->muxes[i].value)
+ continue;
+ if (plla) {
+ cpu_rate = rate;
+ fast_rate = safe_rate / 2;
+ } else {
+ cpu_rate = rate / (sel == 5 ? 3 : (old & BIT(13) ? 1 : 2));
+ cpu_rate /= div;
+ fast_rate = safe_rate / 2 / div;
+ }
+ slow_rate = safe_rate / 5 / div;
+ /* Never increase a CPU's rate without a corresponding voltage vote. */
+ if (fast_rate <= cpu_rate)
+ park->selected[i] = sync->safe_sel;
+ else if (slow_rate <= cpu_rate)
+ park->selected[i] = sync->slow_sel;
+ else
+ return -ERANGE;
+ }
+ /* Both candidate parents are derived from the always-on PLL1. */
+ ret = ccu_update(safe, swcr2, PLL_SAFE_OUTPUT_GATE, PLL_SAFE_OUTPUT_GATE);
+ if (ret)
+ return ret;
+ ret = regmap_update_bits(safe->lock_regmap, sync->reg_safe_gate,
+ PLL_SAFE_MPMU_GATE, PLL_SAFE_MPMU_GATE);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < sync->num_muxes; i++) {
+ const struct ccu_pll_cpu_mux *mux = &sync->muxes[i];
+
+ if ((park->saved[i] & mux->mask) != mux->value)
+ continue;
+ park->parked |= BIT(i);
+ ret = ccu_pll_select_cpu(park->apmu, mux->reg, park->selected[i]);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int ccu_pll_stop(struct ccu_pll *pll, bool plla)
+{
+ struct ccu_common *common = &pll->common;
+ u32 val;
+ int ret;
+
+ ret = plla ? ccu_update(common, swcr2, PLLA_SWCR2_EN, 0) :
+ ccu_update(common, swcr3, PLL_SWCR3_EN, 0);
+ if (ret)
+ return ret;
+ /* Hardware or firmware may override the software enable bit. */
+ return regmap_read_poll_timeout_atomic(common->lock_regmap,
+ pll->config.reg_lock, val,
+ !(val & pll->config.mask_lock),
+ PLL_DELAY_US, PLL_TIMEOUT_US);
+}
+
+static int ccu_pll_sync_init(struct clk_hw *hw, bool plla)
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
+ const struct ccu_pll_sync *sync = pll->config.sync;
+ const struct ccu_pll_rate_tbl *entry = NULL;
+ unsigned long (*calc)(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate);
+ int (*update)(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *params);
+ int (*enable)(struct clk_hw *hw);
+ struct ccu_pll_rate_tbl old;
+ struct ccu_pll_park park = {};
+ struct clk_hw *parent;
+ unsigned long parent_rate, rate;
+ u32 outputs, lock, i;
+ bool enabled;
+ int ret;
- if (ccu_plla_lookup_matched_entry(pll))
+ /* Synchronization is opt-in; in particular PLL1 has no sync descriptor. */
+ if (!sync)
+ return 0;
+ parent = clk_hw_get_parent_by_index(hw, 0);
+ if (!parent)
+ return 0;
+ parent_rate = clk_hw_get_rate(parent);
+ calc = plla ? ccu_plla_calc_rate : ccu_pll_calc_rate;
+ update = plla ? ccu_plla_update_param : ccu_pll_update_param;
+ enable = plla ? ccu_plla_enable : ccu_pll_enable;
+ ret = ccu_pll_get_params(pll, &old, plla);
+ if (ret)
+ goto warn;
+ rate = calc(&old, parent_rate);
+ if (!rate)
return 0;
- ccu_plla_disable(hw);
- ccu_plla_update_param(pll, &pll->config.rate_tbl[0]);
+ /* Compare encoded rates, including fractional-divider quantization. */
+ for (i = 0; i < pll->config.tbl_num; i++) {
+ if (rate == calc(&pll->config.rate_tbl[i], parent_rate)) {
+ entry = &pll->config.rate_tbl[i];
+ break;
+ }
+ }
+ if (!entry || ccu_pll_params_equal(&old, entry, plla))
+ return 0;
+ if ((plla ? old.swcr3 : old.swcr1) & PLL_POWERDOWN_BYPASS)
+ return 0;
+ ret = regmap_read(pll->common.regmap, pll->common.reg_swcr2, &outputs);
+ if (ret)
+ goto warn;
+ /* Do not interrupt peripheral users, including unregistered consumers. */
+ if (outputs & PLL_OUTPUT_GATES & ~sync->cpu_outputs)
+ return 0;
+ ret = regmap_read(pll->common.lock_regmap, pll->config.reg_lock, &lock);
+ if (ret)
+ goto warn;
+ enabled = plla ? old.swcr2 & PLLA_SWCR2_EN : old.swcr3 & PLL_SWCR3_EN;
+ if (enabled != !!(lock & pll->config.mask_lock))
+ return 0;
+ /*
+ * .init runs under the CCF prepare lock, before this PLL is linked to
+ * its children. Restore the hardware muxes before CCF adopts them.
+ */
+ ret = ccu_pll_park_cpus(sync, &park, plla, rate, parent_rate);
+ if (ret)
+ goto unpark;
+ ret = ccu_pll_stop(pll, plla);
+ if (ret)
+ goto restart;
+ ret = update(pll, entry);
+ if (!ret && enabled)
+ ret = enable(hw);
+ if (!ret)
+ goto unpark;
+
+ /* Restore the old parameters before considering the original parents. */
+ if (ccu_pll_stop(pll, plla) || update(pll, &old))
+ goto warn;
+restart:
+ if (enabled && enable(hw))
+ goto warn;
+unpark:
+ ccu_pll_unpark(sync, &park);
+ if (park.parked)
+ ret = -ETIMEDOUT;
+warn:
+ if (ret)
+ pr_warn("%s: PLL synchronization failed: %d; retaining safe clocks\n",
+ clk_hw_get_name(hw), ret);
+ /* Failed synchronization must not unwind clocks needed to keep booting. */
return 0;
}
+static int ccu_pll_init(struct clk_hw *hw)
+{
+ return ccu_pll_sync_init(hw, false);
+}
+
+static int ccu_plla_init(struct clk_hw *hw)
+{
+ return ccu_pll_sync_init(hw, true);
+}
+
const struct clk_ops spacemit_ccu_pll_ops = {
.init = ccu_pll_init,
.enable = ccu_pll_enable,
diff --git a/drivers/clk/spacemit/ccu_pll.h b/drivers/clk/spacemit/ccu_pll.h
index e41db5c97c1a8..f77afafddba55 100644
--- a/drivers/clk/spacemit/ccu_pll.h
+++ b/drivers/clk/spacemit/ccu_pll.h
@@ -46,11 +46,31 @@ struct ccu_pll_rate_tbl {
struct ccu_pll_config {
const struct ccu_pll_rate_tbl *rate_tbl;
+ const struct ccu_pll_sync *sync;
u32 tbl_num;
u32 reg_lock;
u32 mask_lock;
};
+/* A CPU mux selection which consumes this PLL. */
+struct ccu_pll_cpu_mux {
+ u32 reg;
+ u32 mask;
+ u32 value;
+};
+
+/* Only CPU-exclusive outputs may remain gated on during synchronization. */
+struct ccu_pll_sync {
+ struct ccu_pll *safe_pll;
+ const char *apmu_compatible;
+ const struct ccu_pll_cpu_mux *muxes;
+ u32 num_muxes;
+ u32 cpu_outputs;
+ u32 safe_sel;
+ u32 slow_sel;
+ u32 reg_safe_gate;
+};
+
#define CCU_PLL_RATE(_rate, _swcr1, _swcr3) \
{ \
.rate = _rate, \
@@ -71,12 +91,13 @@ struct ccu_pll {
struct ccu_pll_config config;
};
-#define CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock) \
+#define CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock, _sync) \
{ \
.rate_tbl = _table, \
.tbl_num = ARRAY_SIZE(_table), \
.reg_lock = (_reg_lock), \
.mask_lock = (_mask_lock), \
+ .sync = _sync, \
}
#define CCU_PLL_COMMON_HWINIT(_name, _ops, _flags) \
@@ -89,9 +110,9 @@ struct ccu_pll {
})
#define CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, _ops, _flags) \
+ _reg_lock, _mask_lock, _ops, _flags, _sync) \
static struct ccu_pll _name = { \
- .config = CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock), \
+ .config = CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock, _sync), \
.common = { \
.reg_swcr1 = _reg_swcr1, \
.reg_swcr2 = _reg_swcr2, \
@@ -103,12 +124,22 @@ static struct ccu_pll _name = { \
#define CCU_PLL_DEFINE(_name, _table, _reg_swcr1, _reg_swcr3, _reg_lock, \
_mask_lock, _flags) \
CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, 0, _reg_swcr3, \
- _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags)
+ _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags, NULL)
+
+#define CCU_PLL_SYNC_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, \
+ _reg_swcr3, _reg_lock, _mask_lock, _flags, _sync) \
+ CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
+ _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags, _sync)
#define CCU_PLLA_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, _flags) \
+ _reg_lock, _mask_lock, _flags) \
+ CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
+ _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags, NULL)
+
+#define CCU_PLLA_SYNC_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, \
+ _reg_swcr3, _reg_lock, _mask_lock, _flags, _sync) \
CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags)
+ _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags, _sync)
static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw)
{
--
2.55.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/5] clk: spacemit: safely synchronize PLL parameters during init
@ 2026-09-09 14:07 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-09 14:07 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
Firmware PLL settings can be valid without matching a rate-table entry.
Falling back to the first entry during init can halt CPUs by disabling
their clock source.
Replace that fallback with opt-in parameter synchronization. Preserve
unlisted rates and PLLs without a sync descriptor, including PLL1. Leave
already matching parameters unchanged. For differing parameters, require
an entry that decodes to the same rate and no enabled outputs outside
the described CPU paths.
Park mapped CPU consumers on validated PLL1 branches no faster than
their current clock before stopping their PLL and programming the
matching entry. Re-enable previously enabled PLLs and wait for lock
before restoring CPU parents; leave initially disabled PLLs disabled.
Handle K1 PLL3 and K3 PLL3/4/5/8, parking sharing clusters first and
restoring them last. K3 CPU parking still depends on firmware cluster
initialization for FC completion.
After a programming or relock failure, attempt to restore the old PLL
parameters. Leave parked CPUs on PLL1 if PLL recovery fails, and keep
the fallback gates critical in case an FC handshake does not complete.
Keep synchronization in init, before CCF links the PLL to its children.
Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/clk/spacemit/ccu-k1.c | 52 ++++-
drivers/clk/spacemit/ccu-k3.c | 90 +++++++--
drivers/clk/spacemit/ccu_pll.c | 447 +++++++++++++++++++++++++++++++----------
drivers/clk/spacemit/ccu_pll.h | 43 +++-
4 files changed, 485 insertions(+), 147 deletions(-)
diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
index 40cc96c4ee876..0085179f8d7a7 100644
--- a/drivers/clk/spacemit/ccu-k1.c
+++ b/drivers/clk/spacemit/ccu-k1.c
@@ -41,17 +41,47 @@ static const struct ccu_pll_rate_tbl pll3_rate_tbl[] = {
CCU_PLL_RATE(3200000000UL, 0x0050dd67, 0x43eaaaab),
};
-CCU_PLL_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR3, MPMU_POSR, POSR_PLL1_LOCK,
- CLK_SET_RATE_GATE);
-CCU_PLL_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
- CLK_SET_RATE_GATE);
-CCU_PLL_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
- CLK_SET_RATE_GATE);
-
-CCU_FACTOR_GATE_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1);
+/* PLL1 supplies the fallback and must retain its firmware configuration. */
+CCU_PLL_SYNC_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR2,
+ APBS_PLL1_SWCR3, MPMU_POSR, POSR_PLL1_LOCK,
+ CLK_SET_RATE_GATE, NULL);
+
+static const struct ccu_pll_sync pll2_sync = {
+ .safe_pll = &pll1,
+};
+
+static const struct ccu_pll_cpu_mux pll3_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, GENMASK(2, 0), 5 },
+ { APMU_CPU_C1_CLK_CTRL, GENMASK(2, 0), 7 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 5 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_sync pll3_sync = {
+ .safe_pll = &pll1,
+ .apmu_compatible = "spacemit,k1-syscon-apmu",
+ .muxes = pll3_cpu_muxes,
+ .num_muxes = ARRAY_SIZE(pll3_cpu_muxes),
+ .cpu_outputs = GENMASK(2, 0),
+ .safe_sel = 4,
+ .slow_sel = 3,
+ .reg_safe_gate = MPMU_ACGR,
+};
+
+CCU_PLL_SYNC_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2,
+ APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
+ CLK_SET_RATE_GATE, &pll2_sync);
+CCU_PLL_SYNC_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2,
+ APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
+ CLK_SET_RATE_GATE, &pll3_sync);
+
+/* A failed CPU FC handshake must not lose either fallback clock. */
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d3, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(2), 3, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d4, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(3), 4, 1);
-CCU_FACTOR_GATE_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1);
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d6, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(5), 6, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d7, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(6), 7, 1);
CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d8, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(7), 8, 1,
@@ -111,7 +141,7 @@ CCU_FACTOR_DEFINE(pll1_d3072_0p8, CCU_PARENT_HW(pll1_d384_6p4), 8, 1);
CCU_GATE_DEFINE(pll1_d6_409p6, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(0), 0);
CCU_FACTOR_GATE_DEFINE(pll1_d12_204p8, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(5), 2, 1);
-CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), 0);
+CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d10_245p76, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(18), 2, 1);
CCU_GATE_DEFINE(pll1_d4_614p4, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(15), 0);
@@ -120,7 +150,7 @@ CCU_FACTOR_GATE_DEFINE(pll1_d78_31p5, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(6),
CCU_GATE_DEFINE(pll1_d3_819p2, CCU_PARENT_HW(pll1_d3), MPMU_ACGR, BIT(14), 0);
-CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), 0);
+CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), CLK_IS_CRITICAL);
CCU_GATE_DEFINE(slow_uart, CCU_PARENT_NAME(osc), MPMU_ACGR, BIT(1), CLK_IGNORE_UNUSED);
CCU_DDN_DEFINE(slow_uart1_14p74, pll1_d16_153p6, MPMU_SUCCR, 16, 13, 0, 13, 2, 0);
diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
index 2f0bd74f7da4d..b01f2ce5d394f 100644
--- a/drivers/clk/spacemit/ccu-k3.c
+++ b/drivers/clk/spacemit/ccu-k3.c
@@ -55,25 +55,77 @@ static const struct ccu_pll_rate_tbl pll8_rate_tbl[] = {
CCU_PLLA_DEFINE(pll1, pll1_rate_tbl, APBS_PLL1_SWCR1, APBS_PLL1_SWCR2, APBS_PLL1_SWCR3,
MPMU_POSR, POSR_PLL1_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2, APBS_PLL2_SWCR3,
- MPMU_POSR, POSR_PLL2_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2, APBS_PLL3_SWCR3,
- MPMU_POSR, POSR_PLL3_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll4, pll4_rate_tbl, APBS_PLL4_SWCR1, APBS_PLL4_SWCR2, APBS_PLL4_SWCR3,
- MPMU_POSR, POSR_PLL4_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll5, pll5_rate_tbl, APBS_PLL5_SWCR1, APBS_PLL5_SWCR2, APBS_PLL5_SWCR3,
- MPMU_POSR, POSR_PLL5_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll6, pll6_rate_tbl, APBS_PLL6_SWCR1, APBS_PLL6_SWCR2, APBS_PLL6_SWCR3,
- MPMU_POSR, POSR_PLL6_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll7, pll7_rate_tbl, APBS_PLL7_SWCR1, APBS_PLL7_SWCR2, APBS_PLL7_SWCR3,
- MPMU_POSR, POSR_PLL7_LOCK, CLK_SET_RATE_GATE);
-CCU_PLLA_DEFINE(pll8, pll8_rate_tbl, APBS_PLL8_SWCR1, APBS_PLL8_SWCR2, APBS_PLL8_SWCR3,
- MPMU_POSR, POSR_PLL8_LOCK, CLK_SET_RATE_GATE);
-
-CCU_FACTOR_GATE_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1);
+
+/* PLLs without a managed CPU path require all output gates to be off. */
+static const struct ccu_pll_sync idle_pll_sync = {
+ .safe_pll = &pll1,
+};
+
+/* Park the secondary cluster first when it shares the primary cluster PLL. */
+static const struct ccu_pll_cpu_mux pll3_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, BIT(13) | GENMASK(2, 0), BIT(13) | 7 },
+ { APMU_CPU_C0_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll4_cpu_muxes[] = {
+ { APMU_CPU_C1_CLK_CTRL, BIT(13) | GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll5_cpu_muxes[] = {
+ { APMU_CPU_C3_CLK_CTRL, BIT(13) | GENMASK(2, 0), BIT(13) | 7 },
+ { APMU_CPU_C2_CLK_CTRL, GENMASK(2, 0), 7 },
+};
+
+static const struct ccu_pll_cpu_mux pll8_cpu_muxes[] = {
+ { APMU_CPU_C3_CLK_CTRL, BIT(13) | GENMASK(2, 0), 7 },
+};
+
+#define K3_CPU_PLL_SYNC(_pll) \
+static const struct ccu_pll_sync _pll##_sync = { \
+ .safe_pll = &pll1, \
+ .apmu_compatible = "spacemit,k3-syscon-apmu", \
+ .muxes = _pll##_cpu_muxes, \
+ .num_muxes = ARRAY_SIZE(_pll##_cpu_muxes), \
+ .cpu_outputs = BIT(0), \
+ .safe_sel = 5, \
+ .slow_sel = 1, \
+ .reg_safe_gate = MPMU_ACGR, \
+}
+
+K3_CPU_PLL_SYNC(pll3);
+K3_CPU_PLL_SYNC(pll4);
+K3_CPU_PLL_SYNC(pll5);
+K3_CPU_PLL_SYNC(pll8);
+
+CCU_PLLA_SYNC_DEFINE(pll2, pll2_rate_tbl, APBS_PLL2_SWCR1, APBS_PLL2_SWCR2,
+ APBS_PLL2_SWCR3, MPMU_POSR, POSR_PLL2_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll3, pll3_rate_tbl, APBS_PLL3_SWCR1, APBS_PLL3_SWCR2,
+ APBS_PLL3_SWCR3, MPMU_POSR, POSR_PLL3_LOCK,
+ CLK_SET_RATE_GATE, &pll3_sync);
+CCU_PLLA_SYNC_DEFINE(pll4, pll4_rate_tbl, APBS_PLL4_SWCR1, APBS_PLL4_SWCR2,
+ APBS_PLL4_SWCR3, MPMU_POSR, POSR_PLL4_LOCK,
+ CLK_SET_RATE_GATE, &pll4_sync);
+CCU_PLLA_SYNC_DEFINE(pll5, pll5_rate_tbl, APBS_PLL5_SWCR1, APBS_PLL5_SWCR2,
+ APBS_PLL5_SWCR3, MPMU_POSR, POSR_PLL5_LOCK,
+ CLK_SET_RATE_GATE, &pll5_sync);
+CCU_PLLA_SYNC_DEFINE(pll6, pll6_rate_tbl, APBS_PLL6_SWCR1, APBS_PLL6_SWCR2,
+ APBS_PLL6_SWCR3, MPMU_POSR, POSR_PLL6_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll7, pll7_rate_tbl, APBS_PLL7_SWCR1, APBS_PLL7_SWCR2,
+ APBS_PLL7_SWCR3, MPMU_POSR, POSR_PLL7_LOCK,
+ CLK_SET_RATE_GATE, &idle_pll_sync);
+CCU_PLLA_SYNC_DEFINE(pll8, pll8_rate_tbl, APBS_PLL8_SWCR1, APBS_PLL8_SWCR2,
+ APBS_PLL8_SWCR3, MPMU_POSR, POSR_PLL8_LOCK,
+ CLK_SET_RATE_GATE, &pll8_sync);
+
+/* A failed CPU FC handshake must not lose either fallback clock. */
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d2, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(1), 2, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d3, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(2), 3, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d4, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(3), 4, 1);
-CCU_FACTOR_GATE_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1);
+CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d5, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(4), 5, 1,
+ CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d6, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(5), 6, 1);
CCU_FACTOR_GATE_DEFINE(pll1_d7, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(6), 7, 1);
CCU_FACTOR_GATE_FLAGS_DEFINE(pll1_d8, CCU_PARENT_HW(pll1), APBS_PLL1_SWCR2, BIT(7), 8, 1,
@@ -178,7 +230,7 @@ CCU_FACTOR_DEFINE(pll1_d3072_0p8, CCU_PARENT_HW(pll1_d384_6p4), 8, 1);
CCU_GATE_DEFINE(pll1_d6_409p6, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(0), 0);
CCU_FACTOR_GATE_DEFINE(pll1_d12_204p8, CCU_PARENT_HW(pll1_d6), MPMU_ACGR, BIT(5), 2, 1);
-CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), 0);
+CCU_GATE_DEFINE(pll1_d5_491p52, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(21), CLK_IS_CRITICAL);
CCU_FACTOR_GATE_DEFINE(pll1_d10_245p76, CCU_PARENT_HW(pll1_d5), MPMU_ACGR, BIT(18), 2, 1);
CCU_GATE_DEFINE(pll1_d4_614p4, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(15), 0);
@@ -187,7 +239,7 @@ CCU_FACTOR_GATE_DEFINE(pll1_d78_31p5, CCU_PARENT_HW(pll1_d4), MPMU_ACGR, BIT(6),
CCU_GATE_DEFINE(pll1_d3_819p2, CCU_PARENT_HW(pll1_d3), MPMU_ACGR, BIT(14), 0);
-CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), 0);
+CCU_GATE_DEFINE(pll1_d2_1228p8, CCU_PARENT_HW(pll1_d2), MPMU_ACGR, BIT(16), CLK_IS_CRITICAL);
static const struct clk_parent_data apb_parents[] = {
CCU_PARENT_HW(pll1_d96_25p6),
diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
index 2573b8396cefa..3d8624c4ce5d1 100644
--- a/drivers/clk/spacemit/ccu_pll.c
+++ b/drivers/clk/spacemit/ccu_pll.c
@@ -8,6 +8,8 @@
#include <linux/clk-provider.h>
#include <linux/math.h>
#include <linux/math64.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
#include <linux/regmap.h>
#include "ccu_common.h"
@@ -34,6 +36,15 @@
#define PLLA_SWCR3_PREDIV GENMASK(21, 20)
#define PLL_FRAC_BITS 22
+#define PLL_OUTPUT_GATES GENMASK(7, 0)
+#define PLL_POWERDOWN_BYPASS BIT(23)
+#define PLL_SAFE_OUTPUT_GATE (BIT(1) | BIT(4))
+#define PLL_SAFE_MPMU_GATE (BIT(16) | BIT(21))
+#define PLL_CPU_SEL GENMASK(2, 0)
+#define PLL_CPU_FC BIT(12)
+#define PLL_CPU_TIMEOUT_US 10000
+#define PLL_MAX_CPU_MUXES 4
+
static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *pll,
unsigned long rate)
{
@@ -55,26 +66,6 @@ static const struct ccu_pll_rate_tbl *ccu_pll_lookup_best_rate(struct ccu_pll *p
return best_entry;
}
-static const struct ccu_pll_rate_tbl *ccu_pll_lookup_matched_entry(struct ccu_pll *pll)
-{
- struct ccu_pll_config *config = &pll->config;
- u32 swcr1, swcr3;
- int i;
-
- swcr1 = ccu_read(&pll->common, swcr1);
- swcr3 = ccu_read(&pll->common, swcr3);
- swcr3 &= PLL_SWCR3_MASK;
-
- for (i = 0; i < config->tbl_num; i++) {
- const struct ccu_pll_rate_tbl *entry = &config->rate_tbl[i];
-
- if (swcr1 == entry->swcr1 && swcr3 == entry->swcr3)
- return entry;
- }
-
- return NULL;
-}
-
static int ccu_pll_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
@@ -158,24 +149,6 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
return ccu_pll_update_param(pll, entry);
}
-static int ccu_pll_get_params(struct ccu_pll *pll,
- struct ccu_pll_rate_tbl *params, bool plla)
-{
- struct ccu_common *common = &pll->common;
- int ret;
-
- ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
- if (ret)
- return ret;
- params->swcr2 = 0;
- if (plla) {
- ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
- if (ret)
- return ret;
- }
- return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
-}
-
static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
unsigned long parent_rate)
{
@@ -199,39 +172,6 @@ static unsigned long ccu_pll_calc_rate(const struct ccu_pll_rate_tbl *params,
return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
}
-static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
- unsigned long parent_rate)
-{
- u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
- u32 swcr3 = params->swcr3, prediv, frac;
- u64 divider, rate;
-
- /* Decode the software-controlled mode described by the PLL calculator. */
- if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
- (swcr1 & PLLA_SWCR1_REFSEL))
- return 0;
-
- prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
- frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
- frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
- divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
- divider += frac;
-
- /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
- rate = (u64)parent_rate * prediv * divider;
- return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
-}
-
-static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
- unsigned long parent_rate)
-{
- struct ccu_pll_rate_tbl params;
-
- if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
- return 0;
- return ccu_pll_calc_rate(¶ms, parent_rate);
-}
-
static int ccu_pll_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
@@ -246,42 +186,6 @@ static int ccu_pll_determine_rate(struct clk_hw *hw,
return 0;
}
-static int ccu_pll_init(struct clk_hw *hw)
-{
- struct ccu_pll *pll = hw_to_ccu_pll(hw);
-
- if (ccu_pll_lookup_matched_entry(pll))
- return 0;
-
- ccu_pll_disable(hw);
- ccu_pll_update_param(pll, &pll->config.rate_tbl[0]);
-
- return 0;
-}
-
-static const struct ccu_pll_rate_tbl *ccu_plla_lookup_matched_entry(struct ccu_pll *pll)
-{
- struct ccu_pll_config *config = &pll->config;
- const struct ccu_pll_rate_tbl *entry;
- u32 i, swcr1, swcr2, swcr3;
-
- swcr1 = ccu_read(&pll->common, swcr1);
- swcr2 = ccu_read(&pll->common, swcr2);
- swcr2 &= PLLA_SWCR2_MASK;
- swcr3 = ccu_read(&pll->common, swcr3);
-
- for (i = 0; i < config->tbl_num; i++) {
- entry = &config->rate_tbl[i];
-
- if (swcr1 == entry->swcr1 &&
- swcr2 == entry->swcr2 &&
- swcr3 == entry->swcr3)
- return entry;
- }
-
- return NULL;
-}
-
static int ccu_plla_update_param(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *entry)
{
struct ccu_common *common = &pll->common;
@@ -356,6 +260,68 @@ static int ccu_plla_set_rate(struct clk_hw *hw, unsigned long rate,
return ccu_plla_update_param(pll, entry);
}
+static unsigned long ccu_plla_calc_rate(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate)
+{
+ u32 swcr1 = params->swcr1, swcr2 = params->swcr2;
+ u32 swcr3 = params->swcr3, prediv, frac;
+ u64 divider, rate;
+
+ /* Decode the software-controlled mode described by the PLL calculator. */
+ if (!(swcr1 & PLLA_SWCR1_USER_MODE) ||
+ (swcr1 & PLLA_SWCR1_REFSEL))
+ return 0;
+
+ prediv = FIELD_GET(PLLA_SWCR3_PREDIV, swcr3) + 1;
+ frac = FIELD_GET(PLLA_SWCR1_FRAC, swcr1) << 8;
+ frac |= FIELD_GET(PLLA_SWCR2_MASK, swcr2);
+ divider = (u64)FIELD_GET(PLLA_SWCR1_INT, swcr1) << PLL_FRAC_BITS;
+ divider += frac;
+
+ /* Fvco = Fref * Npre * (Nint + Nfrac), with an unsigned fraction. */
+ rate = (u64)parent_rate * prediv * divider;
+ return DIV_ROUND_CLOSEST_ULL(rate, BIT_ULL(PLL_FRAC_BITS));
+}
+
+static int ccu_pll_get_params(struct ccu_pll *pll,
+ struct ccu_pll_rate_tbl *params, bool plla)
+{
+ struct ccu_common *common = &pll->common;
+ int ret;
+
+ ret = regmap_read(common->regmap, common->reg_swcr1, ¶ms->swcr1);
+ if (ret)
+ return ret;
+ params->swcr2 = 0;
+ if (plla) {
+ ret = regmap_read(common->regmap, common->reg_swcr2, ¶ms->swcr2);
+ if (ret)
+ return ret;
+ }
+ return regmap_read(common->regmap, common->reg_swcr3, ¶ms->swcr3);
+}
+
+static bool ccu_pll_params_equal(const struct ccu_pll_rate_tbl *a,
+ const struct ccu_pll_rate_tbl *b, bool plla)
+{
+ if (a->swcr1 != b->swcr1)
+ return false;
+ if (plla)
+ return a->swcr3 == b->swcr3 &&
+ !((a->swcr2 ^ b->swcr2) & PLLA_SWCR2_MASK);
+ return !((a->swcr3 ^ b->swcr3) & PLL_SWCR3_MASK);
+}
+
+static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ccu_pll_rate_tbl params;
+
+ if (ccu_pll_get_params(hw_to_ccu_pll(hw), ¶ms, false))
+ return 0;
+ return ccu_pll_calc_rate(¶ms, parent_rate);
+}
+
static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
@@ -366,19 +332,278 @@ static unsigned long ccu_plla_recalc_rate(struct clk_hw *hw,
return ccu_plla_calc_rate(¶ms, parent_rate);
}
-static int ccu_plla_init(struct clk_hw *hw)
+struct ccu_pll_park {
+ struct regmap *apmu;
+ u32 saved[PLL_MAX_CPU_MUXES];
+ u32 selected[PLL_MAX_CPU_MUXES];
+ unsigned long parked;
+};
+
+static int ccu_pll_select_cpu(struct regmap *regmap, u32 reg, u32 sel)
+{
+ u32 val;
+ int ret;
+
+ ret = regmap_update_bits(regmap, reg, PLL_CPU_SEL, sel);
+ if (ret)
+ return ret;
+ ret = regmap_update_bits(regmap, reg, PLL_CPU_FC, PLL_CPU_FC);
+ if (ret)
+ return ret;
+ ret = regmap_read_poll_timeout_atomic(regmap, reg, val,
+ !(val & PLL_CPU_FC), PLL_DELAY_US,
+ PLL_CPU_TIMEOUT_US);
+ if (ret)
+ return ret;
+ return (val & PLL_CPU_SEL) == sel ? 0 : -EIO;
+}
+
+static void ccu_pll_unpark(const struct ccu_pll_sync *sync,
+ struct ccu_pll_park *park)
+{
+ int i;
+
+ /* Restore sharing-capable secondary clusters last. */
+ for (i = sync->num_muxes - 1; i >= 0; i--) {
+ if (!(park->parked & BIT(i)))
+ continue;
+ if (!ccu_pll_select_cpu(park->apmu, sync->muxes[i].reg,
+ park->saved[i] & PLL_CPU_SEL))
+ park->parked &= ~BIT(i);
+ }
+
+ /*
+ * The fallback gates remain critical: after an FC timeout the selector
+ * register alone cannot prove that the CPU has left the temporary path.
+ */
+}
+
+static int ccu_pll_park_cpus(const struct ccu_pll_sync *sync,
+ struct ccu_pll_park *park, bool plla,
+ unsigned long rate, unsigned long parent_rate)
+{
+ struct ccu_pll *pll1 = sync->safe_pll;
+ struct ccu_common *safe = &pll1->common;
+ struct ccu_pll_rate_tbl params;
+ struct device_node *np;
+ unsigned long safe_rate;
+ u32 val;
+ u32 i;
+ int ret;
+ bool needed = false;
+
+ if (!sync->num_muxes)
+ return 0;
+ if (sync->num_muxes > PLL_MAX_CPU_MUXES)
+ return -EINVAL;
+ /* Like CCU probe, create the regmap before its clocks are registered. */
+ np = of_find_compatible_node(NULL, NULL, sync->apmu_compatible);
+ if (!np)
+ return -ENODEV;
+ park->apmu = device_node_to_regmap(np);
+ of_node_put(np);
+ if (IS_ERR(park->apmu))
+ return PTR_ERR(park->apmu);
+
+ for (i = 0; i < sync->num_muxes; i++) {
+ const struct ccu_pll_cpu_mux *mux = &sync->muxes[i];
+
+ ret = regmap_read(park->apmu, mux->reg, &park->saved[i]);
+ if (ret)
+ return ret;
+ if (park->saved[i] & PLL_CPU_FC)
+ return -EBUSY;
+ /* K3 selector 4 is unmodeled; do not infer a live parent. */
+ if (plla && (park->saved[i] & PLL_CPU_SEL) == 4)
+ return -EINVAL;
+ needed |= (park->saved[i] & mux->mask) == mux->value;
+ }
+ if (!needed)
+ return 0;
+
+ /* PLL1 is never repaired here, nor used as a fallback if unrecognized. */
+ if (!safe->regmap || !safe->lock_regmap)
+ return -ENODEV;
+ if (!pll1->config.tbl_num)
+ return -EINVAL;
+ ret = ccu_pll_get_params(pll1, ¶ms, plla);
+ if (ret)
+ return ret;
+ if (!ccu_pll_params_equal(¶ms, &pll1->config.rate_tbl[0], plla))
+ return -EINVAL;
+ ret = regmap_read(safe->lock_regmap, pll1->config.reg_lock, &val);
+ if (ret)
+ return ret;
+ if (!(val & pll1->config.mask_lock))
+ return -EBUSY;
+
+ safe_rate = plla ? ccu_plla_calc_rate(¶ms, parent_rate) :
+ ccu_pll_calc_rate(¶ms, parent_rate);
+ for (i = 0; i < sync->num_muxes; i++) {
+ u32 old = park->saved[i], sel = old & PLL_CPU_SEL;
+ u32 div = ((old >> 3) & 7) + 1;
+ unsigned long cpu_rate, fast_rate, slow_rate;
+
+ if ((old & sync->muxes[i].mask) != sync->muxes[i].value)
+ continue;
+ if (plla) {
+ cpu_rate = rate;
+ fast_rate = safe_rate / 2;
+ } else {
+ cpu_rate = rate / (sel == 5 ? 3 : (old & BIT(13) ? 1 : 2));
+ cpu_rate /= div;
+ fast_rate = safe_rate / 2 / div;
+ }
+ slow_rate = safe_rate / 5 / div;
+ /* Never increase a CPU's rate without a corresponding voltage vote. */
+ if (fast_rate <= cpu_rate)
+ park->selected[i] = sync->safe_sel;
+ else if (slow_rate <= cpu_rate)
+ park->selected[i] = sync->slow_sel;
+ else
+ return -ERANGE;
+ }
+ /* Both candidate parents are derived from the always-on PLL1. */
+ ret = ccu_update(safe, swcr2, PLL_SAFE_OUTPUT_GATE, PLL_SAFE_OUTPUT_GATE);
+ if (ret)
+ return ret;
+ ret = regmap_update_bits(safe->lock_regmap, sync->reg_safe_gate,
+ PLL_SAFE_MPMU_GATE, PLL_SAFE_MPMU_GATE);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < sync->num_muxes; i++) {
+ const struct ccu_pll_cpu_mux *mux = &sync->muxes[i];
+
+ if ((park->saved[i] & mux->mask) != mux->value)
+ continue;
+ park->parked |= BIT(i);
+ ret = ccu_pll_select_cpu(park->apmu, mux->reg, park->selected[i]);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int ccu_pll_stop(struct ccu_pll *pll, bool plla)
+{
+ struct ccu_common *common = &pll->common;
+ u32 val;
+ int ret;
+
+ ret = plla ? ccu_update(common, swcr2, PLLA_SWCR2_EN, 0) :
+ ccu_update(common, swcr3, PLL_SWCR3_EN, 0);
+ if (ret)
+ return ret;
+ /* Hardware or firmware may override the software enable bit. */
+ return regmap_read_poll_timeout_atomic(common->lock_regmap,
+ pll->config.reg_lock, val,
+ !(val & pll->config.mask_lock),
+ PLL_DELAY_US, PLL_TIMEOUT_US);
+}
+
+static int ccu_pll_sync_init(struct clk_hw *hw, bool plla)
{
struct ccu_pll *pll = hw_to_ccu_pll(hw);
+ const struct ccu_pll_sync *sync = pll->config.sync;
+ const struct ccu_pll_rate_tbl *entry = NULL;
+ unsigned long (*calc)(const struct ccu_pll_rate_tbl *params,
+ unsigned long parent_rate);
+ int (*update)(struct ccu_pll *pll, const struct ccu_pll_rate_tbl *params);
+ int (*enable)(struct clk_hw *hw);
+ struct ccu_pll_rate_tbl old;
+ struct ccu_pll_park park = {};
+ struct clk_hw *parent;
+ unsigned long parent_rate, rate;
+ u32 outputs, lock, i;
+ bool enabled;
+ int ret;
- if (ccu_plla_lookup_matched_entry(pll))
+ /* Synchronization is opt-in; in particular PLL1 has no sync descriptor. */
+ if (!sync)
+ return 0;
+ parent = clk_hw_get_parent_by_index(hw, 0);
+ if (!parent)
+ return 0;
+ parent_rate = clk_hw_get_rate(parent);
+ calc = plla ? ccu_plla_calc_rate : ccu_pll_calc_rate;
+ update = plla ? ccu_plla_update_param : ccu_pll_update_param;
+ enable = plla ? ccu_plla_enable : ccu_pll_enable;
+ ret = ccu_pll_get_params(pll, &old, plla);
+ if (ret)
+ goto warn;
+ rate = calc(&old, parent_rate);
+ if (!rate)
return 0;
- ccu_plla_disable(hw);
- ccu_plla_update_param(pll, &pll->config.rate_tbl[0]);
+ /* Compare encoded rates, including fractional-divider quantization. */
+ for (i = 0; i < pll->config.tbl_num; i++) {
+ if (rate == calc(&pll->config.rate_tbl[i], parent_rate)) {
+ entry = &pll->config.rate_tbl[i];
+ break;
+ }
+ }
+ if (!entry || ccu_pll_params_equal(&old, entry, plla))
+ return 0;
+ if ((plla ? old.swcr3 : old.swcr1) & PLL_POWERDOWN_BYPASS)
+ return 0;
+ ret = regmap_read(pll->common.regmap, pll->common.reg_swcr2, &outputs);
+ if (ret)
+ goto warn;
+ /* Do not interrupt peripheral users, including unregistered consumers. */
+ if (outputs & PLL_OUTPUT_GATES & ~sync->cpu_outputs)
+ return 0;
+ ret = regmap_read(pll->common.lock_regmap, pll->config.reg_lock, &lock);
+ if (ret)
+ goto warn;
+ enabled = plla ? old.swcr2 & PLLA_SWCR2_EN : old.swcr3 & PLL_SWCR3_EN;
+ if (enabled != !!(lock & pll->config.mask_lock))
+ return 0;
+ /*
+ * .init runs under the CCF prepare lock, before this PLL is linked to
+ * its children. Restore the hardware muxes before CCF adopts them.
+ */
+ ret = ccu_pll_park_cpus(sync, &park, plla, rate, parent_rate);
+ if (ret)
+ goto unpark;
+ ret = ccu_pll_stop(pll, plla);
+ if (ret)
+ goto restart;
+ ret = update(pll, entry);
+ if (!ret && enabled)
+ ret = enable(hw);
+ if (!ret)
+ goto unpark;
+
+ /* Restore the old parameters before considering the original parents. */
+ if (ccu_pll_stop(pll, plla) || update(pll, &old))
+ goto warn;
+restart:
+ if (enabled && enable(hw))
+ goto warn;
+unpark:
+ ccu_pll_unpark(sync, &park);
+ if (park.parked)
+ ret = -ETIMEDOUT;
+warn:
+ if (ret)
+ pr_warn("%s: PLL synchronization failed: %d; retaining safe clocks\n",
+ clk_hw_get_name(hw), ret);
+ /* Failed synchronization must not unwind clocks needed to keep booting. */
return 0;
}
+static int ccu_pll_init(struct clk_hw *hw)
+{
+ return ccu_pll_sync_init(hw, false);
+}
+
+static int ccu_plla_init(struct clk_hw *hw)
+{
+ return ccu_pll_sync_init(hw, true);
+}
+
const struct clk_ops spacemit_ccu_pll_ops = {
.init = ccu_pll_init,
.enable = ccu_pll_enable,
diff --git a/drivers/clk/spacemit/ccu_pll.h b/drivers/clk/spacemit/ccu_pll.h
index e41db5c97c1a8..f77afafddba55 100644
--- a/drivers/clk/spacemit/ccu_pll.h
+++ b/drivers/clk/spacemit/ccu_pll.h
@@ -46,11 +46,31 @@ struct ccu_pll_rate_tbl {
struct ccu_pll_config {
const struct ccu_pll_rate_tbl *rate_tbl;
+ const struct ccu_pll_sync *sync;
u32 tbl_num;
u32 reg_lock;
u32 mask_lock;
};
+/* A CPU mux selection which consumes this PLL. */
+struct ccu_pll_cpu_mux {
+ u32 reg;
+ u32 mask;
+ u32 value;
+};
+
+/* Only CPU-exclusive outputs may remain gated on during synchronization. */
+struct ccu_pll_sync {
+ struct ccu_pll *safe_pll;
+ const char *apmu_compatible;
+ const struct ccu_pll_cpu_mux *muxes;
+ u32 num_muxes;
+ u32 cpu_outputs;
+ u32 safe_sel;
+ u32 slow_sel;
+ u32 reg_safe_gate;
+};
+
#define CCU_PLL_RATE(_rate, _swcr1, _swcr3) \
{ \
.rate = _rate, \
@@ -71,12 +91,13 @@ struct ccu_pll {
struct ccu_pll_config config;
};
-#define CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock) \
+#define CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock, _sync) \
{ \
.rate_tbl = _table, \
.tbl_num = ARRAY_SIZE(_table), \
.reg_lock = (_reg_lock), \
.mask_lock = (_mask_lock), \
+ .sync = _sync, \
}
#define CCU_PLL_COMMON_HWINIT(_name, _ops, _flags) \
@@ -89,9 +110,9 @@ struct ccu_pll {
})
#define CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, _ops, _flags) \
+ _reg_lock, _mask_lock, _ops, _flags, _sync) \
static struct ccu_pll _name = { \
- .config = CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock), \
+ .config = CCU_PLL_CONFIG(_table, _reg_lock, _mask_lock, _sync), \
.common = { \
.reg_swcr1 = _reg_swcr1, \
.reg_swcr2 = _reg_swcr2, \
@@ -103,12 +124,22 @@ static struct ccu_pll _name = { \
#define CCU_PLL_DEFINE(_name, _table, _reg_swcr1, _reg_swcr3, _reg_lock, \
_mask_lock, _flags) \
CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, 0, _reg_swcr3, \
- _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags)
+ _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags, NULL)
+
+#define CCU_PLL_SYNC_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, \
+ _reg_swcr3, _reg_lock, _mask_lock, _flags, _sync) \
+ CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
+ _reg_lock, _mask_lock, &spacemit_ccu_pll_ops, _flags, _sync)
#define CCU_PLLA_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, _flags) \
+ _reg_lock, _mask_lock, _flags) \
+ CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
+ _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags, NULL)
+
+#define CCU_PLLA_SYNC_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, \
+ _reg_swcr3, _reg_lock, _mask_lock, _flags, _sync) \
CCU_PLL_X_DEFINE(_name, _table, _reg_swcr1, _reg_swcr2, _reg_swcr3, \
- _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags)
+ _reg_lock, _mask_lock, &spacemit_ccu_plla_ops, _flags, _sync)
static inline struct ccu_pll *hw_to_ccu_pll(struct clk_hw *hw)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes
2026-09-09 14:07 ` Troy Mitchell
(?)
@ 2026-09-09 14:21 ` sashiko-bot
2026-09-10 3:23 ` Troy Mitchell
-1 siblings, 1 reply; 25+ messages in thread
From: sashiko-bot @ 2026-09-09 14:21 UTC (permalink / raw)
To: Troy Mitchell; +Cc: linux-clk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `.set_rate_and_parent` implementation causes non-atomic updates of clock mux and divider, risking severe CPU over-clocking and crashes during frequency scaling.
--
commit caf894b03e44c8fc393822573e3c7ad1a85fe9f1
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>
clk: spacemit: describe CPU clock dividers and shared PLL muxes
This commit models the K1 and K3 CPU clock dividers and bypass conditions.
It registers sharing muxes as internal clocks and excludes K3 selector 4
from the modeled parents to match BSP policy.
> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
> index dee14d25f75d4..40cc96c4ee876 100644
> --- a/drivers/clk/spacemit/ccu-k1.c
> +++ b/drivers/clk/spacemit/ccu-k1.c
> @@ -391,8 +391,8 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
> CCU_PARENT_HW(pll2_d3),
> CCU_PARENT_HW(cpu_c0_hi_clk),
> };
> -CCU_MUX_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL, BIT(12), 0, 3,
> - CLK_IS_CRITICAL);
> +CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
> + 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
[Severity: High]
Does introducing the divider model to K1's CPU clock here risk severe CPU
over-clocking and crashes during frequency scaling?
The operations struct spacemit_ccu_mux_div_ops in
drivers/clk/spacemit/ccu_mix.c appears to lack .set_rate_and_parent,
which is required by the Common Clock Framework (CCF) to atomically update
both mux and divider.
When cpufreq scales down frequency (e.g., from 614.4 MHz to 409.6 MHz), it
decreases the frequency before lowering the voltage. In K1,
ccu_mix_calc_best_rate() iterates parents and stops at the first exact match,
for example index 1 (pll1_d3_819p2, 819.2 MHz) with div=2.
Because spacemit_ccu_mux_div_ops lacks .set_rate_and_parent, CCF's
clk_change_rate() executes updates sequentially, calling .set_parent
before .set_rate. .set_parent changes the hardware mux to 819.2 MHz
while the divider remains at its previous value (div=1). The CPU temporarily
runs at 819.2 MHz at a voltage only sufficient for 614.4 MHz, causing an
immediate undervoltage crash.
Could spacemit_ccu_mux_div_ops be updated to include a
.set_rate_and_parent implementation to ensure the mux and divider are
updated atomically?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-spacemit-pll-init-v1-0-b3065ad5a4ac@linux.spacemit.com?part=3
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
2026-09-09 14:07 ` Troy Mitchell
(?)
@ 2026-09-09 14:22 ` sashiko-bot
-1 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-09-09 14:22 UTC (permalink / raw)
To: Troy Mitchell; +Cc: linux-clk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling clk_set_rate() immediately after clk_disable() may intermittently fail with -EBUSY because the hardware PLL lock bit is checked without polling.
--
commit a90cd779d21914719856a7e102c5e63e9892f73b
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>
clk: spacemit: reject rate changes to running firmware PLLs
CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
running by firmware can have a zero prepare count, so this flag alone
cannot prevent set_rate() from reprogramming a live PLL.
> diff --git a/drivers/clk/spacemit/ccu_pll.c b/drivers/clk/spacemit/ccu_pll.c
> index c9852fed31017..2573b8396cefa 100644
> --- a/drivers/clk/spacemit/ccu_pll.c
> +++ b/drivers/clk/spacemit/ccu_pll.c
[ ... ]
> @@ -113,6 +122,17 @@ static void ccu_pll_disable(struct clk_hw *hw)
> ccu_update(common, swcr3, PLL_SWCR3_EN, 0);
> }
>
> +static int ccu_pll_check_stopped(struct ccu_pll *pll)
> +{
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(pll->common.lock_regmap, pll->config.reg_lock, &val);
> + if (ret)
> + return ret;
> + return val & pll->config.mask_lock ? -EBUSY : 0;
> +}
[Severity: High]
Can this cause clk_set_rate() to spuriously fail with -EBUSY?
If a driver calls clk_disable_unprepare() followed immediately by
clk_set_rate(), the .disable callback will clear the software enable bit
and return instantly.
When .set_rate calls ccu_pll_check_stopped(), it performs a single
non-blocking read of the hardware PLL lock bit here. Since the analog
PLL requires time to lose lock, wouldn't the lock bit often still be
set, causing the valid rate change to be rejected?
A follow-up patch "clk: spacemit: safely synchronize PLL parameters during
init" seems to acknowledge this behavior by adding a ccu_pll_stop() function
with a polling timeout. Should a similar polling mechanism be used here
instead of a single read?
> +
> /*
> * PLLs must be gated before changing rate, which is ensured by
> * flag CLK_SET_RATE_GATE.
> @@ -122,11 +142,20 @@ static int ccu_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> {
> struct ccu_pll *pll = hw_to_ccu_pll(hw);
> const struct ccu_pll_rate_tbl *entry;
> + int ret;
>
> - entry = ccu_pll_lookup_best_rate(pll, rate);
> - ccu_pll_update_param(pll, entry);
> + /* CLK_SET_RATE_GATE does not account for firmware-only users. */
> + ret = ccu_pll_is_enabled(hw);
> + if (ret)
> + return ret < 0 ? ret : -EBUSY;
> + ret = ccu_pll_check_stopped(pll);
> + if (ret)
> + return ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-spacemit-pll-init-v1-0-b3065ad5a4ac@linux.spacemit.com?part=4
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes
2026-09-09 14:21 ` sashiko-bot
@ 2026-09-10 3:23 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-10 3:23 UTC (permalink / raw)
To: sashiko-reviews, Troy Mitchell; +Cc: linux-clk
[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]
On Wed Sep 9, 2026 at 10:21 PM +08, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `.set_rate_and_parent` implementation causes non-atomic updates of clock mux and divider, risking severe CPU over-clocking and crashes during frequency scaling.
> --
>
> commit caf894b03e44c8fc393822573e3c7ad1a85fe9f1
> Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>
> clk: spacemit: describe CPU clock dividers and shared PLL muxes
>
> This commit models the K1 and K3 CPU clock dividers and bypass conditions.
> It registers sharing muxes as internal clocks and excludes K3 selector 4
> from the modeled parents to match BSP policy.
>
>> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
>> index dee14d25f75d4..40cc96c4ee876 100644
>> --- a/drivers/clk/spacemit/ccu-k1.c
>> +++ b/drivers/clk/spacemit/ccu-k1.c
>> @@ -391,8 +391,8 @@ static const struct clk_parent_data cpu_c0_clk_parents[] = {
>> CCU_PARENT_HW(pll2_d3),
>> CCU_PARENT_HW(cpu_c0_hi_clk),
>> };
>> -CCU_MUX_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL, BIT(12), 0, 3,
>> - CLK_IS_CRITICAL);
>> +CCU_MUX_DIV_FC_DEFINE(cpu_c0_core_clk, cpu_c0_clk_parents, APMU_CPU_C0_CLK_CTRL,
>> + 3, 3, BIT(12), 0, 3, CLK_IS_CRITICAL);
>
> [Severity: High]
> Does introducing the divider model to K1's CPU clock here risk severe CPU
> over-clocking and crashes during frequency scaling?
>
> The operations struct spacemit_ccu_mux_div_ops in
> drivers/clk/spacemit/ccu_mix.c appears to lack .set_rate_and_parent,
> which is required by the Common Clock Framework (CCF) to atomically update
> both mux and divider.
>
> When cpufreq scales down frequency (e.g., from 614.4 MHz to 409.6 MHz), it
> decreases the frequency before lowering the voltage. In K1,
> ccu_mix_calc_best_rate() iterates parents and stops at the first exact match,
> for example index 1 (pll1_d3_819p2, 819.2 MHz) with div=2.
>
> Because spacemit_ccu_mux_div_ops lacks .set_rate_and_parent, CCF's
> clk_change_rate() executes updates sequentially, calling .set_parent
> before .set_rate. .set_parent changes the hardware mux to 819.2 MHz
> while the divider remains at its previous value (div=1). The CPU temporarily
> runs at 819.2 MHz at a voltage only sufficient for 614.4 MHz, causing an
> immediate undervoltage crash.
>
> Could spacemit_ccu_mux_div_ops be updated to include a
> .set_rate_and_parent implementation to ensure the mux and divider are
> updated atomically?
The intermediate 819.2 MHz state is possible with this callback ordering.
I will add .set_rate_and_parent() in the next revision to update the mux
and divider fields together, followed by a single FC handshake.
--
Troy Mitchell
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-10 13:01 ` Yao Zi
-1 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-10 13:01 UTC (permalink / raw)
To: Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Yao Zi
On Wed, Sep 09, 2026 at 10:07:02PM +0800, Troy Mitchell wrote:
> CCF passes the selected parent's rate to set_rate(). Searching other
> parents at that point can produce a divider for a different source,
> making the programmed rate disagree with CCF's selection.
>
> Restrict divider selection to the supplied parent rate and use the same
> rounding as divider_recalc_rate(). Track the best error separately so
> low-rate requests do not leave the initial zero-Hz candidate selected.
> Skip zero-rate parents and have determine_rate() reject requests when no
> usable parent exists.
>
> Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> drivers/clk/spacemit/ccu_mix.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
> index a8b407049bf4d..da3c5685d4f65 100644
> --- a/drivers/clk/spacemit/ccu_mix.c
> +++ b/drivers/clk/spacemit/ccu_mix.c
> @@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
> struct ccu_mix *mix = hw_to_ccu_mix(hw);
> unsigned int parent_num = clk_hw_get_num_parents(hw);
> struct ccu_div_config *div = &mix->div;
> - u32 div_max = 1 << div->width;
> unsigned long best_rate = 0;
> + unsigned long best_delta = ULONG_MAX;
>
> for (int i = 0; i < parent_num; i++) {
> struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> unsigned long parent_rate;
> + u32 div_max = 1 << div->width;
div_max should be invariant across iterations. Is there a reason moving
it inside the loop?
> if (!parent)
> continue;
>
> parent_rate = clk_hw_get_rate(parent);
> + if (!parent_rate)
> + continue;
>
> for (int j = 1; j <= div_max; j++) {
> - unsigned long tmp = DIV_ROUND_CLOSEST_ULL(parent_rate, j);
> + unsigned long tmp = DIV_ROUND_UP_ULL(parent_rate, j);
> + unsigned long delta = abs_diff(tmp, rate);
>
> - if (abs(tmp - rate) < abs(best_rate - rate)) {
> + if (delta < best_delta) {
> + best_delta = delta;
> best_rate = tmp;
>
> if (div_val)
Regards,
Yao Zi
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
@ 2026-09-10 13:01 ` Yao Zi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-10 13:01 UTC (permalink / raw)
To: Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Yao Zi
On Wed, Sep 09, 2026 at 10:07:02PM +0800, Troy Mitchell wrote:
> CCF passes the selected parent's rate to set_rate(). Searching other
> parents at that point can produce a divider for a different source,
> making the programmed rate disagree with CCF's selection.
>
> Restrict divider selection to the supplied parent rate and use the same
> rounding as divider_recalc_rate(). Track the best error separately so
> low-rate requests do not leave the initial zero-Hz candidate selected.
> Skip zero-rate parents and have determine_rate() reject requests when no
> usable parent exists.
>
> Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> drivers/clk/spacemit/ccu_mix.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
> index a8b407049bf4d..da3c5685d4f65 100644
> --- a/drivers/clk/spacemit/ccu_mix.c
> +++ b/drivers/clk/spacemit/ccu_mix.c
> @@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
> struct ccu_mix *mix = hw_to_ccu_mix(hw);
> unsigned int parent_num = clk_hw_get_num_parents(hw);
> struct ccu_div_config *div = &mix->div;
> - u32 div_max = 1 << div->width;
> unsigned long best_rate = 0;
> + unsigned long best_delta = ULONG_MAX;
>
> for (int i = 0; i < parent_num; i++) {
> struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> unsigned long parent_rate;
> + u32 div_max = 1 << div->width;
div_max should be invariant across iterations. Is there a reason moving
it inside the loop?
> if (!parent)
> continue;
>
> parent_rate = clk_hw_get_rate(parent);
> + if (!parent_rate)
> + continue;
>
> for (int j = 1; j <= div_max; j++) {
> - unsigned long tmp = DIV_ROUND_CLOSEST_ULL(parent_rate, j);
> + unsigned long tmp = DIV_ROUND_UP_ULL(parent_rate, j);
> + unsigned long delta = abs_diff(tmp, rate);
>
> - if (abs(tmp - rate) < abs(best_rate - rate)) {
> + if (delta < best_delta) {
> + best_delta = delta;
> best_rate = tmp;
>
> if (div_val)
Regards,
Yao Zi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
2026-09-09 14:07 ` Troy Mitchell
@ 2026-09-10 13:31 ` Yao Zi
-1 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-10 13:31 UTC (permalink / raw)
To: Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Yao Zi
On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
> running by firmware can have a zero prepare count, so this flag alone
> cannot prevent set_rate() from reprogramming a live PLL.
Would it be a better idea to simply turn off the PLL before reprogramming,
since protected by CLK_SET_RATE_GATE, re-programming never happens when
the PLL is required by downstream? This also seems to be simpler.
> Check the hardware state and reject set_rate() while either the enable
> or lock bit is set on K1 PLLs and K3 PLLAs. Propagate register read/write
> failures and reject rate requests with no table candidate. Callers must
> still move consumers away and stop the PLL before changing its rate.
>
> Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Best regards,
Yao Zi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
@ 2026-09-10 13:31 ` Yao Zi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-10 13:31 UTC (permalink / raw)
To: Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Yao Zi
On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
> running by firmware can have a zero prepare count, so this flag alone
> cannot prevent set_rate() from reprogramming a live PLL.
Would it be a better idea to simply turn off the PLL before reprogramming,
since protected by CLK_SET_RATE_GATE, re-programming never happens when
the PLL is required by downstream? This also seems to be simpler.
> Check the hardware state and reject set_rate() while either the enable
> or lock bit is set on K1 PLLs and K3 PLLAs. Propagate register read/write
> failures and reject rate requests with no table candidate. Callers must
> still move consumers away and stop the PLL before changing its rate.
>
> Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Fixes: 3a086236c600 ("clk: spacemit: ccu_pll: add plla type clock")
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Best regards,
Yao Zi
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
2026-09-10 13:01 ` Yao Zi
@ 2026-09-10 14:19 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-10 14:19 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell,
Yao Zi
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
On Thu, Sep 10, 2026 at 01:01:30PM +0000, Yao Zi wrote:
> [...]
>
> > @@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
> > struct ccu_mix *mix = hw_to_ccu_mix(hw);
> > unsigned int parent_num = clk_hw_get_num_parents(hw);
> > struct ccu_div_config *div = &mix->div;
> > - u32 div_max = 1 << div->width;
> > unsigned long best_rate = 0;
> > + unsigned long best_delta = ULONG_MAX;
> >
> > for (int i = 0; i < parent_num; i++) {
> > struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> > unsigned long parent_rate;
> > + u32 div_max = 1 << div->width;
>
> div_max should be invariant across iterations. Is there a reason moving
> it inside the loop?
It is invariant in this patch. Moving the declaration was preparation
for patch 3, which makes the limit depend on the parent being considered:
u32 div_max = div->bypass & BIT(i) ? 1 : 1 << div->width;
K3 bypasses the divider for some parents, so those parents must only be
considered with a divisor of one.
I will keep the declaration outside the loop in patch 2 and move it
inside when introducing the bypass handling in patch 3. This does not
change the final code.
- Troy
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/5] clk: spacemit: make MIX rate selection consistent
@ 2026-09-10 14:19 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-10 14:19 UTC (permalink / raw)
To: Stephen Boyd, Brian Masney, Jerome Brunet, Yixun Lan, Alex Elder,
Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell,
Yao Zi
[-- Attachment #1.1: Type: text/plain, Size: 1271 bytes --]
On Thu, Sep 10, 2026 at 01:01:30PM +0000, Yao Zi wrote:
> [...]
>
> > @@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
> > struct ccu_mix *mix = hw_to_ccu_mix(hw);
> > unsigned int parent_num = clk_hw_get_num_parents(hw);
> > struct ccu_div_config *div = &mix->div;
> > - u32 div_max = 1 << div->width;
> > unsigned long best_rate = 0;
> > + unsigned long best_delta = ULONG_MAX;
> >
> > for (int i = 0; i < parent_num; i++) {
> > struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> > unsigned long parent_rate;
> > + u32 div_max = 1 << div->width;
>
> div_max should be invariant across iterations. Is there a reason moving
> it inside the loop?
It is invariant in this patch. Moving the declaration was preparation
for patch 3, which makes the limit depend on the parent being considered:
u32 div_max = div->bypass & BIT(i) ? 1 : 1 << div->width;
K3 bypasses the divider for some parents, so those parents must only be
considered with a divisor of one.
I will keep the declaration outside the loop in patch 2 and move it
inside when introducing the bypass handling in patch 3. This does not
change the final code.
- Troy
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
2026-09-10 13:31 ` Yao Zi
@ 2026-09-11 2:02 ` Troy Mitchell
-1 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-11 2:02 UTC (permalink / raw)
To: Yao Zi, Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 900 bytes --]
On Thu Sep 10, 2026 at 9:31 PM +08, Yao Zi wrote:
> On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
>> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
>> running by firmware can have a zero prepare count, so this flag alone
>> cannot prevent set_rate() from reprogramming a live PLL.
>
> Would it be a better idea to simply turn off the PLL before reprogramming,
> since protected by CLK_SET_RATE_GATE, re-programming never happens when
> the PLL is required by downstream? This also seems to be simpler.
Then assigned-clock-rates on the PLL provider node? I tested this
on K3: .set_rate() was called during provider registration, with PLL3's
prepare count still zero while the CPUs were running on it. Disabling
PLL3 hung the board. CLK_SET_RATE_GATE therefore does not protect users
that CCF has not yet accounted for.
--
Troy Mitchell
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
@ 2026-09-11 2:02 ` Troy Mitchell
0 siblings, 0 replies; 25+ messages in thread
From: Troy Mitchell @ 2026-09-11 2:02 UTC (permalink / raw)
To: Yao Zi, Troy Mitchell, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 900 bytes --]
On Thu Sep 10, 2026 at 9:31 PM +08, Yao Zi wrote:
> On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
>> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
>> running by firmware can have a zero prepare count, so this flag alone
>> cannot prevent set_rate() from reprogramming a live PLL.
>
> Would it be a better idea to simply turn off the PLL before reprogramming,
> since protected by CLK_SET_RATE_GATE, re-programming never happens when
> the PLL is required by downstream? This also seems to be simpler.
Then assigned-clock-rates on the PLL provider node? I tested this
on K3: .set_rate() was called during provider registration, with PLL3's
prepare count still zero while the CPUs were running on it. Disabling
PLL3 hung the board. CLK_SET_RATE_GATE therefore does not protect users
that CCF has not yet accounted for.
--
Troy Mitchell
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
2026-09-11 2:02 ` Troy Mitchell
@ 2026-09-11 6:08 ` Yao Zi
-1 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-11 6:08 UTC (permalink / raw)
To: Troy Mitchell, Yao Zi, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel
On Fri, Sep 11, 2026 at 10:02:47AM +0800, Troy Mitchell wrote:
> On Thu Sep 10, 2026 at 9:31 PM +08, Yao Zi wrote:
> > On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
> >> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
> >> running by firmware can have a zero prepare count, so this flag alone
> >> cannot prevent set_rate() from reprogramming a live PLL.
> >
> > Would it be a better idea to simply turn off the PLL before reprogramming,
> > since protected by CLK_SET_RATE_GATE, re-programming never happens when
> > the PLL is required by downstream? This also seems to be simpler.
> Then assigned-clock-rates on the PLL provider node?
Protecting PLL from rate-changing when it's enabled, and re-programming
the PLL to a recommended rate, are separate goals. This review comment
only focuses on the former (i.e. changes in this patch).
> I tested this
> on K3: .set_rate() was called during provider registration, with PLL3's
> prepare count still zero while the CPUs were running on it. Disabling
> PLL3 hung the board. CLK_SET_RATE_GATE therefore does not protect users
> that CCF has not yet accounted for.
I think it's caused by asynchronous probing. The APMU controller might
probe and register clocks after the PLL one, so the prepare/enable
counters of the PLLs might not match the reality during the gap.
>
> --
> Troy Mitchell
>
Best regards,
Yao Zi
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs
@ 2026-09-11 6:08 ` Yao Zi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Zi @ 2026-09-11 6:08 UTC (permalink / raw)
To: Troy Mitchell, Yao Zi, Stephen Boyd, Brian Masney, Jerome Brunet,
Yixun Lan, Alex Elder, Inochi Amaoto, Haylen Chu
Cc: linux-clk, linux-riscv, spacemit, linux-kernel
On Fri, Sep 11, 2026 at 10:02:47AM +0800, Troy Mitchell wrote:
> On Thu Sep 10, 2026 at 9:31 PM +08, Yao Zi wrote:
> > On Wed, Sep 09, 2026 at 10:07:04PM +0800, Troy Mitchell wrote:
> >> CLK_SET_RATE_GATE only protects clocks prepared through CCF. A PLL left
> >> running by firmware can have a zero prepare count, so this flag alone
> >> cannot prevent set_rate() from reprogramming a live PLL.
> >
> > Would it be a better idea to simply turn off the PLL before reprogramming,
> > since protected by CLK_SET_RATE_GATE, re-programming never happens when
> > the PLL is required by downstream? This also seems to be simpler.
> Then assigned-clock-rates on the PLL provider node?
Protecting PLL from rate-changing when it's enabled, and re-programming
the PLL to a recommended rate, are separate goals. This review comment
only focuses on the former (i.e. changes in this patch).
> I tested this
> on K3: .set_rate() was called during provider registration, with PLL3's
> prepare count still zero while the CPUs were running on it. Disabling
> PLL3 hung the board. CLK_SET_RATE_GATE therefore does not protect users
> that CCF has not yet accounted for.
I think it's caused by asynchronous probing. The APMU controller might
probe and register clocks after the PLL one, so the prepare/enable
counters of the PLLs might not match the reality during the gap.
>
> --
> Troy Mitchell
>
Best regards,
Yao Zi
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-09-11 6:09 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 14:07 [PATCH 0/5] clk: spacemit: preserve and safely synchronize firmware PLLs Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
2026-09-09 14:07 ` [PATCH 1/5] clk: spacemit: derive PLL rates from hardware Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
2026-09-09 14:07 ` [PATCH 2/5] clk: spacemit: make MIX rate selection consistent Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
2026-09-10 13:01 ` Yao Zi
2026-09-10 13:01 ` Yao Zi
2026-09-10 14:19 ` Troy Mitchell
2026-09-10 14:19 ` Troy Mitchell
2026-09-09 14:07 ` [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
2026-09-09 14:21 ` sashiko-bot
2026-09-10 3:23 ` Troy Mitchell
2026-09-09 14:07 ` [PATCH 4/5] clk: spacemit: reject rate changes to running firmware PLLs Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
2026-09-09 14:22 ` sashiko-bot
2026-09-10 13:31 ` Yao Zi
2026-09-10 13:31 ` Yao Zi
2026-09-11 2:02 ` Troy Mitchell
2026-09-11 2:02 ` Troy Mitchell
2026-09-11 6:08 ` Yao Zi
2026-09-11 6:08 ` Yao Zi
2026-09-09 14:07 ` [PATCH 5/5] clk: spacemit: safely synchronize PLL parameters during init Troy Mitchell
2026-09-09 14:07 ` Troy Mitchell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.