Skip to content

Commit

Permalink
ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle
Browse files Browse the repository at this point in the history
The ACPI SCI (System Control Interrupt) is set up as a wakeup IRQ
during suspend-to-idle transitions and, consequently, any events
signaled through it wake up the system from that state.  However,
on some systems some of the events signaled via the ACPI SCI while
suspended to idle should not cause the system to wake up.  In fact,
quite often they should just be discarded.

Arguably, systems should not resume entirely on such events, but in
order to decide which events really should cause the system to resume
and which are spurious, it is necessary to resume up to the point
when ACPI SCIs are actually handled and processed, which is after
executing dpm_resume_noirq() in the system resume path.

For this reasons, add a loop around freeze_enter() in which the
platforms can process events signaled via multiplexed IRQ lines
like the ACPI SCI and add suspend-to-idle hooks that can be
used for this purpose to struct platform_freeze_ops.

In the ACPI case, the ->wake hook is used for checking if the SCI
has triggered while suspended and deferring the interrupt-induced
system wakeup until the events signaled through it are actually
processed sufficiently to decide whether or not the system should
resume.  In turn, the ->sync hook allows all of the relevant event
queues to be flushed so as to prevent events from being missed due
to race conditions.

In addition to that, some ACPI code processing wakeup events needs
to be modified to use the "hard" version of wakeup triggers, so that
it will cause a system resume to happen on device-induced wakeup
events even if the "soft" mechanism to prevent the system from
suspending is not enabled (that also helps to catch device-induced
wakeup events occurring during suspend transitions in progress).

Signed-off-by: Rafael J. Wysocki <[email protected]>
  • Loading branch information
rafaeljw committed May 5, 2017
1 parent 8a537ec commit eed4d47
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 22 deletions.
2 changes: 1 addition & 1 deletion drivers/acpi/battery.c
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume)
if ((battery->state & ACPI_BATTERY_STATE_CRITICAL) ||
(test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) &&
(battery->capacity_now <= battery->alarm)))
pm_wakeup_event(&battery->device->dev, 0);
pm_wakeup_hard_event(&battery->device->dev);

return result;
}
Expand Down
5 changes: 3 additions & 2 deletions drivers/acpi/button.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)
}

if (state)
pm_wakeup_event(&device->dev, 0);
pm_wakeup_hard_event(&device->dev);

ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
if (ret == NOTIFY_DONE)
Expand Down Expand Up @@ -398,7 +398,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
} else {
int keycode;

pm_wakeup_event(&device->dev, 0);
pm_wakeup_hard_event(&device->dev);
if (button->suspended)
break;

Expand Down Expand Up @@ -530,6 +530,7 @@ static int acpi_button_add(struct acpi_device *device)
lid_device = device;
}

device_init_wakeup(&device->dev, true);
printk(KERN_INFO PREFIX "%s [%s]\n", name, acpi_device_bid(device));
return 0;

Expand Down
3 changes: 2 additions & 1 deletion drivers/acpi/device_pm.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <linux/pm_qos.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/suspend.h>

#include "internal.h"

Expand Down Expand Up @@ -399,7 +400,7 @@ static void acpi_pm_notify_handler(acpi_handle handle, u32 val, void *not_used)
mutex_lock(&acpi_pm_notifier_lock);

if (adev->wakeup.flags.notifier_present) {
__pm_wakeup_event(adev->wakeup.ws, 0);
pm_wakeup_ws_event(adev->wakeup.ws, 0, true);
if (adev->wakeup.context.work.func)
queue_pm_work(&adev->wakeup.context.work);
}
Expand Down
28 changes: 28 additions & 0 deletions drivers/acpi/sleep.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,14 +662,40 @@ static int acpi_freeze_prepare(void)
acpi_os_wait_events_complete();
if (acpi_sci_irq_valid())
enable_irq_wake(acpi_sci_irq);

return 0;
}

static void acpi_freeze_wake(void)
{
/*
* If IRQD_WAKEUP_ARMED is not set for the SCI at this point, it means
* that the SCI has triggered while suspended, so cancel the wakeup in
* case it has not been a wakeup event (the GPEs will be checked later).
*/
if (acpi_sci_irq_valid() &&
!irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
pm_system_cancel_wakeup();
}

static void acpi_freeze_sync(void)
{
/*
* Process all pending events in case there are any wakeup ones.
*
* The EC driver uses the system workqueue, so that one needs to be
* flushed too.
*/
acpi_os_wait_events_complete();
flush_scheduled_work();
}

static void acpi_freeze_restore(void)
{
acpi_disable_wakeup_devices(ACPI_STATE_S0);
if (acpi_sci_irq_valid())
disable_irq_wake(acpi_sci_irq);

acpi_enable_all_runtime_gpes();
}

Expand All @@ -681,6 +707,8 @@ static void acpi_freeze_end(void)
static const struct platform_freeze_ops acpi_freeze_ops = {
.begin = acpi_freeze_begin,
.prepare = acpi_freeze_prepare,
.wake = acpi_freeze_wake,
.sync = acpi_freeze_sync,
.restore = acpi_freeze_restore,
.end = acpi_freeze_end,
};
Expand Down
5 changes: 0 additions & 5 deletions drivers/base/power/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1091,11 +1091,6 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
if (async_error)
goto Complete;

if (pm_wakeup_pending()) {
async_error = -EBUSY;
goto Complete;
}

if (dev->power.syscore || dev->power.direct_complete)
goto Complete;

Expand Down
18 changes: 12 additions & 6 deletions drivers/base/power/wakeup.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ bool events_check_enabled __read_mostly;
/* First wakeup IRQ seen by the kernel in the last cycle. */
unsigned int pm_wakeup_irq __read_mostly;

/* If set and the system is suspending, terminate the suspend. */
static bool pm_abort_suspend __read_mostly;
/* If greater than 0 and the system is suspending, terminate the suspend. */
static atomic_t pm_abort_suspend __read_mostly;

/*
* Combined counters of registered wakeup events and wakeup events in progress.
Expand Down Expand Up @@ -856,20 +856,26 @@ bool pm_wakeup_pending(void)
pm_print_active_wakeup_sources();
}

return ret || pm_abort_suspend;
return ret || atomic_read(&pm_abort_suspend) > 0;
}

void pm_system_wakeup(void)
{
pm_abort_suspend = true;
atomic_inc(&pm_abort_suspend);
freeze_wake();
}
EXPORT_SYMBOL_GPL(pm_system_wakeup);

void pm_wakeup_clear(void)
void pm_system_cancel_wakeup(void)
{
atomic_dec(&pm_abort_suspend);
}

void pm_wakeup_clear(bool reset)
{
pm_abort_suspend = false;
pm_wakeup_irq = 0;
if (reset)
atomic_set(&pm_abort_suspend, 0);
}

void pm_system_irq_wakeup(unsigned int irq_number)
Expand Down
7 changes: 5 additions & 2 deletions include/linux/suspend.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ struct platform_suspend_ops {
struct platform_freeze_ops {
int (*begin)(void);
int (*prepare)(void);
void (*wake)(void);
void (*sync)(void);
void (*restore)(void);
void (*end)(void);
};
Expand Down Expand Up @@ -428,7 +430,8 @@ extern unsigned int pm_wakeup_irq;

extern bool pm_wakeup_pending(void);
extern void pm_system_wakeup(void);
extern void pm_wakeup_clear(void);
extern void pm_system_cancel_wakeup(void);
extern void pm_wakeup_clear(bool reset);
extern void pm_system_irq_wakeup(unsigned int irq_number);
extern bool pm_get_wakeup_count(unsigned int *count, bool block);
extern bool pm_save_wakeup_count(unsigned int count);
Expand Down Expand Up @@ -478,7 +481,7 @@ static inline int unregister_pm_notifier(struct notifier_block *nb)

static inline bool pm_wakeup_pending(void) { return false; }
static inline void pm_system_wakeup(void) {}
static inline void pm_wakeup_clear(void) {}
static inline void pm_wakeup_clear(bool reset) {}
static inline void pm_system_irq_wakeup(unsigned int irq_number) {}

static inline void lock_system_sleep(void) {}
Expand Down
2 changes: 1 addition & 1 deletion kernel/power/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ int freeze_processes(void)
if (!pm_freezing)
atomic_inc(&system_freezing_cnt);

pm_wakeup_clear();
pm_wakeup_clear(true);
pr_info("Freezing user space processes ... ");
pm_freezing = true;
error = try_to_freeze_tasks(true);
Expand Down
29 changes: 25 additions & 4 deletions kernel/power/suspend.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ static void freeze_begin(void)

static void freeze_enter(void)
{
trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, true);

spin_lock_irq(&suspend_freeze_lock);
if (pm_wakeup_pending())
goto out;
Expand All @@ -98,6 +100,27 @@ static void freeze_enter(void)
out:
suspend_freeze_state = FREEZE_STATE_NONE;
spin_unlock_irq(&suspend_freeze_lock);

trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, false);
}

static void s2idle_loop(void)
{
do {
freeze_enter();

if (freeze_ops && freeze_ops->wake)
freeze_ops->wake();

dpm_resume_noirq(PMSG_RESUME);
if (freeze_ops && freeze_ops->sync)
freeze_ops->sync();

if (pm_wakeup_pending())
break;

pm_wakeup_clear(false);
} while (!dpm_suspend_noirq(PMSG_SUSPEND));
}

void freeze_wake(void)
Expand Down Expand Up @@ -371,10 +394,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
* all the devices are suspended.
*/
if (state == PM_SUSPEND_FREEZE) {
trace_suspend_resume(TPS("machine_suspend"), state, true);
freeze_enter();
trace_suspend_resume(TPS("machine_suspend"), state, false);
goto Platform_wake;
s2idle_loop();
goto Platform_early_resume;
}

error = disable_nonboot_cpus();
Expand Down

0 comments on commit eed4d47

Please sign in to comment.