Skip to contentSkip to conversation

clk: handle fixed-rate clocks correctly in clk_core_round_rate_nolock

PATCH In review Heiko Stübner 4 participants 4 messages Jul 25, 2015 – Jul 28, 2015 Common clk

lore
Share & tools
lore

Heiko Stübner

author
#0 lore raw

PATCH 1/1 clk: handle fixed-rate clocks correctly in clk_core_round_rate_nolock

Commit 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype") changed the behaviour of clk_core_round_rate_nolock as it forgot to also include the else conditional simply returning the clock rate for clocks that neither have a parent or can determine their rate - for example said fixed clocks.

This resulted in failures to set pll rates on rockchip socs, as it returned the target pll rate as suitable rate for the 24MHz xin24m clock, thus making the ccf want to set this fixed clock to 1.6GHz or similar.

Fixes 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype")
Reported-by Romain Perier <[email protected]>
Signed-off-by Heiko Stuebner <[email protected]>
---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/clk/clk.c +2 -0
index 67f778b..819ffa6 100644
@@ -802,6 +802,8 @@ static int clk_core_round_rate_nolock(struct clk_core *core,
 		req->rate = rate;
 	} else if (core->flags & CLK_SET_RATE_PARENT) {
 		return clk_core_round_rate_nolock(parent, req);
+	} else {
+		req->rate = core->rate;
 	}
 
 	return 0;
Signature · 1 line
2.1.4
Acked-by on v1 1/1
#1 lore raw

Hi Heiko,

Quoted 5 lines · On Sun, 26 Jul 2015 00:18:40 +0200 Heiko St=C3=BCbner <[email protected]> wrote:

Commit 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype") changed the behaviour of clk_core_round_rate_nolock as it forgot to also include the else conditional simply returning the clock rate for clocks that neither have a parent or can determine their rate - for example said fixed clocks.

Indeed.

Quoted 8 lines

=20
This resulted in failures to set pll rates on rockchip socs, as it returned the target pll rate as suitable rate for the 24MHz xin24m clock, thus making the ccf want to set this fixed clock to 1.6GHz or similar. =20

Fixes 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype")
Reported-by Romain Perier <[email protected]>
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Boris Brezillon <[email protected]>

Thanks,

Boris

drivers/clk/clk.c static int clk_core_round_rate_nolock(struct clk_core=

Quoted 9 lines
---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)

=20

drivers/clk/clk.c +0 -0
index 67f778b..819ffa6 100644
@@ -802,6 +802,8 @@ static int clk_core_round_rate_nolock(struct clk_core=

*core,

Quoted 8 lines
drivers/clk/clk.c +2 -0
 		req->rate =3D rate;
 	} else if (core->flags & CLK_SET_RATE_PARENT) {
 		return clk_core_round_rate_nolock(parent, req);
+	} else {
+		req->rate =3D core->rate;
 	}

=20
return 0;

--=20
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Tested-by
#2 lore raw

Hi all,

A side effect produced by this issue was cpufreq complaining about incorrect clk rate when it tried to change cpu frequency (on cpu0). It is fixed now.

Tested-by Romain Perier <[email protected]>

2015-07-26 9:27 GMT+02:00 Boris Brezillon <[email protected]= om>:

Quoted 6 lines

Hi Heiko,

Quoted 1 line · On Sun, 26 Jul 2015 00:18:40 +0200 Heiko St=C3=BCbner <[email protected]> wrote:

Commit 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype"=

)

Quoted 10 lines
Quoted 4 lines

changed the behaviour of clk_core_round_rate_nolock as it forgot to also include the else conditional simply returning the clock rate for clocks that neither have a parent or can determine their rate - for example said fixed clocks.

Indeed.

Quoted 3 lines

This resulted in failures to set pll rates on rockchip socs, as it returned the target pll rate as suitable rate for the 24MHz xin24m clock=

,

Quoted 3 lines
Quoted 3 lines

thus making the ccf want to set this fixed clock to 1.6GHz or similar.

Fixes 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype"=

)

Quoted 13 lines
Quoted 2 lines
Reported-by Romain Perier <[email protected]>
Signed-off-by Heiko Stuebner <[email protected]>
Acked-by Boris Brezillon <[email protected]>

Thanks,

Boris

Quoted 4 lines
---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)
Quoted 5 lines

drivers/clk/clk.c static int clk_core_round_rate_nolock(struct clk_cor=

Quoted 5 lines
drivers/clk/clk.c +0 -0
index 67f778b..819ffa6 100644
@@ -802,6 +802,8 @@ static int clk_core_round_rate_nolock(struct clk_cor=

e *core,

Quoted 15 lines

drivers/clk/clk.c static int clk_core_round_rate_nolock(struct clk_core *core, · line 809

Quoted 8 lines
drivers/clk/clk.c +2 -0
              req->rate =3D rate;
      } else if (core->flags & CLK_SET_RATE_PARENT) {
              return clk_core_round_rate_nolock(parent, req);
+     } else {
+             req->rate =3D core->rate;
      }
 
      return 0;

--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

#3 lore raw
In reply to Heiko Stübner
Quoted 6 lines · Heiko Stübner

Commit 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype") changed the behaviour of clk_core_round_rate_nolock as it forgot to also include the else conditional simply returning the clock rate for clocks that neither have a parent or can determine their rate - for example said fixed clocks.

Quoted 8 lines

This resulted in failures to set pll rates on rockchip socs, as it returned the target pll rate as suitable rate for the 24MHz xin24m clock, thus making the ccf want to set this fixed clock to 1.6GHz or similar.

Fixes 6edc753d8ecc ("clk: change clk_ops' ->determine_rate() prototype")
Reported-by Romain Perier <[email protected]>
Signed-off-by Heiko Stuebner <[email protected]>

---

Thanks I folded this into the original patch and pushed it out to -next.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project