From d5a64513c6a171262082c250592c062e97a2c693 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" <rjw@sisk.pl> Date: Fri, 9 Apr 2010 01:39:40 +0200 Subject: ACPI / EC / PM: Fix race between EC transactions and system suspend There still is a race that may result in suspending the system in the middle of an EC transaction in progress, which leads to problems (like the kernel thinking that the ACPI global lock is held during resume while in fact it's not). To remove the race condition, modify the ACPI platform suspend and hibernate callbacks so that EC transactions are blocked right after executing the _PTS global control method and are allowed to happen again right after the low-level wakeup. Introduce acpi_pm_freeze() that will disable GPEs, wait until the event queues are empty and block EC transactions. Use it wherever GPEs are disabled in preparation for switching local interrupts off. Introduce acpi_pm_thaw() that will allow EC transactions to happen again and enable runtime GPEs. Use it to balance acpi_pm_freeze() wherever necessary. In addition to that use acpi_ec_resume_transactions_early() to unblock EC transactions as early as reasonably possible during resume. Also unblock EC transactions in acpi_hibernation_finish() and in the analogous suspend routine to make sure that the EC transactions are enabled in all error paths. Fixes https://bugzilla.kernel.org/show_bug.cgi?id=14668 Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Reported-and-tested-by: Maxim Levitsky <maximlevitsky@gmail.com> Signed-off-by: Len Brown <len.brown@intel.com> --- drivers/acpi/internal.h | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/acpi/internal.h') diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index e28411367239..0ec48c7efa9b 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -51,6 +51,7 @@ int acpi_ec_ecdt_probe(void); int acpi_boot_ec_enable(void); void acpi_ec_suspend_transactions(void); void acpi_ec_resume_transactions(void); +void acpi_ec_resume_transactions_early(void); /*-------------------------------------------------------------------------- Suspend/Resume -- cgit From fe955682d2153b35dffcf1673dff0491096a3f0a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" <rjw@sisk.pl> Date: Fri, 9 Apr 2010 01:40:38 +0200 Subject: ACPI / EC / PM: Fix names of functions that block/unblock EC transactions The names of the functions used for blocking/unblocking EC transactions during suspend/hibernation suggest that the transactions are suspended and resumed by them, while in fact they are disabled and enabled. Rename the functions (and the flag used by them) to better reflect what they really do. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Signed-off-by: Len Brown <len.brown@intel.com> --- drivers/acpi/ec.c | 16 ++++++++-------- drivers/acpi/internal.h | 6 +++--- drivers/acpi/sleep.c | 12 ++++++------ 3 files changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers/acpi/internal.h') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 2c2b73a2a7c2..3f01f065b533 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -79,7 +79,7 @@ enum { EC_FLAGS_GPE_STORM, /* GPE storm detected */ EC_FLAGS_HANDLERS_INSTALLED, /* Handlers for GPE and * OpReg are installed */ - EC_FLAGS_FROZEN, /* Transactions are suspended */ + EC_FLAGS_BLOCKED, /* Transactions are blocked */ }; /* If we find an EC via the ECDT, we need to keep a ptr to its context */ @@ -293,7 +293,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) if (t->rdata) memset(t->rdata, 0, t->rlen); mutex_lock(&ec->lock); - if (test_bit(EC_FLAGS_FROZEN, &ec->flags)) { + if (test_bit(EC_FLAGS_BLOCKED, &ec->flags)) { status = -EINVAL; goto unlock; } @@ -459,7 +459,7 @@ int ec_transaction(u8 command, EXPORT_SYMBOL(ec_transaction); -void acpi_ec_suspend_transactions(void) +void acpi_ec_block_transactions(void) { struct acpi_ec *ec = first_ec; @@ -468,11 +468,11 @@ void acpi_ec_suspend_transactions(void) mutex_lock(&ec->lock); /* Prevent transactions from being carried out */ - set_bit(EC_FLAGS_FROZEN, &ec->flags); + set_bit(EC_FLAGS_BLOCKED, &ec->flags); mutex_unlock(&ec->lock); } -void acpi_ec_resume_transactions(void) +void acpi_ec_unblock_transactions(void) { struct acpi_ec *ec = first_ec; @@ -481,18 +481,18 @@ void acpi_ec_resume_transactions(void) mutex_lock(&ec->lock); /* Allow transactions to be carried out again */ - clear_bit(EC_FLAGS_FROZEN, &ec->flags); + clear_bit(EC_FLAGS_BLOCKED, &ec->flags); mutex_unlock(&ec->lock); } -void acpi_ec_resume_transactions_early(void) +void acpi_ec_unblock_transactions_early(void) { /* * Allow transactions to happen again (this function is called from * atomic context during wakeup, so we don't need to acquire the mutex). */ if (first_ec) - clear_bit(EC_FLAGS_FROZEN, &first_ec->flags); + clear_bit(EC_FLAGS_BLOCKED, &first_ec->flags); } static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 * data) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 0ec48c7efa9b..f8f190ec066e 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -49,9 +49,9 @@ void acpi_early_processor_set_pdc(void); int acpi_ec_init(void); int acpi_ec_ecdt_probe(void); int acpi_boot_ec_enable(void); -void acpi_ec_suspend_transactions(void); -void acpi_ec_resume_transactions(void); -void acpi_ec_resume_transactions_early(void); +void acpi_ec_block_transactions(void); +void acpi_ec_unblock_transactions(void); +void acpi_ec_unblock_transactions_early(void); /*-------------------------------------------------------------------------- Suspend/Resume diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 24741ac65897..504a55edac49 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -116,7 +116,7 @@ static int acpi_pm_freeze(void) { acpi_disable_all_gpes(); acpi_os_wait_events_complete(NULL); - acpi_ec_suspend_transactions(); + acpi_ec_block_transactions(); return 0; } @@ -279,7 +279,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) */ acpi_disable_all_gpes(); /* Allow EC transactions to happen. */ - acpi_ec_resume_transactions_early(); + acpi_ec_unblock_transactions_early(); local_irq_restore(flags); printk(KERN_DEBUG "Back to C!\n"); @@ -293,7 +293,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) static void acpi_suspend_finish(void) { - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_pm_finish(); } @@ -597,7 +597,7 @@ static int acpi_hibernation_enter(void) static void acpi_hibernation_finish(void) { hibernate_nvs_free(); - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_pm_finish(); } @@ -619,12 +619,12 @@ static void acpi_hibernation_leave(void) /* Restore the NVS memory area */ hibernate_nvs_restore(); /* Allow EC transactions to happen. */ - acpi_ec_resume_transactions_early(); + acpi_ec_unblock_transactions_early(); } static void acpi_pm_thaw(void) { - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_enable_all_runtime_gpes(); } -- cgit