Thread (14 messages) 14 messages, 4 authors, 2014-02-28

[PATCH 1/5] clk: sun6i: Protect CPU clock

From: Russell King - ARM Linux <hidden>
Date: 2014-02-24 16:30:54
Also in: linux-devicetree, lkml

On Mon, Feb 24, 2014 at 05:22:43PM +0100, Maxime Ripard wrote:
quoted hunk ↗ jump to hunk
Right now, AHB is an indirect child clock of the CPU clock. If that happens to
change, since the CPU clock has no other consumers declared in Linux, it would
be shut down, which is not really a good idea.

Prevent this by forcing it enabled.

Signed-off-by: Maxime Ripard <redacted>
---
 drivers/clk/sunxi/clk-sunxi.c | 8 ++++++++
 1 file changed, 8 insertions(+)
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 23baad9..cedaf4b 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1301,6 +1301,14 @@ static void __init sunxi_clock_protect(void)
 		clk_prepare_enable(clk);
 		clk_put(clk);
 	}
+
+	/* CPU clocks - sun6i */
+	clk = clk_get(NULL, "cpu");
+	if (!IS_ERR(clk)) {
+		clk_prepare_enable(clk);
+		clk_put(clk);
+	}
This is broken.  I'm not sure what's difficult to grasp about the concept
of "while a clock is in use, you should keep a reference to that clock".

That implies that if you get a clock, and then enable it, you don't
put the clock until you've disabled it.

The only reason the core doesn't check for this kind of thing is that
a clock may be shared, so it's entirely possible for a correctly written
driver to have a clock which is still enabled at put time - but enabled
by an entirely different driver.

However, that's no excuse for this kind of sloppiness.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help