Re: [PATCH] clocksource: Add debugfs support

[Thierry Reding]

On Wed, Apr 01, 2020 at 12:06:37AM +0200, Thomas Gleixner wrote:
> Thierry,
> 
> Thierry Reding <thierry.reding@xxxxxxxxx> writes:
> > Add a top-level "clocksource" directory to debugfs. For each clocksource
> > registered with the system, a subdirectory will be added with attributes
> > that can be queried to obtain information about the clocksource.
> 
> first of all this does tell what this patch does but omits the more
> important information about the WHY.
> 
> What's even worse is that the changelog is blantantly wrong.
> 
> > +static int clocksource_debugfs_counter_show(struct seq_file *s, void *data)
> > +{
> > +	struct clocksource *cs = s->private;
> > +
> > +	seq_printf(s, "%llu\n", cs->read(cs));
> > +
> > +	return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(clocksource_debugfs_counter);
> > +
> > +static void clocksource_debugfs_add(struct clocksource *cs)
> > +{
> > +	if (!debugfs_root)
> > +		return;
> > +
> > +	cs->debugfs = debugfs_create_dir(cs->name, debugfs_root);
> > +
> > +	debugfs_create_file("counter", 0444, cs->debugfs, cs,
> > +			    &clocksource_debugfs_counter_fops);
> > +}
> 
> It does not provide any information about the clocksource, it provides
> an interface to read the counter - nothing else.

The counter is part of the information about a clocksource, isn't it?
But yes, frankly I had anticipated that I'd be adding more files here
and when I ended up not doing that I forgot to update the patch
description.

I can also add some information about what I intend to use this for,
though it'll be a bit boring because I really only want this as a way
of testing that I'm reading from the right registers and that these
counters are running. A debugfs interface seemed like a better and more
widely useful way to achieve that than implementing some one-off hack to
poll those registers.

Thierry

Attachment: signature.asc
Description: PGP signature