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

[Greg KH <greg@kroah.com>]

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