ACPI / PM: Fixes and simplifications
cover ACPI / PM: Fixes and simplifications
Hi,
The following patches fix a few bugs in the ACPI platform suspend/hibernate code and simplify it quite a bit.
[1/5] - Fixes a theoretical issue in acpi_suspend_begin() related to the NVS saving.
[2/5] - Moves acpi_enable_wakeup_device() to acpi_sleep_prepare(), so that it
can be merged with acpi_enable_wakeup_device_prep().
[3/5] - Merges acpi_enable_wakeup_device() and acpi_enable_wakeup_device_prep()
and drops some unnecessary code.
[4/5] - Gets rid of some code duplication in drivers/acpi/sleep.c.
[5/5] - Drops an unnecessary routine from drivers/acpi/sleep.c.
Please apply.
Rafael
PATCH 1/5 ACPI / Sleep: Do not allocate memory for saving NVS in advance
From: Rafael J. Wysocki <[email protected]>
We only can try to allocate memory for saving the NVS region if it is known that the target system sleep state is valid. Otherwise we will leak memory if suspend_nvs_alloc() is successful and the target state is invalid, because suspend_nvs_free() will not be called in that case.
--- drivers/acpi/sleep.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/acpi/sleep.c +2 -7
@@ -195,20 +195,15 @@ static u32 acpi_suspend_states[] = {static int acpi_suspend_begin (suspend_state_t pm_state) {u32 acpi_state = acpi_suspend_states[pm_state]; -int error =0 ; - - error =suspend_nvs_alloc (); - -if (error) -return error; +int error = -ENOSYS ;if (sleep_states[acpi_state]) { acpi_target_sleep_state = acpi_state;acpi_sleep_tts_switch (acpi_target_sleep_state); + error =suspend_nvs_alloc (); }else {printk (KERN_ERR "ACPI does not support this state: %d\n" , pm_state); - error = -ENOSYS ; }return error; }
PATCH 1/5 ACPI / Sleep: Do not allocate memory for saving NVS in advance
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
We only can try to allocate memory for saving the NVS region if it is known that the target system sleep state is valid. Otherwise we will leak memory if suspend_nvs_alloc() is successful and the target state is invalid, because suspend_nvs_free() will not be called in that case.
--- drivers/acpi/sleep.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/acpi/sleep.c +2 -7
@@ -195,20 +195,15 @@ static u32 acpi_suspend_states[] = {static int acpi_suspend_begin (suspend_state_t pm_state) {u32 acpi_state = acpi_suspend_states[pm_state]; -int error =0 ; - - error =suspend_nvs_alloc (); - -if (error) -return error; +int error = -ENOSYS ;if (sleep_states[acpi_state]) { acpi_target_sleep_state = acpi_state;acpi_sleep_tts_switch (acpi_target_sleep_state); + error =suspend_nvs_alloc (); }else {printk (KERN_ERR "ACPI does not support this state: %d\n" , pm_state); - error = -ENOSYS ; }return error; }
Re: [PATCH 1/5] ACPI / Sleep: Do not allocate memory for saving NVS in advance
Quoted 9 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
We only can try to allocate memory for saving the NVS region if it is known that the target system sleep state is valid. Otherwise we will leak memory if
suspend_nvs_alloc()is successful and the target state is invalid, becausesuspend_nvs_free()will not be called in that case.Signed-off-by Rafael J. Wysocki <[email protected]>
Well, the $subject patch doesn't cover a real problem where the suspending of devices fails (or suspend is tested in the "devices" mode) and acpi_pm_finish() is not called at all. So, the appended patch should be used instead.
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Sleep: Free NVS copy if suspending of devices fails
If suspending of devices fails or system suspend is tested in the "devices" mode, the memory allocated for storing a copy of the ACPI NVS area will not be freed, because acpi_pm_finish() is not called in that case. Fix this by moving the suspend_nvs_free() call to acpi_pm_end().
--- drivers/acpi/sleep.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/acpi/sleep.c +1 -1
@@ -145,7 +145,6 @@ static void acpi_pm_finish(void) {u32 acpi_state = acpi_target_sleep_state; -suspend_nvs_free ();acpi_ec_unblock_transactions ();if (acpi_state ==ACPI_STATE_S0 )
@@ -167,6 +166,7 @@ static void acpi_pm_finish(void)*/ static void acpi_pm_end (void ) { +suspend_nvs_free ();/* * This is necessary in case acpi_pm_finish() is not called during a * failing transition to a sleep state.
Re: [linux-pm] [PATCH 1/5] ACPI / Sleep: Do not allocate memory for saving NVS in advance
In reply to Rafael J. WysockiQuoted 9 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
We only can try to allocate memory for saving the NVS region if it is known that the target system sleep state is valid. Otherwise we will leak memory if
suspend_nvs_alloc()is successful and the target state is invalid, becausesuspend_nvs_free()will not be called in that case.Signed-off-by Rafael J. Wysocki <[email protected]>
Well, the $subject patch doesn't cover a real problem where the suspending of devices fails (or suspend is tested in the "devices" mode) and acpi_pm_finish() is not called at all. So, the appended patch should be used instead.
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Sleep: Free NVS copy if suspending of devices fails
If suspending of devices fails or system suspend is tested in the "devices" mode, the memory allocated for storing a copy of the ACPI NVS area will not be freed, because acpi_pm_finish() is not called in that case. Fix this by moving the suspend_nvs_free() call to acpi_pm_end().
--- drivers/acpi/sleep.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/acpi/sleep.c +1 -1
@@ -145,7 +145,6 @@ static void acpi_pm_finish(void) {u32 acpi_state = acpi_target_sleep_state; -suspend_nvs_free ();acpi_ec_unblock_transactions ();if (acpi_state ==ACPI_STATE_S0 )
@@ -167,6 +166,7 @@ static void acpi_pm_finish(void)*/ static void acpi_pm_end (void ) { +suspend_nvs_free ();/* * This is necessary in case acpi_pm_finish() is not called during a * failing transition to a sleep state.
Len Brown
reviewerRe: [PATCH 1/5] ACPI / Sleep: Do not allocate memory for saving NVS in advance
In reply to Rafael J. Wysockiapplied the new version
thanks,
Len Brown, Intel Open Source Technology Center
Len Brown
reviewerRe: [PATCH 1/5] ACPI / Sleep: Do not allocate memory for saving NVS in advance
In reply to Rafael J. Wysockiapplied the new version
thanks,
Len Brown, Intel Open Source Technology Center
PATCH 2/5 ACPI / Sleep: Rework enabling wakeup devices
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
There is no reason why acpi_enable_wakeup_device() should be called with interrupts disabled, because it doesn't access hardware. Thus it is possible to move it next to acpi_enable_wakeup_device_prep() and make the ACPI suspend, hibernate and poweroff code more straightforward.
--- drivers/acpi/sleep.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/acpi/sleep.c +2 -4
@@ -70,10 +70,11 @@ static int acpi_sleep_prepare(u32 acpi_s }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_enable_wakeup_device_prep (acpi_state); +acpi_enable_wakeup_device (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -233,7 +234,6 @@ static int acpi_suspend_enter(suspend_st }local_irq_save (flags); -acpi_enable_wakeup_device (acpi_state);switch (acpi_state) {case ACPI_STATE_S1 :barrier ();
@@ -437,7 +437,6 @@ static int acpi_hibernation_enter(void)ACPI_FLUSH_CPU_CACHE ();local_irq_save (flags); -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 */
@@ -682,7 +681,6 @@ static void acpi_power_off(void)/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */ printk (KERN_DEBUG "%s called\n" , __func__);local_irq_disable (); -acpi_enable_wakeup_device (ACPI_STATE_S5 );acpi_enter_sleep_state (ACPI_STATE_S5 ); }
PATCH 2/5 ACPI / Sleep: Rework enabling wakeup devices
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
There is no reason why acpi_enable_wakeup_device() should be called with interrupts disabled, because it doesn't access hardware. Thus it is possible to move it next to acpi_enable_wakeup_device_prep() and make the ACPI suspend, hibernate and poweroff code more straightforward.
--- drivers/acpi/sleep.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/acpi/sleep.c +2 -4
@@ -70,10 +70,11 @@ static int acpi_sleep_prepare(u32 acpi_s }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_enable_wakeup_device_prep (acpi_state); +acpi_enable_wakeup_device (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -233,7 +234,6 @@ static int acpi_suspend_enter(suspend_st }local_irq_save (flags); -acpi_enable_wakeup_device (acpi_state);switch (acpi_state) {case ACPI_STATE_S1 :barrier ();
@@ -437,7 +437,6 @@ static int acpi_hibernation_enter(void)ACPI_FLUSH_CPU_CACHE ();local_irq_save (flags); -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 */
@@ -682,7 +681,6 @@ static void acpi_power_off(void)/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */ printk (KERN_DEBUG "%s called\n" , __func__);local_irq_disable (); -acpi_enable_wakeup_device (ACPI_STATE_S5 );acpi_enter_sleep_state (ACPI_STATE_S5 ); }
Re: [PATCH 2/5] ACPI / Sleep: Rework enabling wakeup devices
applied
thanks,
Len Brown, Intel Open Source Technology Center
Re: [PATCH 2/5] ACPI / Sleep: Rework enabling wakeup devices
In reply to Rafael J. Wysockiapplied
thanks,
Len Brown, Intel Open Source Technology Center
PATCH 3/5 ACPI / Wakeup: Simplify enabling of wakeup devices
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
To simplify the enabling of wakeup devices during system suspend and hibernation, merge acpi_enable_wakeup_device_prep() with acpi_disable_wakeup_device() and remove unnecessary (and no longer valid) comments from the latter. Rename acpi_enable_wakeup_device() to acpi_enable_wakeup_devices() and acpi_disable_wakeup_device() to acpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.
--- drivers/acpi/sleep.c | 5 ++--- drivers/acpi/sleep.h | 5 ++--- drivers/acpi/wakeup.c | 50 +++++++++++--------------------------------------- 3 files changed, 15 insertions(+), 45 deletions(-)
drivers/acpi/wakeup.c +11 -39
@@ -21,46 +21,18 @@ACPI_MODULE_NAME ("wakeup_devices" )/** -* acpi_enable_wakeup_device_prep - Prepare wake-up devices. +* acpi_enable_wakeup_devices - Enable wake-up device GPEs. * @sleep_state: ACPI system sleep state. * -* Enable all wake-up devices' power, unless the requested system sleep state is -* too deep. +* Enable wakeup device power of devices with the state.enable flag set and set +* the wakeup enable mask bits in the GPE registers that correspond to wakeup +* devices. */ -void acpi_enable_wakeup_device_prep (u8 sleep_state) +void acpi_enable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;list_for_each_safe (node, next, &acpi_wakeup_device_list) { -struct acpi_device *dev =container_of (node, - struct acpi_device, - wakeup_list); - -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) -continue ; - -acpi_enable_wakeup_device_power (dev, sleep_state); - } -} - -/** -* acpi_enable_wakeup_device - Enable wake-up device GPEs. -* @sleep_state: ACPI system sleep state. -* -* Enable all wake-up devices' GPEs, with the assumption that -* acpi_disable_all_gpes() was executed before, so we don't need to disable any -* GPEs here. -*/ -void acpi_enable_wakeup_device (u8 sleep_state) -{ -struct list_head *node, *next; - -/* -* Caution: this routine must be invoked when interrupt is disabled -* Refer ACPI2.0: P212 -*/ -list_for_each_safe (node, next, &acpi_wakeup_device_list) {struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list);
@@ -69,6 +41,9 @@ void acpi_enable_wakeup_device(u8 sleep_ || sleep_state > (u32) dev->wakeup.sleep_state)continue ; +if (dev->wakeup.state.enabled) +acpi_enable_wakeup_device_power (dev, sleep_state); +/* The wake-up power should have been enabled already. */ acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number,ACPI_GPE_ENABLE );
@@ -76,13 +51,10 @@ void acpi_enable_wakeup_device(u8 sleep_ }/** -* acpi_disable_wakeup_device - Disable devices' wakeup capability. +* acpi_disable_wakeup_devices - Disable devices' wakeup capability. * @sleep_state: ACPI system sleep state. -* -* This function only affects devices with wakeup.state.enabled set, which means -* that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ -void acpi_disable_wakeup_device (u8 sleep_state) +void acpi_disable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;
@@ -92,7 +64,7 @@ void acpi_disable_wakeup_device(u8 sleepif (!dev->wakeup.flags.valid || !(dev->wakeup.state.enabled || dev->wakeup.prepare_count) - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) + || sleep_state > (u32 ) dev->wakeup.sleep_state)continue ; acpi_gpe_wakeup(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
drivers/acpi/sleep.h +2 -3
@@ -2,9 +2,8 @@extern u8 sleep_states[];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 void acpi_enable_wakeup_devices (u8 sleep_state); +extern void acpi_disable_wakeup_devices (u8 sleep_state);extern struct list_head acpi_wakeup_device_list;extern struct mutex acpi_device_lock;
drivers/acpi/sleep.c +2 -3
@@ -73,8 +73,7 @@ static int acpi_sleep_prepare(u32 acpi_s#endif printk(KERN_INFO PREFIX "Preparing to enter system sleep state S%d\n", acpi_state); -acpi_enable_wakeup_device_prep (acpi_state); -acpi_enable_wakeup_device (acpi_state); +acpi_enable_wakeup_devices (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -154,7 +153,7 @@ static void acpi_pm_finish(void) printk(KERN_INFO PREFIX "Waking up from system sleep state S%d\n", acpi_state); -acpi_disable_wakeup_device (acpi_state); +acpi_disable_wakeup_devices (acpi_state);acpi_leave_sleep_state (acpi_state);/* reset firmware waking vector */
PATCH 3/5 ACPI / Wakeup: Simplify enabling of wakeup devices
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
To simplify the enabling of wakeup devices during system suspend and hibernation, merge acpi_enable_wakeup_device_prep() with acpi_disable_wakeup_device() and remove unnecessary (and no longer valid) comments from the latter. Rename acpi_enable_wakeup_device() to acpi_enable_wakeup_devices() and acpi_disable_wakeup_device() to acpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.
--- drivers/acpi/sleep.c | 5 ++--- drivers/acpi/sleep.h | 5 ++--- drivers/acpi/wakeup.c | 50 +++++++++++--------------------------------------- 3 files changed, 15 insertions(+), 45 deletions(-)
drivers/acpi/wakeup.c +11 -39
@@ -21,46 +21,18 @@ACPI_MODULE_NAME ("wakeup_devices" )/** -* acpi_enable_wakeup_device_prep - Prepare wake-up devices. +* acpi_enable_wakeup_devices - Enable wake-up device GPEs. * @sleep_state: ACPI system sleep state. * -* Enable all wake-up devices' power, unless the requested system sleep state is -* too deep. +* Enable wakeup device power of devices with the state.enable flag set and set +* the wakeup enable mask bits in the GPE registers that correspond to wakeup +* devices. */ -void acpi_enable_wakeup_device_prep (u8 sleep_state) +void acpi_enable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;list_for_each_safe (node, next, &acpi_wakeup_device_list) { -struct acpi_device *dev =container_of (node, - struct acpi_device, - wakeup_list); - -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) -continue ; - -acpi_enable_wakeup_device_power (dev, sleep_state); - } -} - -/** -* acpi_enable_wakeup_device - Enable wake-up device GPEs. -* @sleep_state: ACPI system sleep state. -* -* Enable all wake-up devices' GPEs, with the assumption that -* acpi_disable_all_gpes() was executed before, so we don't need to disable any -* GPEs here. -*/ -void acpi_enable_wakeup_device (u8 sleep_state) -{ -struct list_head *node, *next; - -/* -* Caution: this routine must be invoked when interrupt is disabled -* Refer ACPI2.0: P212 -*/ -list_for_each_safe (node, next, &acpi_wakeup_device_list) {struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list);
@@ -69,6 +41,9 @@ void acpi_enable_wakeup_device(u8 sleep_ || sleep_state > (u32) dev->wakeup.sleep_state)continue ; +if (dev->wakeup.state.enabled) +acpi_enable_wakeup_device_power (dev, sleep_state); +/* The wake-up power should have been enabled already. */ acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number,ACPI_GPE_ENABLE );
@@ -76,13 +51,10 @@ void acpi_enable_wakeup_device(u8 sleep_ }/** -* acpi_disable_wakeup_device - Disable devices' wakeup capability. +* acpi_disable_wakeup_devices - Disable devices' wakeup capability. * @sleep_state: ACPI system sleep state. -* -* This function only affects devices with wakeup.state.enabled set, which means -* that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ -void acpi_disable_wakeup_device (u8 sleep_state) +void acpi_disable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;
@@ -92,7 +64,7 @@ void acpi_disable_wakeup_device(u8 sleepif (!dev->wakeup.flags.valid || !(dev->wakeup.state.enabled || dev->wakeup.prepare_count) - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) + || sleep_state > (u32 ) dev->wakeup.sleep_state)continue ; acpi_gpe_wakeup(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
drivers/acpi/sleep.h +2 -3
@@ -2,9 +2,8 @@extern u8 sleep_states[];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 void acpi_enable_wakeup_devices (u8 sleep_state); +extern void acpi_disable_wakeup_devices (u8 sleep_state);extern struct list_head acpi_wakeup_device_list;extern struct mutex acpi_device_lock;
drivers/acpi/sleep.c +2 -3
@@ -73,8 +73,7 @@ static int acpi_sleep_prepare(u32 acpi_s#endif printk(KERN_INFO PREFIX "Preparing to enter system sleep state S%d\n", acpi_state); -acpi_enable_wakeup_device_prep (acpi_state); -acpi_enable_wakeup_device (acpi_state); +acpi_enable_wakeup_devices (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -154,7 +153,7 @@ static void acpi_pm_finish(void) printk(KERN_INFO PREFIX "Waking up from system sleep state S%d\n", acpi_state); -acpi_disable_wakeup_device (acpi_state); +acpi_disable_wakeup_devices (acpi_state);acpi_leave_sleep_state (acpi_state);/* reset firmware waking vector */
Re: [PATCH 3/5] ACPI / Wakeup: Simplify enabling of wakeup devices
Quoted 11 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
To simplify the enabling of wakeup devices during system suspend and hibernation, merge
acpi_enable_wakeup_device_prep()withacpi_disable_wakeup_device()and remove unnecessary (and no longer valid) comments from the latter. Renameacpi_enable_wakeup_device()toacpi_enable_wakeup_devices()andacpi_disable_wakeup_device()toacpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.Signed-off-by Rafael J. Wysocki <[email protected]>
Below is a version without the last hunk in wakeup.c (that didin't apply and was cosmetic-only).
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Wakeup: Simplify enabling of wakeup devices
To simplify the enabling of wakeup devices during system suspend and hibernation, merge acpi_enable_wakeup_device_prep() with acpi_disable_wakeup_device() and remove unnecessary (and no longer valid) comments from the latter. Rename acpi_enable_wakeup_device() to acpi_enable_wakeup_devices() and acpi_disable_wakeup_device() to acpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.
--- drivers/acpi/sleep.c | 5 ++--- drivers/acpi/sleep.h | 5 ++--- drivers/acpi/wakeup.c | 50 +++++++++++--------------------------------------- 3 files changed, 15 insertions(+), 45 deletions(-)
drivers/acpi/wakeup.c +10 -38
@@ -21,46 +21,18 @@ACPI_MODULE_NAME ("wakeup_devices" )/** -* acpi_enable_wakeup_device_prep - Prepare wake-up devices. +* acpi_enable_wakeup_devices - Enable wake-up device GPEs. * @sleep_state: ACPI system sleep state. * -* Enable all wake-up devices' power, unless the requested system sleep state is -* too deep. +* Enable wakeup device power of devices with the state.enable flag set and set +* the wakeup enable mask bits in the GPE registers that correspond to wakeup +* devices. */ -void acpi_enable_wakeup_device_prep (u8 sleep_state) +void acpi_enable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;list_for_each_safe (node, next, &acpi_wakeup_device_list) { -struct acpi_device *dev =container_of (node, - struct acpi_device, - wakeup_list); - -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) -continue ; - -acpi_enable_wakeup_device_power (dev, sleep_state); - } -} - -/** -* acpi_enable_wakeup_device - Enable wake-up device GPEs. -* @sleep_state: ACPI system sleep state. -* -* Enable all wake-up devices' GPEs, with the assumption that -* acpi_disable_all_gpes() was executed before, so we don't need to disable any -* GPEs here. -*/ -void acpi_enable_wakeup_device (u8 sleep_state) -{ -struct list_head *node, *next; - -/* -* Caution: this routine must be invoked when interrupt is disabled -* Refer ACPI2.0: P212 -*/ -list_for_each_safe (node, next, &acpi_wakeup_device_list) {struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list);
@@ -69,6 +41,9 @@ void acpi_enable_wakeup_device(u8 sleep_ || sleep_state > (u32) dev->wakeup.sleep_state)continue ; +if (dev->wakeup.state.enabled) +acpi_enable_wakeup_device_power (dev, sleep_state); +/* The wake-up power should have been enabled already. */ acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number,ACPI_GPE_ENABLE );
@@ -76,13 +51,10 @@ void acpi_enable_wakeup_device(u8 sleep_ }/** -* acpi_disable_wakeup_device - Disable devices' wakeup capability. +* acpi_disable_wakeup_devices - Disable devices' wakeup capability. * @sleep_state: ACPI system sleep state. -* -* This function only affects devices with wakeup.state.enabled set, which means -* that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ -void acpi_disable_wakeup_device (u8 sleep_state) +void acpi_disable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;
drivers/acpi/sleep.h +2 -3
@@ -2,9 +2,8 @@extern u8 sleep_states[];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 void acpi_enable_wakeup_devices (u8 sleep_state); +extern void acpi_disable_wakeup_devices (u8 sleep_state);extern struct list_head acpi_wakeup_device_list;extern struct mutex acpi_device_lock;
drivers/acpi/sleep.c +2 -3
@@ -73,8 +73,7 @@ static int acpi_sleep_prepare(u32 acpi_s#endif printk(KERN_INFO PREFIX "Preparing to enter system sleep state S%d\n", acpi_state); -acpi_enable_wakeup_device_prep (acpi_state); -acpi_enable_wakeup_device (acpi_state); +acpi_enable_wakeup_devices (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -153,7 +152,7 @@ static void acpi_pm_finish(void) printk(KERN_INFO PREFIX "Waking up from system sleep state S%d\n", acpi_state); -acpi_disable_wakeup_device (acpi_state); +acpi_disable_wakeup_devices (acpi_state);acpi_leave_sleep_state (acpi_state);/* reset firmware waking vector */
Re: [linux-pm] [PATCH 3/5] ACPI / Wakeup: Simplify enabling of wakeup devices
In reply to Rafael J. WysockiQuoted 11 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
To simplify the enabling of wakeup devices during system suspend and hibernation, merge
acpi_enable_wakeup_device_prep()withacpi_disable_wakeup_device()and remove unnecessary (and no longer valid) comments from the latter. Renameacpi_enable_wakeup_device()toacpi_enable_wakeup_devices()andacpi_disable_wakeup_device()toacpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.Signed-off-by Rafael J. Wysocki <[email protected]>
Below is a version without the last hunk in wakeup.c (that didin't apply and was cosmetic-only).
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / Wakeup: Simplify enabling of wakeup devices
To simplify the enabling of wakeup devices during system suspend and hibernation, merge acpi_enable_wakeup_device_prep() with acpi_disable_wakeup_device() and remove unnecessary (and no longer valid) comments from the latter. Rename acpi_enable_wakeup_device() to acpi_enable_wakeup_devices() and acpi_disable_wakeup_device() to acpi_disable_wakeup_devices(), because these functions usually operate on multiple device objects.
--- drivers/acpi/sleep.c | 5 ++--- drivers/acpi/sleep.h | 5 ++--- drivers/acpi/wakeup.c | 50 +++++++++++--------------------------------------- 3 files changed, 15 insertions(+), 45 deletions(-)
drivers/acpi/wakeup.c +10 -38
@@ -21,46 +21,18 @@ACPI_MODULE_NAME ("wakeup_devices" )/** -* acpi_enable_wakeup_device_prep - Prepare wake-up devices. +* acpi_enable_wakeup_devices - Enable wake-up device GPEs. * @sleep_state: ACPI system sleep state. * -* Enable all wake-up devices' power, unless the requested system sleep state is -* too deep. +* Enable wakeup device power of devices with the state.enable flag set and set +* the wakeup enable mask bits in the GPE registers that correspond to wakeup +* devices. */ -void acpi_enable_wakeup_device_prep (u8 sleep_state) +void acpi_enable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;list_for_each_safe (node, next, &acpi_wakeup_device_list) { -struct acpi_device *dev =container_of (node, - struct acpi_device, - wakeup_list); - -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled - || (sleep_state > (u32 ) dev->wakeup.sleep_state)) -continue ; - -acpi_enable_wakeup_device_power (dev, sleep_state); - } -} - -/** -* acpi_enable_wakeup_device - Enable wake-up device GPEs. -* @sleep_state: ACPI system sleep state. -* -* Enable all wake-up devices' GPEs, with the assumption that -* acpi_disable_all_gpes() was executed before, so we don't need to disable any -* GPEs here. -*/ -void acpi_enable_wakeup_device (u8 sleep_state) -{ -struct list_head *node, *next; - -/* -* Caution: this routine must be invoked when interrupt is disabled -* Refer ACPI2.0: P212 -*/ -list_for_each_safe (node, next, &acpi_wakeup_device_list) {struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list);
@@ -69,6 +41,9 @@ void acpi_enable_wakeup_device(u8 sleep_ || sleep_state > (u32) dev->wakeup.sleep_state)continue ; +if (dev->wakeup.state.enabled) +acpi_enable_wakeup_device_power (dev, sleep_state); +/* The wake-up power should have been enabled already. */ acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number,ACPI_GPE_ENABLE );
@@ -76,13 +51,10 @@ void acpi_enable_wakeup_device(u8 sleep_ }/** -* acpi_disable_wakeup_device - Disable devices' wakeup capability. +* acpi_disable_wakeup_devices - Disable devices' wakeup capability. * @sleep_state: ACPI system sleep state. -* -* This function only affects devices with wakeup.state.enabled set, which means -* that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ -void acpi_disable_wakeup_device (u8 sleep_state) +void acpi_disable_wakeup_devices (u8 sleep_state) {struct list_head *node, *next;
drivers/acpi/sleep.h +2 -3
@@ -2,9 +2,8 @@extern u8 sleep_states[];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 void acpi_enable_wakeup_devices (u8 sleep_state); +extern void acpi_disable_wakeup_devices (u8 sleep_state);extern struct list_head acpi_wakeup_device_list;extern struct mutex acpi_device_lock;
drivers/acpi/sleep.c +2 -3
@@ -73,8 +73,7 @@ static int acpi_sleep_prepare(u32 acpi_s#endif printk(KERN_INFO PREFIX "Preparing to enter system sleep state S%d\n", acpi_state); -acpi_enable_wakeup_device_prep (acpi_state); -acpi_enable_wakeup_device (acpi_state); +acpi_enable_wakeup_devices (acpi_state);acpi_enter_sleep_state_prep (acpi_state);return 0 ; }
@@ -153,7 +152,7 @@ static void acpi_pm_finish(void) printk(KERN_INFO PREFIX "Waking up from system sleep state S%d\n", acpi_state); -acpi_disable_wakeup_device (acpi_state); +acpi_disable_wakeup_devices (acpi_state);acpi_leave_sleep_state (acpi_state);/* reset firmware waking vector */
Re: [PATCH 3/5] ACPI / Wakeup: Simplify enabling of wakeup devices
applied
thanks,
Len Brown, Intel Open Source Technology Center
Re: [linux-pm] [PATCH 3/5] ACPI / Wakeup: Simplify enabling of wakeup devices
In reply to Rafael J. Wysockiapplied
thanks,
Len Brown, Intel Open Source Technology Center
PATCH 4/5 ACPI / Sleep: Consolidate suspend and hibernation routines
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
Some hibernation and suspend routines can be merged, which reduces the complexity of code a bit.
--- drivers/acpi/sleep.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
drivers/acpi/sleep.c +16 -26
@@ -105,6 +105,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
@@ -113,11 +123,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; }
@@ -128,9 +136,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; }
@@ -317,9 +324,9 @@ static struct platform_suspend_ops acpi_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; }
@@ -330,7 +337,7 @@ static int acpi_suspend_begin_old(suspenstatic 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,
@@ -418,16 +425,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 ;
@@ -475,7 +472,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,
@@ -511,13 +508,6 @@ static int acpi_hibernation_begin_old(voreturn 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.
@@ -525,7 +515,7 @@ static int acpi_hibernation_pre_snapshotstatic 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,
PATCH 4/5 ACPI / Sleep: Consolidate suspend and hibernation routines
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
Some hibernation and suspend routines can be merged, which reduces the complexity of code a bit.
--- drivers/acpi/sleep.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
drivers/acpi/sleep.c +16 -26
@@ -105,6 +105,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
@@ -113,11 +123,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; }
@@ -128,9 +136,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; }
@@ -317,9 +324,9 @@ static struct platform_suspend_ops acpi_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; }
@@ -330,7 +337,7 @@ static int acpi_suspend_begin_old(suspenstatic 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,
@@ -418,16 +425,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 ;
@@ -475,7 +472,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,
@@ -511,13 +508,6 @@ static int acpi_hibernation_begin_old(voreturn 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.
@@ -525,7 +515,7 @@ static int acpi_hibernation_pre_snapshotstatic 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,
Re: [PATCH 4/5] ACPI / Sleep: Consolidate suspend and hibernation routines
applied
thanks,
Len Brown, Intel Open Source Technology Center
Re: [PATCH 4/5] ACPI / Sleep: Consolidate suspend and hibernation routines
In reply to Rafael J. Wysockiapplied
thanks,
Len Brown, Intel Open Source Technology Center
PATCH 5/5 ACPI / Sleep: Drop acpi_suspend_finish()
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
The function acpi_suspend_finish() is not necessary any more, because acpi_pm_finish() can be used instead of it just fine. Remove it.
--- drivers/acpi/sleep.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/acpi/sleep.c +2 -7
@@ -285,11 +285,6 @@ static int acpi_suspend_enter(suspend_streturn ACPI_SUCCESS (status) ?0 : -EFAULT ; } -static void acpi_suspend_finish (void ) -{ -acpi_pm_finish (); -} -static int acpi_suspend_state_valid (suspend_state_t pm_state) {u32 acpi_state;
@@ -311,7 +306,7 @@ static struct platform_suspend_ops acpi_ .begin = acpi_suspend_begin, .prepare_late = acpi_pm_prepare, .enter = acpi_suspend_enter, - .wake = acpi_suspend_finish, + .wake = acpi_pm_finish, .end = acpi_pm_end, };
@@ -339,7 +334,7 @@ static struct platform_suspend_ops acpi_ .begin = acpi_suspend_begin_old, .prepare_late = acpi_pm_pre_suspend, .enter = acpi_suspend_enter, - .wake = acpi_suspend_finish, + .wake = acpi_pm_finish, .end = acpi_pm_end, .recover = acpi_pm_finish, };
PATCH 5/5 ACPI / Sleep: Drop acpi_suspend_finish()
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
The function acpi_suspend_finish() is not necessary any more, because acpi_pm_finish() can be used instead of it just fine. Remove it.
--- drivers/acpi/sleep.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/acpi/sleep.c +2 -7
@@ -285,11 +285,6 @@ static int acpi_suspend_enter(suspend_streturn ACPI_SUCCESS (status) ?0 : -EFAULT ; } -static void acpi_suspend_finish (void ) -{ -acpi_pm_finish (); -} -static int acpi_suspend_state_valid (suspend_state_t pm_state) {u32 acpi_state;
@@ -311,7 +306,7 @@ static struct platform_suspend_ops acpi_ .begin = acpi_suspend_begin, .prepare_late = acpi_pm_prepare, .enter = acpi_suspend_enter, - .wake = acpi_suspend_finish, + .wake = acpi_pm_finish, .end = acpi_pm_end, };
@@ -339,7 +334,7 @@ static struct platform_suspend_ops acpi_ .begin = acpi_suspend_begin_old, .prepare_late = acpi_pm_pre_suspend, .enter = acpi_suspend_enter, - .wake = acpi_suspend_finish, + .wake = acpi_pm_finish, .end = acpi_pm_end, .recover = acpi_pm_finish, };
Re: [PATCH 5/5] ACPI / Sleep: Drop acpi_suspend_finish()
applied
thanks,
Len Brown, Intel Open Source Technology Center