diff mbox

[05/14] clkdev: use clk_hw internally

Message ID E1Ye59E-0001BB-7z@rmk-PC.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King April 3, 2015, 5:12 p.m. UTC 
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/clk/clkdev.c   | 24 ++++++++++++++++--------
 include/linux/clkdev.h |  1 +
 2 files changed, 17 insertions(+), 8 deletions(-)

Comments

Stephen Boyd April 6, 2015, 8:18 p.m. UTC | #1 
On 04/03/15 10:12, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Some commit text would be nice for us 5 years from now when we wonder
why this patch was applied.

I suspect the commit text would go like this:

clk_add_alias() calls clk_get() followed by clk_put() but in between
those two calls it saves away the struct clk pointer to a clk_lookup
structure. This leaves the 'clk' member of the clk_lookup pointing at
freed memory on configurations where CONFIG_COMMON_CLK=y. This is a
problem because clk_get_sys() will eventually try to dereference the
freed pointer by calling __clk_get_hw() on it. Fix this by saving away
the struct clk_hw pointer instead of the struct clk pointer so that when
we try to create a per-user struct clk in clk_get_sys() we don't
dereference a junk pointer.

Now the question is does any of this matter for the 4.0 release. From
what I can tell, the answer is no.

$ git grep 'clk_add_alias' v4.0-rc6
v4.0-rc6:arch/arm/mach-davinci/da850.c:         clk_add_alias("async", da850_cpufreq_device.name,
v4.0-rc6:arch/arm/mach-omap1/board-nokia770.c:  clk_add_alias("hwa_sys_ck", NULL, "bclk", NULL);
v4.0-rc6:arch/arm/mach-pxa/eseries.c:   clk_add_alias("CLK_CK48M", e740_t7l66xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/eseries.c:   clk_add_alias("CLK_CK3P6MI", e750_tc6393xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/eseries.c:   clk_add_alias("CLK_CK3P6MI", e800_tc6393xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/lubbock.c:   clk_add_alias("SA1111_CLK", NULL, "GPIO11_CLK", NULL);
v4.0-rc6:arch/arm/mach-pxa/tosa.c:      clk_add_alias("CLK_CK3P6MI", tc6393xb_device.name, "GPIO11_CLK", NULL);
v4.0-rc6:arch/mips/alchemy/common/clock.c:                      clk_add_alias(t->alias, NULL, t->base, NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("wdt", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c:       clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-tmu-sh3.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-tmu.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-tmu.1", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-tmu.2", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-mtu2", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-cmt-16.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("fck", "sh-cmt-32.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c:        clk_add_alias("sci_ick", NULL, "peripheral_clk", NULL);

All of these architectures and platforms have CONFIG_COMMON_CLK=n, so
there doesn't seem to be any regression that these patches are fixing.
That isn't to say the patches are bad, just that they aren't urgent for
the upcoming release.
diff mbox

Patch

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 5d7746d19445..8e676eafc823 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -205,7 +205,7 @@  struct clk *clk_get_sys(const char *dev_id, const char *con_id)
 	if (!cl)
 		goto out;
 
-	clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id);
+	clk = __clk_create_clk(cl->clk_hw, dev_id, con_id);
 	if (IS_ERR(clk))
 		goto out;
 
@@ -243,18 +243,26 @@  void clk_put(struct clk *clk)
 }
 EXPORT_SYMBOL(clk_put);
 
-void clkdev_add(struct clk_lookup *cl)
+static void __clkdev_add(struct clk_lookup *cl)
 {
 	mutex_lock(&clocks_mutex);
 	list_add_tail(&cl->node, &clocks);
 	mutex_unlock(&clocks_mutex);
 }
+
+void clkdev_add(struct clk_lookup *cl)
+{
+	if (!cl->clk_hw)
+		cl->clk_hw = __clk_get_hw(cl->clk);
+	__clkdev_add(cl);
+}
 EXPORT_SYMBOL(clkdev_add);
 
 void clkdev_add_table(struct clk_lookup *cl, size_t num)
 {
 	mutex_lock(&clocks_mutex);
 	while (num--) {
+		cl->clk_hw = __clk_get_hw(cl->clk);
 		list_add_tail(&cl->node, &clocks);
 		cl++;
 	}
@@ -271,7 +279,7 @@  struct clk_lookup_alloc {
 };
 
 static struct clk_lookup * __init_refok
-vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt,
+vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
 	va_list ap)
 {
 	struct clk_lookup_alloc *cla;
@@ -280,7 +288,7 @@  vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt,
 	if (!cla)
 		return NULL;
 
-	cla->cl.clk = clk;
+	cla->cl.clk_hw = hw;
 	if (con_id) {
 		strlcpy(cla->con_id, con_id, sizeof(cla->con_id));
 		cla->cl.con_id = cla->con_id;
@@ -301,7 +309,7 @@  clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...)
 	va_list ap;
 
 	va_start(ap, dev_fmt);
-	cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
+	cl = vclkdev_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap);
 	va_end(ap);
 
 	return cl;
@@ -362,7 +370,7 @@  int clk_register_clkdev(struct clk *clk, const char *con_id,
 		return PTR_ERR(clk);
 
 	va_start(ap, dev_fmt);
-	cl = vclkdev_alloc(clk, con_id, dev_fmt, ap);
+	cl = vclkdev_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap);
 	va_end(ap);
 
 	if (!cl)
@@ -393,8 +401,8 @@  int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num)
 		return PTR_ERR(clk);
 
 	for (i = 0; i < num; i++, cl++) {
-		cl->clk = clk;
-		clkdev_add(cl);
+		cl->clk_hw = __clk_get_hw(clk);
+		__clkdev_add(cl);
 	}
 
 	return 0;
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index eb9b38a79b43..cd93b215e3af 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -22,6 +22,7 @@  struct clk_lookup {
 	const char		*dev_id;
 	const char		*con_id;
 	struct clk		*clk;
+	struct clk_hw		*clk_hw;
 };
 
 #define CLKDEV_INIT(d, n, c)	\