Re: [PATCH 4/6] clk: rockchip: rk3506: Fix CLK_SARADC set rate issues

[Quentin Schulz via U-Boot]

Hi Jonas,

On 8/3/26 9:09 PM, Jonas Karlman wrote:
The set_rate ops for the CLK_SARADC clock in RK3506 clock driver
force use of 32 KHz or 400 KHz rates when any requested rate is
divisible with 32 KHz or 400 KHz.
Adjust logic to better match the 32 KHz, 400 KHz or 1.5-24 GHz rate
limitation of the CLK_SARADC clock in RK3506.


Same typo as previous patch, it's MHz and not GHz. Same remark as 
previous patch, the divider applies to any of the parent clock so the 
1.5-24MHz is kinda misleading.

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

diff --git a/drivers/clk/rockchip/clk_rk3506.c 
b/drivers/clk/rockchip/clk_rk3506.c
index 457082eea87b..e156bf19a6b4 100644
--- a/drivers/clk/rockchip/clk_rk3506.c
+++ b/drivers/clk/rockchip/clk_rk3506.c
@@ -440,22 +440,19 @@ static ulong rk3506_sdmmc_set_rate(struct rk3506_clk_priv 
*priv, ulong clk_id,
  static ulong rk3506_saradc_get_rate(struct rk3506_clk_priv *priv, ulong 
clk_id)
  {
        u32 con, div, sel;
-       ulong prate;
  
  	con = readl(RK3506_CLKSEL_CON(54));
        sel = FIELD_GET(CLK_SARADC_SEL_MASK, con);
        div = FIELD_GET(CLK_SARADC_DIV_MASK, con);
  
  	if (sel == CLK_SARADC_SEL_24M)
-               prate = OSC_HZ;
+               return DIV_TO_RATE(OSC_HZ, div);
        else if (sel == CLK_SARADC_SEL_400K)
-               prate = 400000;
+               return 400000;
        else if (sel == CLK_SARADC_SEL_32K)
-               prate = 32000;
+               return 32000;
        else
                return -EINVAL;
-
-       return DIV_TO_RATE(prate, div);

I'm not sure this is correct. The divider applies to any parent clock 
and this applies the divider only to the 24MHz parent clock.

  }
  
  static ulong rk3506_saradc_set_rate(struct rk3506_clk_priv *priv, ulong clk_id,
@@ -463,10 +460,10 @@ static ulong rk3506_saradc_set_rate(struct 
rk3506_clk_priv *priv, ulong clk_id,
  {
        u32 div, sel;
  
-	if (32000 % rate == 0) {
+       if (rate <= 32768) {
                sel = CLK_SARADC_SEL_32K;
                div = 1;
-       } else if (400000 % rate == 0) {
+       } else if (rate == 400000) {
                sel = CLK_SARADC_SEL_400K;
                div = 1;

This is also not correct.

I think what you want instead is to modify div to not be always 1 but 
rather 32000 / rate and 4000000 / rate.

Cheers,
Quentin