| Message ID | 1409788535-28264-1-git-send-email-computersforpeace@gmail.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
Hi! > 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. When you are doing this kind of analysis, perhaps directly modifying kernel source is the way to go ...? Pavel
On Thu, Sep 04, 2014 at 09:14:12AM +0200, Pavel Machek 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. > > When you are doing this kind of analysis, perhaps directly modifying > kernel source is the way to go ...? That's what I've been doing for now, but I have a few engineers who need to do this sort of testing and aren't kernel developers. I could continue to maintain my own patch for this, but I just thought I'd see what others thought. Is there a good reason this can't be in mainline? These features are hidden behind a Kconfig symbol called PM_DEBUG anyway, and I think this classifies as a pretty simple extension to the limited existing PM debugging options. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 4, 2014 at 10:54 AM, Brian Norris <computersforpeace@gmail.com> wrote: > On Thu, Sep 04, 2014 at 09:14:12AM +0200, Pavel Machek 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. >> >> When you are doing this kind of analysis, perhaps directly modifying >> kernel source is the way to go ...? > > That's what I've been doing for now, but I have a few engineers who need > to do this sort of testing and aren't kernel developers. I could > continue to maintain my own patch for this, but I just thought I'd see > what others thought. > > Is there a good reason this can't be in mainline? These features are > hidden behind a Kconfig symbol called PM_DEBUG anyway, and I think this > classifies as a pretty simple extension to the limited existing PM > debugging options. > We're currently carrying a similar patch in the chrome os kernel tree. I would also like to see this go into mainline. -Chirantan > Regards, > Brian > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, 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. Thanks, Brian > --- > Documentation/power/basic-pm-debugging.txt | 14 ++++++++------ > kernel/power/main.c | 27 +++++++++++++++++++++++++++ > kernel/power/power.h | 5 +++++ > kernel/power/suspend.c | 8 ++++++-- > 4 files changed, 46 insertions(+), 8 deletions(-) > > diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt > index edeecd447d23..bd9f27ae99fe 100644 > --- a/Documentation/power/basic-pm-debugging.txt > +++ b/Documentation/power/basic-pm-debugging.txt > @@ -75,12 +75,14 @@ you should do the following: > # echo platform > /sys/power/disk > # echo disk > /sys/power/state > > -Then, the kernel will try to freeze processes, suspend devices, wait 5 seconds, > -resume devices and thaw processes. If "platform" is written to > -/sys/power/pm_test , then after suspending devices the kernel will additionally > -invoke the global control methods (eg. ACPI global control methods) used to > -prepare the platform firmware for hibernation. Next, it will wait 5 seconds and > -invoke the platform (eg. ACPI) global methods used to cancel hibernation etc. > +Then, the kernel will try to freeze processes, suspend devices, wait a few > +seconds (5 by default, but configurable via /sys/power/pm_test_delay), resume > +devices and thaw processes. If "platform" is written to /sys/power/pm_test, > +then after suspending devices the kernel will additionally invoke the global > +control methods (eg. ACPI global control methods) used to prepare the platform > +firmware for hibernation. Next, it will wait a configurable number of seconds > +and invoke the platform (eg. ACPI) global methods used to cancel hibernation > +etc. > > Writing "none" to /sys/power/pm_test causes the kernel to switch to the normal > hibernation/suspend operations. Also, when open for reading, /sys/power/pm_test > diff --git a/kernel/power/main.c b/kernel/power/main.c > index 9a59d042ea84..4d242c8b43a0 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -73,6 +73,7 @@ power_attr(pm_async); > > #ifdef CONFIG_PM_DEBUG > int pm_test_level = TEST_NONE; > +int pm_test_seconds = PM_TEST_DELAY_DEFAULT; > > static const char * const pm_tests[__TEST_AFTER_LAST] = { > [TEST_NONE] = "none", > @@ -132,6 +133,31 @@ static ssize_t pm_test_store(struct kobject *kobj, struct kobj_attribute *attr, > } > > power_attr(pm_test); > + > +static ssize_t pm_test_delay_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%d\n", pm_test_seconds); > +} > + > +static ssize_t pm_test_delay_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, size_t n) > +{ > + int val; > + > + if (kstrtoint(buf, 10, &val)) > + return -EINVAL; > + > + if (val < 0) > + return -EINVAL; > + > + pm_test_seconds = val; > + > + return n; > +} > + > +power_attr(pm_test_delay); > #endif /* CONFIG_PM_DEBUG */ > > #ifdef CONFIG_DEBUG_FS > @@ -601,6 +627,7 @@ static struct attribute * g[] = { > #endif > #ifdef CONFIG_PM_DEBUG > &pm_test_attr.attr, > + &pm_test_delay_attr.attr, > #endif > #ifdef CONFIG_PM_SLEEP_DEBUG > &pm_print_times_attr.attr, > diff --git a/kernel/power/power.h b/kernel/power/power.h > index 5d49dcac2537..28111795da71 100644 > --- a/kernel/power/power.h > +++ b/kernel/power/power.h > @@ -230,6 +230,11 @@ enum { > > extern int pm_test_level; > > +/* Default to 5 second delay */ > +#define PM_TEST_DELAY_DEFAULT 5 > + > +extern int pm_test_seconds; > + > #ifdef CONFIG_SUSPEND_FREEZER > static inline int suspend_freeze_processes(void) > { > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index 6dadb25cb0d8..2372a99d4356 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -196,8 +196,12 @@ static int suspend_test(int level) > { > #ifdef CONFIG_PM_DEBUG > if (pm_test_level == level) { > - printk(KERN_INFO "suspend debug: Waiting for 5 seconds.\n"); > - mdelay(5000); > + int i; > + > + pr_info("suspend debug: waiting for %d second(s)\n", > + pm_test_seconds); > + for (i = 0; i < pm_test_seconds; i++) > + mdelay(1000); > return 1; > } > #endif /* !CONFIG_PM_DEBUG */ > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 2014-12-12 18:55:30, Brian Norris wrote: > Hi Rafael, > > 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). Does not struck me as a good balance. Pavel
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > 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: I know. I believe your patch is so crazy that no more discussion was neccessary. > 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. We are talking 40 unneccessary lines of source. Imagine everyone who needed to change one value added 40 lines of code. Terabyte kernel. Overengineered. Crazy. Something we'll have to maintain forever. Make it module parameter so that the patch is two lines of code. If that does not work for you, think of something that does. > 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? Good questions. Showing that perhaps you should not patch the upstream. Pavel
On Wed, Dec 17, 2014 at 09:09:47AM +0100, Pavel Machek wrote: > > > > 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: > > I know. I believe your patch is so crazy that no more discussion was neccessary. That's awfully disrespectful. You never responded to my follow up question, and now you ridicule me for asking again. > > 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. > > We are talking 40 unneccessary lines of source. "Unnecessary" is highly subjective. Just because you don't need it doesn't mean no one else does. There are at least two completely different projects that have mentioned the need for it. Thus (as I explained in my "ping") I don't think that qualifies as unnecessary. > Imagine everyone who > needed to change one value added 40 lines of code. Terabyte > kernel. That's an exaggeration. > Overengineered. Crazy. Something we'll have to maintain > forever. It follows the same form and style of the other kernel/power/ debug sysfs code. Its interface is well documented and straightforward. It's really quite obvious. So how is this a maintenance problem? Additionally, because it's explicitly a debug feature (CONFIG_PM_DEBUG), I'm not sure it would really qualify as an unchangeable ABI that can never be touched, if it really became a maintenance problem somehow. > Make it module parameter so that the patch is two lines of code. If > that does not work for you, think of something that does. OK, so that's actually constructive. If lines of code is really the most important factor here, then I suppose I can do that. I'd argue that a module parameter is a much less sensible interface here, though, given that it is coupled with the existing /sys/power/pm_test interface. > > 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? > > Good questions. Showing that perhaps you should not patch the > upstream. You seem to not understand the point behind my rhetorical questions. And I don't feel like you've honestly addressed any of the 4 points I made. That's disappointing. I'd appreciate some feedback from Rafael. Based on his feedback (or lack thereof) my options seem to be: 1. Sit and wait on this patch for another few months 2. Resend this patch as a module parameter instead 3. Forget this entirely Thanks, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Make it module parameter so that the patch is two lines of code. If > > that does not work for you, think of something that does. > > OK, so that's actually constructive. If lines of code is really the most > important factor here, then I suppose I can do that. I'd argue that a > module parameter is a much less sensible interface here, though, given > that it is coupled with the existing /sys/power/pm_test interface. If module parameter works for you, we have a winner, that should be two lines of code and two lines of documentation. Pavel
2014-12-17 1:22 GMT-08:00 Pavel Machek <pavel@ucw.cz>: > >> > Make it module parameter so that the patch is two lines of code. If >> > that does not work for you, think of something that does. >> >> OK, so that's actually constructive. If lines of code is really the most >> important factor here, then I suppose I can do that. I'd argue that a >> module parameter is a much less sensible interface here, though, given >> that it is coupled with the existing /sys/power/pm_test interface. > > If module parameter works for you, we have a winner, that should be > two lines of code and two lines of documentation. I do not think that a module parameter is good, some of the uses cases I can think about (from real people using that facility) involve setting up a Linux system in a lab with multiple measurement equipments, having to reboot Linux to change this delay is going to be a no-go for them because that will break the uptime/endurance/stability/automated testing they might be doing. Having them be able to change the PM delay at runtime is completely satisfactory though. Both module parameters and sysfs entries need to remain stable, and potentially there forever, once introduced, yet the sysfs entries are a lot more flexible. Considering that Brian's change are enclosed within a CONFIG_PM_DEBUG ifdef, can we really use the code bloat as a technical argument here? Thanks!
> Considering that Brian's change are enclosed within a CONFIG_PM_DEBUG > ifdef, can we really use the code bloat as a technical argument here? Yes. Pavel
2015-02-21 12:32 GMT-08:00 Pavel Machek <pavel@ucw.cz>: > > > >> Considering that Brian's change are enclosed within a CONFIG_PM_DEBUG >> ifdef, can we really use the code bloat as a technical argument here? > > Yes. Help me understand a few things here: - this particular change is enclosed within a debug option, so only people interested in enabling PM_DEBUG will get access to it - if we need to turn on PM_DEBUG all the time, including mainline distributions, does that mean that: - portions of code existing only in PM_DEBUG should be moved out of it because it is actually useful outside of debug option? - CONFIG_PM itself is not self sufficient and there are still problems that require PM_DEBUG to be turned on? - should there be a second level debug option (e.g: CONFIG_PM_DEBUG_ADV) which gates specific control knobs like PM delay? The current 5 seconds delay is completely arbitrary and goes against the principle of not enforcing a policy, having this configurable brings this back in the mechanism principle.
On Sat 2015-02-21 14:56:01, Florian Fainelli wrote: > 2015-02-21 12:32 GMT-08:00 Pavel Machek <pavel@ucw.cz>: > > > > > > > >> Considering that Brian's change are enclosed within a CONFIG_PM_DEBUG > >> ifdef, can we really use the code bloat as a technical argument here? > > > > Yes. > > Help me understand a few things here: What you are missing is that we try to keep _sources_ small and readable, too. > - if we need to turn on PM_DEBUG all the time, including mainline > distributions, does that mean that: > - portions of code existing only in PM_DEBUG should be > moved out of it because it is actually useful outside of debug option? > - CONFIG_PM itself is not self sufficient and there are > still problems that require PM_DEBUG to be turned on? > - should there be a second level debug option (e.g: > CONFIG_PM_DEBUG_ADV) which gates specific control knobs like PM delay? > > The current 5 seconds delay is completely arbitrary and goes against > the principle of not enforcing a policy, having this configurable > brings this back in the mechanism principle. 5 second delay is arbitrary, and slightly ugly debugging hack, that is acceptable because it is useful, small and unobtrusive. The second you add 40 lines of sysfs glue.. it is no longer small and unobtrusive, and no longer acceptable. And yes, distros probably ship with PM_DEBUG on, and no, adding another config option would not make the big & ugly hack more acceptable. OTOH if you added a hook to kprobes that alowed easy binary patching of the value "5" while not adding 40 lines of overhead, that might be neccessary. Actually, you can probably pull the address from System.map and then just write to it using /dev/mem, no? Just arrange for "5" to be in variable that is in System.map. And you also want to check the kernel parameters, they are modifiable in runtime in at least same cases. But we are not adding 40 lines of uglyness. Clear? Pavel
On Sun, Feb 22, 2015 at 12:24:29AM +0100, Pavel Machek wrote: > And you also want to check the kernel parameters, they are modifiable > in runtime in at least same cases. I forgot about this fact. In this case, I think this satisfies all that we need. So we can do something like this at runtime: # echo 30 > /sys/module/suspend/parameters/pm_test_delay I'll send a patch shortly. Brian -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt index edeecd447d23..bd9f27ae99fe 100644 --- a/Documentation/power/basic-pm-debugging.txt +++ b/Documentation/power/basic-pm-debugging.txt @@ -75,12 +75,14 @@ you should do the following: # echo platform > /sys/power/disk # echo disk > /sys/power/state -Then, the kernel will try to freeze processes, suspend devices, wait 5 seconds, -resume devices and thaw processes. If "platform" is written to -/sys/power/pm_test , then after suspending devices the kernel will additionally -invoke the global control methods (eg. ACPI global control methods) used to -prepare the platform firmware for hibernation. Next, it will wait 5 seconds and -invoke the platform (eg. ACPI) global methods used to cancel hibernation etc. +Then, the kernel will try to freeze processes, suspend devices, wait a few +seconds (5 by default, but configurable via /sys/power/pm_test_delay), resume +devices and thaw processes. If "platform" is written to /sys/power/pm_test, +then after suspending devices the kernel will additionally invoke the global +control methods (eg. ACPI global control methods) used to prepare the platform +firmware for hibernation. Next, it will wait a configurable number of seconds +and invoke the platform (eg. ACPI) global methods used to cancel hibernation +etc. Writing "none" to /sys/power/pm_test causes the kernel to switch to the normal hibernation/suspend operations. Also, when open for reading, /sys/power/pm_test diff --git a/kernel/power/main.c b/kernel/power/main.c index 9a59d042ea84..4d242c8b43a0 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -73,6 +73,7 @@ power_attr(pm_async); #ifdef CONFIG_PM_DEBUG int pm_test_level = TEST_NONE; +int pm_test_seconds = PM_TEST_DELAY_DEFAULT; static const char * const pm_tests[__TEST_AFTER_LAST] = { [TEST_NONE] = "none", @@ -132,6 +133,31 @@ static ssize_t pm_test_store(struct kobject *kobj, struct kobj_attribute *attr, } power_attr(pm_test); + +static ssize_t pm_test_delay_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sprintf(buf, "%d\n", pm_test_seconds); +} + +static ssize_t pm_test_delay_store(struct kobject *kobj, + struct kobj_attribute *attr, + const char *buf, size_t n) +{ + int val; + + if (kstrtoint(buf, 10, &val)) + return -EINVAL; + + if (val < 0) + return -EINVAL; + + pm_test_seconds = val; + + return n; +} + +power_attr(pm_test_delay); #endif /* CONFIG_PM_DEBUG */ #ifdef CONFIG_DEBUG_FS @@ -601,6 +627,7 @@ static struct attribute * g[] = { #endif #ifdef CONFIG_PM_DEBUG &pm_test_attr.attr, + &pm_test_delay_attr.attr, #endif #ifdef CONFIG_PM_SLEEP_DEBUG &pm_print_times_attr.attr, diff --git a/kernel/power/power.h b/kernel/power/power.h index 5d49dcac2537..28111795da71 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -230,6 +230,11 @@ enum { extern int pm_test_level; +/* Default to 5 second delay */ +#define PM_TEST_DELAY_DEFAULT 5 + +extern int pm_test_seconds; + #ifdef CONFIG_SUSPEND_FREEZER static inline int suspend_freeze_processes(void) { diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 6dadb25cb0d8..2372a99d4356 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -196,8 +196,12 @@ static int suspend_test(int level) { #ifdef CONFIG_PM_DEBUG if (pm_test_level == level) { - printk(KERN_INFO "suspend debug: Waiting for 5 seconds.\n"); - mdelay(5000); + int i; + + pr_info("suspend debug: waiting for %d second(s)\n", + pm_test_seconds); + for (i = 0; i < pm_test_seconds; i++) + mdelay(1000); return 1; } #endif /* !CONFIG_PM_DEBUG */
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> --- Documentation/power/basic-pm-debugging.txt | 14 ++++++++------ kernel/power/main.c | 27 +++++++++++++++++++++++++++ kernel/power/power.h | 5 +++++ kernel/power/suspend.c | 8 ++++++-- 4 files changed, 46 insertions(+), 8 deletions(-)