All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] clocksource: Add debugfs support
Date: Wed, 1 Apr 2020 00:29:17 +0200	[thread overview]
Message-ID: <20200331222917.GG2954599@ulmo> (raw)
In-Reply-To: <87lfnguqky.fsf@nanos.tec.linutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]

On Wed, Apr 01, 2020 at 12:06:37AM +0200, Thomas Gleixner wrote:
> Thierry,
> 
> Thierry Reding <thierry.reding@gmail.com> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-03-31 22:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-31 21:40 [PATCH] clocksource: Add debugfs support Thierry Reding
2020-03-31 21:50 ` John Stultz
2020-03-31 22:25   ` Thierry Reding
2020-03-31 22:44     ` John Stultz
2020-03-31 23:02       ` Thierry Reding
2020-03-31 22:06 ` Thomas Gleixner
2020-03-31 22:29   ` Thierry Reding [this message]
2020-03-31 22:49     ` Thierry Reding
2020-03-31 23:01     ` Thomas Gleixner
2020-03-31 23:06       ` Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200331222917.GG2954599@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.