* [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
@ 2008-01-05 22:38 ` Rafael J. Wysocki
2008-01-06 0:31 ` David Brownell
2008-01-05 22:41 ` [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK Rafael J. Wysocki
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:38 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett, David Brownell
From: Rafael J. Wysocki <rjw@sisk.pl>
On ACPI systems the target state set by acpi_pm_set_target() is
reset by acpi_pm_finish(), but that need not be called in the
suspend fails. All platforms that use the .set_target() global
suspend callback are affected by analogous issues.
For this reason, we need an additional global suspend callback that
will reset the target state regardless of whether or not the suspend
is successful. Also, it is reasonable to rename the .set_target()
callback, since it will be used for a different purpose on ACPI
systems (due to ACPI 1.0x code ordering requirements).
Introduce the global suspend callback .close() to be executed at the
end of the suspend sequence and rename the .set_target() global
suspend callback to .open().
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
arch/arm/mach-at91/pm.c | 17 +++++++++++++----
arch/powerpc/platforms/52xx/lite5200_pm.c | 10 ++++++++--
drivers/acpi/sleep/main.c | 22 ++++++++++++++++++----
include/linux/suspend.h | 24 ++++++++++++++----------
kernel/power/main.c | 9 ++++++---
5 files changed, 59 insertions(+), 23 deletions(-)
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -38,18 +38,16 @@ typedef int __bitwise suspend_state_t;
* There is the %suspend_valid_only_mem function available that can be
* assigned to this if the platform only supports mem sleep.
*
- * @set_target: Tell the platform which system sleep state is going to be
- * entered.
- * @set_target() is executed right prior to suspending devices. The
- * information conveyed to the platform code by @set_target() should be
- * disregarded by the platform as soon as @finish() is executed and if
- * @prepare() fails. If @set_target() fails (ie. returns nonzero),
+ * @open: Initialise a transition to given system sleep state.
+ * @open() is executed right prior to suspending devices. The information
+ * conveyed to the platform code by @open() should be disregarded by it as
+ * soon as @close() is executed. If @open() fails (ie. returns nonzero),
* @prepare(), @enter() and @finish() will not be called by the PM core.
* This callback is optional. However, if it is implemented, the argument
* passed to @enter() is meaningless and should be ignored.
*
* @prepare: Prepare the platform for entering the system sleep state indicated
- * by @set_target().
+ * by @open().
* @prepare() is called right after devices have been suspended (ie. the
* appropriate .suspend() method has been executed for each device) and
* before the nonboot CPUs are disabled (it is executed with IRQs enabled).
@@ -57,8 +55,8 @@ typedef int __bitwise suspend_state_t;
* error code otherwise, in which case the system cannot enter the desired
* sleep state (@enter() and @finish() will not be called in that case).
*
- * @enter: Enter the system sleep state indicated by @set_target() or
- * represented by the argument if @set_target() is not implemented.
+ * @enter: Enter the system sleep state indicated by @open() or represented by
+ * the argument if @open() is not implemented.
* This callback is mandatory. It returns 0 on success or a negative
* error code otherwise, in which case the system cannot enter the desired
* sleep state.
@@ -69,13 +67,19 @@ typedef int __bitwise suspend_state_t;
* This callback is optional, but should be implemented by the platforms
* that implement @prepare(). If implemented, it is always called after
* @enter() (even if @enter() fails).
+ *
+ * @close: Called by the PM core right after resuming devices, to indicate to
+ * the platform that the system has returned to the working state.
+ * This callback is optional, but should be implemented by the platforms
+ * that implement @open().
*/
struct platform_suspend_ops {
int (*valid)(suspend_state_t state);
- int (*set_target)(suspend_state_t state);
+ int (*open)(suspend_state_t state);
int (*prepare)(void);
int (*enter)(suspend_state_t state);
void (*finish)(void);
+ void (*close)(void);
};
#ifdef CONFIG_SUSPEND
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -63,11 +63,11 @@ static u32 acpi_suspend_states[] = {
static int init_8259A_after_S1;
/**
- * acpi_pm_set_target - Set the target system sleep state to the state
+ * acpi_pm_open - Set the target system sleep state to the state
* associated with given @pm_state, if supported.
*/
-static int acpi_pm_set_target(suspend_state_t pm_state)
+static int acpi_pm_open(suspend_state_t pm_state)
{
u32 acpi_state = acpi_suspend_states[pm_state];
int error = 0;
@@ -164,7 +164,7 @@ static int acpi_pm_enter(suspend_state_t
}
/**
- * acpi_pm_finish - Finish up suspend sequence.
+ * acpi_pm_finish - Instruct the platform to leave a sleep state.
*
* This is called after we wake back up (or if entering the sleep state
* failed).
@@ -190,6 +190,19 @@ static void acpi_pm_finish(void)
#endif
}
+/**
+ * acpi_pm_close - Finish up suspend sequence.
+ */
+
+static void acpi_pm_close(void)
+{
+ /*
+ * This is necessary in case acpi_pm_finish() is not called during a
+ * failing transition to a sleep state.
+ */
+ acpi_target_sleep_state = ACPI_STATE_S0;
+}
+
static int acpi_pm_state_valid(suspend_state_t pm_state)
{
u32 acpi_state;
@@ -208,10 +221,11 @@ static int acpi_pm_state_valid(suspend_s
static struct platform_suspend_ops acpi_pm_ops = {
.valid = acpi_pm_state_valid,
- .set_target = acpi_pm_set_target,
+ .open = acpi_pm_open,
.prepare = acpi_pm_prepare,
.enter = acpi_pm_enter,
.finish = acpi_pm_finish,
+ .close = acpi_pm_close,
};
/*
Index: linux-2.6/arch/arm/mach-at91/pm.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-at91/pm.c
+++ linux-2.6/arch/arm/mach-at91/pm.c
@@ -52,7 +52,7 @@ static suspend_state_t target_state;
/*
* Called after processes are frozen, but before we shutdown devices.
*/
-static int at91_pm_set_target(suspend_state_t state)
+static int at91_pm_open(suspend_state_t state)
{
target_state = state;
return 0;
@@ -197,11 +197,20 @@ error:
return 0;
}
+/*
+ * Called right prior to thawing processes.
+ */
+static void at91_pm_close(void)
+{
+ target_state = PM_SUSPEND_ON;
+}
+
static struct platform_suspend_ops at91_pm_ops ={
- .valid = at91_pm_valid_state,
- .set_target = at91_pm_set_target,
- .enter = at91_pm_enter,
+ .valid = at91_pm_valid_state,
+ .open = at91_pm_open,
+ .enter = at91_pm_enter,
+ .close = at91_pm_close,
};
static int __init at91_pm_init(void)
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -253,10 +253,10 @@ int suspend_devices_and_enter(suspend_st
if (!suspend_ops)
return -ENOSYS;
- if (suspend_ops->set_target) {
- error = suspend_ops->set_target(state);
+ if (suspend_ops->open) {
+ error = suspend_ops->open(state);
if (error)
- return error;
+ goto Close;
}
suspend_console();
error = device_suspend(PMSG_SUSPEND);
@@ -289,6 +289,9 @@ int suspend_devices_and_enter(suspend_st
device_resume();
Resume_console:
resume_console();
+ Close:
+ if (suspend_ops->close)
+ suspend_ops->close();
return error;
}
Index: linux-2.6/arch/powerpc/platforms/52xx/lite5200_pm.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/lite5200_pm.c
+++ linux-2.6/arch/powerpc/platforms/52xx/lite5200_pm.c
@@ -31,7 +31,7 @@ static int lite5200_pm_valid(suspend_sta
}
}
-static int lite5200_pm_set_target(suspend_state_t state)
+static int lite5200_pm_open(suspend_state_t state)
{
if (lite5200_pm_valid(state)) {
lite5200_pm_target_state = state;
@@ -208,12 +208,18 @@ static void lite5200_pm_finish(void)
mpc52xx_pm_finish();
}
+static void lite5200_pm_close(void)
+{
+ lite5200_pm_target_state = PM_SUSPEND_ON;
+}
+
static struct platform_suspend_ops lite5200_pm_ops = {
.valid = lite5200_pm_valid,
- .set_target = lite5200_pm_set_target,
+ .open = lite5200_pm_open,
.prepare = lite5200_pm_prepare,
.enter = lite5200_pm_enter,
.finish = lite5200_pm_finish,
+ .close = lite5200_pm_close,
};
int __init lite5200_pm_init(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks
2008-01-05 22:38 ` [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks Rafael J. Wysocki
@ 2008-01-06 0:31 ` David Brownell
2008-01-06 13:15 ` Rafael J. Wysocki
0 siblings, 1 reply; 15+ messages in thread
From: David Brownell @ 2008-01-06 0:31 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, Arjan van de Ven, Carlos Corbacho,
Linus Torvalds, Pavel Machek, pm list, Andrew Morton, Len Brown,
Alexey Starikovskiy, Moore, Robert, Matthew Garrett
On Saturday 05 January 2008, Rafael J. Wysocki wrote:
> + * @open: Initialise a transition to given system sleep state.
> + * @open() is executed right prior to suspending devices. The information
> + * conveyed to the platform code by @open() should be disregarded by it as
> + * soon as @close() is executed. If @open() fails (ie. returns nonzero),
> * @prepare(), @enter() and @finish() will not be called by the PM core.
> * This callback is optional. However, if it is implemented, the argument
> * passed to @enter() is meaningless and should be ignored.
^^^^^^^^^^^
Surely this should say "redundant", not "meaningless"?
> + * @close: Called by the PM core right after resuming devices, to indicate to
> + * the platform that the system has returned to the working state.
Or the state transition has aborted ...
> + * This callback is optional, but should be implemented by the platforms
> + * that implement @open().
"..., but platforms which implement @open() should also provide a @close()
which cleans up transitions which aborted before @enter()."
Otherwise it seems rather unclear why this exists, since all platforms know
that things are "normal" as soon as enter() gets back from its transition.
(What can I say ... I like to see things be clear!) Yes, systems may do more
than that when it gets this call; but the minimum involves just that cleanup.
- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks
2008-01-06 0:31 ` David Brownell
@ 2008-01-06 13:15 ` Rafael J. Wysocki
0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-06 13:15 UTC (permalink / raw)
To: David Brownell
Cc: ACPI Devel Maling List, Arjan van de Ven, Carlos Corbacho,
Linus Torvalds, Pavel Machek, pm list, Andrew Morton, Len Brown,
Alexey Starikovskiy, Moore, Robert, Matthew Garrett
On Sunday, 6 of January 2008, David Brownell wrote:
> On Saturday 05 January 2008, Rafael J. Wysocki wrote:
> > + * @open: Initialise a transition to given system sleep state.
> > + * @open() is executed right prior to suspending devices. The information
> > + * conveyed to the platform code by @open() should be disregarded by it as
> > + * soon as @close() is executed. If @open() fails (ie. returns nonzero),
> > * @prepare(), @enter() and @finish() will not be called by the PM core.
> > * This callback is optional. However, if it is implemented, the argument
> > * passed to @enter() is meaningless and should be ignored.
> ^^^^^^^^^^^
> Surely this should say "redundant", not "meaningless"?
>
>
> > + * @close: Called by the PM core right after resuming devices, to indicate to
> > + * the platform that the system has returned to the working state.
>
> Or the state transition has aborted ...
>
> > + * This callback is optional, but should be implemented by the platforms
> > + * that implement @open().
>
> "..., but platforms which implement @open() should also provide a @close()
> which cleans up transitions which aborted before @enter()."
>
> Otherwise it seems rather unclear why this exists, since all platforms know
> that things are "normal" as soon as enter() gets back from its transition.
> (What can I say ... I like to see things be clear!) Yes, systems may do more
> than that when it gets this call; but the minimum involves just that cleanup.
Thanks for the comments. They will be taken into account in the final version
of the patch.
Greetings,
Rafael
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
2008-01-05 22:38 ` [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks Rafael J. Wysocki
@ 2008-01-05 22:41 ` Rafael J. Wysocki
2008-01-07 21:25 ` Len Brown
2008-01-05 22:42 ` [RFC][PATCH 3/7] ACPI: Separate disabling of GPEs from _PTS Rafael J. Wysocki
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:41 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
The execution of ACPI global control methods _GTS and _BFS is
currently tied to the preparation to enter a sleep state and to the
leaving of the sleep state, respectively. However, these functions
are called before disabling the nonboot CPUs and after enabling
them, respectively (in fact, on ACPI 1.0x systems the first of them
ought to be called before suspending devices), while according to the
ACPI specification, _GTS is to be executed right prior to entering
the system sleep state and _BFS is to be executed right after the
platfor firmware has returned control to the OS on wake up.
Move the execution of _GTS and _BFS to the right places.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
drivers/acpi/hardware/hwsleep.c | 75 ++++++++++++++++++++++++++++++----------
drivers/acpi/sleep/main.c | 7 +++
include/acpi/acpixf.h | 2 +
3 files changed, 66 insertions(+), 18 deletions(-)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -192,18 +192,13 @@ acpi_status acpi_enter_sleep_state_prep(
arg.type = ACPI_TYPE_INTEGER;
arg.integer.value = sleep_state;
- /* Run the _PTS and _GTS methods */
+ /* Run the _PTS method */
status = acpi_evaluate_object(NULL, METHOD_NAME__PTS, &arg_list, NULL);
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
return_ACPI_STATUS(status);
}
- status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- return_ACPI_STATUS(status);
- }
-
/* Setup the argument to _SST */
switch (sleep_state) {
@@ -262,6 +257,8 @@ acpi_status asmlinkage acpi_enter_sleep_
struct acpi_bit_register_info *sleep_type_reg_info;
struct acpi_bit_register_info *sleep_enable_reg_info;
u32 in_value;
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
acpi_status status;
ACPI_FUNCTION_TRACE(acpi_enter_sleep_state);
@@ -307,6 +304,18 @@ acpi_status asmlinkage acpi_enter_sleep_
return_ACPI_STATUS(status);
}
+ /* Execute the _GTS method */
+
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = sleep_state;
+
+ status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+ return_ACPI_STATUS(status);
+ }
+
/* Get current value of PM1A control */
status = acpi_hw_register_read(ACPI_REGISTER_PM1_CONTROL, &PM1Acontrol);
@@ -473,17 +482,18 @@ ACPI_EXPORT_SYMBOL(acpi_enter_sleep_stat
/*******************************************************************************
*
- * FUNCTION: acpi_leave_sleep_state
+ * FUNCTION: acpi_leave_sleep_state_prep
*
- * PARAMETERS: sleep_state - Which sleep state we just exited
+ * PARAMETERS: sleep_state - Which sleep state we are exiting
*
* RETURN: Status
*
- * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
- * Called with interrupts ENABLED.
+ * DESCRIPTION: Perform the first state of OS-independent ACPI cleanup after a
+ * sleep.
+ * Called with interrupts DISABLED.
*
******************************************************************************/
-acpi_status acpi_leave_sleep_state(u8 sleep_state)
+acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
{
struct acpi_object_list arg_list;
union acpi_object arg;
@@ -493,7 +503,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
u32 PM1Acontrol;
u32 PM1Bcontrol;
- ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
+ ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
/*
* Set SLP_TYPE and SLP_EN to state S0.
@@ -540,6 +550,41 @@ acpi_status acpi_leave_sleep_state(u8 sl
}
}
+ /* Execute the _BFS method */
+
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = sleep_state;
+
+ status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
+ if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
+ ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
+ }
+
+ return_ACPI_STATUS(status);
+}
+
+/*******************************************************************************
+ *
+ * FUNCTION: acpi_leave_sleep_state
+ *
+ * PARAMETERS: sleep_state - Which sleep state we just exited
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
+ * Called with interrupts ENABLED.
+ *
+ ******************************************************************************/
+acpi_status acpi_leave_sleep_state(u8 sleep_state)
+{
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
+ acpi_status status;
+
+ ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
+
/* Ensure enter_sleep_state_prep -> enter_sleep_state ordering */
acpi_gbl_sleep_type_a = ACPI_SLEEP_TYPE_INVALID;
@@ -558,12 +603,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
}
- arg.integer.value = sleep_state;
- status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
- }
-
/*
* GPEs must be enabled before _WAK is called as GPEs
* might get fired there
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -139,6 +139,9 @@ static int acpi_pm_enter(suspend_state_t
break;
}
+ /* Reprogram control registers and execute _BFS */
+ acpi_leave_sleep_state_prep(acpi_state);
+
/* ACPI 3.0 specs (P62) says that it's the responsabilty
* of the OSPM to clear the status bit [ implying that the
* POWER_BUTTON event should not reach userspace ]
@@ -272,6 +275,8 @@ static int acpi_hibernation_enter(void)
acpi_enable_wakeup_device(ACPI_STATE_S4);
/* This shouldn't return. If it returns, we have a problem */
status = acpi_enter_sleep_state(ACPI_STATE_S4);
+ /* Reprogram control registers and execute _BFS */
+ acpi_leave_sleep_state_prep(ACPI_STATE_S4);
local_irq_restore(flags);
return ACPI_SUCCESS(status) ? 0 : -EFAULT;
@@ -284,6 +289,8 @@ static void acpi_hibernation_leave(void)
* enable it here.
*/
acpi_enable();
+ /* Reprogram control registers and execute _BFS */
+ acpi_leave_sleep_state_prep(ACPI_STATE_S4);
}
static void acpi_hibernation_finish(void)
Index: linux-2.6/include/acpi/acpixf.h
===================================================================
--- linux-2.6.orig/include/acpi/acpixf.h
+++ linux-2.6/include/acpi/acpixf.h
@@ -335,6 +335,8 @@ acpi_status asmlinkage acpi_enter_sleep_
acpi_status asmlinkage acpi_enter_sleep_state_s4bios(void);
+acpi_status acpi_leave_sleep_state_prep(u8 sleep_state);
+
acpi_status acpi_leave_sleep_state(u8 sleep_state);
#endif /* __ACXFACE_H__ */
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
2008-01-05 22:41 ` [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK Rafael J. Wysocki
@ 2008-01-07 21:25 ` Len Brown
2008-01-07 21:44 ` Rafael J. Wysocki
0 siblings, 1 reply; 15+ messages in thread
From: Len Brown @ 2008-01-07 21:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, Arjan van de Ven, Carlos Corbacho,
Linus Torvalds, Pavel Machek, pm list, Andrew Morton,
Alexey Starikovskiy, Moore, Robert, Matthew Garrett
On Saturday 05 January 2008 17:41, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> The execution of ACPI global control methods _GTS and _BFS is
> currently tied to the preparation to enter a sleep state and to the
> leaving of the sleep state, respectively. However, these functions
> are called before disabling the nonboot CPUs and after enabling
> them, respectively (in fact, on ACPI 1.0x systems the first of them
> ought to be called before suspending devices), while according to the
> ACPI specification, _GTS is to be executed right prior to entering
> the system sleep state and _BFS is to be executed right after the
> platfor firmware has returned control to the OS on wake up.
ACPI 1.0 systems don't have _GTS or _BFS -- they were added
as optional control methods starting in ACPI 2.0.
The ACPI 1.0 installed base of Windows by definition could
not have invoked them -- so vendors could not have possibly
made their invocation mandatory w/o adding special handling
fro the installed base. I guess they never found a reason
to do this, because I've not yet seen a BIOS that implements them.
So the patch looks good, but Linux support for _GTS and _BFS
is purely academic at this point.
Rafael,
Do you grant Intel permission to incorporate this change
into the upstream ACPICA source base and ship it under
the the ACPICA source license to all ACPICA customers?
thanks,
-Len
> Move the execution of _GTS and _BFS to the right places.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@suse.cz>
> ---
> drivers/acpi/hardware/hwsleep.c | 75 ++++++++++++++++++++++++++++++----------
> drivers/acpi/sleep/main.c | 7 +++
> include/acpi/acpixf.h | 2 +
> 3 files changed, 66 insertions(+), 18 deletions(-)
>
> Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> @@ -192,18 +192,13 @@ acpi_status acpi_enter_sleep_state_prep(
> arg.type = ACPI_TYPE_INTEGER;
> arg.integer.value = sleep_state;
>
> - /* Run the _PTS and _GTS methods */
> + /* Run the _PTS method */
>
> status = acpi_evaluate_object(NULL, METHOD_NAME__PTS, &arg_list, NULL);
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> return_ACPI_STATUS(status);
> }
>
> - status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> - return_ACPI_STATUS(status);
> - }
> -
> /* Setup the argument to _SST */
>
> switch (sleep_state) {
> @@ -262,6 +257,8 @@ acpi_status asmlinkage acpi_enter_sleep_
> struct acpi_bit_register_info *sleep_type_reg_info;
> struct acpi_bit_register_info *sleep_enable_reg_info;
> u32 in_value;
> + struct acpi_object_list arg_list;
> + union acpi_object arg;
> acpi_status status;
>
> ACPI_FUNCTION_TRACE(acpi_enter_sleep_state);
> @@ -307,6 +304,18 @@ acpi_status asmlinkage acpi_enter_sleep_
> return_ACPI_STATUS(status);
> }
>
> + /* Execute the _GTS method */
> +
> + arg_list.count = 1;
> + arg_list.pointer = &arg;
> + arg.type = ACPI_TYPE_INTEGER;
> + arg.integer.value = sleep_state;
> +
> + status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> + return_ACPI_STATUS(status);
> + }
> +
> /* Get current value of PM1A control */
>
> status = acpi_hw_register_read(ACPI_REGISTER_PM1_CONTROL, &PM1Acontrol);
> @@ -473,17 +482,18 @@ ACPI_EXPORT_SYMBOL(acpi_enter_sleep_stat
>
> /*******************************************************************************
> *
> - * FUNCTION: acpi_leave_sleep_state
> + * FUNCTION: acpi_leave_sleep_state_prep
> *
> - * PARAMETERS: sleep_state - Which sleep state we just exited
> + * PARAMETERS: sleep_state - Which sleep state we are exiting
> *
> * RETURN: Status
> *
> - * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
> - * Called with interrupts ENABLED.
> + * DESCRIPTION: Perform the first state of OS-independent ACPI cleanup after a
> + * sleep.
> + * Called with interrupts DISABLED.
> *
> ******************************************************************************/
> -acpi_status acpi_leave_sleep_state(u8 sleep_state)
> +acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
> {
> struct acpi_object_list arg_list;
> union acpi_object arg;
> @@ -493,7 +503,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> u32 PM1Acontrol;
> u32 PM1Bcontrol;
>
> - ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
> + ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
>
> /*
> * Set SLP_TYPE and SLP_EN to state S0.
> @@ -540,6 +550,41 @@ acpi_status acpi_leave_sleep_state(u8 sl
> }
> }
>
> + /* Execute the _BFS method */
> +
> + arg_list.count = 1;
> + arg_list.pointer = &arg;
> + arg.type = ACPI_TYPE_INTEGER;
> + arg.integer.value = sleep_state;
> +
> + status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> + ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> + }
> +
> + return_ACPI_STATUS(status);
> +}
> +
> +/*******************************************************************************
> + *
> + * FUNCTION: acpi_leave_sleep_state
> + *
> + * PARAMETERS: sleep_state - Which sleep state we just exited
> + *
> + * RETURN: Status
> + *
> + * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
> + * Called with interrupts ENABLED.
> + *
> + ******************************************************************************/
> +acpi_status acpi_leave_sleep_state(u8 sleep_state)
> +{
> + struct acpi_object_list arg_list;
> + union acpi_object arg;
> + acpi_status status;
> +
> + ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
> +
> /* Ensure enter_sleep_state_prep -> enter_sleep_state ordering */
>
> acpi_gbl_sleep_type_a = ACPI_SLEEP_TYPE_INVALID;
> @@ -558,12 +603,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
> ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
> }
>
> - arg.integer.value = sleep_state;
> - status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> - ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> - }
> -
> /*
> * GPEs must be enabled before _WAK is called as GPEs
> * might get fired there
> Index: linux-2.6/drivers/acpi/sleep/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/sleep/main.c
> +++ linux-2.6/drivers/acpi/sleep/main.c
> @@ -139,6 +139,9 @@ static int acpi_pm_enter(suspend_state_t
> break;
> }
>
> + /* Reprogram control registers and execute _BFS */
> + acpi_leave_sleep_state_prep(acpi_state);
> +
> /* ACPI 3.0 specs (P62) says that it's the responsabilty
> * of the OSPM to clear the status bit [ implying that the
> * POWER_BUTTON event should not reach userspace ]
> @@ -272,6 +275,8 @@ static int acpi_hibernation_enter(void)
> acpi_enable_wakeup_device(ACPI_STATE_S4);
> /* This shouldn't return. If it returns, we have a problem */
> status = acpi_enter_sleep_state(ACPI_STATE_S4);
> + /* Reprogram control registers and execute _BFS */
> + acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> local_irq_restore(flags);
>
> return ACPI_SUCCESS(status) ? 0 : -EFAULT;
> @@ -284,6 +289,8 @@ static void acpi_hibernation_leave(void)
> * enable it here.
> */
> acpi_enable();
> + /* Reprogram control registers and execute _BFS */
> + acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> }
>
> static void acpi_hibernation_finish(void)
> Index: linux-2.6/include/acpi/acpixf.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpixf.h
> +++ linux-2.6/include/acpi/acpixf.h
> @@ -335,6 +335,8 @@ acpi_status asmlinkage acpi_enter_sleep_
>
> acpi_status asmlinkage acpi_enter_sleep_state_s4bios(void);
>
> +acpi_status acpi_leave_sleep_state_prep(u8 sleep_state);
> +
> acpi_status acpi_leave_sleep_state(u8 sleep_state);
>
> #endif /* __ACXFACE_H__ */
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK
2008-01-07 21:25 ` Len Brown
@ 2008-01-07 21:44 ` Rafael J. Wysocki
0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-07 21:44 UTC (permalink / raw)
To: Len Brown
Cc: robert.moore, ACPI Devel Maling List, Arjan van de Ven,
Carlos Corbacho, Linus Torvalds, Pavel Machek, pm list,
Andrew Morton, Alexey Starikovskiy, Matthew Garrett
On Monday, 7 of January 2008, Len Brown wrote:
> On Saturday 05 January 2008 17:41, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > The execution of ACPI global control methods _GTS and _BFS is
> > currently tied to the preparation to enter a sleep state and to the
> > leaving of the sleep state, respectively. However, these functions
> > are called before disabling the nonboot CPUs and after enabling
> > them, respectively (in fact, on ACPI 1.0x systems the first of them
> > ought to be called before suspending devices), while according to the
> > ACPI specification, _GTS is to be executed right prior to entering
> > the system sleep state and _BFS is to be executed right after the
> > platfor firmware has returned control to the OS on wake up.
>
>
> ACPI 1.0 systems don't have _GTS or _BFS -- they were added
> as optional control methods starting in ACPI 2.0.
>
> The ACPI 1.0 installed base of Windows by definition could
> not have invoked them -- so vendors could not have possibly
> made their invocation mandatory w/o adding special handling
> fro the installed base. I guess they never found a reason
> to do this, because I've not yet seen a BIOS that implements them.
>
> So the patch looks good, but Linux support for _GTS and _BFS
> is purely academic at this point.
>
> Rafael,
> Do you grant Intel permission to incorporate this change
> into the upstream ACPICA source base and ship it under
> the the ACPICA source license to all ACPICA customers?
Sure, I do.
Thanks,
Rafael
> > Move the execution of _GTS and _BFS to the right places.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > Acked-by: Pavel Machek <pavel@suse.cz>
> > ---
> > drivers/acpi/hardware/hwsleep.c | 75 ++++++++++++++++++++++++++++++----------
> > drivers/acpi/sleep/main.c | 7 +++
> > include/acpi/acpixf.h | 2 +
> > 3 files changed, 66 insertions(+), 18 deletions(-)
> >
> > Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
> > +++ linux-2.6/drivers/acpi/hardware/hwsleep.c
> > @@ -192,18 +192,13 @@ acpi_status acpi_enter_sleep_state_prep(
> > arg.type = ACPI_TYPE_INTEGER;
> > arg.integer.value = sleep_state;
> >
> > - /* Run the _PTS and _GTS methods */
> > + /* Run the _PTS method */
> >
> > status = acpi_evaluate_object(NULL, METHOD_NAME__PTS, &arg_list, NULL);
> > if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > return_ACPI_STATUS(status);
> > }
> >
> > - status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> > - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > - return_ACPI_STATUS(status);
> > - }
> > -
> > /* Setup the argument to _SST */
> >
> > switch (sleep_state) {
> > @@ -262,6 +257,8 @@ acpi_status asmlinkage acpi_enter_sleep_
> > struct acpi_bit_register_info *sleep_type_reg_info;
> > struct acpi_bit_register_info *sleep_enable_reg_info;
> > u32 in_value;
> > + struct acpi_object_list arg_list;
> > + union acpi_object arg;
> > acpi_status status;
> >
> > ACPI_FUNCTION_TRACE(acpi_enter_sleep_state);
> > @@ -307,6 +304,18 @@ acpi_status asmlinkage acpi_enter_sleep_
> > return_ACPI_STATUS(status);
> > }
> >
> > + /* Execute the _GTS method */
> > +
> > + arg_list.count = 1;
> > + arg_list.pointer = &arg;
> > + arg.type = ACPI_TYPE_INTEGER;
> > + arg.integer.value = sleep_state;
> > +
> > + status = acpi_evaluate_object(NULL, METHOD_NAME__GTS, &arg_list, NULL);
> > + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > + return_ACPI_STATUS(status);
> > + }
> > +
> > /* Get current value of PM1A control */
> >
> > status = acpi_hw_register_read(ACPI_REGISTER_PM1_CONTROL, &PM1Acontrol);
> > @@ -473,17 +482,18 @@ ACPI_EXPORT_SYMBOL(acpi_enter_sleep_stat
> >
> > /*******************************************************************************
> > *
> > - * FUNCTION: acpi_leave_sleep_state
> > + * FUNCTION: acpi_leave_sleep_state_prep
> > *
> > - * PARAMETERS: sleep_state - Which sleep state we just exited
> > + * PARAMETERS: sleep_state - Which sleep state we are exiting
> > *
> > * RETURN: Status
> > *
> > - * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
> > - * Called with interrupts ENABLED.
> > + * DESCRIPTION: Perform the first state of OS-independent ACPI cleanup after a
> > + * sleep.
> > + * Called with interrupts DISABLED.
> > *
> > ******************************************************************************/
> > -acpi_status acpi_leave_sleep_state(u8 sleep_state)
> > +acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
> > {
> > struct acpi_object_list arg_list;
> > union acpi_object arg;
> > @@ -493,7 +503,7 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > u32 PM1Acontrol;
> > u32 PM1Bcontrol;
> >
> > - ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
> > + ACPI_FUNCTION_TRACE(acpi_leave_sleep_state_prep);
> >
> > /*
> > * Set SLP_TYPE and SLP_EN to state S0.
> > @@ -540,6 +550,41 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > }
> > }
> >
> > + /* Execute the _BFS method */
> > +
> > + arg_list.count = 1;
> > + arg_list.pointer = &arg;
> > + arg.type = ACPI_TYPE_INTEGER;
> > + arg.integer.value = sleep_state;
> > +
> > + status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> > + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > + ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> > + }
> > +
> > + return_ACPI_STATUS(status);
> > +}
> > +
> > +/*******************************************************************************
> > + *
> > + * FUNCTION: acpi_leave_sleep_state
> > + *
> > + * PARAMETERS: sleep_state - Which sleep state we just exited
> > + *
> > + * RETURN: Status
> > + *
> > + * DESCRIPTION: Perform OS-independent ACPI cleanup after a sleep
> > + * Called with interrupts ENABLED.
> > + *
> > + ******************************************************************************/
> > +acpi_status acpi_leave_sleep_state(u8 sleep_state)
> > +{
> > + struct acpi_object_list arg_list;
> > + union acpi_object arg;
> > + acpi_status status;
> > +
> > + ACPI_FUNCTION_TRACE(acpi_leave_sleep_state);
> > +
> > /* Ensure enter_sleep_state_prep -> enter_sleep_state ordering */
> >
> > acpi_gbl_sleep_type_a = ACPI_SLEEP_TYPE_INVALID;
> > @@ -558,12 +603,6 @@ acpi_status acpi_leave_sleep_state(u8 sl
> > ACPI_EXCEPTION((AE_INFO, status, "During Method _SST"));
> > }
> >
> > - arg.integer.value = sleep_state;
> > - status = acpi_evaluate_object(NULL, METHOD_NAME__BFS, &arg_list, NULL);
> > - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > - ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
> > - }
> > -
> > /*
> > * GPEs must be enabled before _WAK is called as GPEs
> > * might get fired there
> > Index: linux-2.6/drivers/acpi/sleep/main.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/sleep/main.c
> > +++ linux-2.6/drivers/acpi/sleep/main.c
> > @@ -139,6 +139,9 @@ static int acpi_pm_enter(suspend_state_t
> > break;
> > }
> >
> > + /* Reprogram control registers and execute _BFS */
> > + acpi_leave_sleep_state_prep(acpi_state);
> > +
> > /* ACPI 3.0 specs (P62) says that it's the responsabilty
> > * of the OSPM to clear the status bit [ implying that the
> > * POWER_BUTTON event should not reach userspace ]
> > @@ -272,6 +275,8 @@ static int acpi_hibernation_enter(void)
> > acpi_enable_wakeup_device(ACPI_STATE_S4);
> > /* This shouldn't return. If it returns, we have a problem */
> > status = acpi_enter_sleep_state(ACPI_STATE_S4);
> > + /* Reprogram control registers and execute _BFS */
> > + acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> > local_irq_restore(flags);
> >
> > return ACPI_SUCCESS(status) ? 0 : -EFAULT;
> > @@ -284,6 +289,8 @@ static void acpi_hibernation_leave(void)
> > * enable it here.
> > */
> > acpi_enable();
> > + /* Reprogram control registers and execute _BFS */
> > + acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> > }
> >
> > static void acpi_hibernation_finish(void)
> > Index: linux-2.6/include/acpi/acpixf.h
> > ===================================================================
> > --- linux-2.6.orig/include/acpi/acpixf.h
> > +++ linux-2.6/include/acpi/acpixf.h
> > @@ -335,6 +335,8 @@ acpi_status asmlinkage acpi_enter_sleep_
> >
> > acpi_status asmlinkage acpi_enter_sleep_state_s4bios(void);
> >
> > +acpi_status acpi_leave_sleep_state_prep(u8 sleep_state);
> > +
> > acpi_status acpi_leave_sleep_state(u8 sleep_state);
> >
> > #endif /* __ACXFACE_H__ */
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC][PATCH 3/7] ACPI: Separate disabling of GPEs from _PTS
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
2008-01-05 22:38 ` [RFC][PATCH 1/7] Suspend: Introduce open() and close() callbacks Rafael J. Wysocki
2008-01-05 22:41 ` [RFC][PATCH 2/7] ACPI: Separate invocations of _GTS and _BFS from _PTS and _WAK Rafael J. Wysocki
@ 2008-01-05 22:42 ` Rafael J. Wysocki
2008-01-05 22:48 ` [RFC][PATCH 4/7] ACPI Suspend: Call _PTS before suspending devices Rafael J. Wysocki
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:42 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
The preparation to enter an ACPI system sleep state is now tied to
the disabling of GPEs, but the GPEs should not be disabled before
suspending devices. Since on ACPI 1.0x systems the _PTS global
control method should be executed before suspending devices, we
need to disable GPEs separately.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
drivers/acpi/hardware/hwsleep.c | 4 ----
drivers/acpi/sleep/main.c | 17 +++++++++++++++--
2 files changed, 15 insertions(+), 6 deletions(-)
Index: linux-2.6/drivers/acpi/hardware/hwsleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/hardware/hwsleep.c
+++ linux-2.6/drivers/acpi/hardware/hwsleep.c
@@ -229,10 +229,6 @@ acpi_status acpi_enter_sleep_state_prep(
"While executing method _SST"));
}
- /* Disable/Clear all GPEs */
-
- status = acpi_hw_disable_all_gpes();
-
return_ACPI_STATUS(status);
}
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -91,10 +91,13 @@ static int acpi_pm_open(suspend_state_t
static int acpi_pm_prepare(void)
{
- int error = acpi_sleep_prepare(acpi_target_sleep_state);
+ int error;
+ error = acpi_sleep_prepare(acpi_target_sleep_state);
if (error)
acpi_target_sleep_state = ACPI_STATE_S0;
+ else if (!ACPI_SUCCESS(acpi_hw_disable_all_gpes()))
+ error = -EFAULT;
return error;
}
@@ -261,7 +264,16 @@ static int acpi_hibernation_start(void)
static int acpi_hibernation_prepare(void)
{
- return acpi_sleep_prepare(ACPI_STATE_S4);
+ int error;
+
+ error = acpi_sleep_prepare(ACPI_STATE_S4);
+ if (error)
+ return error;
+
+ if (!ACPI_SUCCESS(acpi_hw_disable_all_gpes()))
+ error = -EFAULT;
+
+ return error;
}
static int acpi_hibernation_enter(void)
@@ -432,6 +444,7 @@ static void acpi_power_off_prepare(void)
{
/* Prepare to power off the system */
acpi_sleep_prepare(ACPI_STATE_S5);
+ acpi_hw_disable_all_gpes();
}
static void acpi_power_off(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC][PATCH 4/7] ACPI Suspend: Call _PTS before suspending devices
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
` (2 preceding siblings ...)
2008-01-05 22:42 ` [RFC][PATCH 3/7] ACPI: Separate disabling of GPEs from _PTS Rafael J. Wysocki
@ 2008-01-05 22:48 ` Rafael J. Wysocki
2008-01-05 22:50 ` [RFC][PATCH 5/7] Hibernation: Introduce open() and close() callbacks Rafael J. Wysocki
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:48 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
The ACPI 1.0 specification wants us to put devices into low power
states after executing the _PTS global control methods, while ACPI
2.0 and later want us to do that in the reverse order. The current
suspend code follows ACPI 2.0 in that respect which causes some
ACPI 1.0x systems to hang during suspend (ref.
http://bugzilla.kernel.org/show_bug.cgi?id=9528).
Make the suspend code execute _PTS before putting devices into low
power (ie. in accordance with ACPI 1.0x) and provide a command line
option to override the default if need be.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
Documentation/kernel-parameters.txt | 5 +++
drivers/acpi/sleep/main.c | 51 ++++++++++++++++++++++++++----------
2 files changed, 43 insertions(+), 13 deletions(-)
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -26,6 +26,21 @@ u8 sleep_states[ACPI_S_STATE_COUNT];
#ifdef CONFIG_PM_SLEEP
static u32 acpi_target_sleep_state = ACPI_STATE_S0;
+static bool acpi_sleep_finish_wake_up;
+
+/*
+ * ACPI 2.0 and later want us to execute _PTS after suspending devices, so we
+ * allow the user to request that behavior by using the 'acpi_new_pts_ordering'
+ * kernel command line option that causes the following variable to be set.
+ */
+static bool new_pts_ordering;
+
+static int __init acpi_new_pts_ordering(char *str)
+{
+ new_pts_ordering = true;
+ return 1;
+}
+__setup("acpi_new_pts_ordering", acpi_new_pts_ordering);
#endif
int acpi_sleep_prepare(u32 acpi_state)
@@ -74,6 +89,14 @@ static int acpi_pm_open(suspend_state_t
if (sleep_states[acpi_state]) {
acpi_target_sleep_state = acpi_state;
+ if (new_pts_ordering)
+ return 0;
+
+ error = acpi_sleep_prepare(acpi_state);
+ if (error)
+ acpi_target_sleep_state = ACPI_STATE_S0;
+ else
+ acpi_sleep_finish_wake_up = true;
} else {
printk(KERN_ERR "ACPI does not support this state: %d\n",
pm_state);
@@ -91,15 +114,17 @@ static int acpi_pm_open(suspend_state_t
static int acpi_pm_prepare(void)
{
- int error;
+ if (new_pts_ordering) {
+ int error = acpi_sleep_prepare(acpi_target_sleep_state);
- error = acpi_sleep_prepare(acpi_target_sleep_state);
- if (error)
- acpi_target_sleep_state = ACPI_STATE_S0;
- else if (!ACPI_SUCCESS(acpi_hw_disable_all_gpes()))
- error = -EFAULT;
+ if (error) {
+ acpi_target_sleep_state = ACPI_STATE_S0;
+ return error;
+ }
+ acpi_sleep_finish_wake_up = true;
+ }
- return error;
+ return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
}
/**
@@ -123,10 +148,8 @@ static int acpi_pm_enter(suspend_state_t
if (acpi_state == ACPI_STATE_S3) {
int error = acpi_save_state_mem();
- if (error) {
- acpi_target_sleep_state = ACPI_STATE_S0;
+ if (error)
return error;
- }
}
local_irq_save(flags);
@@ -187,6 +210,7 @@ static void acpi_pm_finish(void)
acpi_set_firmware_waking_vector((acpi_physical_address) 0);
acpi_target_sleep_state = ACPI_STATE_S0;
+ acpi_sleep_finish_wake_up = false;
#ifdef CONFIG_X86
if (init_8259A_after_S1) {
@@ -203,10 +227,11 @@ static void acpi_pm_finish(void)
static void acpi_pm_close(void)
{
/*
- * This is necessary in case acpi_pm_finish() is not called during a
- * failing transition to a sleep state.
+ * This is necessary in case acpi_pm_finish() is not called directly
+ * during a failing transition to a sleep state.
*/
- acpi_target_sleep_state = ACPI_STATE_S0;
+ if (acpi_sleep_finish_wake_up)
+ acpi_pm_finish();
}
static int acpi_pm_state_valid(suspend_state_t pm_state)
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -167,6 +167,11 @@ and is between 256 and 4096 characters.
acpi_irq_isa= [HW,ACPI] If irq_balance, mark listed IRQs used by ISA
Format: <irq>,<irq>...
+ acpi_new_pts_ordering [HW,ACPI]
+ Enforce the ACPI 2.0 ordering of the _PTS control
+ method wrt putting devices into low power states
+ default: pre ACPI 2.0 ordering of _PTS
+
acpi_no_auto_ssdt [HW,ACPI] Disable automatic loading of SSDT
acpi_os_name= [HW,ACPI] Tell ACPI BIOS the name of the OS
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC][PATCH 5/7] Hibernation: Introduce open() and close() callbacks
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
` (3 preceding siblings ...)
2008-01-05 22:48 ` [RFC][PATCH 4/7] ACPI Suspend: Call _PTS before suspending devices Rafael J. Wysocki
@ 2008-01-05 22:50 ` Rafael J. Wysocki
2008-01-05 22:56 ` [RFC][PATCH 6/7] ACPI hibernation: Call _PTS before suspending devices Rafael J. Wysocki
2008-01-05 22:58 ` [RFC][PATCH 7/7] ACPI: Print message before calling _PTS Rafael J. Wysocki
6 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:50 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
Introduce global hibernation callback .close() and rename global
hibernation callback .start() to .open(), in analogy with the
recent modifications of the global suspend callbacks.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
drivers/acpi/sleep/main.c | 14 ++++++++++++--
include/linux/suspend.h | 8 ++++++--
kernel/power/disk.c | 33 ++++++++++++++++++++++++---------
3 files changed, 42 insertions(+), 13 deletions(-)
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -138,9 +138,12 @@ extern void mark_free_pages(struct zone
*
* All three methods must be assigned.
*
- * @start: Tell the platform driver that we're starting hibernation.
+ * @open: Tell the platform driver that we're starting hibernation.
* Called right after shrinking memory and before freezing devices.
*
+ * @close: Called by the PM core right after resuming devices, to indicate to
+ * the platform that the system has returned to the working state.
+ *
* @pre_snapshot: Prepare the platform for creating the hibernation image.
* Called right after devices have been frozen and before the nonboot
* CPUs are disabled (runs with IRQs on).
@@ -175,7 +178,8 @@ extern void mark_free_pages(struct zone
* thawing devices (runs with IRQs on).
*/
struct platform_hibernation_ops {
- int (*start)(void);
+ int (*open)(void);
+ void (*close)(void);
int (*pre_snapshot)(void);
void (*finish)(void);
int (*prepare)(void);
Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -54,8 +54,8 @@ static struct platform_hibernation_ops *
void hibernation_set_ops(struct platform_hibernation_ops *ops)
{
- if (ops && !(ops->start && ops->pre_snapshot && ops->finish
- && ops->prepare && ops->enter && ops->pre_restore
+ if (ops && !(ops->open && ops->close && ops->pre_snapshot
+ && ops->prepare && ops->finish && ops->enter && ops->pre_restore
&& ops->restore_cleanup)) {
WARN_ON(1);
return;
@@ -100,14 +100,25 @@ static int hibernation_test(int level) {
#endif /* !CONFIG_PM_DEBUG */
/**
- * platform_start - tell the platform driver that we're starting
+ * platform_open - tell the platform driver that we're starting
* hibernation
*/
-static int platform_start(int platform_mode)
+static int platform_open(int platform_mode)
{
return (platform_mode && hibernation_ops) ?
- hibernation_ops->start() : 0;
+ hibernation_ops->open() : 0;
+}
+
+/**
+ * platform_close - tell the platform driver that we've entered the
+ * working state
+ */
+
+static void platform_close(int platform_mode)
+{
+ if (platform_mode && hibernation_ops)
+ hibernation_ops->close();
}
/**
@@ -237,9 +248,9 @@ int hibernation_snapshot(int platform_mo
if (error)
return error;
- error = platform_start(platform_mode);
+ error = platform_open(platform_mode);
if (error)
- return error;
+ goto Close;
suspend_console();
error = device_suspend(PMSG_FREEZE);
@@ -272,6 +283,8 @@ int hibernation_snapshot(int platform_mo
device_resume();
Resume_console:
resume_console();
+ Close:
+ platform_close(platform_mode);
return error;
}
@@ -373,9 +386,9 @@ int hibernation_platform_enter(void)
* hibernation_ops->finish() before saving the image, so we should let
* the firmware know that we're going to enter the sleep state after all
*/
- error = hibernation_ops->start();
+ error = hibernation_ops->open();
if (error)
- return error;
+ goto Close;
suspend_console();
error = device_suspend(PMSG_SUSPEND);
@@ -409,6 +422,8 @@ int hibernation_platform_enter(void)
device_resume();
Resume_console:
resume_console();
+ Close:
+ hibernation_ops->close();
return error;
}
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -281,7 +281,7 @@ static struct dmi_system_id __initdata a
#endif /* CONFIG_SUSPEND */
#ifdef CONFIG_HIBERNATION
-static int acpi_hibernation_start(void)
+static int acpi_hibernation_open(void)
{
acpi_target_sleep_state = ACPI_STATE_S4;
return 0;
@@ -341,6 +341,15 @@ static void acpi_hibernation_finish(void
acpi_target_sleep_state = ACPI_STATE_S0;
}
+static void acpi_hibernation_close(void)
+{
+ /*
+ * This is necessary in case acpi_hibernation_finish() is not called
+ * during a failing transition to the sleep state.
+ */
+ acpi_target_sleep_state = ACPI_STATE_S0;
+}
+
static int acpi_hibernation_pre_restore(void)
{
acpi_status status;
@@ -356,7 +365,8 @@ static void acpi_hibernation_restore_cle
}
static struct platform_hibernation_ops acpi_hibernation_ops = {
- .start = acpi_hibernation_start,
+ .open = acpi_hibernation_open,
+ .close = acpi_hibernation_close,
.pre_snapshot = acpi_hibernation_prepare,
.finish = acpi_hibernation_finish,
.prepare = acpi_hibernation_prepare,
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC][PATCH 6/7] ACPI hibernation: Call _PTS before suspending devices
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
` (4 preceding siblings ...)
2008-01-05 22:50 ` [RFC][PATCH 5/7] Hibernation: Introduce open() and close() callbacks Rafael J. Wysocki
@ 2008-01-05 22:56 ` Rafael J. Wysocki
2008-01-05 22:58 ` [RFC][PATCH 7/7] ACPI: Print message before calling _PTS Rafael J. Wysocki
6 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:56 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
The ACPI 1.0 specification wants us to put devices into low power
states after executing the _PTS global control methods, while ACPI
2.0 and later want us to do that in the reverse order. The current
hibernation code follows ACPI 2.0 in that respect which may cause some
ACPI 1.0x systems to hang during hibernation (ref.
http://bugzilla.kernel.org/show_bug.cgi?id=9528).
Make the hibernation code execute _PTS before putting devices into
low power (ie. in accordance with ACPI 1.0x) with the possibility to
override that using the 'acpi_new_pts_ordering' kernel command line
option.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/acpi/sleep/main.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -283,22 +283,34 @@ static struct dmi_system_id __initdata a
#ifdef CONFIG_HIBERNATION
static int acpi_hibernation_open(void)
{
+ int error;
+
acpi_target_sleep_state = ACPI_STATE_S4;
- return 0;
+ if (new_pts_ordering)
+ return 0;
+
+ error = acpi_sleep_prepare(ACPI_STATE_S4);
+ if (error)
+ acpi_target_sleep_state = ACPI_STATE_S0;
+ else
+ acpi_sleep_finish_wake_up = true;
+
+ return error;
}
static int acpi_hibernation_prepare(void)
{
- int error;
-
- error = acpi_sleep_prepare(ACPI_STATE_S4);
- if (error)
- return error;
+ if (new_pts_ordering) {
+ int error = acpi_sleep_prepare(ACPI_STATE_S4);
- if (!ACPI_SUCCESS(acpi_hw_disable_all_gpes()))
- error = -EFAULT;
+ if (error) {
+ acpi_target_sleep_state = ACPI_STATE_S0;
+ return error;
+ }
+ acpi_sleep_finish_wake_up = true;
+ }
- return error;
+ return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
}
static int acpi_hibernation_enter(void)
@@ -339,15 +351,17 @@ static void acpi_hibernation_finish(void
acpi_set_firmware_waking_vector((acpi_physical_address) 0);
acpi_target_sleep_state = ACPI_STATE_S0;
+ acpi_sleep_finish_wake_up = false;
}
static void acpi_hibernation_close(void)
{
/*
* This is necessary in case acpi_hibernation_finish() is not called
- * during a failing transition to the sleep state.
+ * directly during a failing transition to the sleep state.
*/
- acpi_target_sleep_state = ACPI_STATE_S0;
+ if (acpi_sleep_finish_wake_up)
+ acpi_hibernation_finish();
}
static int acpi_hibernation_pre_restore(void)
^ permalink raw reply [flat|nested] 15+ messages in thread* [RFC][PATCH 7/7] ACPI: Print message before calling _PTS
2008-01-05 22:32 [RFC][PATCH 0/7] Fix the ACPI 1.0 vs ACPI 2.0 suspend ordering issue (rev. 2) Rafael J. Wysocki
` (5 preceding siblings ...)
2008-01-05 22:56 ` [RFC][PATCH 6/7] ACPI hibernation: Call _PTS before suspending devices Rafael J. Wysocki
@ 2008-01-05 22:58 ` Rafael J. Wysocki
6 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2008-01-05 22:58 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Arjan van de Ven, Carlos Corbacho, Linus Torvalds, Pavel Machek,
pm list, Andrew Morton, Len Brown, Alexey Starikovskiy,
Moore, Robert, Matthew Garrett
From: Rafael J. Wysocki <rjw@sisk.pl>
Make acpi_sleep_prepare() static and cause it to print a message
specifying the ACPI system sleep state to be entered (helpful for
debugging the suspend/hibernation code).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
drivers/acpi/sleep/main.c | 4 +++-
drivers/acpi/sleep/sleep.h | 2 --
2 files changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -43,7 +43,7 @@ static int __init acpi_new_pts_ordering(
__setup("acpi_new_pts_ordering", acpi_new_pts_ordering);
#endif
-int acpi_sleep_prepare(u32 acpi_state)
+static int acpi_sleep_prepare(u32 acpi_state)
{
#ifdef CONFIG_ACPI_SLEEP
/* do we have a wakeup address for S2 and S3? */
@@ -59,6 +59,8 @@ int acpi_sleep_prepare(u32 acpi_state)
ACPI_FLUSH_CPU_CACHE();
acpi_enable_wakeup_device_prep(acpi_state);
#endif
+ printk(KERN_INFO PREFIX "Preparing to enter system sleep state S%d\n",
+ acpi_state);
acpi_enter_sleep_state_prep(acpi_state);
return 0;
}
Index: linux-2.6/drivers/acpi/sleep/sleep.h
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/sleep.h
+++ linux-2.6/drivers/acpi/sleep/sleep.h
@@ -5,5 +5,3 @@ extern int acpi_suspend (u32 state);
extern void acpi_enable_wakeup_device_prep(u8 sleep_state);
extern void acpi_enable_wakeup_device(u8 sleep_state);
extern void acpi_disable_wakeup_device(u8 sleep_state);
-
-extern int acpi_sleep_prepare(u32 acpi_state);
^ permalink raw reply [flat|nested] 15+ messages in thread