Re: [PATCH 3/5] clk: spacemit: describe CPU clock dividers and shared PLL muxes

[sashiko-bot@kernel.org]

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