Re: [PATCH] PM / sleep: add configurable delay for pm_test

[Brian Norris <computersforpeace@gmail.com>]

On Sat, Dec 13, 2014 at 09:31:23AM +0100, Pavel Machek wrote:
> On Fri 2014-12-12 18:55:30, Brian Norris wrote:
> > On Wed, Sep 03, 2014 at 04:55:35PM -0700, Brian Norris wrote:
> > > When CONFIG_PM_DEBUG=y, we provide a sysfs file (/sys/power/pm_test) for
> > > selecting one of a few suspend test modes, where rather than entering a
> > > full suspend state, the kernel will perform some subset of suspend
> > > steps, wait 5 seconds, and then resume back to normal operation.
> > > 
> > > This mode is useful for (among other things) observing the state of the
> > > system just before entering a sleep mode, for debugging or analysis
> > > purposes. However, a constant 5 second wait is not sufficient for some
> > > sorts of analysis; for example, on an SoC, one might want to use
> > > external tools to probe the power states of various on-chip controllers
> > > or clocks.
> > > 
> > > This patch adds a companion sysfs file (/sys/power/pm_test_delay) that
> > > allows user-space to configure how long the system waits in this test
> > > state before resuming. It also updates the PM debugging documentation to
> > > mention the new file.
> > > 
> > > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> > 
> > What do you think about this patch? It seems there is at least one other
> > developer who is independently interested in this.
> 
> 40 lines of code, and new sysfs interface for use by someone who puts
> the probes on board, anyway... (so should be able to add the single
> mdelay himself).

I heard your complaint the first time:

  https://lkml.org/lkml/2014/9/4/63

And I responded to it already:

  https://lkml.org/lkml/2014/9/4/494

You did not respond, but Chirantan spoke up saying he wanted such a
patch too. He came up with a very similar solution independently:

  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/15bccc2c63c3475ef61d3187c73ccf1d80b18c7e

But since you've decided to make the same comment again, I will detail
more of the reasons why I think your suggestion ("go add the mdelay
yourself") is off-base.

 1. This is behind a debug config option (CONFIG_PM_DEBUG). So what's
 the problem with improving its usefulness? Non-debug users can easily
 compile it out if they're worried about 40 lines.

 2. The current debug code encodes a particular policy (which kernels
 generally should not). Is it better if I submit a patch that changes
 the current magic delay to 60000 milliseconds? What about 1334
 milliseconds?

 3. To continue your argument: why would I ever try to patch the
 upstream kernel, if I'm perfectly capable of doing this myself?

 4. How does putting probes on a board suddenly qualify someone for
 patching and rebuilding their kernel? I noted that I have *users* who
 want to do this. Hence, I'm patching a *user* interface.

Brian