From 9cd031441a4fd09273b7c2beb337d0b2683d104c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:32:23 +0100 Subject: [PATCH 1/6] ACPI / ACPICA: Fix global lock acquisition There are two problems with the ACPICA's current implementation of the global lock acquisition. First, acpi_ev_global_lock_handler(), which in fact is an interface to the outside of the kernel, doesn't validate its input, so it only works correctly if the other side (i.e. the ACPI firmware) is fully specification-compliant (as far as the global lock is concerned). Unfortunately, that's known not to be the case on some systems (i.e. we get spurious global lock signaling interrupts without the pending flag set on some systems). Second, acpi_ev_global_lock_handler() attempts to acquire the global lock on behalf of a thread waiting for it without checking if there actually is such a thread. Both of these shortcomings need to be addressed to prevent all possible race conditions from happening. Rework acpi_ev_global_lock_handler() so that it doesn't try to acquire the global lock and make it signal the availability of the global lock to the waiting thread instead. Make sure that the availability of the global lock can only be signaled when there is a thread waiting for it and that it can't be signaled more than once in a row (to keep acpi_gbl_global_lock_semaphore in balance). Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/acpica/evmisc.c | 98 +++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 41 deletions(-) diff --git a/drivers/acpi/acpica/evmisc.c b/drivers/acpi/acpica/evmisc.c index fcaed9fb44ff..8e31bb5a973a 100644 --- a/drivers/acpi/acpica/evmisc.c +++ b/drivers/acpi/acpica/evmisc.c @@ -284,41 +284,41 @@ static void ACPI_SYSTEM_XFACE acpi_ev_notify_dispatch(void *context) * RETURN: ACPI_INTERRUPT_HANDLED * * DESCRIPTION: Invoked directly from the SCI handler when a global lock - * release interrupt occurs. Attempt to acquire the global lock, - * if successful, signal the thread waiting for the lock. + * release interrupt occurs. If there's a thread waiting for + * the global lock, signal it. * * NOTE: Assumes that the semaphore can be signaled from interrupt level. If * this is not possible for some reason, a separate thread will have to be * scheduled to do this. * ******************************************************************************/ +static u8 acpi_ev_global_lock_pending; +static spinlock_t _acpi_ev_global_lock_pending_lock; +#define acpi_ev_global_lock_pending_lock &_acpi_ev_global_lock_pending_lock static u32 acpi_ev_global_lock_handler(void *context) { - u8 acquired = FALSE; + acpi_status status; + acpi_cpu_flags flags; - /* - * Attempt to get the lock. - * - * If we don't get it now, it will be marked pending and we will - * take another interrupt when it becomes free. - */ - ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired); - if (acquired) { + flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock); - /* Got the lock, now wake all threads waiting for it */ - - acpi_gbl_global_lock_acquired = TRUE; - /* Send a unit to the semaphore */ - - if (ACPI_FAILURE - (acpi_os_signal_semaphore - (acpi_gbl_global_lock_semaphore, 1))) { - ACPI_ERROR((AE_INFO, - "Could not signal Global Lock semaphore")); - } + if (!acpi_ev_global_lock_pending) { + goto out; } + /* Send a unit to the semaphore */ + + status = acpi_os_signal_semaphore(acpi_gbl_global_lock_semaphore, 1); + if (ACPI_FAILURE(status)) { + ACPI_ERROR((AE_INFO, "Could not signal Global Lock semaphore")); + } + + acpi_ev_global_lock_pending = FALSE; + + out: + acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags); + return (ACPI_INTERRUPT_HANDLED); } @@ -415,6 +415,7 @@ static int acpi_ev_global_lock_acquired; acpi_status acpi_ev_acquire_global_lock(u16 timeout) { + acpi_cpu_flags flags; acpi_status status = AE_OK; u8 acquired = FALSE; @@ -467,32 +468,47 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout) return_ACPI_STATUS(AE_OK); } - /* Attempt to acquire the actual hardware lock */ + flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock); - ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired); - if (acquired) { + do { - /* We got the lock */ + /* Attempt to acquire the actual hardware lock */ + ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired); + if (acquired) { + acpi_gbl_global_lock_acquired = TRUE; + + ACPI_DEBUG_PRINT((ACPI_DB_EXEC, + "Acquired hardware Global Lock\n")); + break; + } + + acpi_ev_global_lock_pending = TRUE; + + acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags); + + /* + * Did not get the lock. The pending bit was set above, and we + * must wait until we get the global lock released interrupt. + */ ACPI_DEBUG_PRINT((ACPI_DB_EXEC, - "Acquired hardware Global Lock\n")); + "Waiting for hardware Global Lock\n")); - acpi_gbl_global_lock_acquired = TRUE; - return_ACPI_STATUS(AE_OK); - } + /* + * Wait for handshake with the global lock interrupt handler. + * This interface releases the interpreter if we must wait. + */ + status = acpi_ex_system_wait_semaphore( + acpi_gbl_global_lock_semaphore, + ACPI_WAIT_FOREVER); - /* - * Did not get the lock. The pending bit was set above, and we must now - * wait until we get the global lock released interrupt. - */ - ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "Waiting for hardware Global Lock\n")); + flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock); - /* - * Wait for handshake with the global lock interrupt handler. - * This interface releases the interpreter if we must wait. - */ - status = acpi_ex_system_wait_semaphore(acpi_gbl_global_lock_semaphore, - ACPI_WAIT_FOREVER); + } while (ACPI_SUCCESS(status)); + + acpi_ev_global_lock_pending = FALSE; + + acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags); return_ACPI_STATUS(status); } From b014f4f1aad3f25d5c7d877a394869645ea0c96b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:33:30 +0100 Subject: [PATCH 2/6] ACPI / PM: Do not enable multiple devices to wake up simultaneously If a device is enabled to wake up the system from sleep states via /proc/acpi/wakeup and there are other devices associated with the same wakeup GPE, all of these devices are automatically enabled to wake up the system. This isn't correct, because the fact the GPE is shared need not imply that wakeup power has to be enabled for all the devices at the same time (i.e. it is possible that one device will have its wakeup power enabled and it will wake up the system from a sleep state if the shared wakeup GPE is enabled, while another device having its wakeup power disabled will not wake up the system even though the GPE is enabled). Rework acpi_system_write_wakeup_device() so that it only enables wakeup for one device at a time. Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/proc.c | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c index afad67769db6..129effbb7bd4 100644 --- a/drivers/acpi/proc.c +++ b/drivers/acpi/proc.c @@ -341,7 +341,6 @@ acpi_system_write_wakeup_device(struct file *file, char strbuf[5]; char str[5] = ""; unsigned int len = count; - struct acpi_device *found_dev = NULL; if (len > 4) len = 4; @@ -363,33 +362,10 @@ acpi_system_write_wakeup_device(struct file *file, if (!strncmp(dev->pnp.bus_id, str, 4)) { dev->wakeup.state.enabled = dev->wakeup.state.enabled ? 0 : 1; - found_dev = dev; + physical_device_enable_wakeup(dev); break; } } - if (found_dev) { - physical_device_enable_wakeup(found_dev); - list_for_each_safe(node, next, &acpi_wakeup_device_list) { - struct acpi_device *dev = container_of(node, - struct - acpi_device, - wakeup_list); - - if ((dev != found_dev) && - (dev->wakeup.gpe_number == - found_dev->wakeup.gpe_number) - && (dev->wakeup.gpe_device == - found_dev->wakeup.gpe_device)) { - printk(KERN_WARNING - "ACPI: '%s' and '%s' have the same GPE, " - "can't disable/enable one separately\n", - dev->pnp.bus_id, found_dev->pnp.bus_id); - dev->wakeup.state.enabled = - found_dev->wakeup.state.enabled; - physical_device_enable_wakeup(dev); - } - } - } mutex_unlock(&acpi_device_lock); return count; } From f2b56bc808addb908a5bf435d9b942c02af9a7c4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:34:22 +0100 Subject: [PATCH 3/6] ACPI / PM: Use device wakeup flags for handling ACPI wakeup devices There are ACPI devices (buttons and the laptop lid) that can wake up the system from sleep states and have no "physical" companion devices. The ACPI subsystem uses two flags, wakeup.state.enabled and wakeup.flags.always_enabled, for handling those devices, but they are not accessible through the standard device wakeup infrastructure. User space can only control them via the /proc/acpi/wakeup interface that is not really convenient (e.g. the way in which devices are enabled to wake up the system is not portable between different systems, because it requires one to know the devices' "names" used in the system's ACPI tables). To address this problem, use standard device wakeup flags instead of the special ACPI flags for handling those devices. In particular, use device_set_wakeup_capable() to mark the ACPI wakeup devices during initialization and use device_set_wakeup_enable() to allow or disallow them to wake up the system from sleep states. Rework the /proc/acpi/wakeup interface to take these changes into account. Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/button.c | 4 ++-- drivers/acpi/proc.c | 19 +++++++++++++------ drivers/acpi/scan.c | 2 +- drivers/acpi/wakeup.c | 18 ++++++++++-------- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 71ef9cd0735f..234c104fcdd8 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -426,7 +426,7 @@ static int acpi_button_add(struct acpi_device *device) acpi_enable_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number); device->wakeup.run_wake_count++; - device->wakeup.state.enabled = 1; + device_set_wakeup_enable(&device->dev, true); } printk(KERN_INFO PREFIX "%s [%s]\n", name, acpi_device_bid(device)); @@ -449,7 +449,7 @@ static int acpi_button_remove(struct acpi_device *device, int type) acpi_disable_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number); device->wakeup.run_wake_count--; - device->wakeup.state.enabled = 0; + device_set_wakeup_enable(&device->dev, false); } acpi_button_remove_fs(device); diff --git a/drivers/acpi/proc.c b/drivers/acpi/proc.c index 129effbb7bd4..f5f986991b52 100644 --- a/drivers/acpi/proc.c +++ b/drivers/acpi/proc.c @@ -311,7 +311,9 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset) dev->pnp.bus_id, (u32) dev->wakeup.sleep_state, dev->wakeup.flags.run_wake ? '*' : ' ', - dev->wakeup.state.enabled ? "enabled" : "disabled"); + (device_may_wakeup(&dev->dev) + || (ldev && device_may_wakeup(ldev))) ? + "enabled" : "disabled"); if (ldev) seq_printf(seq, "%s:%s", ldev->bus ? ldev->bus->name : "no-bus", @@ -328,8 +330,10 @@ static void physical_device_enable_wakeup(struct acpi_device *adev) { struct device *dev = acpi_get_physical_device(adev->handle); - if (dev && device_can_wakeup(dev)) - device_set_wakeup_enable(dev, adev->wakeup.state.enabled); + if (dev && device_can_wakeup(dev)) { + bool enable = !device_may_wakeup(dev); + device_set_wakeup_enable(dev, enable); + } } static ssize_t @@ -360,9 +364,12 @@ acpi_system_write_wakeup_device(struct file *file, continue; if (!strncmp(dev->pnp.bus_id, str, 4)) { - dev->wakeup.state.enabled = - dev->wakeup.state.enabled ? 0 : 1; - physical_device_enable_wakeup(dev); + if (device_can_wakeup(&dev->dev)) { + bool enable = !device_may_wakeup(&dev->dev); + device_set_wakeup_enable(&dev->dev, enable); + } else { + physical_device_enable_wakeup(dev); + } break; } } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 29ef505c487b..bf7acbff1f5f 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -803,7 +803,7 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device) /* Power button, Lid switch always enable wakeup */ if (!acpi_match_device_ids(device, button_device_ids)) { device->wakeup.flags.run_wake = 1; - device->wakeup.flags.always_enabled = 1; + device_set_wakeup_capable(&device->dev, true); return; } diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c index f62a50c3ed34..f252d0de9922 100644 --- a/drivers/acpi/wakeup.c +++ b/drivers/acpi/wakeup.c @@ -37,11 +37,12 @@ void acpi_enable_wakeup_devices(u8 sleep_state) container_of(node, struct acpi_device, wakeup_list); if (!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 + || !(device_may_wakeup(&dev->dev) + || dev->wakeup.prepare_count)) continue; - if (dev->wakeup.state.enabled) + if (device_may_wakeup(&dev->dev)) acpi_enable_wakeup_device_power(dev, sleep_state); /* The wake-up power should have been enabled already. */ @@ -63,14 +64,15 @@ void acpi_disable_wakeup_devices(u8 sleep_state) container_of(node, struct acpi_device, wakeup_list); if (!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 + || !(device_may_wakeup(&dev->dev) + || dev->wakeup.prepare_count)) continue; acpi_gpe_wakeup(dev->wakeup.gpe_device, dev->wakeup.gpe_number, ACPI_GPE_DISABLE); - if (dev->wakeup.state.enabled) + if (device_may_wakeup(&dev->dev)) acpi_disable_wakeup_device_power(dev); } } @@ -84,8 +86,8 @@ int __init acpi_wakeup_device_init(void) struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); - if (dev->wakeup.flags.always_enabled) - dev->wakeup.state.enabled = 1; + if (device_can_wakeup(&dev->dev)) + device_set_wakeup_enable(&dev->dev, true); } mutex_unlock(&acpi_device_lock); return 0; From 7fa69baf29de8c77a6b32c054df2abb8f11f8aa4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:35:10 +0100 Subject: [PATCH 4/6] ACPI / PM: Drop special ACPI wakeup flags Drop special ACPI wakeup flags, wakeup.state.enabled and wakeup.flags.always_enabled, that aren't necessary any more after we've started to use standard device wakeup flags for handling ACPI wakeup devices. Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/glue.c | 5 +---- include/acpi/acpi_bus.h | 6 ------ 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c index 78b0164c35b2..7c47ed55e528 100644 --- a/drivers/acpi/glue.c +++ b/drivers/acpi/glue.c @@ -167,11 +167,8 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle) "firmware_node"); ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj, "physical_node"); - if (acpi_dev->wakeup.flags.valid) { + if (acpi_dev->wakeup.flags.valid) device_set_wakeup_capable(dev, true); - device_set_wakeup_enable(dev, - acpi_dev->wakeup.state.enabled); - } } return 0; diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 359ef11725a6..f2499f572109 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -242,20 +242,14 @@ struct acpi_device_perf { struct acpi_device_wakeup_flags { u8 valid:1; /* Can successfully enable wakeup? */ u8 run_wake:1; /* Run-Wake GPE devices */ - u8 always_enabled:1; /* Run-wake devices that are always enabled */ u8 notifier_present:1; /* Wake-up notify handler has been installed */ }; -struct acpi_device_wakeup_state { - u8 enabled:1; -}; - struct acpi_device_wakeup { acpi_handle gpe_device; u64 gpe_number; u64 sleep_state; struct acpi_handle_list resources; - struct acpi_device_wakeup_state state; struct acpi_device_wakeup_flags flags; int prepare_count; int run_wake_count; From 1f83511bd8f44b8a9e2d82263b2c95f26a625fcc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:36:01 +0100 Subject: [PATCH 5/6] ACPI / PM: Report wakeup events from buttons Since ACPI buttons and lids can be configured to wake up the system from sleep states, report wakeup events from these devices. Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/button.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 234c104fcdd8..76bbb78a5ad9 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -279,6 +279,9 @@ static int acpi_lid_send_state(struct acpi_device *device) input_report_switch(button->input, SW_LID, !state); input_sync(button->input); + if (state) + pm_wakeup_event(&device->dev, 0); + ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device); if (ret == NOTIFY_DONE) ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, @@ -314,6 +317,8 @@ static void acpi_button_notify(struct acpi_device *device, u32 event) input_sync(input); input_report_key(input, keycode, 0); input_sync(input); + + pm_wakeup_event(&device->dev, 0); } acpi_bus_generate_proc_event(device, event, ++button->pushed); From 7b330707dddab1ad772898c1c82516342a551173 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jan 2011 23:37:01 +0100 Subject: [PATCH 6/6] ACPI / PM: Blacklist Averatec machine known to require acpi_sleep=nonvs Apparently, Averatec AV1020-ED2 does not resume correctly without acpi_sleep=nonvs, so add it to the ACPI sleep blacklist. References: https://bugzilla.kernel.org/show_bug.cgi?id=16396#c86 Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/sleep.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index febb153b5a68..ddc5cce508a1 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -435,6 +435,14 @@ static struct dmi_system_id __initdata acpisleep_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "VGN-NW130D"), }, }, + { + .callback = init_nvs_nosave, + .ident = "Averatec AV1020-ED2", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "AVERATEC"), + DMI_MATCH(DMI_PRODUCT_NAME, "1000 Series"), + }, + }, {}, }; #endif /* CONFIG_SUSPEND */