| Message ID | 1295088221-4956-2-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Brad Figg |
| Headers | show |
Seems to do exactly what is described (plus a bit of cleanup). And given the soak testing the potential for regression should be low. Colin, do you happen to know when this was introduced? Do we need to ask Rafael to send it to stable for more than .35? Stefan Acked-by: Stefan Bader <stefan.bader@canonical.com> On 01/15/2011 11:43 AM, Colin King wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > BugLink: http://bugs.launchpad.net/bugs/703228 > > Some hibernation and suspend routines can be merged, which reduces > the complexity of code a bit. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > Signed-off-by: Len Brown <len.brown@intel.com> > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/acpi/sleep.c | 42 ++++++++++++++++-------------------------- > 1 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > index 4882bc1..e0c08ab 100644 > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -119,6 +119,16 @@ static int acpi_pm_freeze(void) > } > > /** > + * acpi_pre_suspend - Enable wakeup devices, "freeze" EC and save NVS. > + */ > +static int acpi_pm_pre_suspend(void) > +{ > + acpi_pm_freeze(); > + suspend_nvs_save(); > + return 0; > +} > + > +/** > * __acpi_pm_prepare - Prepare the platform to enter the target state. > * > * If necessary, set the firmware waking vector and do arch-specific > @@ -127,11 +137,9 @@ static int acpi_pm_freeze(void) > static int __acpi_pm_prepare(void) > { > int error = acpi_sleep_prepare(acpi_target_sleep_state); > - > - suspend_nvs_save(); > - > if (error) > acpi_target_sleep_state = ACPI_STATE_S0; > + > return error; > } > > @@ -142,9 +150,8 @@ static int __acpi_pm_prepare(void) > static int acpi_pm_prepare(void) > { > int error = __acpi_pm_prepare(); > - > if (!error) > - acpi_pm_freeze(); > + acpi_pm_pre_suspend(); > > return error; > } > @@ -336,9 +343,9 @@ static struct platform_suspend_ops acpi_suspend_ops = { > static int acpi_suspend_begin_old(suspend_state_t pm_state) > { > int error = acpi_suspend_begin(pm_state); > - > if (!error) > error = __acpi_pm_prepare(); > + > return error; > } > > @@ -349,7 +356,7 @@ static int acpi_suspend_begin_old(suspend_state_t pm_state) > static struct platform_suspend_ops acpi_suspend_ops_old = { > .valid = acpi_suspend_state_valid, > .begin = acpi_suspend_begin_old, > - .prepare_late = acpi_pm_freeze, > + .prepare_late = acpi_pm_pre_suspend, > .enter = acpi_suspend_enter, > .wake = acpi_suspend_finish, > .end = acpi_pm_end, > @@ -445,16 +452,6 @@ static int acpi_hibernation_begin(void) > return error; > } > > -static int acpi_hibernation_pre_snapshot(void) > -{ > - int error = acpi_pm_prepare(); > - > - if (!error) > - suspend_nvs_save(); > - > - return error; > -} > - > static int acpi_hibernation_enter(void) > { > acpi_status status = AE_OK; > @@ -503,7 +500,7 @@ static void acpi_pm_thaw(void) > static struct platform_hibernation_ops acpi_hibernation_ops = { > .begin = acpi_hibernation_begin, > .end = acpi_pm_end, > - .pre_snapshot = acpi_hibernation_pre_snapshot, > + .pre_snapshot = acpi_pm_prepare, > .finish = acpi_pm_finish, > .prepare = acpi_pm_prepare, > .enter = acpi_hibernation_enter, > @@ -539,13 +536,6 @@ static int acpi_hibernation_begin_old(void) > return error; > } > > -static int acpi_hibernation_pre_snapshot_old(void) > -{ > - acpi_pm_freeze(); > - suspend_nvs_save(); > - return 0; > -} > - > /* > * The following callbacks are used if the pre-ACPI 2.0 suspend ordering has > * been requested. > @@ -553,7 +543,7 @@ static int acpi_hibernation_pre_snapshot_old(void) > static struct platform_hibernation_ops acpi_hibernation_ops_old = { > .begin = acpi_hibernation_begin_old, > .end = acpi_pm_end, > - .pre_snapshot = acpi_hibernation_pre_snapshot_old, > + .pre_snapshot = acpi_pm_pre_suspend, > .prepare = acpi_pm_freeze, > .finish = acpi_pm_finish, > .enter = acpi_hibernation_enter,
On Mon, 2011-01-17 at 10:03 +0100, Stefan Bader wrote: > Seems to do exactly what is described (plus a bit of cleanup). And given the > soak testing the potential for regression should be low. Colin, do you happen to > know when this was introduced? Well, it seems to me that a lot of re-working on the kernel post May 28th could possible have introduced the bug, see commits dd4c4f17d722ffeb2515bf781400675a30fcead7 onwards, so I suspect pre-Maverick to be OK, but it's worth checking manually, when I've got some free cycles today. > Do we need to ask Rafael to send it to stable for > more than .35? Probably, if it applies correctly to .36, .37 > > Stefan > > Acked-by: Stefan Bader <stefan.bader@canonical.com> > > On 01/15/2011 11:43 AM, Colin King wrote: > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > BugLink: http://bugs.launchpad.net/bugs/703228 > > > > Some hibernation and suspend routines can be merged, which reduces > > the complexity of code a bit. > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > Signed-off-by: Len Brown <len.brown@intel.com> > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > --- > > drivers/acpi/sleep.c | 42 ++++++++++++++++-------------------------- > > 1 files changed, 16 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > > index 4882bc1..e0c08ab 100644 > > --- a/drivers/acpi/sleep.c > > +++ b/drivers/acpi/sleep.c > > @@ -119,6 +119,16 @@ static int acpi_pm_freeze(void) > > } > > > > /** > > + * acpi_pre_suspend - Enable wakeup devices, "freeze" EC and save NVS. > > + */ > > +static int acpi_pm_pre_suspend(void) > > +{ > > + acpi_pm_freeze(); > > + suspend_nvs_save(); > > + return 0; > > +} > > + > > +/** > > * __acpi_pm_prepare - Prepare the platform to enter the target state. > > * > > * If necessary, set the firmware waking vector and do arch-specific > > @@ -127,11 +137,9 @@ static int acpi_pm_freeze(void) > > static int __acpi_pm_prepare(void) > > { > > int error = acpi_sleep_prepare(acpi_target_sleep_state); > > - > > - suspend_nvs_save(); > > - > > if (error) > > acpi_target_sleep_state = ACPI_STATE_S0; > > + > > return error; > > } > > > > @@ -142,9 +150,8 @@ static int __acpi_pm_prepare(void) > > static int acpi_pm_prepare(void) > > { > > int error = __acpi_pm_prepare(); > > - > > if (!error) > > - acpi_pm_freeze(); > > + acpi_pm_pre_suspend(); > > > > return error; > > } > > @@ -336,9 +343,9 @@ static struct platform_suspend_ops acpi_suspend_ops = { > > static int acpi_suspend_begin_old(suspend_state_t pm_state) > > { > > int error = acpi_suspend_begin(pm_state); > > - > > if (!error) > > error = __acpi_pm_prepare(); > > + > > return error; > > } > > > > @@ -349,7 +356,7 @@ static int acpi_suspend_begin_old(suspend_state_t pm_state) > > static struct platform_suspend_ops acpi_suspend_ops_old = { > > .valid = acpi_suspend_state_valid, > > .begin = acpi_suspend_begin_old, > > - .prepare_late = acpi_pm_freeze, > > + .prepare_late = acpi_pm_pre_suspend, > > .enter = acpi_suspend_enter, > > .wake = acpi_suspend_finish, > > .end = acpi_pm_end, > > @@ -445,16 +452,6 @@ static int acpi_hibernation_begin(void) > > return error; > > } > > > > -static int acpi_hibernation_pre_snapshot(void) > > -{ > > - int error = acpi_pm_prepare(); > > - > > - if (!error) > > - suspend_nvs_save(); > > - > > - return error; > > -} > > - > > static int acpi_hibernation_enter(void) > > { > > acpi_status status = AE_OK; > > @@ -503,7 +500,7 @@ static void acpi_pm_thaw(void) > > static struct platform_hibernation_ops acpi_hibernation_ops = { > > .begin = acpi_hibernation_begin, > > .end = acpi_pm_end, > > - .pre_snapshot = acpi_hibernation_pre_snapshot, > > + .pre_snapshot = acpi_pm_prepare, > > .finish = acpi_pm_finish, > > .prepare = acpi_pm_prepare, > > .enter = acpi_hibernation_enter, > > @@ -539,13 +536,6 @@ static int acpi_hibernation_begin_old(void) > > return error; > > } > > > > -static int acpi_hibernation_pre_snapshot_old(void) > > -{ > > - acpi_pm_freeze(); > > - suspend_nvs_save(); > > - return 0; > > -} > > - > > /* > > * The following callbacks are used if the pre-ACPI 2.0 suspend ordering has > > * been requested. > > @@ -553,7 +543,7 @@ static int acpi_hibernation_pre_snapshot_old(void) > > static struct platform_hibernation_ops acpi_hibernation_ops_old = { > > .begin = acpi_hibernation_begin_old, > > .end = acpi_pm_end, > > - .pre_snapshot = acpi_hibernation_pre_snapshot_old, > > + .pre_snapshot = acpi_pm_pre_suspend, > > .prepare = acpi_pm_freeze, > > .finish = acpi_pm_finish, > > .enter = acpi_hibernation_enter, > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 01/15/2011 02:43 AM, Colin King wrote: > From: Rafael J. Wysocki<rjw@sisk.pl> > > BugLink: http://bugs.launchpad.net/bugs/703228 > > Some hibernation and suspend routines can be merged, which reduces > the complexity of code a bit. > > Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl> > Signed-off-by: Len Brown<len.brown@intel.com> > Signed-off-by: Colin Ian King<colin.king@canonical.com> > --- > drivers/acpi/sleep.c | 42 ++++++++++++++++-------------------------- > 1 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c > index 4882bc1..e0c08ab 100644 > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -119,6 +119,16 @@ static int acpi_pm_freeze(void) > } > > /** > + * acpi_pre_suspend - Enable wakeup devices, "freeze" EC and save NVS. > + */ > +static int acpi_pm_pre_suspend(void) > +{ > + acpi_pm_freeze(); > + suspend_nvs_save(); > + return 0; > +} > + > +/** > * __acpi_pm_prepare - Prepare the platform to enter the target state. > * > * If necessary, set the firmware waking vector and do arch-specific > @@ -127,11 +137,9 @@ static int acpi_pm_freeze(void) > static int __acpi_pm_prepare(void) > { > int error = acpi_sleep_prepare(acpi_target_sleep_state); > - > - suspend_nvs_save(); > - > if (error) > acpi_target_sleep_state = ACPI_STATE_S0; > + > return error; > } > > @@ -142,9 +150,8 @@ static int __acpi_pm_prepare(void) > static int acpi_pm_prepare(void) > { > int error = __acpi_pm_prepare(); > - > if (!error) > - acpi_pm_freeze(); > + acpi_pm_pre_suspend(); > > return error; > } > @@ -336,9 +343,9 @@ static struct platform_suspend_ops acpi_suspend_ops = { > static int acpi_suspend_begin_old(suspend_state_t pm_state) > { > int error = acpi_suspend_begin(pm_state); > - > if (!error) > error = __acpi_pm_prepare(); > + > return error; > } > > @@ -349,7 +356,7 @@ static int acpi_suspend_begin_old(suspend_state_t pm_state) > static struct platform_suspend_ops acpi_suspend_ops_old = { > .valid = acpi_suspend_state_valid, > .begin = acpi_suspend_begin_old, > - .prepare_late = acpi_pm_freeze, > + .prepare_late = acpi_pm_pre_suspend, > .enter = acpi_suspend_enter, > .wake = acpi_suspend_finish, > .end = acpi_pm_end, > @@ -445,16 +452,6 @@ static int acpi_hibernation_begin(void) > return error; > } > > -static int acpi_hibernation_pre_snapshot(void) > -{ > - int error = acpi_pm_prepare(); > - > - if (!error) > - suspend_nvs_save(); > - > - return error; > -} > - > static int acpi_hibernation_enter(void) > { > acpi_status status = AE_OK; > @@ -503,7 +500,7 @@ static void acpi_pm_thaw(void) > static struct platform_hibernation_ops acpi_hibernation_ops = { > .begin = acpi_hibernation_begin, > .end = acpi_pm_end, > - .pre_snapshot = acpi_hibernation_pre_snapshot, > + .pre_snapshot = acpi_pm_prepare, > .finish = acpi_pm_finish, > .prepare = acpi_pm_prepare, > .enter = acpi_hibernation_enter, > @@ -539,13 +536,6 @@ static int acpi_hibernation_begin_old(void) > return error; > } > > -static int acpi_hibernation_pre_snapshot_old(void) > -{ > - acpi_pm_freeze(); > - suspend_nvs_save(); > - return 0; > -} > - > /* > * The following callbacks are used if the pre-ACPI 2.0 suspend ordering has > * been requested. > @@ -553,7 +543,7 @@ static int acpi_hibernation_pre_snapshot_old(void) > static struct platform_hibernation_ops acpi_hibernation_ops_old = { > .begin = acpi_hibernation_begin_old, > .end = acpi_pm_end, > - .pre_snapshot = acpi_hibernation_pre_snapshot_old, > + .pre_snapshot = acpi_pm_pre_suspend, > .prepare = acpi_pm_freeze, > .finish = acpi_pm_finish, > .enter = acpi_hibernation_enter, Acked-by: Brad Figg <brad.figg@canonical.com> Applied and pushed for Maverick
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 4882bc1..e0c08ab 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -119,6 +119,16 @@ static int acpi_pm_freeze(void) } /** + * acpi_pre_suspend - Enable wakeup devices, "freeze" EC and save NVS. + */ +static int acpi_pm_pre_suspend(void) +{ + acpi_pm_freeze(); + suspend_nvs_save(); + return 0; +} + +/** * __acpi_pm_prepare - Prepare the platform to enter the target state. * * If necessary, set the firmware waking vector and do arch-specific @@ -127,11 +137,9 @@ static int acpi_pm_freeze(void) static int __acpi_pm_prepare(void) { int error = acpi_sleep_prepare(acpi_target_sleep_state); - - suspend_nvs_save(); - if (error) acpi_target_sleep_state = ACPI_STATE_S0; + return error; } @@ -142,9 +150,8 @@ static int __acpi_pm_prepare(void) static int acpi_pm_prepare(void) { int error = __acpi_pm_prepare(); - if (!error) - acpi_pm_freeze(); + acpi_pm_pre_suspend(); return error; } @@ -336,9 +343,9 @@ static struct platform_suspend_ops acpi_suspend_ops = { static int acpi_suspend_begin_old(suspend_state_t pm_state) { int error = acpi_suspend_begin(pm_state); - if (!error) error = __acpi_pm_prepare(); + return error; } @@ -349,7 +356,7 @@ static int acpi_suspend_begin_old(suspend_state_t pm_state) static struct platform_suspend_ops acpi_suspend_ops_old = { .valid = acpi_suspend_state_valid, .begin = acpi_suspend_begin_old, - .prepare_late = acpi_pm_freeze, + .prepare_late = acpi_pm_pre_suspend, .enter = acpi_suspend_enter, .wake = acpi_suspend_finish, .end = acpi_pm_end, @@ -445,16 +452,6 @@ static int acpi_hibernation_begin(void) return error; } -static int acpi_hibernation_pre_snapshot(void) -{ - int error = acpi_pm_prepare(); - - if (!error) - suspend_nvs_save(); - - return error; -} - static int acpi_hibernation_enter(void) { acpi_status status = AE_OK; @@ -503,7 +500,7 @@ static void acpi_pm_thaw(void) static struct platform_hibernation_ops acpi_hibernation_ops = { .begin = acpi_hibernation_begin, .end = acpi_pm_end, - .pre_snapshot = acpi_hibernation_pre_snapshot, + .pre_snapshot = acpi_pm_prepare, .finish = acpi_pm_finish, .prepare = acpi_pm_prepare, .enter = acpi_hibernation_enter, @@ -539,13 +536,6 @@ static int acpi_hibernation_begin_old(void) return error; } -static int acpi_hibernation_pre_snapshot_old(void) -{ - acpi_pm_freeze(); - suspend_nvs_save(); - return 0; -} - /* * The following callbacks are used if the pre-ACPI 2.0 suspend ordering has * been requested. @@ -553,7 +543,7 @@ static int acpi_hibernation_pre_snapshot_old(void) static struct platform_hibernation_ops acpi_hibernation_ops_old = { .begin = acpi_hibernation_begin_old, .end = acpi_pm_end, - .pre_snapshot = acpi_hibernation_pre_snapshot_old, + .pre_snapshot = acpi_pm_pre_suspend, .prepare = acpi_pm_freeze, .finish = acpi_pm_finish, .enter = acpi_hibernation_enter,