Re: [PATCH v1] PM / Domains: Add tracepoints

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	Douglas Anderson <dianders@chromium.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ravi Chandra Sadineni <ravisadineni@chromium.org>
Subject: Re: [PATCH v1] PM / Domains: Add tracepoints
Date: Wed, 2 Oct 2019 09:55:33 +0200	[thread overview]
Message-ID: <CAJZ5v0iepu-WAdT=kLcXruRGAerjfPqzgxK_003OpjRho4OrPg@mail.gmail.com> (raw)
In-Reply-To: <20191001193701.GD87296@google.com>

On Tue, Oct 1, 2019 at 9:37 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Tue, Oct 01, 2019 at 02:08:46PM -0400, Steven Rostedt wrote:
> > On Tue, 1 Oct 2019 10:42:35 -0700
> > Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > > On Tue, Oct 01, 2019 at 01:03:43PM -0400, Steven Rostedt wrote:
> > > > On Tue, 1 Oct 2019 09:35:42 -0700
> > > > Matthias Kaehlcke <mka@chromium.org> wrote:
> > > >
> > > > > How about this instead:
> > > > >
> > > > >   Add tracepoints for genpd_power_on, genpd_power_off and
> > > > >   genpd_set_performance_state. The tracepoints can help with
> > > > >   understanding power domain behavior of a given device, which
> > > > >   may be particularly interesting for battery powered devices
> > > > >   and suspend/resume.
> > > >
> > > > Do you have a use case example to present?
> > >
> > > TBH I'm not looking into a specific use case right now. While
> > > peeking around in /sys/kernel/debug/tracing/events to learn more
> > > about existing tracepoints that might be relevant for my work
> > > I noticed the absence of genpd ones and it seemed a good idea to
> > > add them preemptively. Conceptually they seem similar to the
> > > existing regulator_enable/disable and cpu_idle tracepoints.
> > >
> > > As an abstract use case I could see power analysis on battery
> > > powered devices during suspend. genpd_power_on/off allow to see
> > > which power domains remain on during suspend, and might give
> > > insights for possible power saving options. Examples could be that
> > > a power domain stays unexpectedly on due to a misconfiguration, or
> > > two power domains remain on when it could be only one if you just
> > > moved that one pin/port over to the other domain in the next
> > > hardware revision.
> >
> > If the power management maintainers have no issues with adding these,
> > then neither do I ;-)  It would be them who would pull them in anyway.
>
> Ok, I'll send a new version with the changes you suggested and some more
> info in the commit message, unless PM maintainers raise concerns before
> that.

I have no problems with adding tracepoints (or trace events to be more
precise) in there, but Ulf has been taking care of that code for quite
some time, so his opinion matters more than mine in that respect.