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