Hi Jeremy,
Great ideal to make it beyond ARM specific.
Below is the updated patch as per your comments.
>From 5a3e2d3bf577e3059c9254a61d671910ab170623 Mon Sep 17 00:00:00 2001
From: Yong Shen <yong.s...@linaro.org>
Date: Tue, 16 Nov 2010 15:00:28 +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 debugfs.
Signed-off-by: Yong Shen <yong.s...@linaro.org>
---
arch/arm/common/Kconfig | 7 +++
arch/arm/common/clkdev.c | 28 +++++++++++++
arch/arm/plat-mxc/clock-common.c | 5 ++-
include/linux/clk.h | 19 ++++++++-
kernel/clk.c | 83
++++++++++++++++++++++++++++++++++++++
5 files changed, 139 insertions(+), 3 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 0a34c81..0300c95 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -41,3 +41,10 @@ config SHARP_SCOOP
config COMMON_CLKDEV
bool
select HAVE_CLK
+
+config CLK_DEBUG
+ bool "clock debug information export to user space"
+ depends on COMMON_CLKDEV && PM_DEBUG && DEBUG_FS
+ default n
+ help
+ export clk debug information to user space
diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c
index 9e4c4d9..5f4a309 100644
--- a/arch/arm/common/clkdev.c
+++ b/arch/arm/common/clkdev.c
@@ -19,6 +19,9 @@
#include <linux/mutex.h>
#include <linux/clk.h>
#include <linux/slab.h>
+#ifdef CONFIG_CLK_DEBUG
+#include <linux/debugfs.h>
+#endif
#include <asm/clkdev.h>
@@ -186,3 +189,28 @@ void clkdev_drop(struct clk_lookup *cl)
kfree(cl);
}
EXPORT_SYMBOL(clkdev_drop);
+
+#ifdef CONFIG_CLK_DEBUG
+static int __init clk_debugfs_init(void)
+{
+ struct clk_lookup *cl;
+ struct dentry *d;
+ int err;
+
+ d = debugfs_create_dir("clocks", NULL);
+ if (!d)
+ return -ENOMEM;
+
+ list_for_each_entry(cl, &clocks, node) {
+ err = clk_debugfs_register(cl->clk, d);
+ if (err)
+ goto err_out;
+ }
+ return 0;
+err_out:
+ debugfs_remove_recursive(d);
+ return err;
+}
+late_initcall(clk_debugfs_init);
+
+#endif
diff --git a/arch/arm/plat-mxc/clock-common.c
b/arch/arm/plat-mxc/clock-common.c
index 8911813..20d38a8 100644
--- a/arch/arm/plat-mxc/clock-common.c
+++ b/arch/arm/plat-mxc/clock-common.c
@@ -97,7 +97,10 @@ unsigned long clk_mxc_get_rate(struct clk *_clk)
if (clk->get_rate)
return clk->get_rate(clk);
- return clk_get_rate(clk->parent);
+ if (clk->parent)
+ return clk_get_rate(clk->parent);
+
+ return 0;
}
/* Round the requested clock rate to the nearest supported
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 56416b7..751c086 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -44,16 +44,28 @@ struct device;
* registered with the arch-specific clock management code; the clock
driver
* code does not need to handle these.
*/
+#define CLK_NAME_LEN 32
struct clk {
const struct clk_ops *ops;
unsigned int enable_count;
struct mutex mutex;
+#ifdef CONFIG_CLK_DEBUG
+ char name[CLK_NAME_LEN];
+ struct dentry *dentry;
+#endif
};
-#define INIT_CLK(name, o) { \
+#ifdef CONFIG_CLK_DEBUG
+#define __INIT_CLK_DEBUG(n) .name = #n,
+#else
+#define __INIT_CLK_DEBUG(n)
+#endif
+
+#define INIT_CLK(clk_name, o) { \
.ops = &o, \
.enable_count = 0, \
- .mutex = __MUTEX_INITIALIZER(name.mutex), \
+ .mutex = __MUTEX_INITIALIZER(clk_name.mutex), \
+ __INIT_CLK_DEBUG(clk_name) \
}
struct clk_ops {
@@ -245,4 +257,7 @@ 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
+int clk_debugfs_register(struct clk *clk, struct dentry *root);
+#endif
#endif
diff --git a/kernel/clk.c b/kernel/clk.c
index 32f25ef..97e5e66 100644
--- a/kernel/clk.c
+++ b/kernel/clk.c
@@ -12,6 +12,10 @@
#include <linux/mutex.h>
#include <linux/module.h>
+#ifdef CONFIG_CLK_DEBUG
+#include <linux/debugfs.h>
+#endif
+
int clk_enable(struct clk *clk)
{
int ret = 0;
@@ -113,3 +117,82 @@ 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 int clk_debugfs_register_one(struct clk *clk, struct dentry *root)
+{
+ 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
+ d = debugfs_create_dir(clk->name, 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;
+}
+
+int clk_debugfs_register(struct clk *clk, struct dentry *root)
+{
+ int err;
+ struct clk *pa;
+
+ pa = clk_get_parent(clk);
+
+ if (pa && !IS_ERR(pa) && !pa->dentry) {
+ err = clk_debugfs_register(pa, root);
+ if (err)
+ return err;
+ }
+
+ if (!clk->dentry) {
+ err = clk_debugfs_register_one(clk, root);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(clk_debugfs_register);
+
+#endif /* defined CONFIG_CLK_DEBUG */
--
1.7.0.4
Cheers
Yong
On Tue, Nov 16, 2010 at 5:16 PM, Jeremy Kerr <jeremy.k...@canonical.com>wrote:
> Hi Yong,
>
> Looks good, just a couple of things:
>
> > diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c
> > index 9e4c4d9..cf81e70 100644
> > --- a/arch/arm/common/clkdev.c
> > +++ b/arch/arm/common/clkdev.c
>
> Why not do this in the core clock code (kernel/clk.c) instead? No need to
> make
> it ARM-specific.
>
> > diff --git a/arch/arm/plat-mxc/clock-common.c
> > b/arch/arm/plat-mxc/clock-common.c
> > index 8911813..20d38a8 100644
> > --- a/arch/arm/plat-mxc/clock-common.c
> > +++ b/arch/arm/plat-mxc/clock-common.c
> > @@ -97,7 +97,10 @@ unsigned long clk_mxc_get_rate(struct clk *_clk)
> > if (clk->get_rate)
> > return clk->get_rate(clk);
> >
> > - return clk_get_rate(clk->parent);
> > + if (clk->parent)
> > + return clk_get_rate(clk->parent);
> > +
> > + return 0;
> > }
> >
> > /* Round the requested clock rate to the nearest supported
> > diff --git a/include/linux/clk.h b/include/linux/clk.h
> > index 56416b7..1102c2c 100644
> > --- a/include/linux/clk.h
> > +++ b/include/linux/clk.h
> > @@ -44,17 +44,31 @@ struct device;
> > * registered with the arch-specific clock management code; the clock
> > driver
> > * code does not need to handle these.
> > */
> > +#define CLK_NAME_LEN 16
>
> We might need more than 16 bytes here; 32 should be fine.
>
> > struct clk {
> > const struct clk_ops *ops;
> > unsigned int enable_count;
> > struct mutex mutex;
> > +#ifdef CONFIG_CLK_DEBUG
> > + char name[CLK_NAME_LEN];
> > + struct dentry *dentry;
> > +#endif
> > };
> >
> > -#define INIT_CLK(name, o) { \
> > +#ifndef CONFIG_CLK_DEBUG
> > +#define INIT_CLK(clk_name, o) { \
> > .ops = &o, \
> > .enable_count = 0, \
> > - .mutex = __MUTEX_INITIALIZER(name.mutex), \
> > + .mutex = __MUTEX_INITIALIZER(clk_name.mutex), \
> > }
> > +#else
> > +#define INIT_CLK(clk_name, o) { \
> > + .ops = &o, \
> > + .enable_count = 0, \
> > + .mutex = __MUTEX_INITIALIZER(clk_name.mutex), \
> > + .name = #clk_name, \
> > +}
> > +#endif
>
> How about this instead:
>
> +#ifdef CONFIG_CLK_DEBUG
> +#define __INIT_CLK_DEBUG(n) .name = #n,
> +#else
> +#define __INIT_CLK_DEBUG(n)
> +#endif
> +
> /* static initialiser for non-atomic clocks */
> #define INIT_CLK(name, o) { \
> .ops = &o, \
> .enable_count = 0, \
> .flags = 0, \
> .lock.mutex = __MUTEX_INITIALIZER(name.lock.mutex), \
> + __INIT_CLK_DEBUG(name) \
> }
>
> /* static initialiser for atomic clocks */
>
> - otherwise, you're going to need to add another definition of
> INIT_CLK_ATOMIC
> too, giving four in total.
>
> Cheers,
>
>
> Jeremy
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
