Re: [PATCH 2/6] Clocklib: debugfs support

Greg KH
2008-04-08T01:05:27Z

On Mon, Apr 07, 2008 at 03:59:01PM -0700, Andrew Morton wrote:
> On Thu, 3 Apr 2008 17:23:37 +0400
> Dmitry Baryshkov <dbaryshkov@gmail.com> wrote:
>
> > Provide /sys/kernel/debug/clock to ease debugging.
> >
>
> Plese fully document the proposed kernel->userspace interface in the
> changelog so that we can review it design.

You can do this easily by providing a file for Documentation/ABI/
describing your addition to the kernel ABI.

> > /**
> > diff --git a/kernel/clklib.c b/kernel/clklib.c
> > index 012f845..8d872e1 100644
> > --- a/kernel/clklib.c
> > +++ b/kernel/clklib.c
> > @@ -324,3 +324,77 @@ out:
> > return rc;
> > }
> > EXPORT_SYMBOL(clk_alloc_function);
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +#include <linux/debugfs.h>
> > +#include <linux/seq_file.h>
> > +
> > +#define NAME_FIELD_LEN 20
> > +
> > +static void dump_clocks(struct seq_file *s, struct clk *parent, int nest)
> > +{
> > + struct clk *clk;
> > + int i;
> > +
> > + list_for_each_entry(clk, &clocks, node) {
> > + if (clk->parent == parent) {
> > + for (i = 0; i < nest; i++) {
> > + seq_putc(s, ' ');
> > + seq_putc(s, ' ');
> > + }
> > + seq_puts(s, clk->name);
> > +
> > + i = 2 * nest + strlen(clk->name);
> > + if (i >= NAME_FIELD_LEN)
> > + i = NAME_FIELD_LEN - 1;
> > + for (; i < NAME_FIELD_LEN; i++) {
> > + seq_putc(s, ' ');
> > + }
> > + seq_printf(s, "%c use=%d rate=%10lu Hz",
> > + clk->ops && clk->ops->set_parent ? '*' : ' ',
> > + clk->users,
> > + __clk_get_rate(clk));
> > + if (clk->ops && clk->ops->format)
> > + clk->ops->format(clk, s);
> > + seq_putc(s, '\n');
> > +
> > + dump_clocks(s, clk, nest + 1);
> > + }
> > + }
> > +}
>
> hm, this would break the one-value-per-file rule, except that applies to
> sysfs, but this interface is implemented via debug fs, but it is proposed
> that it appear under /sys. Tricky.

Huh?

How can a seq_file entry show up in sysfs?

Or do you mean /sys/kernel/debug/ ?
That's where debugfs is supposed to be mounted so that's fine, it's
really a debugfs file, not a sysfs one.

thanks,

greg k-h