Hi Jeremy,
This is latest one for review. I add some minor changes in.
Cheers Yong
From efe7fa8bea67f9bf532c0074a92d938e6d6f4c5d Mon Sep 17 00:00:00 2001
From: Yong Shen yong.shen@linaro.org Date: Thu, 18 Nov 2010 14:54:49 +0800 Subject: [PATCH] export clock debug information to user space
create a tree-like directory structure in debugfs so user space tools like powerdebug can generate readable clock information. more functions tend to be add in, like individual clock enable/disable by writing to this debug interface.
Signed-off-by: Yong Shen yong.shen@linaro.org --- arch/arm/common/Kconfig | 6 ++ arch/arm/common/clkdev.c | 3 + include/linux/clk.h | 18 +++++++ kernel/clk.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 0 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig index 0a34c81..13d7cf9 100644 --- a/arch/arm/common/Kconfig +++ b/arch/arm/common/Kconfig @@ -41,3 +41,9 @@ config SHARP_SCOOP config COMMON_CLKDEV bool select HAVE_CLK + +config CLK_DEBUG + bool "clock debug information export to user space" + depends on USE_COMMON_STRUCT_CLK && PM_DEBUG && DEBUG_FS + help + export clk debug information to user space diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c index 9e4c4d9..1d08fb3 100644 --- a/arch/arm/common/clkdev.c +++ b/arch/arm/common/clkdev.c @@ -19,6 +19,7 @@ #include <linux/mutex.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/debugfs.h>
#include <asm/clkdev.h>
@@ -104,6 +105,7 @@ EXPORT_SYMBOL(clk_put);
void clkdev_add(struct clk_lookup *cl) { + clk_debug_register(cl->clk); mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); @@ -114,6 +116,7 @@ void __init clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) { + clk_debug_register(cl->clk); list_add_tail(&cl->node, &clocks); cl++; } diff --git a/include/linux/clk.h b/include/linux/clk.h index 56416b7..4aaddea 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -48,12 +48,24 @@ struct clk { const struct clk_ops *ops; unsigned int enable_count; struct mutex mutex; +#ifdef CONFIG_CLK_DEBUG +#define CLK_NAME_LEN 32 + char name[CLK_NAME_LEN]; + struct dentry *dentry; +#endif };
+#ifdef CONFIG_CLK_DEBUG +#define __INIT_CLK_DEBUG(n) .name = #n, +#else +#define __INIT_CLK_DEBUG(n) +#endif + #define INIT_CLK(name, o) { \ .ops = &o, \ .enable_count = 0, \ .mutex = __MUTEX_INITIALIZER(name.mutex), \ + __INIT_CLK_DEBUG(name) \ }
struct clk_ops { @@ -245,4 +257,10 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id); int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, struct device *dev);
+#ifdef CONFIG_CLK_DEBUG +void clk_debug_register(struct clk *clk); +#else +static inline void clk_debug_register(struct clk *clk) {} +#endif + #endif diff --git a/kernel/clk.c b/kernel/clk.c index 32f25ef..7f8bea9 100644 --- a/kernel/clk.c +++ b/kernel/clk.c @@ -11,6 +11,8 @@ #include <linux/clk.h> #include <linux/mutex.h> #include <linux/module.h> +#include <linux/slab.h> +#include <linux/debugfs.h>
int clk_enable(struct clk *clk) { @@ -113,3 +115,122 @@ struct clk_ops clk_fixed_ops = { .get_rate = clk_fixed_get_rate, }; EXPORT_SYMBOL_GPL(clk_fixed_ops); + +#ifdef CONFIG_CLK_DEBUG +/* + * debugfs support to trace clock tree hierarchy and attributes + */ +static int clk_debug_rate_get(void *data, u64 *val) +{ + struct clk *clk = data; + + *val = (u64)clk_get_rate(clk); + return 0; +} +DEFINE_SIMPLE_ATTRIBUTE(clk_debug_rate_fops, clk_debug_rate_get, NULL, + "%llu\n"); + + +static struct dentry *clk_root; +static int clk_debug_register_one(struct clk *clk) +{ + int err; + struct dentry *d, *child, *child_tmp; + struct clk *pa = clk_get_parent(clk); + + if (pa && !IS_ERR(pa)) + d = debugfs_create_dir(clk->name, pa->dentry); + else { + if (!clk_root) + clk_root = debugfs_create_dir("clocks", NULL); + if (!clk_root) + return -ENOMEM; + d = debugfs_create_dir(clk->name, clk_root); + } + + if (!d) + return -ENOMEM; + + clk->dentry = d; + + d = debugfs_create_u32("enable_count", S_IRUGO, clk->dentry, + (u32 *)&clk->enable_count); + if (!d) { + err = -ENOMEM; + goto err_out; + } + + d = debugfs_create_file("rate", S_IRUGO, clk->dentry, (void *)clk, + &clk_debug_rate_fops); + if (!d) { + err = -ENOMEM; + goto err_out; + } + + return 0; + +err_out: + d = clk->dentry; + list_for_each_entry_safe(child, child_tmp, &d->d_subdirs, d_u.d_child) + debugfs_remove(child); + debugfs_remove(clk->dentry); + return err; +} + +struct preinit_clk { + struct list_head list; + struct clk *clk; +}; +static LIST_HEAD(preinit_clks); +static DEFINE_MUTEX(preinit_lock); +static int init_done; + +void clk_debug_register(struct clk *clk) +{ + int err; + struct clk *pa; + + if (init_done) { + pa = clk_get_parent(clk); + + if (pa && !IS_ERR(pa) && !pa->dentry) + clk_debug_register(pa); + + if (!clk->dentry) { + err = clk_debug_register_one(clk); + if (err) + return; + } + } else { + struct preinit_clk *p; + mutex_lock(&preinit_lock); + p = kmalloc(sizeof(*p), GFP_KERNEL); + if (!p) + goto unlock; + p->clk = clk; + list_add(&p->list, &preinit_clks); +unlock: + mutex_unlock(&preinit_lock); + } +} +EXPORT_SYMBOL_GPL(clk_debug_register); + +static int __init clk_debugfs_init(void) +{ + struct preinit_clk *pclk, *tmp; + + if (debugfs_initialized()) + init_done = 1; + + list_for_each_entry(pclk, &preinit_clks, list) { + clk_debug_register(pclk->clk); + } + + list_for_each_entry_safe(pclk, tmp, &preinit_clks, list) { + list_del(&pclk->list); + kfree(pclk); + } + return 0; +} +late_initcall(clk_debugfs_init); +#endif
On Fri, Dec 3, 2010 at 1:30 PM, Yong Shen yong.shen@linaro.org wrote
Hi Jeremy,
This is latest one for review. I add some minor changes in.
Cheers Yong
Yong,
It helps reviewers if you version your patches (PATCHv1, PATCHv2, etc.) and you list the changes made in each version in the changelog.
See http://kerneltrap.org/mailarchive/linux-kernel/2010/9/27/4624656/thread for a random example.
Regards, Amit
From efe7fa8bea67f9bf532c0074a92d938e6d6f4c5d Mon Sep 17 00:00:00 2001 From: Yong Shen yong.shen@linaro.org Date: Thu, 18 Nov 2010 14:54:49 +0800 Subject: [PATCH] export clock debug information to user space
create a tree-like directory structure in debugfs so user space tools like powerdebug can generate readable clock information. more functions tend to be add in, like individual clock enable/disable by writing to this debug interface.
Signed-off-by: Yong Shen yong.shen@linaro.org
arch/arm/common/Kconfig | 6 ++ arch/arm/common/clkdev.c | 3 + include/linux/clk.h | 18 +++++++ kernel/clk.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 0 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig index 0a34c81..13d7cf9 100644 --- a/arch/arm/common/Kconfig +++ b/arch/arm/common/Kconfig @@ -41,3 +41,9 @@ config SHARP_SCOOP config COMMON_CLKDEV bool select HAVE_CLK
+config CLK_DEBUG
- bool "clock debug information export to user space"
- depends on USE_COMMON_STRUCT_CLK && PM_DEBUG && DEBUG_FS
- help
- export clk debug information to user space
diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c index 9e4c4d9..1d08fb3 100644 --- a/arch/arm/common/clkdev.c +++ b/arch/arm/common/clkdev.c @@ -19,6 +19,7 @@ #include <linux/mutex.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/debugfs.h>
#include <asm/clkdev.h>
@@ -104,6 +105,7 @@ EXPORT_SYMBOL(clk_put);
void clkdev_add(struct clk_lookup *cl) {
- clk_debug_register(cl->clk);
mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); @@ -114,6 +116,7 @@ void __init clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) {
- clk_debug_register(cl->clk);
list_add_tail(&cl->node, &clocks); cl++; } diff --git a/include/linux/clk.h b/include/linux/clk.h index 56416b7..4aaddea 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -48,12 +48,24 @@ struct clk { const struct clk_ops *ops; unsigned int enable_count; struct mutex mutex; +#ifdef CONFIG_CLK_DEBUG +#define CLK_NAME_LEN 32
- char name[CLK_NAME_LEN];
- struct dentry *dentry;
+#endif };
+#ifdef CONFIG_CLK_DEBUG +#define __INIT_CLK_DEBUG(n) .name = #n, +#else +#define __INIT_CLK_DEBUG(n) +#endif
#define INIT_CLK(name, o) { \ .ops = &o, \ .enable_count = 0, \ .mutex = __MUTEX_INITIALIZER(name.mutex), \
- __INIT_CLK_DEBUG(name) \
}
struct clk_ops { @@ -245,4 +257,10 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id); int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, struct device *dev);
+#ifdef CONFIG_CLK_DEBUG +void clk_debug_register(struct clk *clk); +#else +static inline void clk_debug_register(struct clk *clk) {} +#endif
#endif diff --git a/kernel/clk.c b/kernel/clk.c index 32f25ef..7f8bea9 100644 --- a/kernel/clk.c +++ b/kernel/clk.c @@ -11,6 +11,8 @@ #include <linux/clk.h> #include <linux/mutex.h> #include <linux/module.h> +#include <linux/slab.h> +#include <linux/debugfs.h>
int clk_enable(struct clk *clk) { @@ -113,3 +115,122 @@ struct clk_ops clk_fixed_ops = { .get_rate = clk_fixed_get_rate, }; EXPORT_SYMBOL_GPL(clk_fixed_ops);
+#ifdef CONFIG_CLK_DEBUG +/*
- debugfs support to trace clock tree hierarchy and attributes
- */
+static int clk_debug_rate_get(void *data, u64 *val) +{
- struct clk *clk = data;
- *val = (u64)clk_get_rate(clk);
- return 0;
+} +DEFINE_SIMPLE_ATTRIBUTE(clk_debug_rate_fops, clk_debug_rate_get, NULL,
- "%llu\n");
+static struct dentry *clk_root; +static int clk_debug_register_one(struct clk *clk) +{
- int err;
- struct dentry *d, *child, *child_tmp;
- struct clk *pa = clk_get_parent(clk);
- if (pa && !IS_ERR(pa))
- d = debugfs_create_dir(clk->name, pa->dentry);
- else {
- if (!clk_root)
- clk_root = debugfs_create_dir("clocks", NULL);
- if (!clk_root)
- return -ENOMEM;
- d = debugfs_create_dir(clk->name, clk_root);
- }
- if (!d)
- return -ENOMEM;
- clk->dentry = d;
- d = debugfs_create_u32("enable_count", S_IRUGO, clk->dentry,
- (u32 *)&clk->enable_count);
- if (!d) {
- err = -ENOMEM;
- goto err_out;
- }
- d = debugfs_create_file("rate", S_IRUGO, clk->dentry, (void *)clk,
- &clk_debug_rate_fops);
- if (!d) {
- err = -ENOMEM;
- goto err_out;
- }
- return 0;
+err_out:
- d = clk->dentry;
- list_for_each_entry_safe(child, child_tmp, &d->d_subdirs, d_u.d_child)
- debugfs_remove(child);
- debugfs_remove(clk->dentry);
- return err;
+}
+struct preinit_clk {
- struct list_head list;
- struct clk *clk;
+}; +static LIST_HEAD(preinit_clks); +static DEFINE_MUTEX(preinit_lock); +static int init_done;
+void clk_debug_register(struct clk *clk) +{
- int err;
- struct clk *pa;
- if (init_done) {
- pa = clk_get_parent(clk);
- if (pa && !IS_ERR(pa) && !pa->dentry)
- clk_debug_register(pa);
- if (!clk->dentry) {
- err = clk_debug_register_one(clk);
- if (err)
- return;
- }
- } else {
- struct preinit_clk *p;
- mutex_lock(&preinit_lock);
- p = kmalloc(sizeof(*p), GFP_KERNEL);
- if (!p)
- goto unlock;
- p->clk = clk;
- list_add(&p->list, &preinit_clks);
+unlock:
- mutex_unlock(&preinit_lock);
- }
+} +EXPORT_SYMBOL_GPL(clk_debug_register);
+static int __init clk_debugfs_init(void) +{
- struct preinit_clk *pclk, *tmp;
- if (debugfs_initialized())
- init_done = 1;
- list_for_each_entry(pclk, &preinit_clks, list) {
- clk_debug_register(pclk->clk);
- }
- list_for_each_entry_safe(pclk, tmp, &preinit_clks, list) {
- list_del(&pclk->list);
- kfree(pclk);
- }
- return 0;
+} +late_initcall(clk_debugfs_init);
+#endif
1.7.0.4
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Hi Amit,
That's a good reminding. About version number, I am using it in all my code reviews in different mail list now. About this patch, I planed to send out a 'v2', however it had been a while since last review, so I expect a new start. But I do need to mention the changes. Changes from last round: 1. add memory free afte clock initialization 2. Move CLK_NAME_LEN into #ifdef.
Thanks Yong
On Fri, Dec 3, 2010 at 5:06 PM, Amit Kucheria amit.kucheria@linaro.org wrote:
On Fri, Dec 3, 2010 at 1:30 PM, Yong Shen yong.shen@linaro.org wrote
Hi Jeremy,
This is latest one for review. I add some minor changes in.
Cheers Yong
Yong,
It helps reviewers if you version your patches (PATCHv1, PATCHv2, etc.) and you list the changes made in each version in the changelog.
See http://kerneltrap.org/mailarchive/linux-kernel/2010/9/27/4624656/thread for a random example.
Regards, Amit
From efe7fa8bea67f9bf532c0074a92d938e6d6f4c5d Mon Sep 17 00:00:00 2001 From: Yong Shen yong.shen@linaro.org Date: Thu, 18 Nov 2010 14:54:49 +0800 Subject: [PATCH] export clock debug information to user space
create a tree-like directory structure in debugfs so user space tools like powerdebug can generate readable clock information. more functions tend to be add in, like individual clock enable/disable by writing to this debug interface.
Signed-off-by: Yong Shen yong.shen@linaro.org
arch/arm/common/Kconfig | 6 ++ arch/arm/common/clkdev.c | 3 + include/linux/clk.h | 18 +++++++ kernel/clk.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 0 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig index 0a34c81..13d7cf9 100644 --- a/arch/arm/common/Kconfig +++ b/arch/arm/common/Kconfig @@ -41,3 +41,9 @@ config SHARP_SCOOP config COMMON_CLKDEV bool select HAVE_CLK
+config CLK_DEBUG
- bool "clock debug information export to user space"
- depends on USE_COMMON_STRUCT_CLK && PM_DEBUG && DEBUG_FS
- help
- export clk debug information to user space
diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c index 9e4c4d9..1d08fb3 100644 --- a/arch/arm/common/clkdev.c +++ b/arch/arm/common/clkdev.c @@ -19,6 +19,7 @@ #include <linux/mutex.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/debugfs.h>
#include <asm/clkdev.h>
@@ -104,6 +105,7 @@ EXPORT_SYMBOL(clk_put);
void clkdev_add(struct clk_lookup *cl) {
- clk_debug_register(cl->clk);
mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); @@ -114,6 +116,7 @@ void __init clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) {
- clk_debug_register(cl->clk);
list_add_tail(&cl->node, &clocks); cl++; } diff --git a/include/linux/clk.h b/include/linux/clk.h index 56416b7..4aaddea 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -48,12 +48,24 @@ struct clk { const struct clk_ops *ops; unsigned int enable_count; struct mutex mutex; +#ifdef CONFIG_CLK_DEBUG +#define CLK_NAME_LEN 32
- char name[CLK_NAME_LEN];
- struct dentry *dentry;
+#endif };
+#ifdef CONFIG_CLK_DEBUG +#define __INIT_CLK_DEBUG(n) .name = #n, +#else +#define __INIT_CLK_DEBUG(n) +#endif
#define INIT_CLK(name, o) { \ .ops = &o, \ .enable_count = 0, \ .mutex = __MUTEX_INITIALIZER(name.mutex), \
- __INIT_CLK_DEBUG(name) \
}
struct clk_ops { @@ -245,4 +257,10 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id); int clk_add_alias(const char *alias, const char *alias_dev_name, char *id, struct device *dev);
+#ifdef CONFIG_CLK_DEBUG +void clk_debug_register(struct clk *clk); +#else +static inline void clk_debug_register(struct clk *clk) {} +#endif
#endif diff --git a/kernel/clk.c b/kernel/clk.c index 32f25ef..7f8bea9 100644 --- a/kernel/clk.c +++ b/kernel/clk.c @@ -11,6 +11,8 @@ #include <linux/clk.h> #include <linux/mutex.h> #include <linux/module.h> +#include <linux/slab.h> +#include <linux/debugfs.h>
int clk_enable(struct clk *clk) { @@ -113,3 +115,122 @@ struct clk_ops clk_fixed_ops = { .get_rate = clk_fixed_get_rate, }; EXPORT_SYMBOL_GPL(clk_fixed_ops);
+#ifdef CONFIG_CLK_DEBUG +/*
- debugfs support to trace clock tree hierarchy and attributes
- */
+static int clk_debug_rate_get(void *data, u64 *val) +{
- struct clk *clk = data;
- *val = (u64)clk_get_rate(clk);
- return 0;
+} +DEFINE_SIMPLE_ATTRIBUTE(clk_debug_rate_fops, clk_debug_rate_get, NULL,
- "%llu\n");
+static struct dentry *clk_root; +static int clk_debug_register_one(struct clk *clk) +{
- int err;
- struct dentry *d, *child, *child_tmp;
- struct clk *pa = clk_get_parent(clk);
- if (pa && !IS_ERR(pa))
- d = debugfs_create_dir(clk->name, pa->dentry);
- else {
- if (!clk_root)
- clk_root = debugfs_create_dir("clocks", NULL);
- if (!clk_root)
- return -ENOMEM;
- d = debugfs_create_dir(clk->name, clk_root);
- }
- if (!d)
- return -ENOMEM;
- clk->dentry = d;
- d = debugfs_create_u32("enable_count", S_IRUGO, clk->dentry,
- (u32 *)&clk->enable_count);
- if (!d) {
- err = -ENOMEM;
- goto err_out;
- }
- d = debugfs_create_file("rate", S_IRUGO, clk->dentry, (void *)clk,
- &clk_debug_rate_fops);
- if (!d) {
- err = -ENOMEM;
- goto err_out;
- }
- return 0;
+err_out:
- d = clk->dentry;
- list_for_each_entry_safe(child, child_tmp, &d->d_subdirs, d_u.d_child)
- debugfs_remove(child);
- debugfs_remove(clk->dentry);
- return err;
+}
+struct preinit_clk {
- struct list_head list;
- struct clk *clk;
+}; +static LIST_HEAD(preinit_clks); +static DEFINE_MUTEX(preinit_lock); +static int init_done;
+void clk_debug_register(struct clk *clk) +{
- int err;
- struct clk *pa;
- if (init_done) {
- pa = clk_get_parent(clk);
- if (pa && !IS_ERR(pa) && !pa->dentry)
- clk_debug_register(pa);
- if (!clk->dentry) {
- err = clk_debug_register_one(clk);
- if (err)
- return;
- }
- } else {
- struct preinit_clk *p;
- mutex_lock(&preinit_lock);
- p = kmalloc(sizeof(*p), GFP_KERNEL);
- if (!p)
- goto unlock;
- p->clk = clk;
- list_add(&p->list, &preinit_clks);
+unlock:
- mutex_unlock(&preinit_lock);
- }
+} +EXPORT_SYMBOL_GPL(clk_debug_register);
+static int __init clk_debugfs_init(void) +{
- struct preinit_clk *pclk, *tmp;
- if (debugfs_initialized())
- init_done = 1;
- list_for_each_entry(pclk, &preinit_clks, list) {
- clk_debug_register(pclk->clk);
- }
- list_for_each_entry_safe(pclk, tmp, &preinit_clks, list) {
- list_del(&pclk->list);
- kfree(pclk);
- }
- return 0;
+} +late_initcall(clk_debugfs_init);
+#endif
1.7.0.4
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Hi Yong,
We're getting pretty close - just a couple of changes:
} else {struct preinit_clk *p;mutex_lock(&preinit_lock);p = kmalloc(sizeof(*p), GFP_KERNEL);if (!p)goto unlock;p->clk = clk;list_add(&p->list, &preinit_clks);+unlock:
mutex_unlock(&preinit_lock);}
Might be best to avoid the goto:
} else { struct preinit_clk *p; mutex_lock(&preinit_lock); p = kmalloc(sizeof(*p), GFP_KERNEL); if (p) { p->clk = clk; list_add(&p->list, &preinit_clks); } mutex_unlock(&preinit_lock); }
+static int __init clk_debugfs_init(void) +{
- struct preinit_clk *pclk, *tmp;
- if (debugfs_initialized())
init_done = 1;
No need to check debugfs_initialised() here; if it's not initialised we're in trouble anyway.
- list_for_each_entry(pclk, &preinit_clks, list) {
clk_debug_register(pclk->clk);- }
You should hold preinit_lock while traversing the list here.
Cheers,
Jeremy
hi Jeremy,
It's nice to be close :). And it seems that many people in arm kernel list are also intested in the clock debug information, maybe we can post next version to that list and let them know how things are going.
On Thu, Dec 9, 2010 at 10:02 AM, Jeremy Kerr jeremy.kerr@canonical.com wrote:
Hi Yong,
We're getting pretty close - just a couple of changes:
- } else {
- struct preinit_clk *p;
- mutex_lock(&preinit_lock);
- p = kmalloc(sizeof(*p), GFP_KERNEL);
- if (!p)
- goto unlock;
- p->clk = clk;
- list_add(&p->list, &preinit_clks);
+unlock:
- mutex_unlock(&preinit_lock);
- }
Might be best to avoid the goto:
Acked.
} else { struct preinit_clk *p; mutex_lock(&preinit_lock); p = kmalloc(sizeof(*p), GFP_KERNEL); if (p) { p->clk = clk; list_add(&p->list, &preinit_clks); } mutex_unlock(&preinit_lock); }
+static int __init clk_debugfs_init(void) +{
- struct preinit_clk *pclk, *tmp;
- if (debugfs_initialized())
- init_done = 1;
No need to check debugfs_initialised() here; if it's not initialised we're in trouble anyway.
IMO, it is still needed. If debugfs is not initialised, we should return directly, otherwise, it will redo clk_debug_register() again. How about: if (!debugfs_initialized()) return 0;
init_done = 1;
- list_for_each_entry(pclk, &preinit_clks, list) {
- clk_debug_register(pclk->clk);
- }
You should hold preinit_lock while traversing the list here.
Cheers,
Jeremy
Thanks Yong
Hi Yong,
+static int __init clk_debugfs_init(void) +{
struct preinit_clk *pclk, *tmp;if (debugfs_initialized())init_done = 1;No need to check debugfs_initialised() here; if it's not initialised we're in trouble anyway.
IMO, it is still needed. If debugfs is not initialised, we should return directly, otherwise, it will redo clk_debug_register() again.
How about: if (!debugfs_initialized()) return 0;
init_done = 1;
debugfs is initialised as a core_initcall (and clk_debugfs_init as a late_initcall), so we should never see this case.
However, if we ever do the 'return 0', there is no mechanism to retry the initialisation (ie, clk_debugfs_register will only be called once), so we'll just keep adding clocks to preinit_list.
If anything, return an error code (-EBUSY maybe?) so that we get a useful message with initcall_debug. But I don't think this is necessary.
Cheers,
Jeremy
-
Amit Kucheria -
Jeremy Kerr -
Yong Shen