[PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes

[PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes

[Jonas Karlman]

This series fixes a few trivial copy-paste mistakes and other misc clock
issues for RK35xx SoCs.

Patch 1-3 fixes minor and very unlikely issues for plls
Patch 4 fixes PPLL mode handling on RK3588/RK3576
Patch 5 disable 'special pll handling' for RK3576 plls
Patch 6-7,11-13 fixes trivial clock configuration errors
Patch 8-9 fixes less trivial clock configuration errors
Patch 14-15 remove unused GRF syscon lookups

Some of these fixes has only been compile tested as it involves clocks
or rates that is rarely used in U-Boot.

The 'special pll handling' should ideally be moved into clk_rk3588
driver, something that can be done in a future follow-up series.

Jonas Karlman (15):
  clk: rockchip: pll: Fix double use of postdiv1
  clk: rockchip: pll: Always write the dsmpd flag
  clk: rockchip: pll: Always write the k param
  clk: rockchip: pll: Use PLL_FIXED_MODE flag on rk3588/rk3576 plls
  clk: rockchip: pll: Limit special rk3588_pll handling to RK3588
  clk: rockchip: rk3568: Fix trivial clock configuration errors
  clk: rockchip: rk3588: Fix trivial clock configuration errors
  clk: rockchip: rk3588: Fix possible divide by zero
  clk: rockchip: rk3588: Fix ACLK_BUS_ROOT rate set during probe
  clk: rockchip: rk3588: Use SPLL_HZ constant
  clk: rockchip: rk3576: Fix trivial clock configuration errors
  clk: rockchip: rk3528: Fix trivial clock configuration errors
  clk: rockchip: rk3506: Fix trivial clock configuration errors
  clk: rockchip: rk3568: Drop unused GRF syscon lookup
  clk: rockchip: rk3588: Drop unused GRF syscon lookup

 .../include/asm/arch-rockchip/cru_rk3568.h    |  1 -
 .../include/asm/arch-rockchip/cru_rk3576.h    |  5 +-
 .../include/asm/arch-rockchip/cru_rk3588.h    |  6 +-
 drivers/clk/rockchip/clk_pll.c                | 56 ++++++++++---------
 drivers/clk/rockchip/clk_rk3506.c             |  2 +
 drivers/clk/rockchip/clk_rk3528.c             |  7 ++-
 drivers/clk/rockchip/clk_rk3568.c             | 12 ++--
 drivers/clk/rockchip/clk_rk3576.c             | 26 ++++-----
 drivers/clk/rockchip/clk_rk3588.c             | 39 ++++++-------
 9 files changed, 72 insertions(+), 82 deletions(-)

-- 
2.54.0

[PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

[Jonas Karlman]

The possible frac_rate part of rk3036_pll plls is divided two times by
postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
this by using postdiv2 in the second do_div() call.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk_pll.c b/drivers/clk/rockchip/clk_pll.c
index 9dec40b1fe83..4fe67c889376 100644
--- a/drivers/clk/rockchip/clk_pll.c
+++ b/drivers/clk/rockchip/clk_pll.c
@@ -405,7 +405,7 @@ static ulong rk3036_pll_get_rate(struct rockchip_pll_clock *pll,
 			do_div(frac_rate, refdiv);
 			frac_rate >>= 24;
 			do_div(frac_rate, postdiv1);
-			do_div(frac_rate, postdiv1);
+			do_div(frac_rate, postdiv2);
 			rate += frac_rate;
 		}
 		return rate;
-- 
2.54.0

Re: [PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

[Quentin Schulz via U-Boot]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The possible frac_rate part of rk3036_pll plls is divided two times by
> postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
> this by using postdiv2 in the second do_div() call.
> 

I... cannot find which SoC(s) actually makes use of that function. It's 
only called when pll->type = rk3036 (which is 0, so anything that 
doesn't explicitly set ->type will have its type be rk3036), but all the 
drivers declaring a rockchip_pll_clock array specify something that 
isn't rk3036. So... is this dead code or what did I miss :)?

Where did you get the formula also, I couldn't quickly find a publicly 
leaked TRM for RK3036 (and since I couldn't figure out which SoC(s) use 
the same formula...).

Cheers,
Quentin

Re: [PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

[Jonas Karlman]

Hi Quentin,

On 8/7/2026 11:36 AM, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 7/30/26 4:12 PM, Jonas Karlman wrote:
>> The possible frac_rate part of rk3036_pll plls is divided two times by
>> postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
>> this by using postdiv2 in the second do_div() call.
>>
> 
> I... cannot find which SoC(s) actually makes use of that function. It's 
> only called when pll->type = rk3036 (which is 0, so anything that 
> doesn't explicitly set ->type will have its type be rk3036), but all the 
> drivers declaring a rockchip_pll_clock array specify something that 
> isn't rk3036. So... is this dead code or what did I miss :)?

It is also called for the pll_rk3328 type, i.e. rk3308, rk3506, rk3528,
rk3588 and rv1126, strangely not for rk3328 (in U-Boot).

I do think there is a very low chance we actually use the fractal pll
rates in U-Boot but the double use of postdiv1 seems (and should be)
wrong.

> 
> Where did you get the formula also, I couldn't quickly find a publicly 
> leaked TRM for RK3036 (and since I couldn't figure out which SoC(s) use 
> the same formula...).

I think they are very similar as most fractional plls used by RK.

Following is a snippet from Rockchip RK3036 TRM V1.0 20150907-Part1
(that used to exist in a GitHub repo at Poco-Ye/rk-datasheet)

  The Fractional PLL output frequency can be calculated using some
  simple formulas.
  
  If DSMPD = 1 (DSM is disabled, "integer mode")
   FOUTVCO = FREF / REFDIV * FBDIV
   FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
  
  If DSMPD = 0 (DSM is enabled, "fractional mode")
   FOUTVCO = FREF / REFDIV * (FBDIV + FRAC / 224)
   FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
  
  Where:
   FOUTVCO = Fractional PLL non-divided output frequency
   FOUTPOSTDIV = Fractional PLL divided output frequency (output of
                 second post divider)
   FREF = Fractional PLL input reference frequency
   REFDIV = Fractional PLL input reference clock divider
   FVCO = Frequency of internal VCO
   FBDIV = Integer value programmed into feedback divide
   FRAC = Fractional value programmed into DSM

And Linux use following in rockchip_rk3036_pll_recalc_rate():

	rate64 *= cur.fbdiv;
	do_div(rate64, cur.refdiv);

	if (cur.dsmpd == 0) {
		/* fractional mode */
		u64 frac_rate64 = prate * cur.frac;

		do_div(frac_rate64, cur.refdiv);
		rate64 += frac_rate64 >> 24;
	}

	do_div(rate64, cur.postdiv1);
	do_div(rate64, cur.postdiv2);

In U-Boot the postdiv part seem to be handled for the integer and
fractal parts separately. Maybe we should just try to adopt something
similar/closer to Linux?

Regards,
Jonas

> 
> Cheers,
> Quentin

Re: [PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

[Quentin Schulz via U-Boot]

Hi Jonas,

On 8/7/26 12:38 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 8/7/2026 11:36 AM, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 7/30/26 4:12 PM, Jonas Karlman wrote:
>>> The possible frac_rate part of rk3036_pll plls is divided two times by
>>> postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
>>> this by using postdiv2 in the second do_div() call.
>>>
>>
>> I... cannot find which SoC(s) actually makes use of that function. It's
>> only called when pll->type = rk3036 (which is 0, so anything that
>> doesn't explicitly set ->type will have its type be rk3036), but all the
>> drivers declaring a rockchip_pll_clock array specify something that
>> isn't rk3036. So... is this dead code or what did I miss :)?
> 
> It is also called for the pll_rk3328 type, i.e. rk3308, rk3506, rk3528,
> rk3588 and rv1126, strangely not for rk3328 (in U-Boot).
> 

s/rk3588/rk3568/ above.

Thanks for the pointer, I missed the second call to 
rk3036_pll_[gs]et_rate() when pll->type = pll_rk3328.

As for rk3328, it isn't using the common PLL core from 
drivers/clk/rockchip/clk-pll.c but it is indeed weird to have it named 
following the name of an SoC that doesn't actually make use of it.

> I do think there is a very low chance we actually use the fractal pll
> rates in U-Boot but the double use of postdiv1 seems (and should be)
> wrong.
> 

Alexey started to look into supporting them for RK3576 and found a few 
issues, so I'm guessing we are going to start seeing more users.

>>
>> Where did you get the formula also, I couldn't quickly find a publicly
>> leaked TRM for RK3036 (and since I couldn't figure out which SoC(s) use
>> the same formula...).
> 
> I think they are very similar as most fractional plls used by RK.
> 
> Following is a snippet from Rockchip RK3036 TRM V1.0 20150907-Part1
> (that used to exist in a GitHub repo at Poco-Ye/rk-datasheet)
> 
>    The Fractional PLL output frequency can be calculated using some
>    simple formulas.
>    
>    If DSMPD = 1 (DSM is disabled, "integer mode")
>     FOUTVCO = FREF / REFDIV * FBDIV
>     FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
>    
>    If DSMPD = 0 (DSM is enabled, "fractional mode")
>     FOUTVCO = FREF / REFDIV * (FBDIV + FRAC / 224)

This one's funny because I think it's supposed to be 2^24 and not 224. 
They made the same mistake in the RK3308 TRM. It's fine in other TRMs.

>     FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
>    
>    Where:
>     FOUTVCO = Fractional PLL non-divided output frequency
>     FOUTPOSTDIV = Fractional PLL divided output frequency (output of
>                   second post divider)
>     FREF = Fractional PLL input reference frequency
>     REFDIV = Fractional PLL input reference clock divider
>     FVCO = Frequency of internal VCO
>     FBDIV = Integer value programmed into feedback divide
>     FRAC = Fractional value programmed into DSM
> 
> And Linux use following in rockchip_rk3036_pll_recalc_rate():
> 
> 	rate64 *= cur.fbdiv;
> 	do_div(rate64, cur.refdiv);
> 
> 	if (cur.dsmpd == 0) {
> 		/* fractional mode */
> 		u64 frac_rate64 = prate * cur.frac;
> 
> 		do_div(frac_rate64, cur.refdiv);
> 		rate64 += frac_rate64 >> 24;
> 	}
> 
> 	do_div(rate64, cur.postdiv1);
> 	do_div(rate64, cur.postdiv2);
> 
> In U-Boot the postdiv part seem to be handled for the integer and
> fractal parts separately. Maybe we should just try to adopt something
> similar/closer to Linux?
> 

Casey has posted a series[1] porting some parts of the Linux's CCF to 
U-Boot which should allow us to import the Linux kernel driver(s) with 
minimal adaptation[2]. We would still need a minimal clock driver for 
xPL (and currently also proper pre-relocation) if I understood 
correctly, but we hopefully would need so little that it wouldn't cost 
us much to effectively have two separate clock drivers depending on the 
boot stage. I believe she's looking into making it work in proper pre-reloc.

I'm honestly not sure if I'll have the time to give this a try any time 
soon but maybe someone else would like to see if this can work out for 
us. I would also feel much better if we had CI with Rockchip boards so 
it feels less like a coin toss to do such big changes.

[1] 
https://lore.kernel.org/u-boot/20260720-casey-ccf-upstream-july-v1-0-8fece1b0333d@linaro.org/
[2] 
https://lore.kernel.org/u-boot/20260720-casey-ccf-upstream-july-v1-33-8fece1b0333d@linaro.org/

Cheers,
Quentin

Re: [PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The possible frac_rate part of rk3036_pll plls is divided two times by
> postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
> this by using postdiv2 in the second do_div() call.
> 

Fixes: bbda2ed5840c ("rockchip: clk: pll: add common pll setting funcs")
Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

I've checked RK3308, RK3506, RK3568 and RV1126 TRMs, they all have this 
formula (well, RK3308 divides FRAC by 224 instead of 2^24 but I'm 
assuming this is a typo in the TRM). I couldn't find a TRM for RK3528 
yet but I'm assuming it's going to follow the same formula.

Cheers,
Quentin

[PATCH 02/15] clk: rockchip: pll: Always write the dsmpd flag

[Jonas Karlman]

The dsmpd flag determines when a rk3036_pll fracpll should enable or
bypass use of fractal mode.

Change to always write the value of the dsmpd flag to reg to allow
fractal re-configuration and closer match how Linux behaves.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_pll.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/rockchip/clk_pll.c b/drivers/clk/rockchip/clk_pll.c
index 4fe67c889376..a135fad3de0c 100644
--- a/drivers/clk/rockchip/clk_pll.c
+++ b/drivers/clk/rockchip/clk_pll.c
@@ -326,13 +326,12 @@ static int rk3036_pll_set_rate(struct rockchip_pll_clock *pll,
 		     rate->fbdiv);
 	rk_clrsetreg(base + pll->con_offset + 0x4,
 		     (RK3036_PLLCON1_POSTDIV2_MASK |
-		     RK3036_PLLCON1_REFDIV_MASK),
+		     RK3036_PLLCON1_REFDIV_MASK |
+		     RK3036_PLLCON1_DSMPD_MASK),
 		     (rate->postdiv2 << RK3036_PLLCON1_POSTDIV2_SHIFT |
-		     rate->refdiv << RK3036_PLLCON1_REFDIV_SHIFT));
+		     rate->refdiv << RK3036_PLLCON1_REFDIV_SHIFT |
+		     rate->dsmpd << RK3036_PLLCON1_DSMPD_SHIFT));
 	if (!rate->dsmpd) {
-		rk_clrsetreg(base + pll->con_offset + 0x4,
-			     RK3036_PLLCON1_DSMPD_MASK,
-			     rate->dsmpd << RK3036_PLLCON1_DSMPD_SHIFT);
 		writel((readl(base + pll->con_offset + 0x8) &
 			(~RK3036_PLLCON2_FRAC_MASK)) |
 			    (rate->frac << RK3036_PLLCON2_FRAC_SHIFT),
-- 
2.54.0

Re: [PATCH 02/15] clk: rockchip: pll: Always write the dsmpd flag

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The dsmpd flag determines when a rk3036_pll fracpll should enable or
> bypass use of fractal mode.
> 
> Change to always write the value of the dsmpd flag to reg to allow
> fractal re-configuration and closer match how Linux behaves.
> 

nitpick: fractional, not fractal :) (don't resend for this)

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 03/15] clk: rockchip: pll: Always write the k param

[Jonas Karlman]

The k param help determine when a rk3588_pll fracpll should enable or
bypass use of fractal mode.

Change to always write the value of the k param to reg to allow fractal
re-configuration and closer match how Linux behaves.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_pll.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/rockchip/clk_pll.c b/drivers/clk/rockchip/clk_pll.c
index a135fad3de0c..7dbe77d74b8a 100644
--- a/drivers/clk/rockchip/clk_pll.c
+++ b/drivers/clk/rockchip/clk_pll.c
@@ -484,11 +484,9 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 		     RK3588_PLLCON1_S_MASK),
 		     (rate->p << RK3588_PLLCON1_P_SHIFT |
 		     rate->s << RK3588_PLLCON1_S_SHIFT));
-	if (rate->k) {
-		rk_clrsetreg(base + pll->con_offset + RK3588_PLLCON(2),
-			     RK3588_PLLCON2_K_MASK,
-			     rate->k << RK3588_PLLCON2_K_SHIFT);
-	}
+	rk_clrsetreg(base + pll->con_offset + RK3588_PLLCON(2),
+		     RK3588_PLLCON2_K_MASK,
+		     rate->k << RK3588_PLLCON2_K_SHIFT);
 	/* Power up */
 	rk_clrreg(base + pll->con_offset + RK3588_PLLCON(1),
 		  RK3588_PLLCON1_PWRDOWN);
-- 
2.54.0

Re: [PATCH 03/15] clk: rockchip: pll: Always write the k param

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The k param help determine when a rk3588_pll fracpll should enable or
> bypass use of fractal mode.
> 
> Change to always write the value of the k param to reg to allow fractal
> re-configuration and closer match how Linux behaves.
> 

There's just one worry that we're writing to undocumented registers when 
this is called on an integer PLL for which PLLCON2 is not defined in the 
TRM (from the discussion we had with Alexey some weeks ago). However, 
the kernel does this already so I guess it's fine.

Fixes: b851c006a150 ("clk: rockchip: pll: Add pll_rk3588 type for rk3588")
Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 04/15] clk: rockchip: pll: Use PLL_FIXED_MODE flag on rk3588/rk3576 plls

[Jonas Karlman]

The PPLL of RK3588 and RK3576 does not have a pll mode reg to switch
between slow, deep slow or normal mode.

Extend support for use of the PLL_FIXED_MODE flag to the rk3588_pll type
and update the PPLL to use this flag to signal that it does not need to
switch to slow mode before the rate is changed.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_pll.c    | 21 +++++++++++++--------
 drivers/clk/rockchip/clk_rk3576.c |  3 ++-
 drivers/clk/rockchip/clk_rk3588.c |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/rockchip/clk_pll.c b/drivers/clk/rockchip/clk_pll.c
index 7dbe77d74b8a..5f035eb98adc 100644
--- a/drivers/clk/rockchip/clk_pll.c
+++ b/drivers/clk/rockchip/clk_pll.c
@@ -456,9 +456,11 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 	if (pll_id == 3)
 		rk_clrsetreg(base + 0x84c, 0x1 << 1, 0x1 << 1);
 
-	rk_clrsetreg(base + pll->mode_offset,
-		     pll->mode_mask << pll->mode_shift,
-		     RKCLK_PLL_MODE_SLOW << pll->mode_shift);
+	if (!(pll->pll_flags & ROCKCHIP_PLL_FIXED_MODE)) {
+		rk_clrsetreg(base + pll->mode_offset,
+			     pll->mode_mask << pll->mode_shift,
+			     RKCLK_PLL_MODE_SLOW << pll->mode_shift);
+	}
 	if (pll_id == 0)
 		rk_clrsetreg(base + RK3588_B0PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
@@ -498,8 +500,11 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 		debug("%s: wait pll lock, pll_id=%ld\n", __func__, pll_id);
 	}
 
-	rk_clrsetreg(base + pll->mode_offset, pll->mode_mask << pll->mode_shift,
-		     RKCLK_PLL_MODE_NORMAL << pll->mode_shift);
+	if (!(pll->pll_flags & ROCKCHIP_PLL_FIXED_MODE)) {
+		rk_clrsetreg(base + pll->mode_offset,
+			     pll->mode_mask << pll->mode_shift,
+			     RKCLK_PLL_MODE_NORMAL << pll->mode_shift);
+	}
 	if (pll_id == 0) {
 		rk_clrsetreg(base + RK3588_B0PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
@@ -559,10 +564,10 @@ static ulong rk3588_pll_get_rate(struct rockchip_pll_clock *pll,
 
 	con = readl(base + pll->mode_offset);
 	shift = pll->mode_shift;
-	if (pll_id == 8)
-		mode = RKCLK_PLL_MODE_NORMAL;
-	else
+	if (!(pll->pll_flags & ROCKCHIP_PLL_FIXED_MODE))
 		mode = (con & (pll->mode_mask << shift)) >> shift;
+	else
+		mode = RKCLK_PLL_MODE_NORMAL;
 	switch (mode) {
 	case RKCLK_PLL_MODE_SLOW:
 		return OSC_HZ;
diff --git a/drivers/clk/rockchip/clk_rk3576.c b/drivers/clk/rockchip/clk_rk3576.c
index db8ce25852fd..92bde425b0ee 100644
--- a/drivers/clk/rockchip/clk_rk3576.c
+++ b/drivers/clk/rockchip/clk_rk3576.c
@@ -57,7 +57,8 @@ static struct rockchip_pll_clock rk3576_pll_clks[] = {
 	[GPLL] = PLL(pll_rk3588, PLL_GPLL, RK3576_PLL_CON(112),
 		     RK3576_MODE_CON0, 2, 15, 0, rk3576_24m_pll_rates),
 	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PMU_PLL_CON(128),
-		     RK3576_MODE_CON0, 10, 15, 0, rk3576_24m_pll_rates),
+		     RK3576_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,
+		     rk3576_24m_pll_rates),
 };
 
 #ifdef CONFIG_SPL_BUILD
diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
index be401a9faeec..b9fd4bec311b 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -62,7 +62,8 @@ static struct rockchip_pll_clock rk3588_pll_clks[] = {
 	[NPLL] = PLL(pll_rk3588, PLL_NPLL, RK3588_PLL_CON(120),
 		     RK3588_MODE_CON0, 0, 15, 0, rk3588_pll_rates),
 	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3588_PMU_PLL_CON(128),
-		     RK3588_MODE_CON0, 10, 15, 0, rk3588_pll_rates),
+		     RK3588_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,
+		     rk3588_pll_rates),
 #ifdef CONFIG_XPL_BUILD
 	/*
 	 * The SPLL is part of the SBUSCRU, not the main CRU and as
-- 
2.54.0

Re: [PATCH 04/15] clk: rockchip: pll: Use PLL_FIXED_MODE flag on rk3588/rk3576 plls

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The PPLL of RK3588 and RK3576 does not have a pll mode reg to switch
> between slow, deep slow or normal mode.
> 
> Extend support for use of the PLL_FIXED_MODE flag to the rk3588_pll type
> and update the PPLL to use this flag to signal that it does not need to
> switch to slow mode before the rate is changed.
> 

At a glance, it seems the Linux kernel is doing something different. 
Instead, it checks whether the current mode is normal (0x1) if not, then 
it sets it to slow while changing the rate (and then revert back to 
normal mode). I'm assuming they rely on undocumented registers (the PLL 
that don't have a slow mode for example) returning zeroes (i.e., slow 
mode) so they don't do this dance for when slow mode (or undocumented 
registers) is set. Did you send patches to the Linux kernel for this, 
are you planning to maybe?

This change here seems fine to me but we continue diverging from design 
decisions made in the Linux kernel (which aren't necessarily correct). 
Anyway:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 05/15] clk: rockchip: pll: Limit special rk3588_pll handling to RK3588

[Jonas Karlman]

The rk3588_pll_set_rate() function contains special handling intended
only for PLLs on RK3588 that are also applied to PLLs on RK3576.

This special handling should ideally be moved from the common pll funcs
into the clk_rk3588 driver. Add IS_ENABLED() guards as an initial step
to limit this special handling to PLLs on RK3588 targets.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_pll.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/rockchip/clk_pll.c b/drivers/clk/rockchip/clk_pll.c
index 5f035eb98adc..76276e1ac1f2 100644
--- a/drivers/clk/rockchip/clk_pll.c
+++ b/drivers/clk/rockchip/clk_pll.c
@@ -453,7 +453,7 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 	 * When power on or changing PLL setting,
 	 * we must force PLL into slow mode to ensure output stable clock.
 	 */
-	if (pll_id == 3)
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 3)
 		rk_clrsetreg(base + 0x84c, 0x1 << 1, 0x1 << 1);
 
 	if (!(pll->pll_flags & ROCKCHIP_PLL_FIXED_MODE)) {
@@ -461,15 +461,15 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 			     pll->mode_mask << pll->mode_shift,
 			     RKCLK_PLL_MODE_SLOW << pll->mode_shift);
 	}
-	if (pll_id == 0)
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 0)
 		rk_clrsetreg(base + RK3588_B0PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
 			     RKCLK_PLL_MODE_SLOW << 6);
-	else if (pll_id == 1)
+	else if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 1)
 		rk_clrsetreg(base + RK3588_B1PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
 			     RKCLK_PLL_MODE_SLOW << 6);
-	else if (pll_id == 2)
+	else if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 2)
 		rk_clrsetreg(base + RK3588_LPLL_CLKSEL_CON(5),
 			     pll->mode_mask << 14,
 			     RKCLK_PLL_MODE_SLOW << 14);
@@ -505,7 +505,7 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 			     pll->mode_mask << pll->mode_shift,
 			     RKCLK_PLL_MODE_NORMAL << pll->mode_shift);
 	}
-	if (pll_id == 0) {
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 0) {
 		rk_clrsetreg(base + RK3588_B0PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
 			     2 << 6);
@@ -515,7 +515,7 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 		rk_clrsetreg(base + RK3588_B0PLL_CLKSEL_CON(1),
 			     RK3588_CORE_DIV_MASK << RK3588_CORE_B13_DIV_SHIFT,
 			     0 << RK3588_CORE_B13_DIV_SHIFT);
-	} else if (pll_id == 1) {
+	} else if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 1) {
 		rk_clrsetreg(base + RK3588_B1PLL_CLKSEL_CON(0),
 			     pll->mode_mask << 6,
 			     2 << 6);
@@ -525,7 +525,7 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 		rk_clrsetreg(base + RK3588_B1PLL_CLKSEL_CON(1),
 			     RK3588_CORE_DIV_MASK << RK3588_CORE_B13_DIV_SHIFT,
 			     0 << RK3588_CORE_B13_DIV_SHIFT);
-	} else if (pll_id == 2) {
+	} else if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 2) {
 		rk_clrsetreg(base + RK3588_LPLL_CLKSEL_CON(5),
 			     pll->mode_mask << 14,
 			     2 << 14);
@@ -543,7 +543,7 @@ static int rk3588_pll_set_rate(struct rockchip_pll_clock *pll,
 			     0 << RK3588_CORE_L02_DIV_SHIFT);
 	}
 
-	if (pll_id == 3)
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && pll_id == 3)
 		rk_clrsetreg(base + 0x84c, 0x1 << 1, 0);
 
 	debug("PLL at %p: con0=%x con1= %x con2= %x mode= %x\n",
-- 
2.54.0

Re: [PATCH 05/15] clk: rockchip: pll: Limit special rk3588_pll handling to RK3588

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The rk3588_pll_set_rate() function contains special handling intended
> only for PLLs on RK3588 that are also applied to PLLs on RK3576.
> 
> This special handling should ideally be moved from the common pll funcs
> into the clk_rk3588 driver. Add IS_ENABLED() guards as an initial step
> to limit this special handling to PLLs on RK3588 targets.
> 

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Fixes: 3919310b37ec ("clk: rockchip: Add rk3576 clk support")

Thanks!
Quentin

[PATCH 06/15] clk: rockchip: rk3568: Fix trivial clock configuration errors

[Jonas Karlman]

The RK3568 clock driver has a few trivial copy-paste mistakes in its
clock handling.

Fix the trivial clock configuration errors:
- use correct pll_id for GPLL
- use correct shift macro for PWM0 div
- use correct pll rate for GMAC src clock

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_rk3568.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
index bb49af358e60..940b824103cc 100644
--- a/drivers/clk/rockchip/clk_rk3568.c
+++ b/drivers/clk/rockchip/clk_rk3568.c
@@ -77,7 +77,7 @@ static struct rockchip_pll_clock rk3568_pll_clks[] = {
 		     RK3568_MODE_CON, 2, 10, 0, NULL),
 	[CPLL] = PLL(pll_rk3328, PLL_CPLL, RK3568_PLL_CON(24),
 		     RK3568_MODE_CON, 4, 10, 0, rk3568_pll_rates),
-	[GPLL] = PLL(pll_rk3328, PLL_HPLL, RK3568_PLL_CON(16),
+	[GPLL] = PLL(pll_rk3328, PLL_GPLL, RK3568_PLL_CON(16),
 		     RK3568_MODE_CON, 6, 10, 0, rk3568_pll_rates),
 	[NPLL] = PLL(pll_rk3328, PLL_NPLL, RK3568_PLL_CON(32),
 		     RK3568_MODE_CON, 10, 10, 0, rk3568_pll_rates),
@@ -294,12 +294,12 @@ static ulong rk3568_pwm_set_pmuclk(struct rk3568_pmuclk_priv *priv,
 				     CLK_PWM0_SEL_MASK | CLK_PWM0_DIV_MASK,
 				     (CLK_PWM0_SEL_XIN24M <<
 				      CLK_PWM0_SEL_SHIFT) |
-				     0 << CLK_PWM0_SEL_SHIFT);
+				     0 << CLK_PWM0_DIV_SHIFT);
 		} else {
 			src_clk_div = DIV_ROUND_UP(priv->ppll_hz, rate);
 			assert(src_clk_div - 1 <= 127);
 			rk_clrsetreg(&pmucru->pmu_clksel_con[6],
-				     CLK_PWM0_DIV_MASK | CLK_PWM0_DIV_MASK,
+				     CLK_PWM0_SEL_MASK | CLK_PWM0_DIV_MASK,
 				     (CLK_PWM0_SEL_PPLL << CLK_PWM0_SEL_SHIFT) |
 				     (src_clk_div - 1) << CLK_PWM0_DIV_SHIFT);
 		}
@@ -1882,7 +1882,7 @@ static ulong rk3568_gmac_src_get_clk(struct rk3568_clk_priv *priv,
 	case CLK_MAC0_2TOP_SEL_25M:
 		return 25 * MHz;
 	case CLK_MAC0_2TOP_SEL_PPLL:
-		return rk3568_pmu_pll_get_rate(priv, HPLL);
+		return rk3568_pmu_pll_get_rate(priv, PPLL);
 	default:
 		return -ENOENT;
 	}
-- 
2.54.0

Re: [PATCH 06/15] clk: rockchip: rk3568: Fix trivial clock configuration errors

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The RK3568 clock driver has a few trivial copy-paste mistakes in its
> clock handling.
> 
> Fix the trivial clock configuration errors:
> - use correct pll_id for GPLL
> - use correct shift macro for PWM0 div
> - use correct pll rate for GMAC src clock
> 

If you need to list things you're doing in a commit log, it means you 
need to split the changes in multiple commits.

Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")

Please split in four and add my Reviewed-by:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 07/15] clk: rockchip: rk3588: Fix trivial clock configuration errors

[Jonas Karlman]

The RK3588 clock driver has a few trivial copy-paste mistakes in its
clock handling.

Fix the trivial clock configuration errors:
- remove duplicate PLL_CON/MODE_CON defines
- rename and use PHP_PLL_CON macro
- use correct parent rate for TSADC clock
- use correct parent rate for UART clocks
- align BxPLL configuration to match other PLLs

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 arch/arm/include/asm/arch-rockchip/cru_rk3588.h |  5 +----
 drivers/clk/rockchip/clk_rk3588.c               | 12 +++++-------
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
index afce8a44af3b..39295adeb38a 100644
--- a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
+++ b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
@@ -105,9 +105,6 @@ struct pll_rate_table {
 	unsigned int k;
 };
 
-#define RK3588_PLL_CON(x)		((x) * 0x4)
-#define RK3588_MODE_CON			0x280
-
 #define RK3588_PHP_CRU_BASE		0x8000
 #define RK3588_PMU_CRU_BASE		0x30000
 #define RK3588_BIGCORE0_CRU_BASE	0x50000
@@ -129,10 +126,10 @@ struct pll_rate_table {
 #define RK3588_SDMMC_CON0		0xC30
 #define RK3588_SDMMC_CON1		0xC34
 
+#define RK3588_PHP_PLL_CON(x)		((x) * 0x4 + RK3588_PHP_CRU_BASE)
 #define RK3588_PHP_CLKGATE_CON(x)	((x) * 0x4 + RK3588_PHP_CRU_BASE + 0x800)
 #define RK3588_PHP_SOFTRST_CON(x)	((x) * 0x4 + RK3588_PHP_CRU_BASE + 0xa00)
 
-#define RK3588_PMU_PLL_CON(x)		((x) * 0x4 + RK3588_PHP_CRU_BASE)
 #define RK3588_PMU_CLKSEL_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0x300)
 #define RK3588_PMU_CLKGATE_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0x800)
 #define RK3588_PMU_SOFTRST_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0xa00)
diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
index b9fd4bec311b..157a02e011a5 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -44,11 +44,9 @@ static struct rockchip_pll_rate_table rk3588_pll_rates[] = {
 
 static struct rockchip_pll_clock rk3588_pll_clks[] = {
 	[B0PLL] = PLL(pll_rk3588, PLL_B0PLL, RK3588_B0_PLL_CON(0),
-		      RK3588_B0_PLL_MODE_CON, 0, 15, 0,
-		      rk3588_pll_rates),
+		      RK3588_B0_PLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
 	[B1PLL] = PLL(pll_rk3588, PLL_B1PLL, RK3588_B1_PLL_CON(8),
-		      RK3588_B1_PLL_MODE_CON, 0, 15, 0,
-		      rk3588_pll_rates),
+		      RK3588_B1_PLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
 	[LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3588_LPLL_CON(16),
 		     RK3588_LPLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
 	[V0PLL] = PLL(pll_rk3588, PLL_V0PLL, RK3588_PLL_CON(88),
@@ -61,7 +59,7 @@ static struct rockchip_pll_clock rk3588_pll_clks[] = {
 		     RK3588_MODE_CON0, 2, 15, 0, rk3588_pll_rates),
 	[NPLL] = PLL(pll_rk3588, PLL_NPLL, RK3588_PLL_CON(120),
 		     RK3588_MODE_CON0, 0, 15, 0, rk3588_pll_rates),
-	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3588_PMU_PLL_CON(128),
+	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3588_PHP_PLL_CON(128),
 		     RK3588_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,
 		     rk3588_pll_rates),
 #ifdef CONFIG_XPL_BUILD
@@ -655,7 +653,7 @@ static ulong rk3588_adc_get_clk(struct rk3588_clk_priv *priv, ulong clk_id)
 		if (sel == CLK_TSADC_SEL_24M)
 			prate = OSC_HZ;
 		else
-			prate = 100 * MHz;
+			prate = priv->gpll_hz;
 		return DIV_TO_RATE(prate, div);
 	default:
 		return -ENOENT;
@@ -1343,7 +1341,7 @@ static ulong rk3588_uart_set_rate(struct rk3588_clk_priv *priv,
 	} else if (priv->cpll_hz % rate == 0) {
 		clk_src = CLK_UART_SRC_SEL_CPLL;
 		uart_src = CLK_UART_SEL_SRC;
-		div = DIV_ROUND_UP(priv->gpll_hz, rate);
+		div = DIV_ROUND_UP(priv->cpll_hz, rate);
 	} else if (rate == OSC_HZ) {
 		clk_src = CLK_UART_SRC_SEL_GPLL;
 		uart_src = CLK_UART_SEL_XIN24M;
-- 
2.54.0

Re: [PATCH 07/15] clk: rockchip: rk3588: Fix trivial clock configuration errors

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The RK3588 clock driver has a few trivial copy-paste mistakes in its
> clock handling.
> 
> Fix the trivial clock configuration errors:
> - remove duplicate PLL_CON/MODE_CON defines

RK3588_MODE_CON is not duplicated. However, it is unused 
(RK3588_MODE_CON0 was preferred) so its removal is fine.

> - rename and use PHP_PLL_CON macro
> - use correct parent rate for TSADC clock
> - use correct parent rate for UART clocks
> - align BxPLL configuration to match other PLLs
> 

Please no cosmetic changes in a patch that has logic changes, split them 
in their own commit.

For the rest, same remark as the previous patch, when you need to list 
unrelated things, you need to split in separate commits. The individual 
changes are mostly fine, see my remark for the PHP_PLL_CON macro a few 
lines after.

> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   arch/arm/include/asm/arch-rockchip/cru_rk3588.h |  5 +----
>   drivers/clk/rockchip/clk_rk3588.c               | 12 +++++-------
>   2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
> index afce8a44af3b..39295adeb38a 100644
> --- a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
> +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
> @@ -105,9 +105,6 @@ struct pll_rate_table {
>   	unsigned int k;
>   };
>   
> -#define RK3588_PLL_CON(x)		((x) * 0x4)
> -#define RK3588_MODE_CON			0x280
> -

Fixes: 165d258386a7 ("arm: rockchip: Add cru header for rk3588")

>   #define RK3588_PHP_CRU_BASE		0x8000
>   #define RK3588_PMU_CRU_BASE		0x30000
>   #define RK3588_BIGCORE0_CRU_BASE	0x50000
> @@ -129,10 +126,10 @@ struct pll_rate_table {
>   #define RK3588_SDMMC_CON0		0xC30
>   #define RK3588_SDMMC_CON1		0xC34
>   
> +#define RK3588_PHP_PLL_CON(x)		((x) * 0x4 + RK3588_PHP_CRU_BASE)

Shouldn't that be PHP_PPLL_CON(x)?

Also, please take this opportunity to actually fix the offset and have 
0x200 added to it already as otherwise we need to write

RK3588_PHP_PLL_CON(128)

when wanting to interact with PHPTOPCRU_PPLL_CON0, and I would much prefer

RK3588_PHP_PLL_CON(0)

instead.

Fixes: 165d258386a7 ("arm: rockchip: Add cru header for rk3588")

>   #define RK3588_PHP_CLKGATE_CON(x)	((x) * 0x4 + RK3588_PHP_CRU_BASE + 0x800)
>   #define RK3588_PHP_SOFTRST_CON(x)	((x) * 0x4 + RK3588_PHP_CRU_BASE + 0xa00)
>   
> -#define RK3588_PMU_PLL_CON(x)		((x) * 0x4 + RK3588_PHP_CRU_BASE)
>   #define RK3588_PMU_CLKSEL_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0x300)
>   #define RK3588_PMU_CLKGATE_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0x800)
>   #define RK3588_PMU_SOFTRST_CON(x)	((x) * 0x4 + RK3588_PMU_CRU_BASE + 0xa00)
> diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
> index b9fd4bec311b..157a02e011a5 100644
> --- a/drivers/clk/rockchip/clk_rk3588.c
> +++ b/drivers/clk/rockchip/clk_rk3588.c
> @@ -44,11 +44,9 @@ static struct rockchip_pll_rate_table rk3588_pll_rates[] = {
>   
>   static struct rockchip_pll_clock rk3588_pll_clks[] = {
>   	[B0PLL] = PLL(pll_rk3588, PLL_B0PLL, RK3588_B0_PLL_CON(0),
> -		      RK3588_B0_PLL_MODE_CON, 0, 15, 0,
> -		      rk3588_pll_rates),
> +		      RK3588_B0_PLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
>   	[B1PLL] = PLL(pll_rk3588, PLL_B1PLL, RK3588_B1_PLL_CON(8),
> -		      RK3588_B1_PLL_MODE_CON, 0, 15, 0,
> -		      rk3588_pll_rates),
> +		      RK3588_B1_PLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
>   	[LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3588_LPLL_CON(16),
>   		     RK3588_LPLL_MODE_CON, 0, 15, 0, rk3588_pll_rates),
>   	[V0PLL] = PLL(pll_rk3588, PLL_V0PLL, RK3588_PLL_CON(88),
> @@ -61,7 +59,7 @@ static struct rockchip_pll_clock rk3588_pll_clks[] = {
>   		     RK3588_MODE_CON0, 2, 15, 0, rk3588_pll_rates),
>   	[NPLL] = PLL(pll_rk3588, PLL_NPLL, RK3588_PLL_CON(120),
>   		     RK3588_MODE_CON0, 0, 15, 0, rk3588_pll_rates),
> -	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3588_PMU_PLL_CON(128),
> +	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3588_PHP_PLL_CON(128),
>   		     RK3588_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,
>   		     rk3588_pll_rates),
>   #ifdef CONFIG_XPL_BUILD
> @@ -655,7 +653,7 @@ static ulong rk3588_adc_get_clk(struct rk3588_clk_priv *priv, ulong clk_id)
>   		if (sel == CLK_TSADC_SEL_24M)
>   			prate = OSC_HZ;
>   		else
> -			prate = 100 * MHz;
> +			prate = priv->gpll_hz;

We also have a few cases of *PLL_HZ being used instead of priv->*pll_hz. 
Technically, they should be the same based on the implementation  in 
rk3588_clk_init() but we're not being consistent. In any case, not a 
blocker for this here.

Fixes: 7a474df74023 ("clk: rockchip: Add rk3588 clk support")

>   		return DIV_TO_RATE(prate, div);
>   	default:
>   		return -ENOENT;
> @@ -1343,7 +1341,7 @@ static ulong rk3588_uart_set_rate(struct rk3588_clk_priv *priv,
>   	} else if (priv->cpll_hz % rate == 0) {
>   		clk_src = CLK_UART_SRC_SEL_CPLL;
>   		uart_src = CLK_UART_SEL_SRC;
> -		div = DIV_ROUND_UP(priv->gpll_hz, rate);
> +		div = DIV_ROUND_UP(priv->cpll_hz, rate);

Fixes: 7a474df74023 ("clk: rockchip: Add rk3588 clk support")

>   	} else if (rate == OSC_HZ) {
>   		clk_src = CLK_UART_SRC_SEL_GPLL;
>   		uart_src = CLK_UART_SEL_XIN24M;

[PATCH 08/15] clk: rockchip: rk3588: Fix possible divide by zero

[Jonas Karlman]

The best rate handling of rk3588_dclk_vop_set_clk() may unintentionally
trigger a divide by zero when V0PLL is tested as parent. Instead, fully
skip rate calculation when testing using V0PLL as parent to avoid a
divide by zero error.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_rk3588.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
index 157a02e011a5..d03ca9c4ac8d 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -1165,8 +1165,8 @@ static ulong rk3588_dclk_vop_set_clk(struct rk3588_clk_priv *priv,
 				pll_rate = priv->aupll_hz;
 				break;
 			case DCLK_VOP_SRC_SEL_V0PLL:
-				pll_rate = 0;
-				break;
+				/* Skip V0PLL as parent for best_rate mode */
+				continue;
 			default:
 				printf("do not support this vop pll sel\n");
 				return -EINVAL;
-- 
2.54.0

Re: [PATCH 08/15] clk: rockchip: rk3588: Fix possible divide by zero

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The best rate handling of rk3588_dclk_vop_set_clk() may unintentionally
> trigger a divide by zero when V0PLL is tested as parent. Instead, fully
> skip rate calculation when testing using V0PLL as parent to avoid a
> divide by zero error.
> 

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

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

[Jonas Karlman]

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.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 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);
 
 	if (priv->cpll_hz != CPLL_HZ) {
 		ret = rockchip_pll_set_rate(&rk3588_pll_clks[CPLL], priv->cru,
-- 
2.54.0

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

[Quentin Schulz]

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 <jonas@kwiboo.se>
> ---
>   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 <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 10/15] clk: rockchip: rk3588: Use SPLL_HZ constant

[Jonas Karlman]

The SPLL rate is set to 702 MHz in SPL. Replace '702 * MHz' with use of
the SPLL_HZ constant to enhance readability.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 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 c48743d8296f..1006102f4cf4 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -777,7 +777,7 @@ static ulong rk3588_mmc_get_clk(struct rk3588_clk_priv *priv, ulong clk_id)
 		sel = (con & DCLK_DECOM_SEL_MASK) >>
 		      DCLK_DECOM_SEL_SHIFT;
 		if (sel == DCLK_DECOM_SEL_SPLL)
-			prate = 702 * MHz;
+			prate = SPLL_HZ;
 		else
 			prate = priv->gpll_hz;
 		return DIV_TO_RATE(prate, div);
@@ -817,9 +817,9 @@ static ulong rk3588_mmc_set_clk(struct rk3588_clk_priv *priv,
 		}
 		break;
 	case DCLK_DECOM:
-		if (!(702 * MHz % rate)) {
+		if (!(SPLL_HZ % rate)) {
 			src_clk = DCLK_DECOM_SEL_SPLL;
-			div = DIV_ROUND_UP(702 * MHz, rate);
+			div = DIV_ROUND_UP(SPLL_HZ, rate);
 		} else {
 			src_clk = DCLK_DECOM_SEL_GPLL;
 			div = DIV_ROUND_UP(priv->gpll_hz, rate);
@@ -942,7 +942,7 @@ static ulong rk3588_aclk_vop_get_clk(struct rk3588_clk_priv *priv, ulong clk_id)
 		else if (sel == ACLK_VOP_ROOT_SEL_NPLL)
 			parent = priv->npll_hz;
 		else
-			parent = 702 * MHz;
+			parent = SPLL_HZ;
 		return DIV_TO_RATE(parent, div);
 	case ACLK_VOP_LOW_ROOT:
 		con = readl(&cru->clksel_con[110]);
-- 
2.54.0

Re: [PATCH 10/15] clk: rockchip: rk3588: Use SPLL_HZ constant

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:12 PM, Jonas Karlman wrote:
> The SPLL rate is set to 702 MHz in SPL. Replace '702 * MHz' with use of
> the SPLL_HZ constant to enhance readability.
> 

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors

[Jonas Karlman]

The RK3576 clock driver has a few trivial copy-paste mistakes in its
clock handling.

Fix the trivial clock configuration errors:
- use correct VPLL mode reg
- rename and use PHP_PLL_CON macro
- set correct parent for ACLK_TOP clocks
- avoid overriding the selected I2C parent clock
- stop CLK_I2C8 from falling through into CLK_I2C9
- use correct SARADC and TSADC clksel regs
- use correct parent pll rate for UART clocks
- align BPLL configuration to match other PLLs
- remove unused BPLL_CON macro

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 .../include/asm/arch-rockchip/cru_rk3576.h    |  5 ++--
 drivers/clk/rockchip/clk_rk3576.c             | 23 +++++++++----------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
index fb77fbd7307a..41e225245843 100644
--- a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
+++ b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
@@ -127,24 +127,23 @@ struct pll_rate_table {
 #define RK3576_SDMMC_CON0		0xC30
 #define RK3576_SDMMC_CON1		0xC34
 
+#define RK3576_PHP_PLL_CON(x)		((x) * 0x4 + RK3576_PHP_CRU_BASE)
 #define RK3576_PHP_CLKSEL_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x300)
 #define RK3576_PHP_CLKGATE_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x800)
 #define RK3576_PHP_SOFTRST_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0xa00)
 
-#define RK3576_PMU_PLL_CON(x)		((x) * 0x4 + RK3576_PHP_CRU_BASE)
 #define RK3576_PMU_CLKSEL_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x300)
 #define RK3576_PMU_CLKGATE_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x800)
 #define RK3576_PMU_SOFTRST_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0xa00)
 
+#define RK3576_LPLL_CON(x)		((x) * 0x4 + RK3576_CCI_CRU_BASE)
 #define RK3576_CCI_CLKSEL_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x300)
 #define RK3576_CCI_CLKGATE_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x800)
 #define RK3576_CCI_SOFTRST_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0xa00)
 
-#define RK3576_BPLL_CON(x)		((x) * 0x4 + RK3576_BIGCORE_CRU_BASE)
 #define RK3576_BIGCORE_CLKSEL_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x300)
 #define RK3576_BIGCORE_CLKGATE_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x800)
 #define RK3576_BIGCORE_SOFTRST_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0xa00)
-#define RK3576_LPLL_CON(x)		((x) * 0x4 + RK3576_CCI_CRU_BASE)
 #define RK3576_LITCORE_CLKSEL_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x300)
 #define RK3576_LITCORE_CLKGATE_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x800)
 #define RK3576_LITCORE_SOFTRST_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0xa00)
diff --git a/drivers/clk/rockchip/clk_rk3576.c b/drivers/clk/rockchip/clk_rk3576.c
index 92bde425b0ee..75b705ffba2f 100644
--- a/drivers/clk/rockchip/clk_rk3576.c
+++ b/drivers/clk/rockchip/clk_rk3576.c
@@ -44,19 +44,18 @@ static struct rockchip_pll_rate_table rk3576_24m_pll_rates[] = {
 
 static struct rockchip_pll_clock rk3576_pll_clks[] = {
 	[BPLL] = PLL(pll_rk3588, PLL_BPLL, RK3576_PLL_CON(0),
-		      RK3576_BPLL_MODE_CON0, 0, 15, 0,
-		      rk3576_24m_pll_rates),
+		     RK3576_BPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates),
 	[LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3576_LPLL_CON(16),
 		     RK3576_LPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates),
 	[VPLL] = PLL(pll_rk3588, PLL_VPLL, RK3576_PLL_CON(88),
-		      RK3576_LPLL_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates),
+		     RK3576_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates),
 	[AUPLL] = PLL(pll_rk3588, PLL_AUPLL, RK3576_PLL_CON(96),
 		      RK3576_MODE_CON0, 6, 15, 0, rk3576_24m_pll_rates),
 	[CPLL] = PLL(pll_rk3588, PLL_CPLL, RK3576_PLL_CON(104),
 		     RK3576_MODE_CON0, 8, 15, 0, rk3576_24m_pll_rates),
 	[GPLL] = PLL(pll_rk3588, PLL_GPLL, RK3576_PLL_CON(112),
 		     RK3576_MODE_CON0, 2, 15, 0, rk3576_24m_pll_rates),
-	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PMU_PLL_CON(128),
+	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PHP_PLL_CON(128),
 		     RK3576_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,
 		     rk3576_24m_pll_rates),
 };
@@ -320,8 +319,7 @@ static ulong rk3576_top_set_clk(struct rk3576_clk_priv *priv,
 		rk_clrsetreg(&cru->clksel_con[10],
 			     ACLK_TOP_MID_DIV_MASK |
 			     ACLK_TOP_MID_SEL_MASK,
-			     (ACLK_TOP_MID_SEL_GPLL <<
-			      ACLK_TOP_MID_SEL_SHIFT) |
+			     (src_clk << ACLK_TOP_MID_SEL_SHIFT) |
 			     (src_clk_div - 1) << ACLK_TOP_MID_DIV_SHIFT);
 		break;
 	case PCLK_TOP_ROOT:
@@ -429,7 +427,7 @@ static ulong rk3576_i2c_set_clk(struct rk3576_clk_priv *priv, ulong clk_id,
 		src_clk = CLK_I2C_SEL_200M;
 	else if (rate >= 99 * MHz)
 		src_clk = CLK_I2C_SEL_100M;
-	if (rate >= 50 * MHz)
+	else if (rate >= 50 * MHz)
 		src_clk = CLK_I2C_SEL_50M;
 	else
 		src_clk = CLK_I2C_SEL_OSC;
@@ -470,6 +468,7 @@ static ulong rk3576_i2c_set_clk(struct rk3576_clk_priv *priv, ulong clk_id,
 	case CLK_I2C8:
 		rk_clrsetreg(&cru->clksel_con[57], CLK_I2C8_SEL_MASK,
 			     src_clk << CLK_I2C8_SEL_SHIFT);
+		break;
 	case CLK_I2C9:
 		rk_clrsetreg(&cru->clksel_con[58], CLK_I2C9_SEL_MASK,
 			     src_clk << CLK_I2C9_SEL_SHIFT);
@@ -691,7 +690,7 @@ static ulong rk3576_adc_set_clk(struct rk3576_clk_priv *priv,
 		} else {
 			src_clk_div = DIV_ROUND_UP(priv->gpll_hz, rate);
 			assert(src_clk_div - 1 <= 255);
-			rk_clrsetreg(&cru->clksel_con[59],
+			rk_clrsetreg(&cru->clksel_con[58],
 				     CLK_SARADC_SEL_MASK |
 				     CLK_SARADC_DIV_MASK,
 				     (CLK_SARADC_SEL_GPLL <<
@@ -703,7 +702,7 @@ static ulong rk3576_adc_set_clk(struct rk3576_clk_priv *priv,
 	case CLK_TSADC:
 		src_clk_div = DIV_ROUND_UP(OSC_HZ, rate);
 		assert(src_clk_div - 1 <= 255);
-		rk_clrsetreg(&cru->clksel_con[58],
+		rk_clrsetreg(&cru->clksel_con[59],
 			     CLK_TSADC_DIV_MASK,
 			     (src_clk_div - 1) <<
 			     CLK_TSADC_DIV_SHIFT);
@@ -1715,7 +1714,7 @@ static ulong rk3576_uart_frac_set_rate(struct rk3576_clk_priv *priv,
 		p_rate = OSC_HZ;
 	} else {
 		clk_src = CLK_UART_SRC_SEL_GPLL;
-		p_rate = priv->cpll_hz;
+		p_rate = priv->gpll_hz;
 	}
 
 	rational_best_approximation(rate, p_rate, GENMASK(16 - 1, 0),
@@ -1843,7 +1842,7 @@ static ulong rk3576_uart_set_rate(struct rk3576_clk_priv *priv,
 		div = DIV_ROUND_UP(priv->gpll_hz, rate);
 	} else if (!(priv->cpll_hz % rate)) {
 		clk_src = CLK_UART_SEL_CPLL;
-		div = DIV_ROUND_UP(priv->gpll_hz, rate);
+		div = DIV_ROUND_UP(priv->cpll_hz, rate);
 	} else if (!(rk3576_uart_frac_get_rate(priv, CLK_UART_FRAC_0) % rate)) {
 		clk_src = CLK_UART_SEL_FRAC0;
 		div = DIV_ROUND_UP(rk3576_uart_frac_get_rate(priv, CLK_UART_FRAC_0), rate);
@@ -2458,7 +2457,7 @@ static int rk3576_clk_probe(struct udevice *dev)
 	       RK3576_SCRU_BASE + RK3576_MODE_CON0);
 	/* fix ppll\aupll\cpll */
 	writel(BITS_WITH_WMASK(2, 0x7U, 6),
-	       RK3576_CRU_BASE + RK3576_PMU_PLL_CON(129));
+	       RK3576_CRU_BASE + RK3576_PHP_PLL_CON(129));
 	writel(BITS_WITH_WMASK(2, 0x7U, 6),
 	       RK3576_CRU_BASE + RK3576_PLL_CON(97));
 	writel(BITS_WITH_WMASK(2, 0x7U, 6),
-- 
2.54.0

Re: [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors

[Quentin Schulz via U-Boot]

Hi Jonas,

On 7/30/26 4:13 PM, Jonas Karlman wrote:
> The RK3576 clock driver has a few trivial copy-paste mistakes in its
> clock handling.
> 
> Fix the trivial clock configuration errors:
> - use correct VPLL mode reg
> - rename and use PHP_PLL_CON macro
> - set correct parent for ACLK_TOP clocks
> - avoid overriding the selected I2C parent clock
> - stop CLK_I2C8 from falling through into CLK_I2C9
> - use correct SARADC and TSADC clksel regs
> - use correct parent pll rate for UART clocks
> - align BPLL configuration to match other PLLs
> - remove unused BPLL_CON macro
> 

Please split those into separate commits. All changes are fine 
individually. See small remark below for a change I believe would help 
with reading the code more easily.

> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   .../include/asm/arch-rockchip/cru_rk3576.h    |  5 ++--
>   drivers/clk/rockchip/clk_rk3576.c             | 23 +++++++++----------
>   2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
> index fb77fbd7307a..41e225245843 100644
> --- a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
> +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h
> @@ -127,24 +127,23 @@ struct pll_rate_table {
>   #define RK3576_SDMMC_CON0		0xC30
>   #define RK3576_SDMMC_CON1		0xC34
>   
> +#define RK3576_PHP_PLL_CON(x)		((x) * 0x4 + RK3576_PHP_CRU_BASE)

Same remark as for the RK3588 patch, please add 0x200 so we can use 
RK3576_PHP_PLL_CON(0) when we want to interact with PHPTOPCRU_PPLL_CON0.

>   #define RK3576_PHP_CLKSEL_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x300)
>   #define RK3576_PHP_CLKGATE_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x800)
>   #define RK3576_PHP_SOFTRST_CON(x)	((x) * 0x4 + RK3576_PHP_CRU_BASE + 0xa00)
>   
> -#define RK3576_PMU_PLL_CON(x)		((x) * 0x4 + RK3576_PHP_CRU_BASE)
>   #define RK3576_PMU_CLKSEL_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x300)
>   #define RK3576_PMU_CLKGATE_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x800)
>   #define RK3576_PMU_SOFTRST_CON(x)	((x) * 0x4 + RK3576_PMU_CRU_BASE + 0xa00)
>   
> +#define RK3576_LPLL_CON(x)		((x) * 0x4 + RK3576_CCI_CRU_BASE)

Please add 0x40 so we can do RK3576_LPLL_CON(0) to interact with 
CCICRU_LPLL_CON0.

>   #define RK3576_CCI_CLKSEL_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x300)
>   #define RK3576_CCI_CLKGATE_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x800)
>   #define RK3576_CCI_SOFTRST_CON(x)	((x) * 0x4 + RK3576_CCI_CRU_BASE + 0xa00)
>   
> -#define RK3576_BPLL_CON(x)		((x) * 0x4 + RK3576_BIGCORE_CRU_BASE)
>   #define RK3576_BIGCORE_CLKSEL_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x300)
>   #define RK3576_BIGCORE_CLKGATE_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x800)
>   #define RK3576_BIGCORE_SOFTRST_CON(x)	((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0xa00)
> -#define RK3576_LPLL_CON(x)		((x) * 0x4 + RK3576_CCI_CRU_BASE)
>   #define RK3576_LITCORE_CLKSEL_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x300)
>   #define RK3576_LITCORE_CLKGATE_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x800)
>   #define RK3576_LITCORE_SOFTRST_CON(x)	((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0xa00)
> diff --git a/drivers/clk/rockchip/clk_rk3576.c b/drivers/clk/rockchip/clk_rk3576.c
> index 92bde425b0ee..75b705ffba2f 100644
> --- a/drivers/clk/rockchip/clk_rk3576.c
> +++ b/drivers/clk/rockchip/clk_rk3576.c
> @@ -44,19 +44,18 @@ static struct rockchip_pll_rate_table rk3576_24m_pll_rates[] = {
>   
>   static struct rockchip_pll_clock rk3576_pll_clks[] = {
>   	[BPLL] = PLL(pll_rk3588, PLL_BPLL, RK3576_PLL_CON(0),
> -		      RK3576_BPLL_MODE_CON0, 0, 15, 0,
> -		      rk3576_24m_pll_rates),
> +		     RK3576_BPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates),
>   	[LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3576_LPLL_CON(16),
>   		     RK3576_LPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates),
>   	[VPLL] = PLL(pll_rk3588, PLL_VPLL, RK3576_PLL_CON(88),
> -		      RK3576_LPLL_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates),
> +		     RK3576_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates),
>   	[AUPLL] = PLL(pll_rk3588, PLL_AUPLL, RK3576_PLL_CON(96),
>   		      RK3576_MODE_CON0, 6, 15, 0, rk3576_24m_pll_rates),
>   	[CPLL] = PLL(pll_rk3588, PLL_CPLL, RK3576_PLL_CON(104),
>   		     RK3576_MODE_CON0, 8, 15, 0, rk3576_24m_pll_rates),
>   	[GPLL] = PLL(pll_rk3588, PLL_GPLL, RK3576_PLL_CON(112),
>   		     RK3576_MODE_CON0, 2, 15, 0, rk3576_24m_pll_rates),
> -	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PMU_PLL_CON(128),
> +	[PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PHP_PLL_CON(128),
>   		     RK3576_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE,

RK3576_MODE_CON0 is incorrect here, but since ROCKCHIP_PLL_FIXED_MODE is 
set, this won't be used as far as I could tell. I'm wondering whether we 
should have a new macros that wouldn't force us to define something 
necessarily incorrect. Something for later though.

Cheers,
Quentin

[PATCH 12/15] clk: rockchip: rk3528: Fix trivial clock configuration errors

[Jonas Karlman]

The RK3528 clock driver has a few trivial copy-paste mistakes in its
clock handling.

Fix the trivial clock configuration errors:
- avoid reporting incorrect PWM clock rate
- use correct mask and value for VOP clocks
- align PPLL configuration to match other PLLs

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_rk3528.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3528.c b/drivers/clk/rockchip/clk_rk3528.c
index cf8c3a623495..83e302dbc3c4 100644
--- a/drivers/clk/rockchip/clk_rk3528.c
+++ b/drivers/clk/rockchip/clk_rk3528.c
@@ -72,7 +72,8 @@ static struct rockchip_pll_clock rk3528_pll_clks[] = {
 		     RK3528_MODE_CON, 4, 10, 0, rk3528_pll_rates),
 
 	[PPLL] = PLL(pll_rk3328, PLL_PPLL, RK3528_PCIE_PLL_CON(32),
-		     RK3528_MODE_CON, 6, 10, ROCKCHIP_PLL_FIXED_MODE, rk3528_pll_rates),
+		     RK3528_MODE_CON, 6, 10, ROCKCHIP_PLL_FIXED_MODE,
+		     rk3528_pll_rates),
 
 	[DPLL] = PLL(pll_rk3328, PLL_DPLL, RK3528_DDRPHY_PLL_CON(16),
 		     RK3528_DDRPHY_MODE_CON, 0, 10, 0, rk3528_pll_rates),
@@ -770,7 +771,7 @@ static ulong rk3528_pwm_get_clk(struct rk3528_clk_priv *priv, ulong clk_id)
 	sel = (con & mask) >> shift;
 	if (sel == CLK_PWM0_SEL_CLK_MATRIX_100M_SRC)
 		rate = 100 * MHz;
-	if (sel == CLK_PWM0_SEL_CLK_MATRIX_50M_SRC)
+	else if (sel == CLK_PWM0_SEL_CLK_MATRIX_50M_SRC)
 		rate = 50 * MHz;
 	else
 		rate = OSC_HZ;
@@ -1097,7 +1098,7 @@ static ulong rk3528_dclk_vop_set_clk(struct rk3528_clk_priv *priv,
 	}
 
 	div = ((DIV_ROUND_UP(prate, rate) - 1) << div_shift) & div_mask;
-	rk_clrsetreg(&cru->clksel_con[id], sel, div);
+	rk_clrsetreg(&cru->clksel_con[id], sel_mask | div_mask, sel | div);
 
 	return rk3528_dclk_vop_get_clk(priv, clk_id);
 }
-- 
2.54.0

Re: [PATCH 12/15] clk: rockchip: rk3528: Fix trivial clock configuration errors

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:13 PM, Jonas Karlman wrote:
> The RK3528 clock driver has a few trivial copy-paste mistakes in its
> clock handling.
> 
> Fix the trivial clock configuration errors:
> - avoid reporting incorrect PWM clock rate
> - use correct mask and value for VOP clocks
> - align PPLL configuration to match other PLLs
> 

Individual commits please.

Fixes: 5a7a856b132f ("clk: rockchip: Add support for RK3528")

Once split, you can add my:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

for each commit.

Thanks!
Quentin

[PATCH 13/15] clk: rockchip: rk3506: Fix trivial clock configuration errors

[Jonas Karlman]

The RK3506 clock driver has a few trivial copy-paste mistakes in its
clock handling.

Fix the trivial clock configuration error:
- stop CLK_I2Cx from falling through into CLK_I2C2

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/rockchip/clk_rk3506.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3506.c b/drivers/clk/rockchip/clk_rk3506.c
index 38066c5c3e3c..457082eea87b 100644
--- a/drivers/clk/rockchip/clk_rk3506.c
+++ b/drivers/clk/rockchip/clk_rk3506.c
@@ -537,10 +537,12 @@ static ulong rk3506_i2c_get_rate(struct rk3506_clk_priv *priv, ulong clk_id)
 		con = readl(RK3506_CLKSEL_CON(32));
 		sel = FIELD_GET(CLK_I2C0_SEL_MASK, con);
 		div = FIELD_GET(CLK_I2C0_DIV_MASK, con);
+		break;
 	case CLK_I2C1:
 		con = readl(RK3506_CLKSEL_CON(32));
 		sel = FIELD_GET(CLK_I2C1_SEL_MASK, con);
 		div = FIELD_GET(CLK_I2C1_DIV_MASK, con);
+		break;
 	case CLK_I2C2:
 		con = readl(RK3506_CLKSEL_CON(33));
 		sel = FIELD_GET(CLK_I2C2_SEL_MASK, con);
-- 
2.54.0

Re: [PATCH 13/15] clk: rockchip: rk3506: Fix trivial clock configuration errors

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:13 PM, Jonas Karlman wrote:
> The RK3506 clock driver has a few trivial copy-paste mistakes in its
> clock handling.
> 
> Fix the trivial clock configuration error:
> - stop CLK_I2Cx from falling through into CLK_I2C2
> 

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Fixes: fbf72dce915a ("clk: rockchip: Add support for RK3506")

Thanks!
Quentin

[PATCH 14/15] clk: rockchip: rk3568: Drop unused GRF syscon lookup

[Jonas Karlman]

The RK3568 clock driver lookup and stores a reference to the GRF regs.
However, the grf field is never dereferenced, let's drop it.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 arch/arm/include/asm/arch-rockchip/cru_rk3568.h | 1 -
 drivers/clk/rockchip/clk_rk3568.c               | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3568.h b/arch/arm/include/asm/arch-rockchip/cru_rk3568.h
index 9c7ddd751f72..d72f241d0150 100644
--- a/arch/arm/include/asm/arch-rockchip/cru_rk3568.h
+++ b/arch/arm/include/asm/arch-rockchip/cru_rk3568.h
@@ -44,7 +44,6 @@ struct rk3568_pmuclk_priv {
 
 struct rk3568_clk_priv {
 	struct rk3568_cru *cru;
-	struct rk3568_grf *grf;
 	ulong ppll_hz;
 	ulong hpll_hz;
 	ulong gpll_hz;
diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
index 940b824103cc..9f3e9322161e 100644
--- a/drivers/clk/rockchip/clk_rk3568.c
+++ b/drivers/clk/rockchip/clk_rk3568.c
@@ -2907,10 +2907,6 @@ static int rk3568_clk_probe(struct udevice *dev)
 	struct rk3568_clk_priv *priv = dev_get_priv(dev);
 	int ret;
 
-	priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
-	if (IS_ERR(priv->grf))
-		return PTR_ERR(priv->grf);
-
 	rk3568_clk_init(priv);
 
 	/* Process 'assigned-{clocks/clock-parents/clock-rates}' properties */
-- 
2.54.0

Re: [PATCH 14/15] clk: rockchip: rk3568: Drop unused GRF syscon lookup

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:13 PM, Jonas Karlman wrote:
> The RK3568 clock driver lookup and stores a reference to the GRF regs.
> However, the grf field is never dereferenced, let's drop it.
> 

The kernel has the clk_ddr1x and pclk_edpphy_grf using the GRF I 
believe. Those are unlikely to be supported in U-Boot I guess. We can 
always add them back anyways, so:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

[PATCH 15/15] clk: rockchip: rk3588: Drop unused GRF syscon lookup

[Jonas Karlman]

The RK3588 clock driver lookup and stores a reference to the GRF regs.
However, the grf field is never dereferenced and struct rk3588_grf is
not defined in any header, let's drop it.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 arch/arm/include/asm/arch-rockchip/cru_rk3588.h | 1 -
 drivers/clk/rockchip/clk_rk3588.c               | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
index 39295adeb38a..85cc17840436 100644
--- a/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
+++ b/arch/arm/include/asm/arch-rockchip/cru_rk3588.h
@@ -41,7 +41,6 @@ struct rk3588_clk_info {
 
 struct rk3588_clk_priv {
 	struct rk3588_cru *cru;
-	struct rk3588_grf *grf;
 	ulong ppll_hz;
 	ulong gpll_hz;
 	ulong cpll_hz;
diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c
index 1006102f4cf4..5236c3fa985e 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -1977,10 +1977,6 @@ static int rk3588_clk_probe(struct udevice *dev)
 	}
 #endif
 
-	priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
-	if (IS_ERR(priv->grf))
-		return PTR_ERR(priv->grf);
-
 	rk3588_clk_init(priv);
 
 	/* Process 'assigned-{clocks/clock-parents/clock-rates}' properties */
-- 
2.54.0

Re: [PATCH 15/15] clk: rockchip: rk3588: Drop unused GRF syscon lookup

[Quentin Schulz]

Hi Jonas,

On 7/30/26 4:13 PM, Jonas Karlman wrote:
> The RK3588 clock driver lookup and stores a reference to the GRF regs.
> However, the grf field is never dereferenced and struct rk3588_grf is
> not defined in any header, let's drop it.
> 

In the Linux kernel, they seem to be referencing the GRF for I2S*, 
PCLK_VO[01]GRF. We may eventually need the latter for video output in 
U-Boot but it can always be added again once we actually make use of it, so:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

Re: [PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes

[Simon Glass]

Hi Jonas,

On Thu, 30 Jul 2026 at 08:13, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> This series fixes a few trivial copy-paste mistakes and other misc clock
> issues for RK35xx SoCs.
>
> Patch 1-3 fixes minor and very unlikely issues for plls
> Patch 4 fixes PPLL mode handling on RK3588/RK3576
> Patch 5 disable 'special pll handling' for RK3576 plls
> Patch 6-7,11-13 fixes trivial clock configuration errors
> Patch 8-9 fixes less trivial clock configuration errors
> Patch 14-15 remove unused GRF syscon lookups
>
> Some of these fixes has only been compile tested as it involves clocks
> or rates that is rarely used in U-Boot.
>
> The 'special pll handling' should ideally be moved into clk_rk3588
> driver, something that can be done in a future follow-up series.
>
> Jonas Karlman (15):
>   clk: rockchip: pll: Fix double use of postdiv1
>   clk: rockchip: pll: Always write the dsmpd flag
>   clk: rockchip: pll: Always write the k param
>   clk: rockchip: pll: Use PLL_FIXED_MODE flag on rk3588/rk3576 plls
>   clk: rockchip: pll: Limit special rk3588_pll handling to RK3588
>   clk: rockchip: rk3568: Fix trivial clock configuration errors
>   clk: rockchip: rk3588: Fix trivial clock configuration errors
>   clk: rockchip: rk3588: Fix possible divide by zero
>   clk: rockchip: rk3588: Fix ACLK_BUS_ROOT rate set during probe
>   clk: rockchip: rk3588: Use SPLL_HZ constant
>   clk: rockchip: rk3576: Fix trivial clock configuration errors
>   clk: rockchip: rk3528: Fix trivial clock configuration errors
>   clk: rockchip: rk3506: Fix trivial clock configuration errors
>   clk: rockchip: rk3568: Drop unused GRF syscon lookup
>   clk: rockchip: rk3588: Drop unused GRF syscon lookup
>
>  .../include/asm/arch-rockchip/cru_rk3568.h    |  1 -
>  .../include/asm/arch-rockchip/cru_rk3576.h    |  5 +-
>  .../include/asm/arch-rockchip/cru_rk3588.h    |  6 +-
>  drivers/clk/rockchip/clk_pll.c                | 56 ++++++++++---------
>  drivers/clk/rockchip/clk_rk3506.c             |  2 +
>  drivers/clk/rockchip/clk_rk3528.c             |  7 ++-
>  drivers/clk/rockchip/clk_rk3568.c             | 12 ++--
>  drivers/clk/rockchip/clk_rk3576.c             | 26 ++++-----
>  drivers/clk/rockchip/clk_rk3588.c             | 39 ++++++-------
>  9 files changed, 72 insertions(+), 82 deletions(-)
>
> --
> 2.54.0
>

Just to note that Quentin has done a very thorough review and I didn't
see anything else to mention, so I'm not going to send review tags on
this one.

Regards,
Simon