Re: [PATCH] clk: spacemit: k3: add 2.0 GHz PLL3 and PLL4 rates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Dangerous Clock Initialization Causing Kernel Panic on Unmatched Rates
--

commit 3c3a6a2413682e8647573306b4c79569d6255e7d
Author: Aurelien Jarno <aurelien@aurel32.net>

clk: spacemit: k3: add 2.0 GHz PLL3 and PLL4 rates
    
Starting with version k3-br-v1.0.7, the vendor U-Boot configures the C0
and C1 CPU clock domains (X100 cores) to 2.0 GHz instead of 2.2 GHz.
PLL3 feeds C0 and PLL4 feeds C1, but their rate tables currently contain
only the 2.2 GHz overclocking operating point. This causes the linux
kernel to hang at boot.

> diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
> index cb0c4277f72a8..e42a1404aca89 100644
> --- a/drivers/clk/spacemit/ccu-k3.c
> +++ b/drivers/clk/spacemit/ccu-k3.c
> @@ -30,10 +30,12 @@ static const struct ccu_pll_rate_tbl pll2_rate_tbl[] = {
>  };
>  
>  static const struct ccu_pll_rate_tbl pll3_rate_tbl[] = {
> +	CCU_PLLA_RATE(2000000000UL, 0x0b292aaa, 0x0000ab00, 0xa0558686),

[Severity: High]
This isn't a bug introduced by this patch, but this patch only masks the
issue for U-Boot versions currently using 2.0 GHz by ensuring a match is
found. The underlying structural flaw remains: unconditionally disabling a
potentially vital clock during init just because its rate is unmatched is
dangerous.

If the bootloader configures a PLL to a frequency not explicitly defined in
the driver's static rate table, ccu_plla_lookup_matched_entry() returns a
null pointer. When this occurs, the initialization logic falls back to
unconditionally calling ccu_plla_disable() directly, bypassing
framework-level protections:

drivers/clk/spacemit/ccu_pll.c:ccu_plla_init() {
	...
	if (ccu_plla_lookup_matched_entry(pll))
		return 0;

	ccu_plla_disable(hw);
	ccu_plla_update_param(pll, &pll->config.rate_tbl[0]);
	...
}

This exhibits the same unsafe fallback logic for standard PLLs in
ccu_pll_init(), blindly shutting off the clock if it doesn't recognize the
hardware's configuration:

drivers/clk/spacemit/ccu_pll.c:ccu_pll_init() {
	...
	if (ccu_pll_lookup_matched_entry(pll))
		return 0;

	ccu_pll_disable(hw);
	ccu_pll_update_param(pll, &pll->config.rate_tbl[0]);
	...
}

Should the initialization logic be updated to handle unknown rates without
abruptly disabling vital clocks like the CPU clock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830172925.3245444-1-aurelien@aurel32.net?part=1