* [PATCH 0/3] PM / Sleep: Suspend core code cleanups
@ 2012-02-11 23:03 Rafael J. Wysocki
2012-02-11 23:04 ` [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c Rafael J. Wysocki
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-02-11 23:03 UTC (permalink / raw)
To: Linux PM list; +Cc: LKML, Srivatsa S. Bhat, Randy Dunlap
Hi,
The following three patches clean up the core system suspend code.
[1/3] - Unify and improve kerneldoc comments in kernel/power/suspend.c.
[2/3] - Make enter_state() in kernel/power/suspend.c static.
[3/3] - Drop suspend_stats_update() and move its code to the caller.
They are on top of the patch at:
http://marc.info/?l=linux-kernel&m=132891673021129&w=4
and are v3.4 candidates.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c 2012-02-11 23:03 [PATCH 0/3] PM / Sleep: Suspend core code cleanups Rafael J. Wysocki @ 2012-02-11 23:04 ` Rafael J. Wysocki 2012-02-12 14:49 ` Srivatsa S. Bhat 2012-02-11 23:04 ` [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static Rafael J. Wysocki 2012-02-11 23:06 ` [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() Rafael J. Wysocki 2 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-11 23:04 UTC (permalink / raw) To: Linux PM list; +Cc: LKML, Srivatsa S. Bhat, Randy Dunlap From: Rafael J. Wysocki <rjw@sisk.pl> The kerneldoc comments in kernel/power/suspend.c are not formatted in the same way and the quality of some of them is questionable. Unify the formatting and improve the contents. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- kernel/power/suspend.c | 56 +++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 29 deletions(-) Index: linux/kernel/power/suspend.c =================================================================== --- linux.orig/kernel/power/suspend.c +++ linux/kernel/power/suspend.c @@ -37,8 +37,8 @@ const char *const pm_states[PM_SUSPEND_M static const struct platform_suspend_ops *suspend_ops; /** - * suspend_set_ops - Set the global suspend method table. - * @ops: Pointer to ops structure. + * suspend_set_ops - Set the global suspend method table. + * @ops: Suspend operations to use. */ void suspend_set_ops(const struct platform_suspend_ops *ops) { @@ -58,11 +58,11 @@ bool valid_state(suspend_state_t state) } /** - * suspend_valid_only_mem - generic memory-only valid callback + * suspend_valid_only_mem - Generic memory-only valid callback. * - * Platform drivers that implement mem suspend only and only need - * to check for that in their .valid callback can use this instead - * of rolling their own .valid callback. + * Platform drivers that implement mem suspend only and only need to check for + * that in their .valid() callback can use this instead of rolling their own + * .valid() callback. */ int suspend_valid_only_mem(suspend_state_t state) { @@ -83,10 +83,11 @@ static int suspend_test(int level) } /** - * suspend_prepare - Do prep work before entering low-power state. + * suspend_prepare - Prepare for entering system sleep state. * - * This is common code that is called for each state that we're entering. - * Run suspend notifiers, allocate a console and stop all processes. + * Common code run for every system sleep state that can be entered (except for + * hibernation). Run suspend notifiers, allocate the "suspend" console and + * freeze processes. */ static int suspend_prepare(void) { @@ -131,9 +132,9 @@ void __attribute__ ((weak)) arch_suspend } /** - * suspend_enter - enter the desired system sleep state. - * @state: State to enter - * @wakeup: Returns information that suspend should not be entered again. + * suspend_enter - Make the system enter the given sleep state. + * @state: System sleep state to enter. + * @wakeup: Returns information that the sleep state should not be re-entered. * * This function should be called after devices have been suspended. */ @@ -199,9 +200,8 @@ static int suspend_enter(suspend_state_t } /** - * suspend_devices_and_enter - suspend devices and enter the desired system - * sleep state. - * @state: state to enter + * suspend_devices_and_enter - Suspend devices and enter system sleep state. + * @state: System sleep state to enter. */ int suspend_devices_and_enter(suspend_state_t state) { @@ -251,10 +251,10 @@ int suspend_devices_and_enter(suspend_st } /** - * suspend_finish - Do final work before exiting suspend sequence. + * suspend_finish - Clean up before finishing the suspend sequence. * - * Call platform code to clean up, restart processes, and free the - * console that we've allocated. This is not called for suspend-to-disk. + * Call platform code to clean up, restart processes, and free the console that + * we've allocated. This routine is not called for hibernation. */ static void suspend_finish(void) { @@ -265,14 +265,12 @@ static void suspend_finish(void) } /** - * enter_state - Do common work of entering low-power state. - * @state: pm_state structure for state we're entering. + * enter_state - Do common work needed to enter system sleep state. + * @state: System sleep state to enter. * - * Make sure we're the only ones trying to enter a sleep state. Fail - * if someone has beat us to it, since we don't want anything weird to - * happen when we wake up. - * Then, do the setup for suspend, enter the state, and cleaup (after - * we've woken up). + * Make sure that no one else is trying to put the system into a sleep state. + * Fail if that's not the case. Otherwise, prepare for system suspend, make the + * system enter the given sleep state and clean up after wakeup. */ int enter_state(suspend_state_t state) { @@ -310,11 +308,11 @@ int enter_state(suspend_state_t state) } /** - * pm_suspend - Externally visible function for suspending system. - * @state: Enumerated value of state to enter. + * pm_suspend - Externally visible function for suspending the system. + * @state: System sleep state to enter. * - * Determine whether or not value is within range, get state - * structure, and enter (above). + * Check if the value of @state represents one of the supported states, + * execute enter_state() and update system suspend statistics. */ int pm_suspend(suspend_state_t state) { ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c 2012-02-11 23:04 ` [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c Rafael J. Wysocki @ 2012-02-12 14:49 ` Srivatsa S. Bhat 0 siblings, 0 replies; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-12 14:49 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > The kerneldoc comments in kernel/power/suspend.c are not formatted > in the same way and the quality of some of them is questionable. > Unify the formatting and improve the contents. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Regards, Srivatsa S. Bhat > kernel/power/suspend.c | 56 +++++++++++++++++++++++-------------------------- > 1 file changed, 27 insertions(+), 29 deletions(-) > > Index: linux/kernel/power/suspend.c > =================================================================== > --- linux.orig/kernel/power/suspend.c > +++ linux/kernel/power/suspend.c > @@ -37,8 +37,8 @@ const char *const pm_states[PM_SUSPEND_M > static const struct platform_suspend_ops *suspend_ops; > > /** > - * suspend_set_ops - Set the global suspend method table. > - * @ops: Pointer to ops structure. > + * suspend_set_ops - Set the global suspend method table. > + * @ops: Suspend operations to use. > */ > void suspend_set_ops(const struct platform_suspend_ops *ops) > { > @@ -58,11 +58,11 @@ bool valid_state(suspend_state_t state) > } > > /** > - * suspend_valid_only_mem - generic memory-only valid callback > + * suspend_valid_only_mem - Generic memory-only valid callback. > * > - * Platform drivers that implement mem suspend only and only need > - * to check for that in their .valid callback can use this instead > - * of rolling their own .valid callback. > + * Platform drivers that implement mem suspend only and only need to check for > + * that in their .valid() callback can use this instead of rolling their own > + * .valid() callback. > */ > int suspend_valid_only_mem(suspend_state_t state) > { > @@ -83,10 +83,11 @@ static int suspend_test(int level) > } > > /** > - * suspend_prepare - Do prep work before entering low-power state. > + * suspend_prepare - Prepare for entering system sleep state. > * > - * This is common code that is called for each state that we're entering. > - * Run suspend notifiers, allocate a console and stop all processes. > + * Common code run for every system sleep state that can be entered (except for > + * hibernation). Run suspend notifiers, allocate the "suspend" console and > + * freeze processes. > */ > static int suspend_prepare(void) > { > @@ -131,9 +132,9 @@ void __attribute__ ((weak)) arch_suspend > } > > /** > - * suspend_enter - enter the desired system sleep state. > - * @state: State to enter > - * @wakeup: Returns information that suspend should not be entered again. > + * suspend_enter - Make the system enter the given sleep state. > + * @state: System sleep state to enter. > + * @wakeup: Returns information that the sleep state should not be re-entered. > * > * This function should be called after devices have been suspended. > */ > @@ -199,9 +200,8 @@ static int suspend_enter(suspend_state_t > } > > /** > - * suspend_devices_and_enter - suspend devices and enter the desired system > - * sleep state. > - * @state: state to enter > + * suspend_devices_and_enter - Suspend devices and enter system sleep state. > + * @state: System sleep state to enter. > */ > int suspend_devices_and_enter(suspend_state_t state) > { > @@ -251,10 +251,10 @@ int suspend_devices_and_enter(suspend_st > } > > /** > - * suspend_finish - Do final work before exiting suspend sequence. > + * suspend_finish - Clean up before finishing the suspend sequence. > * > - * Call platform code to clean up, restart processes, and free the > - * console that we've allocated. This is not called for suspend-to-disk. > + * Call platform code to clean up, restart processes, and free the console that > + * we've allocated. This routine is not called for hibernation. > */ > static void suspend_finish(void) > { > @@ -265,14 +265,12 @@ static void suspend_finish(void) > } > > /** > - * enter_state - Do common work of entering low-power state. > - * @state: pm_state structure for state we're entering. > + * enter_state - Do common work needed to enter system sleep state. > + * @state: System sleep state to enter. > * > - * Make sure we're the only ones trying to enter a sleep state. Fail > - * if someone has beat us to it, since we don't want anything weird to > - * happen when we wake up. > - * Then, do the setup for suspend, enter the state, and cleaup (after > - * we've woken up). > + * Make sure that no one else is trying to put the system into a sleep state. > + * Fail if that's not the case. Otherwise, prepare for system suspend, make the > + * system enter the given sleep state and clean up after wakeup. > */ > int enter_state(suspend_state_t state) > { > @@ -310,11 +308,11 @@ int enter_state(suspend_state_t state) > } > > /** > - * pm_suspend - Externally visible function for suspending system. > - * @state: Enumerated value of state to enter. > + * pm_suspend - Externally visible function for suspending the system. > + * @state: System sleep state to enter. > * > - * Determine whether or not value is within range, get state > - * structure, and enter (above). > + * Check if the value of @state represents one of the supported states, > + * execute enter_state() and update system suspend statistics. > */ > int pm_suspend(suspend_state_t state) > { > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-11 23:03 [PATCH 0/3] PM / Sleep: Suspend core code cleanups Rafael J. Wysocki 2012-02-11 23:04 ` [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c Rafael J. Wysocki @ 2012-02-11 23:04 ` Rafael J. Wysocki 2012-02-12 14:50 ` Srivatsa S. Bhat 2012-02-11 23:06 ` [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() Rafael J. Wysocki 2 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-11 23:04 UTC (permalink / raw) To: Linux PM list; +Cc: LKML, Srivatsa S. Bhat, Randy Dunlap From: Rafael J. Wysocki <rjw@sisk.pl> The enter_state() function in kernel/power/suspend.c should be static and state_store() in kernel/power/suspend.c should call pm_suspend() instead of it, so make that happen (which also reduces code duplication related to suspend statistics). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- kernel/power/main.c | 6 +----- kernel/power/power.h | 2 -- kernel/power/suspend.c | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) Index: linux/kernel/power/main.c =================================================================== --- linux.orig/kernel/power/main.c +++ linux/kernel/power/main.c @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec #ifdef CONFIG_SUSPEND for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) - break; - } - if (state < PM_SUSPEND_MAX && *s) { - error = enter_state(state); - suspend_stats_update(error); + error = pm_suspend(state); } #endif Index: linux/kernel/power/power.h =================================================================== --- linux.orig/kernel/power/power.h +++ linux/kernel/power/power.h @@ -177,13 +177,11 @@ extern const char *const pm_states[]; extern bool valid_state(suspend_state_t state); extern int suspend_devices_and_enter(suspend_state_t state); -extern int enter_state(suspend_state_t state); #else /* !CONFIG_SUSPEND */ static inline int suspend_devices_and_enter(suspend_state_t state) { return -ENOSYS; } -static inline int enter_state(suspend_state_t state) { return -ENOSYS; } static inline bool valid_state(suspend_state_t state) { return false; } #endif /* !CONFIG_SUSPEND */ Index: linux/kernel/power/suspend.c =================================================================== --- linux.orig/kernel/power/suspend.c +++ linux/kernel/power/suspend.c @@ -272,7 +272,7 @@ static void suspend_finish(void) * Fail if that's not the case. Otherwise, prepare for system suspend, make the * system enter the given sleep state and clean up after wakeup. */ -int enter_state(suspend_state_t state) +static int enter_state(suspend_state_t state) { int error; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-11 23:04 ` [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static Rafael J. Wysocki @ 2012-02-12 14:50 ` Srivatsa S. Bhat 2012-02-12 21:24 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-12 14:50 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > The enter_state() function in kernel/power/suspend.c should be > static and state_store() in kernel/power/suspend.c should call > pm_suspend() instead of it, so make that happen (which also reduces > code duplication related to suspend statistics). > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- > kernel/power/main.c | 6 +----- > kernel/power/power.h | 2 -- > kernel/power/suspend.c | 2 +- > 3 files changed, 2 insertions(+), 8 deletions(-) > > Index: linux/kernel/power/main.c > =================================================================== > --- linux.orig/kernel/power/main.c > +++ linux/kernel/power/main.c > @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec > #ifdef CONFIG_SUSPEND > for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > - break; > - } > - if (state < PM_SUSPEND_MAX && *s) { > - error = enter_state(state); > - suspend_stats_update(error); > + error = pm_suspend(state); This code will not stop after calling pm_suspend(). It will try more iterations in the for loop right? We can probably keep the 'for' loop as it is, and just replace the 'if' block following the 'for' loop by: error = pm_suspend(state); Regards, Srivatsa S. Bhat ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-12 14:50 ` Srivatsa S. Bhat @ 2012-02-12 21:24 ` Rafael J. Wysocki 2012-02-13 5:21 ` Srivatsa S. Bhat 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-12 21:24 UTC (permalink / raw) To: Srivatsa S. Bhat; +Cc: Linux PM list, LKML, Randy Dunlap On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: > On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > The enter_state() function in kernel/power/suspend.c should be > > static and state_store() in kernel/power/suspend.c should call > > pm_suspend() instead of it, so make that happen (which also reduces > > code duplication related to suspend statistics). > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > kernel/power/main.c | 6 +----- > > kernel/power/power.h | 2 -- > > kernel/power/suspend.c | 2 +- > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > Index: linux/kernel/power/main.c > > =================================================================== > > --- linux.orig/kernel/power/main.c > > +++ linux/kernel/power/main.c > > @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec > > #ifdef CONFIG_SUSPEND > > for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > > if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > > - break; > > - } > > - if (state < PM_SUSPEND_MAX && *s) { > > - error = enter_state(state); > > - suspend_stats_update(error); > > + error = pm_suspend(state); > > > This code will not stop after calling pm_suspend(). It will try more iterations > in the for loop right? Well, only one string in pm_states[] can be matched at a time, but I agree that it doesn't make sense to continue the loop after we've found a match. > We can probably keep the 'for' loop as it is, and just replace the 'if' block > following the 'for' loop by: error = pm_suspend(state); I think the patch below is correct. Thanks, Rafael --- From: Rafael J. Wysocki <rjw@sisk.pl> Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static The enter_state() function in kernel/power/suspend.c should be static and state_store() in kernel/power/suspend.c should call pm_suspend() instead of it, so make that happen (which also reduces code duplication related to suspend statistics). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- kernel/power/main.c | 8 +++----- kernel/power/power.h | 2 -- kernel/power/suspend.c | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) Index: linux/kernel/power/main.c =================================================================== --- linux.orig/kernel/power/main.c +++ linux/kernel/power/main.c @@ -291,12 +291,10 @@ static ssize_t state_store(struct kobjec #ifdef CONFIG_SUSPEND for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { - if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) + if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) { + error = pm_suspend(state); break; - } - if (state < PM_SUSPEND_MAX && *s) { - error = enter_state(state); - suspend_stats_update(error); + } } #endif Index: linux/kernel/power/power.h =================================================================== --- linux.orig/kernel/power/power.h +++ linux/kernel/power/power.h @@ -177,13 +177,11 @@ extern const char *const pm_states[]; extern bool valid_state(suspend_state_t state); extern int suspend_devices_and_enter(suspend_state_t state); -extern int enter_state(suspend_state_t state); #else /* !CONFIG_SUSPEND */ static inline int suspend_devices_and_enter(suspend_state_t state) { return -ENOSYS; } -static inline int enter_state(suspend_state_t state) { return -ENOSYS; } static inline bool valid_state(suspend_state_t state) { return false; } #endif /* !CONFIG_SUSPEND */ Index: linux/kernel/power/suspend.c =================================================================== --- linux.orig/kernel/power/suspend.c +++ linux/kernel/power/suspend.c @@ -272,7 +272,7 @@ static void suspend_finish(void) * Fail if that's not the case. Otherwise, prepare for system suspend, make the * system enter the given sleep state and clean up after wakeup. */ -int enter_state(suspend_state_t state) +static int enter_state(suspend_state_t state) { int error; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-12 21:24 ` Rafael J. Wysocki @ 2012-02-13 5:21 ` Srivatsa S. Bhat 2012-02-13 15:12 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-13 5:21 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote: > On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: >> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: >> >>> From: Rafael J. Wysocki <rjw@sisk.pl> >>> >>> The enter_state() function in kernel/power/suspend.c should be >>> static and state_store() in kernel/power/suspend.c should call >>> pm_suspend() instead of it, so make that happen (which also reduces >>> code duplication related to suspend statistics). >>> >>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >>> --- >>> kernel/power/main.c | 6 +----- >>> kernel/power/power.h | 2 -- >>> kernel/power/suspend.c | 2 +- >>> 3 files changed, 2 insertions(+), 8 deletions(-) >>> >>> Index: linux/kernel/power/main.c >>> =================================================================== >>> --- linux.orig/kernel/power/main.c >>> +++ linux/kernel/power/main.c >>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec >>> #ifdef CONFIG_SUSPEND >>> for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { >>> if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) >>> - break; >>> - } >>> - if (state < PM_SUSPEND_MAX && *s) { >>> - error = enter_state(state); >>> - suspend_stats_update(error); >>> + error = pm_suspend(state); >> >> >> This code will not stop after calling pm_suspend(). It will try more iterations >> in the for loop right? > > Well, only one string in pm_states[] can be matched at a time, but I agree that > it doesn't make sense to continue the loop after we've found a match. > >> We can probably keep the 'for' loop as it is, and just replace the 'if' block >> following the 'for' loop by: error = pm_suspend(state); > > I think the patch below is correct. > > Thanks, > Rafael > > --- > From: Rafael J. Wysocki <rjw@sisk.pl> > Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static > > The enter_state() function in kernel/power/suspend.c should be > static and state_store() in kernel/power/suspend.c should call > pm_suspend() instead of it, so make that happen (which also reduces > code duplication related to suspend statistics). > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- Yeah, this version of the patch will work fine. Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Regards, Srivatsa S. Bhat > kernel/power/main.c | 8 +++----- > kernel/power/power.h | 2 -- > kernel/power/suspend.c | 2 +- > 3 files changed, 4 insertions(+), 8 deletions(-) > > Index: linux/kernel/power/main.c > =================================================================== > --- linux.orig/kernel/power/main.c > +++ linux/kernel/power/main.c > @@ -291,12 +291,10 @@ static ssize_t state_store(struct kobjec > > #ifdef CONFIG_SUSPEND > for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > - if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > + if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) { > + error = pm_suspend(state); > break; > - } > - if (state < PM_SUSPEND_MAX && *s) { > - error = enter_state(state); > - suspend_stats_update(error); > + } > } > #endif > > Index: linux/kernel/power/power.h > =================================================================== > --- linux.orig/kernel/power/power.h > +++ linux/kernel/power/power.h > @@ -177,13 +177,11 @@ extern const char *const pm_states[]; > > extern bool valid_state(suspend_state_t state); > extern int suspend_devices_and_enter(suspend_state_t state); > -extern int enter_state(suspend_state_t state); > #else /* !CONFIG_SUSPEND */ > static inline int suspend_devices_and_enter(suspend_state_t state) > { > return -ENOSYS; > } > -static inline int enter_state(suspend_state_t state) { return -ENOSYS; } > static inline bool valid_state(suspend_state_t state) { return false; } > #endif /* !CONFIG_SUSPEND */ > > Index: linux/kernel/power/suspend.c > =================================================================== > --- linux.orig/kernel/power/suspend.c > +++ linux/kernel/power/suspend.c > @@ -272,7 +272,7 @@ static void suspend_finish(void) > * Fail if that's not the case. Otherwise, prepare for system suspend, make the > * system enter the given sleep state and clean up after wakeup. > */ > -int enter_state(suspend_state_t state) > +static int enter_state(suspend_state_t state) > { > int error; > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-13 5:21 ` Srivatsa S. Bhat @ 2012-02-13 15:12 ` Rafael J. Wysocki 2012-02-13 15:34 ` Srivatsa S. Bhat 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-13 15:12 UTC (permalink / raw) To: Srivatsa S. Bhat; +Cc: Linux PM list, LKML, Randy Dunlap On Monday, February 13, 2012, Srivatsa S. Bhat wrote: > On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote: > > > On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: > >> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: > >> > >>> From: Rafael J. Wysocki <rjw@sisk.pl> > >>> > >>> The enter_state() function in kernel/power/suspend.c should be > >>> static and state_store() in kernel/power/suspend.c should call > >>> pm_suspend() instead of it, so make that happen (which also reduces > >>> code duplication related to suspend statistics). > >>> > >>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > >>> --- > >>> kernel/power/main.c | 6 +----- > >>> kernel/power/power.h | 2 -- > >>> kernel/power/suspend.c | 2 +- > >>> 3 files changed, 2 insertions(+), 8 deletions(-) > >>> > >>> Index: linux/kernel/power/main.c > >>> =================================================================== > >>> --- linux.orig/kernel/power/main.c > >>> +++ linux/kernel/power/main.c > >>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec > >>> #ifdef CONFIG_SUSPEND > >>> for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > >>> if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > >>> - break; > >>> - } > >>> - if (state < PM_SUSPEND_MAX && *s) { > >>> - error = enter_state(state); > >>> - suspend_stats_update(error); > >>> + error = pm_suspend(state); > >> > >> > >> This code will not stop after calling pm_suspend(). It will try more iterations > >> in the for loop right? > > > > Well, only one string in pm_states[] can be matched at a time, but I agree that > > it doesn't make sense to continue the loop after we've found a match. > > > >> We can probably keep the 'for' loop as it is, and just replace the 'if' block > >> following the 'for' loop by: error = pm_suspend(state); > > > > I think the patch below is correct. > > > > Thanks, > > Rafael > > > > --- > > From: Rafael J. Wysocki <rjw@sisk.pl> > > Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static > > > > The enter_state() function in kernel/power/suspend.c should be > > static and state_store() in kernel/power/suspend.c should call > > pm_suspend() instead of it, so make that happen (which also reduces > > code duplication related to suspend statistics). > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > > Yeah, this version of the patch will work fine. > > Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Thanks! I wonder if that should be Reviewed-by, tough? You evidently have reviewed the patch (actually, all three of them). Rafael > > kernel/power/main.c | 8 +++----- > > kernel/power/power.h | 2 -- > > kernel/power/suspend.c | 2 +- > > 3 files changed, 4 insertions(+), 8 deletions(-) > > > > Index: linux/kernel/power/main.c > > =================================================================== > > --- linux.orig/kernel/power/main.c > > +++ linux/kernel/power/main.c > > @@ -291,12 +291,10 @@ static ssize_t state_store(struct kobjec > > > > #ifdef CONFIG_SUSPEND > > for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > > - if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > > + if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) { > > + error = pm_suspend(state); > > break; > > - } > > - if (state < PM_SUSPEND_MAX && *s) { > > - error = enter_state(state); > > - suspend_stats_update(error); > > + } > > } > > #endif > > > > Index: linux/kernel/power/power.h > > =================================================================== > > --- linux.orig/kernel/power/power.h > > +++ linux/kernel/power/power.h > > @@ -177,13 +177,11 @@ extern const char *const pm_states[]; > > > > extern bool valid_state(suspend_state_t state); > > extern int suspend_devices_and_enter(suspend_state_t state); > > -extern int enter_state(suspend_state_t state); > > #else /* !CONFIG_SUSPEND */ > > static inline int suspend_devices_and_enter(suspend_state_t state) > > { > > return -ENOSYS; > > } > > -static inline int enter_state(suspend_state_t state) { return -ENOSYS; } > > static inline bool valid_state(suspend_state_t state) { return false; } > > #endif /* !CONFIG_SUSPEND */ > > > > Index: linux/kernel/power/suspend.c > > =================================================================== > > --- linux.orig/kernel/power/suspend.c > > +++ linux/kernel/power/suspend.c > > @@ -272,7 +272,7 @@ static void suspend_finish(void) > > * Fail if that's not the case. Otherwise, prepare for system suspend, make the > > * system enter the given sleep state and clean up after wakeup. > > */ > > -int enter_state(suspend_state_t state) > > +static int enter_state(suspend_state_t state) > > { > > int error; > > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-13 15:12 ` Rafael J. Wysocki @ 2012-02-13 15:34 ` Srivatsa S. Bhat 2012-02-13 15:53 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-13 15:34 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/13/2012 08:42 PM, Rafael J. Wysocki wrote: > On Monday, February 13, 2012, Srivatsa S. Bhat wrote: >> On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote: >> >>> On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: >>>> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: >>>> >>>>> From: Rafael J. Wysocki <rjw@sisk.pl> >>>>> >>>>> The enter_state() function in kernel/power/suspend.c should be >>>>> static and state_store() in kernel/power/suspend.c should call >>>>> pm_suspend() instead of it, so make that happen (which also reduces >>>>> code duplication related to suspend statistics). >>>>> >>>>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >>>>> --- >>>>> kernel/power/main.c | 6 +----- >>>>> kernel/power/power.h | 2 -- >>>>> kernel/power/suspend.c | 2 +- >>>>> 3 files changed, 2 insertions(+), 8 deletions(-) >>>>> >>>>> Index: linux/kernel/power/main.c >>>>> =================================================================== >>>>> --- linux.orig/kernel/power/main.c >>>>> +++ linux/kernel/power/main.c >>>>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec >>>>> #ifdef CONFIG_SUSPEND >>>>> for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { >>>>> if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) >>>>> - break; >>>>> - } >>>>> - if (state < PM_SUSPEND_MAX && *s) { >>>>> - error = enter_state(state); >>>>> - suspend_stats_update(error); >>>>> + error = pm_suspend(state); >>>> >>>> >>>> This code will not stop after calling pm_suspend(). It will try more iterations >>>> in the for loop right? >>> >>> Well, only one string in pm_states[] can be matched at a time, but I agree that >>> it doesn't make sense to continue the loop after we've found a match. >>> >>>> We can probably keep the 'for' loop as it is, and just replace the 'if' block >>>> following the 'for' loop by: error = pm_suspend(state); >>> >>> I think the patch below is correct. >>> >>> Thanks, >>> Rafael >>> >>> --- >>> From: Rafael J. Wysocki <rjw@sisk.pl> >>> Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static >>> >>> The enter_state() function in kernel/power/suspend.c should be >>> static and state_store() in kernel/power/suspend.c should call >>> pm_suspend() instead of it, so make that happen (which also reduces >>> code duplication related to suspend statistics). >>> >>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >>> --- >> >> >> Yeah, this version of the patch will work fine. >> >> Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > > Thanks! > > I wonder if that should be Reviewed-by, tough? You evidently have reviewed > the patch (actually, all three of them). > Anything is fine :-) It is not very clear to me when to use Reviewed-by and when to use Acked-by.. so I randomly chose one of them.. :-) But please enlighten me as to when to use which one, so that in the future, I can use the right one :-) Regards, Srivatsa S. Bhat ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-13 15:34 ` Srivatsa S. Bhat @ 2012-02-13 15:53 ` Rafael J. Wysocki 2012-02-13 16:41 ` Srivatsa S. Bhat 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-13 15:53 UTC (permalink / raw) To: Srivatsa S. Bhat; +Cc: Linux PM list, LKML, Randy Dunlap On Monday, February 13, 2012, Srivatsa S. Bhat wrote: > On 02/13/2012 08:42 PM, Rafael J. Wysocki wrote: > > > On Monday, February 13, 2012, Srivatsa S. Bhat wrote: > >> On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote: > >> > >>> On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: > >>>> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: > >>>> > >>>>> From: Rafael J. Wysocki <rjw@sisk.pl> > >>>>> > >>>>> The enter_state() function in kernel/power/suspend.c should be > >>>>> static and state_store() in kernel/power/suspend.c should call > >>>>> pm_suspend() instead of it, so make that happen (which also reduces > >>>>> code duplication related to suspend statistics). > >>>>> > >>>>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > >>>>> --- > >>>>> kernel/power/main.c | 6 +----- > >>>>> kernel/power/power.h | 2 -- > >>>>> kernel/power/suspend.c | 2 +- > >>>>> 3 files changed, 2 insertions(+), 8 deletions(-) > >>>>> > >>>>> Index: linux/kernel/power/main.c > >>>>> =================================================================== > >>>>> --- linux.orig/kernel/power/main.c > >>>>> +++ linux/kernel/power/main.c > >>>>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec > >>>>> #ifdef CONFIG_SUSPEND > >>>>> for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { > >>>>> if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) > >>>>> - break; > >>>>> - } > >>>>> - if (state < PM_SUSPEND_MAX && *s) { > >>>>> - error = enter_state(state); > >>>>> - suspend_stats_update(error); > >>>>> + error = pm_suspend(state); > >>>> > >>>> > >>>> This code will not stop after calling pm_suspend(). It will try more iterations > >>>> in the for loop right? > >>> > >>> Well, only one string in pm_states[] can be matched at a time, but I agree that > >>> it doesn't make sense to continue the loop after we've found a match. > >>> > >>>> We can probably keep the 'for' loop as it is, and just replace the 'if' block > >>>> following the 'for' loop by: error = pm_suspend(state); > >>> > >>> I think the patch below is correct. > >>> > >>> Thanks, > >>> Rafael > >>> > >>> --- > >>> From: Rafael J. Wysocki <rjw@sisk.pl> > >>> Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static > >>> > >>> The enter_state() function in kernel/power/suspend.c should be > >>> static and state_store() in kernel/power/suspend.c should call > >>> pm_suspend() instead of it, so make that happen (which also reduces > >>> code duplication related to suspend statistics). > >>> > >>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > >>> --- > >> > >> > >> Yeah, this version of the patch will work fine. > >> > >> Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > > > > Thanks! > > > > I wonder if that should be Reviewed-by, tough? You evidently have reviewed > > the patch (actually, all three of them). > > > > > Anything is fine :-) It is not very clear to me when to use Reviewed-by and > when to use Acked-by.. so I randomly chose one of them.. :-) > But please enlighten me as to when to use which one, so that in the future, I > can use the right one :-) "Acked-by" means, roughly, "I have no objection" or "looks good to me", depending on who's saying that, but it doesn't imply that you've had more than a cursory look at the patch in question. "Reviewied-by", in contrast, means something like "I have reviewed the patch in detail and haven't found anything wrong in it" (which obviously means that you have no objection too, because otherwise you'd have found _something_ wrong in the patch). So, "Acked-by" from anyone other than the relevant maintainer is just an "I'm for" declaration, while "Reviewied-by" from _anyone_ carries some actual weight. Thanks, Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static 2012-02-13 15:53 ` Rafael J. Wysocki @ 2012-02-13 16:41 ` Srivatsa S. Bhat 0 siblings, 0 replies; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-13 16:41 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/13/2012 09:23 PM, Rafael J. Wysocki wrote: > On Monday, February 13, 2012, Srivatsa S. Bhat wrote: >> On 02/13/2012 08:42 PM, Rafael J. Wysocki wrote: >> >>> On Monday, February 13, 2012, Srivatsa S. Bhat wrote: >>>> On 02/13/2012 02:54 AM, Rafael J. Wysocki wrote: >>>> >>>>> On Sunday, February 12, 2012, Srivatsa S. Bhat wrote: >>>>>> On 02/12/2012 04:34 AM, Rafael J. Wysocki wrote: >>>>>> >>>>>>> From: Rafael J. Wysocki <rjw@sisk.pl> >>>>>>> >>>>>>> The enter_state() function in kernel/power/suspend.c should be >>>>>>> static and state_store() in kernel/power/suspend.c should call >>>>>>> pm_suspend() instead of it, so make that happen (which also reduces >>>>>>> code duplication related to suspend statistics). >>>>>>> >>>>>>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >>>>>>> --- >>>>>>> kernel/power/main.c | 6 +----- >>>>>>> kernel/power/power.h | 2 -- >>>>>>> kernel/power/suspend.c | 2 +- >>>>>>> 3 files changed, 2 insertions(+), 8 deletions(-) >>>>>>> >>>>>>> Index: linux/kernel/power/main.c >>>>>>> =================================================================== >>>>>>> --- linux.orig/kernel/power/main.c >>>>>>> +++ linux/kernel/power/main.c >>>>>>> @@ -292,11 +292,7 @@ static ssize_t state_store(struct kobjec >>>>>>> #ifdef CONFIG_SUSPEND >>>>>>> for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) { >>>>>>> if (*s && len == strlen(*s) && !strncmp(buf, *s, len)) >>>>>>> - break; >>>>>>> - } >>>>>>> - if (state < PM_SUSPEND_MAX && *s) { >>>>>>> - error = enter_state(state); >>>>>>> - suspend_stats_update(error); >>>>>>> + error = pm_suspend(state); >>>>>> >>>>>> >>>>>> This code will not stop after calling pm_suspend(). It will try more iterations >>>>>> in the for loop right? >>>>> >>>>> Well, only one string in pm_states[] can be matched at a time, but I agree that >>>>> it doesn't make sense to continue the loop after we've found a match. >>>>> >>>>>> We can probably keep the 'for' loop as it is, and just replace the 'if' block >>>>>> following the 'for' loop by: error = pm_suspend(state); >>>>> >>>>> I think the patch below is correct. >>>>> >>>>> Thanks, >>>>> Rafael >>>>> >>>>> --- >>>>> From: Rafael J. Wysocki <rjw@sisk.pl> >>>>> Subject: PM / Sleep: Make enter_state() in kernel/power/suspend.c static >>>>> >>>>> The enter_state() function in kernel/power/suspend.c should be >>>>> static and state_store() in kernel/power/suspend.c should call >>>>> pm_suspend() instead of it, so make that happen (which also reduces >>>>> code duplication related to suspend statistics). >>>>> >>>>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> >>>>> --- >>>> >>>> >>>> Yeah, this version of the patch will work fine. >>>> >>>> Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> >>> >>> Thanks! >>> >>> I wonder if that should be Reviewed-by, tough? You evidently have reviewed >>> the patch (actually, all three of them). >>> >> >> >> Anything is fine :-) It is not very clear to me when to use Reviewed-by and >> when to use Acked-by.. so I randomly chose one of them.. :-) >> But please enlighten me as to when to use which one, so that in the future, I >> can use the right one :-) > > "Acked-by" means, roughly, "I have no objection" or "looks good to me", > depending on who's saying that, but it doesn't imply that you've had more than > a cursory look at the patch in question. "Reviewied-by", in contrast, means > something like "I have reviewed the patch in detail and haven't found anything > wrong in it" (which obviously means that you have no objection too, because > otherwise you'd have found _something_ wrong in the patch). > > So, "Acked-by" from anyone other than the relevant maintainer is just an > "I'm for" declaration, while "Reviewied-by" from _anyone_ carries some actual > weight. > Wow! Nice :-) Thanks for the great explanation! I'll keep this in mind from the next time onwards :-) Regards, Srivatsa S. Bhat ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() 2012-02-11 23:03 [PATCH 0/3] PM / Sleep: Suspend core code cleanups Rafael J. Wysocki 2012-02-11 23:04 ` [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c Rafael J. Wysocki 2012-02-11 23:04 ` [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static Rafael J. Wysocki @ 2012-02-11 23:06 ` Rafael J. Wysocki 2012-02-12 14:50 ` Srivatsa S. Bhat 2 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2012-02-11 23:06 UTC (permalink / raw) To: Linux PM list; +Cc: LKML, Srivatsa S. Bhat, Randy Dunlap From: Rafael J. Wysocki <rjw@sisk.pl> Since suspend_stats_update() is only called from pm_suspend(), move its code directly into that function and remove the static inline definition from include/linux/suspend.h. Clean up pm_suspend() in the process. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- include/linux/suspend.h | 16 ---------------- kernel/power/suspend.c | 18 ++++++++++++------ 2 files changed, 12 insertions(+), 22 deletions(-) Index: linux/include/linux/suspend.h =================================================================== --- linux.orig/include/linux/suspend.h +++ linux/include/linux/suspend.h @@ -95,22 +95,6 @@ static inline void dpm_save_failed_step( } /** - * suspend_stats_update - Update success/failure statistics of suspend-to-ram - * - * @error: Value returned by enter_state() function - */ -static inline void suspend_stats_update(int error) -{ - if (error) { - suspend_stats.fail++; - dpm_save_failed_errno(error); - } else { - suspend_stats.success++; - } -} - - -/** * struct platform_suspend_ops - Callbacks for managing platform dependent * system sleep states. * Index: linux/kernel/power/suspend.c =================================================================== --- linux.orig/kernel/power/suspend.c +++ linux/kernel/power/suspend.c @@ -316,12 +316,18 @@ static int enter_state(suspend_state_t s */ int pm_suspend(suspend_state_t state) { - int ret; - if (state > PM_SUSPEND_ON && state < PM_SUSPEND_MAX) { - ret = enter_state(state); - suspend_stats_update(ret); - return ret; + int error; + + if (state <= PM_SUSPEND_ON || state >= PM_SUSPEND_MAX) + return -EINVAL; + + error = enter_state(state); + if (error) { + suspend_stats.fail++; + dpm_save_failed_errno(error); + } else { + suspend_stats.success++; } - return -EINVAL; + return error; } EXPORT_SYMBOL(pm_suspend); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() 2012-02-11 23:06 ` [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() Rafael J. Wysocki @ 2012-02-12 14:50 ` Srivatsa S. Bhat 0 siblings, 0 replies; 13+ messages in thread From: Srivatsa S. Bhat @ 2012-02-12 14:50 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux PM list, LKML, Randy Dunlap On 02/12/2012 04:36 AM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > Since suspend_stats_update() is only called from pm_suspend(), > move its code directly into that function and remove the static > inline definition from include/linux/suspend.h. Clean up > pm_suspend() in the process. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Regards, Srivatsa S. Bhat > include/linux/suspend.h | 16 ---------------- > kernel/power/suspend.c | 18 ++++++++++++------ > 2 files changed, 12 insertions(+), 22 deletions(-) > > Index: linux/include/linux/suspend.h > =================================================================== > --- linux.orig/include/linux/suspend.h > +++ linux/include/linux/suspend.h > @@ -95,22 +95,6 @@ static inline void dpm_save_failed_step( > } > > /** > - * suspend_stats_update - Update success/failure statistics of suspend-to-ram > - * > - * @error: Value returned by enter_state() function > - */ > -static inline void suspend_stats_update(int error) > -{ > - if (error) { > - suspend_stats.fail++; > - dpm_save_failed_errno(error); > - } else { > - suspend_stats.success++; > - } > -} > - > - > -/** > * struct platform_suspend_ops - Callbacks for managing platform dependent > * system sleep states. > * > Index: linux/kernel/power/suspend.c > =================================================================== > --- linux.orig/kernel/power/suspend.c > +++ linux/kernel/power/suspend.c > @@ -316,12 +316,18 @@ static int enter_state(suspend_state_t s > */ > int pm_suspend(suspend_state_t state) > { > - int ret; > - if (state > PM_SUSPEND_ON && state < PM_SUSPEND_MAX) { > - ret = enter_state(state); > - suspend_stats_update(ret); > - return ret; > + int error; > + > + if (state <= PM_SUSPEND_ON || state >= PM_SUSPEND_MAX) > + return -EINVAL; > + > + error = enter_state(state); > + if (error) { > + suspend_stats.fail++; > + dpm_save_failed_errno(error); > + } else { > + suspend_stats.success++; > } > - return -EINVAL; > + return error; > } > EXPORT_SYMBOL(pm_suspend); > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-02-13 16:42 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-02-11 23:03 [PATCH 0/3] PM / Sleep: Suspend core code cleanups Rafael J. Wysocki 2012-02-11 23:04 ` [PATCH 1/3] PM / Sleep: Unify kerneldoc comments in kernel/power/suspend.c Rafael J. Wysocki 2012-02-12 14:49 ` Srivatsa S. Bhat 2012-02-11 23:04 ` [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static Rafael J. Wysocki 2012-02-12 14:50 ` Srivatsa S. Bhat 2012-02-12 21:24 ` Rafael J. Wysocki 2012-02-13 5:21 ` Srivatsa S. Bhat 2012-02-13 15:12 ` Rafael J. Wysocki 2012-02-13 15:34 ` Srivatsa S. Bhat 2012-02-13 15:53 ` Rafael J. Wysocki 2012-02-13 16:41 ` Srivatsa S. Bhat 2012-02-11 23:06 ` [PATCH 3/3] PM / Sleep: Drop suspend_stats_update() Rafael J. Wysocki 2012-02-12 14:50 ` Srivatsa S. Bhat
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).