Re: [PATCH 09/15] clk: rockchip: rk3588: Fix ACLK_BUS_ROOT rate set during probe

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The ACLK_BUS_ROOT is typically using CPLL as parent clock and running at
> 500 MHz out of reset when CPLL is running at 1.5 GHz.
> 
> The parent and rate of ACLK_BUS_ROOT is changed to use GPLL and to run
> at 237 MHz during clock driver probe. However, the clock rate is
> hardcoded to be reported as 375 MHz.
> 
> Change to explicitly use CPLL as parent and set the rate to 375 MHz, to
> match the reported rate and closer match how the Linux configures the
> clock.
> 

For those wondering, it is statically configured to 375MHz in the Linux 
kernel via the assigned-clock-rates property in 
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi for the clock controller.

> Signed-off-by: Jonas Karlman <[email protected]>
> ---
>   drivers/clk/rockchip/clk_rk3588.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
> index d03ca9c4ac8d..c48743d8296f 100644
> --- a/drivers/clk/rockchip/clk_rk3588.c
> +++ b/drivers/clk/rockchip/clk_rk3588.c
> @@ -1921,11 +1921,11 @@ static void rk3588_clk_init(struct rk3588_clk_priv *priv)
>   {
>   	int ret, div;
>   
> -	div = DIV_ROUND_UP(GPLL_HZ, 300 * MHz);
> +	div = DIV_ROUND_UP(CPLL_HZ, 375 * MHz);
>   	rk_clrsetreg(&priv->cru->clksel_con[38],
> -		     ACLK_BUS_ROOT_SEL_MASK |
> -		     ACLK_BUS_ROOT_DIV_MASK,
> -		     div << ACLK_BUS_ROOT_DIV_SHIFT);
> +		     ACLK_BUS_ROOT_SEL_MASK | ACLK_BUS_ROOT_DIV_MASK,
> +		     (ACLK_BUS_ROOT_SEL_CPLL << ACLK_BUS_ROOT_SEL_SHIFT) |
> +		     (div - 1) << ACLK_BUS_ROOT_DIV_SHIFT);
>   

This is a bug that you fix and haven't reported in the commit log. The 
[4:0] bitfield stores div+1, so we need to remove 1 to div when writing 
it to the register. I would say this warrants its own commit. Please split.

We switch to CPLL because GPLL is set to 1.188GHz which cannot derive 
375MHz since there isn't a fractional divider for aclk_bus_root. CPLL is 
set to 1.5GHz which cleanly divides by 4 to give 375MHz.

For the div -1 fix:
Fixes: 7a474df74023 ("clk: rockchip: Add rk3588 clk support")

For the GPLL->CPLL switch:
Fixes: 716ed2a8c0bb ("clk: rockchip: rk3588: add hardcoded assigned 
clocks values")

Reviewed-by: Quentin Schulz <[email protected]>

Thanks!
Quentin