On 8/12/26 12:40 PM, Quentin Schulz wrote:
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 <[email protected]>
---
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.
Is any of those two clocks externally provided by any chance? Heiko told
me the 32K clock is generally externally provided, usually by an RTC,
and thus I believe we really shouldn't allow those clocks to be selected
if they indeed are external. I don't think we have a mechanism in place
to enable the external clock and make sure it's running at the expected
clock rate. So maybe the answer is to actually remove support for
anything but the 24MHz clock for the SARADC (and other IPs whose parent
clocks can be clk_rc and/or clk_deepslow).
Cheers,
Quentin
