Amit Daniel Kachhap wrote: > > Hi Mr Kim, > > Thanks for your comments. Please see the inline comments below. > Hi Amit,
I didn't get your updated patch, but it was small things. So I applied this in my tree for this merge window. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene....@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. > On 1 December 2010 13:08, Kukjin Kim <kgene....@samsung.com> wrote: > > > > Amit Daniel Kachhap wrote: > > > > > > This patch adds support for clock information exposed to debug-fs > > interface. > > > > > > Signed-off-by: Amit Daniel Kachhap <amit.dan...@samsung.com> > > > --- > > > Code modified for V2 version are, > > > a)Inserted the debug-fs code inside macro CONFIG_PM_DEBUG as suggested by > > > yong.s...@linaro.org. > > > > Could you please let me know why? > Actually this patch is mostly taken from omap architecture, so using > this macro in addition > to CONFIG_DEBUG_FS macro. Also it is better to enable all PM debugging > features with > one configuration parameter. > > > > > > b)Removed macro CONFIG_PLAT_SAMSUNG and implemented other coding > standards > > > as suggested by kgene....@samsung.com > > > > > > arch/arm/plat-samsung/clock.c | 91 > > > ++++++++++++++++++++++++++++ > > > arch/arm/plat-samsung/include/plat/clock.h | 3 + > > > 2 files changed, 94 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat- > samsung/clock.c > > > index e8d20b0..029a49d 100644 > > > --- a/arch/arm/plat-samsung/clock.c > > > +++ b/arch/arm/plat-samsung/clock.c > > > @@ -39,6 +39,9 @@ > > > #include <linux/clk.h> > > > #include <linux/spinlock.h> > > > #include <linux/io.h> > > > +#if defined(CONFIG_DEBUG_FS) > > > +#include <linux/debugfs.h> > > > +#endif > > > > > > #include <mach/hardware.h> > > > #include <asm/irq.h> > > > @@ -447,3 +450,91 @@ int __init s3c24xx_register_baseclocks(unsigned long > > > xtal) > > > return 0; > > > } > > > > > > +#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) > > > > Hmm...please let me know the reason... > same reason as above. > > > > > +/* debugfs support to trace clock tree hierarchy and attributes */ > > > + > > > +static struct dentry *clk_debugfs_root; > > > + > > > +static int clk_debugfs_register_one(struct clk *c) > > > +{ > > > + int err; > > > + struct dentry *d, *child, *child_tmp; > > > + struct clk *pa = c->parent; > > > + char s[255]; > > > + char *p = s; > > > + > > > + p += sprintf(p, "%s", c->name); > > > + /*Append id field with name also*/ > > > > No need above comment. If required, please add other comments also. > ok, will remove this comment in next patch. > > > > > + if (c->id >= 0) > > > + sprintf(p, ":%d", c->id); > > > + > > > + d = debugfs_create_dir(s, pa ? pa->dent : clk_debugfs_root); > > > + if (!d) > > > + return -ENOMEM; > > > + > > > + c->dent = d; > > > + > > > + d = debugfs_create_u8("usecount", S_IRUGO, c->dent, (u8 > > *)&c->usage); > > > + if (!d) { > > > + err = -ENOMEM; > > > + goto err_out; > > > + } > > > > 1 empty line would be helpful to read. > ok, will update this in next patch. > > > > > + d = debugfs_create_u32("rate", S_IRUGO, c->dent, (u32 *)&c->rate); > > > + if (!d) { > > > + err = -ENOMEM; > > > + goto err_out; > > > + } > > > + return 0; > > > + > > > +err_out: > > > + d = c->dent; > > > + list_for_each_entry_safe(child, child_tmp, &d->d_subdirs, > > d_u.d_child) > > > + debugfs_remove(child); > > > + debugfs_remove(c->dent); > > > + return err; > > > +} > > > + > > > +static int clk_debugfs_register(struct clk *c) > > > +{ > > > + int err; > > > + struct clk *pa = c->parent; > > > + > > > + if (pa && !pa->dent) { > > > + err = clk_debugfs_register(pa); > > > + if (err) > > > + return err; > > > + } > > > + > > > + if (!c->dent) { > > > + err = clk_debugfs_register_one(c); > > > + if (err) > > > + return err; > > > + } > > > + return 0; > > > +} > > > + > > > +static int __init clk_debugfs_init(void) > > > +{ > > > + struct clk *c; > > > + struct dentry *d; > > > + int err; > > > + > > > + d = debugfs_create_dir("clock", NULL); > > > + if (!d) > > > + return -ENOMEM; > > > + clk_debugfs_root = d; > > > + > > > + list_for_each_entry(c, &clocks, list) { > > > + err = clk_debugfs_register(c); > > > + if (err) > > > + goto err_out; > > > + } > > > + return 0; > > > +err_out: > > > + debugfs_remove_recursive(clk_debugfs_root); > > > + return err; > > > +} > > > +late_initcall(clk_debugfs_init); > > > + > > > +#endif /* defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) */ > > > + > > > > No need last one empty line. > ok, will update this comment in next patch. > > > > > diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat- > > > samsung/include/plat/clock.h > > > index 0fbcd0e..9a82b88 100644 > > > --- a/arch/arm/plat-samsung/include/plat/clock.h > > > +++ b/arch/arm/plat-samsung/include/plat/clock.h > > > @@ -47,6 +47,9 @@ struct clk { > > > > > > struct clk_ops *ops; > > > int (*enable)(struct clk *, int enable); > > > +#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) > > > + struct dentry *dent; /* For visible tree hierarchy */ > > > +#endif > > > }; > > > > > > /* other clocks which may be registered by board support */ > > > -- _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev