From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Linux PM list <linux-pm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Randy Dunlap <rdunlap@xenotime.net>
Subject: Re: [PATCH 2/3] PM / Sleep: Make enter_state() in kernel/power/suspend.c static
Date: Mon, 13 Feb 2012 10:51:36 +0530 [thread overview]
Message-ID: <4F389DE0.6020904@linux.vnet.ibm.com> (raw)
In-Reply-To: <201202122224.14640.rjw@sisk.pl>
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;
>
>
next prev parent reply other threads:[~2012-02-13 5:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
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=4F389DE0.6020904@linux.vnet.ibm.com \
--to=srivatsa.bhat@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rdunlap@xenotime.net \
--cc=rjw@sisk.pl \
/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 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).