From 99d8845e756cb91e2865f430401d084cd6a8ccc9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 21 Jul 2017 14:40:49 +0200 Subject: ACPI / PM: Split acpi_device_wakeup() To prepare for a subsequent change and make the code somewhat easier to follow, do the following in the ACPI device wakeup handling code: * Replace wakeup.flags.enabled under struct acpi_device with wakeup.enable_count as that will be necessary going forward. For now, wakeup.enable_count is not allowed to grow beyond 1, so the current behavior is retained. * Split acpi_device_wakeup() into acpi_device_wakeup_enable() and acpi_device_wakeup_disable() and modify the callers of it accordingly. * Introduce a new acpi_wakeup_lock mutex to protect the wakeup enabling/disabling code from races in case it is executed more than once in parallel for the same device (which may happen for bridges theoretically). Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg --- include/acpi/acpi_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/acpi') diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 68bc6be447fd..b7df95dbe7e9 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -316,7 +316,6 @@ struct acpi_device_perf { struct acpi_device_wakeup_flags { u8 valid:1; /* Can successfully enable wakeup? */ u8 notifier_present:1; /* Wake-up notify handler has been installed */ - u8 enabled:1; /* Enabled for wakeup */ }; struct acpi_device_wakeup_context { @@ -333,6 +332,7 @@ struct acpi_device_wakeup { struct acpi_device_wakeup_context context; struct wakeup_source *ws; int prepare_count; + int enable_count; }; struct acpi_device_physical_node { -- cgit v1.2.3-59-g8ed1b From 1ba51a7c1496fd8f6d5bdd58dafcb1894275b7f0 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 1 Aug 2017 02:56:18 +0200 Subject: ACPI / PCI / PM: Rework acpi_pci_propagate_wakeup() The acpi_pci_propagate_wakeup() routine is there to handle cases in which PCI bridges (or PCIe ports) are expected to signal wakeup for devices below them, but currently it doesn't do that correctly. The problem is that acpi_pci_propagate_wakeup() uses acpi_pm_set_device_wakeup() for bridges and if that routine is called for multiple times to disable wakeup for the same device, it will disable it on the first invocation and the next calls will have no effect (it works analogously when called to enable wakeup, but that is not a problem). Now, say acpi_pci_propagate_wakeup() has been called for two different devices under the same bridge and it has called acpi_pm_set_device_wakeup() for that bridge each time. The bridge is now enabled to generate wakeup signals. Next, suppose that one of the devices below it resumes and acpi_pci_propagate_wakeup() is called to disable wakeup for that device. It will then call acpi_pm_set_device_wakeup() for the bridge and that will effectively disable remote wakeup for all devices under it even though some of them may still be suspended and remote wakeup may be expected to work for them. To address this (arguably theoretical) issue, allow wakeup.enable_count under struct acpi_device to grow beyond 1 in certain situations. In particular, allow that to happen in acpi_pci_propagate_wakeup() when wakeup is enabled or disabled for PCI bridges, so that wakeup is actually disabled for the bridge when all devices under it resume and not when just one of them does that. Signed-off-by: Rafael J. Wysocki Reviewed-by: Andy Shevchenko Reviewed-by: Mika Westerberg Acked-by: Bjorn Helgaas --- drivers/acpi/device_pm.c | 72 +++++++++++++++++++++++++++++++++--------------- drivers/pci/pci-acpi.c | 4 +-- include/acpi/acpi_bus.h | 5 ++++ 3 files changed, 57 insertions(+), 24 deletions(-) (limited to 'include/acpi') diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 4cd4bdab053d..112fd6c55c2c 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -684,19 +684,8 @@ static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context) static DEFINE_MUTEX(acpi_wakeup_lock); -/** - * acpi_device_wakeup_enable - Enable wakeup functionality for device. - * @adev: ACPI device to enable wakeup functionality for. - * @target_state: State the system is transitioning into. - * - * Enable the GPE associated with @adev so that it can generate wakeup signals - * for the device in response to external (remote) events and enable wakeup - * power for it. - * - * Callers must ensure that @adev is a valid ACPI device node before executing - * this function. - */ -static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state) +static int __acpi_device_wakeup_enable(struct acpi_device *adev, + u32 target_state, int max_count) { struct acpi_device_wakeup *wakeup = &adev->wakeup; acpi_status status; @@ -704,9 +693,12 @@ static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state) mutex_lock(&acpi_wakeup_lock); - if (wakeup->enable_count > 0) + if (wakeup->enable_count >= max_count) goto out; + if (wakeup->enable_count > 0) + goto inc; + error = acpi_enable_wakeup_device_power(adev, target_state); if (error) goto out; @@ -718,6 +710,7 @@ static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state) goto out; } +inc: wakeup->enable_count++; out: @@ -725,6 +718,23 @@ out: return error; } +/** + * acpi_device_wakeup_enable - Enable wakeup functionality for device. + * @adev: ACPI device to enable wakeup functionality for. + * @target_state: State the system is transitioning into. + * + * Enable the GPE associated with @adev so that it can generate wakeup signals + * for the device in response to external (remote) events and enable wakeup + * power for it. + * + * Callers must ensure that @adev is a valid ACPI device node before executing + * this function. + */ +static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state) +{ + return __acpi_device_wakeup_enable(adev, target_state, 1); +} + /** * acpi_device_wakeup_disable - Disable wakeup functionality for device. * @adev: ACPI device to disable wakeup functionality for. @@ -752,12 +762,8 @@ out: mutex_unlock(&acpi_wakeup_lock); } -/** - * acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device. - * @dev: Device to enable/disable to generate wakeup events. - * @enable: Whether to enable or disable the wakeup functionality. - */ -int acpi_pm_set_device_wakeup(struct device *dev, bool enable) +static int __acpi_pm_set_device_wakeup(struct device *dev, bool enable, + int max_count) { struct acpi_device *adev; int error; @@ -777,13 +783,35 @@ int acpi_pm_set_device_wakeup(struct device *dev, bool enable) return 0; } - error = acpi_device_wakeup_enable(adev, acpi_target_system_state()); + error = __acpi_device_wakeup_enable(adev, acpi_target_system_state(), + max_count); if (!error) dev_dbg(dev, "Wakeup enabled by ACPI\n"); return error; } -EXPORT_SYMBOL(acpi_pm_set_device_wakeup); + +/** + * acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device. + * @dev: Device to enable/disable to generate wakeup events. + * @enable: Whether to enable or disable the wakeup functionality. + */ +int acpi_pm_set_device_wakeup(struct device *dev, bool enable) +{ + return __acpi_pm_set_device_wakeup(dev, enable, 1); +} +EXPORT_SYMBOL_GPL(acpi_pm_set_device_wakeup); + +/** + * acpi_pm_set_bridge_wakeup - Enable/disable remote wakeup for given bridge. + * @dev: Bridge device to enable/disable to generate wakeup events. + * @enable: Whether to enable or disable the wakeup functionality. + */ +int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable) +{ + return __acpi_pm_set_device_wakeup(dev, enable, INT_MAX); +} +EXPORT_SYMBOL_GPL(acpi_pm_set_bridge_wakeup); /** * acpi_dev_pm_low_power - Put ACPI device into a low-power state. diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index e70c1c7ba1bf..a8da543b3814 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -573,7 +573,7 @@ static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable) { while (bus->parent) { if (acpi_pm_device_can_wakeup(&bus->self->dev)) - return acpi_pm_set_device_wakeup(&bus->self->dev, enable); + return acpi_pm_set_bridge_wakeup(&bus->self->dev, enable); bus = bus->parent; } @@ -581,7 +581,7 @@ static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable) /* We have reached the root bus. */ if (bus->bridge) { if (acpi_pm_device_can_wakeup(bus->bridge)) - return acpi_pm_set_device_wakeup(bus->bridge, enable); + return acpi_pm_set_bridge_wakeup(bus->bridge, enable); } return 0; } diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index b7df95dbe7e9..de7e8ac9d806 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -606,6 +606,7 @@ acpi_status acpi_remove_pm_notifier(struct acpi_device *adev); bool acpi_pm_device_can_wakeup(struct device *dev); int acpi_pm_device_sleep_state(struct device *, int *, int); int acpi_pm_set_device_wakeup(struct device *dev, bool enable); +int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable); #else static inline void acpi_pm_wakeup_event(struct device *dev) { @@ -636,6 +637,10 @@ static inline int acpi_pm_set_device_wakeup(struct device *dev, bool enable) { return -ENODEV; } +static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable) +{ + return -ENODEV; +} #endif #ifdef CONFIG_ACPI_SLEEP -- cgit v1.2.3-59-g8ed1b From 1312b7e0caca44e7ff312bc2eaa888943384e3e1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 10 Aug 2017 00:31:58 +0200 Subject: ACPICA: Make it possible to enable runtime GPEs earlier Runtime GPEs have corresponding _Lxx/_Exx methods and are enabled automatically during the initialization of the ACPI subsystem through acpi_update_all_gpes() with the assumption that acpi_setup_gpe_for_wake() will be called in advance for all of the GPEs pointed to by _PRW objects in the namespace that may be affected by acpi_update_all_gpes(). That is, acpi_ev_initialize_gpe_block() can only be called for a GPE block after acpi_setup_gpe_for_wake() has been called for all of the _PRW (wakeup) GPEs in it. The platform firmware on some systems, however, expects GPEs to be enabled before the enumeration of devices which is when acpi_setup_gpe_for_wake() is called and that goes against the above assumption. For this reason, introduce a new flag to be set by acpi_ev_initialize_gpe_block() when automatically enabling a GPE to indicate to acpi_setup_gpe_for_wake() that it needs to drop the reference to the GPE coming from acpi_ev_initialize_gpe_block() and modify acpi_setup_gpe_for_wake() accordingly. These changes allow acpi_setup_gpe_for_wake() and acpi_ev_initialize_gpe_block() to be invoked in any order. Signed-off-by: Rafael J. Wysocki Tested-by: Mika Westerberg --- drivers/acpi/acpica/evgpeblk.c | 2 ++ drivers/acpi/acpica/evxfgpe.c | 8 ++++++++ include/acpi/actypes.h | 3 ++- 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'include/acpi') diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c index c27386cca493..3a3cb8624f41 100644 --- a/drivers/acpi/acpica/evgpeblk.c +++ b/drivers/acpi/acpica/evgpeblk.c @@ -496,6 +496,8 @@ acpi_ev_initialize_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info, continue; } + gpe_event_info->flags |= ACPI_GPE_AUTO_ENABLED; + if (event_status & ACPI_EVENT_FLAG_STATUS_SET) { ACPI_INFO(("GPE 0x%02X active on init", gpe_number)); diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c index 57718a3e029a..67c7c4ce276c 100644 --- a/drivers/acpi/acpica/evxfgpe.c +++ b/drivers/acpi/acpica/evxfgpe.c @@ -435,6 +435,14 @@ acpi_setup_gpe_for_wake(acpi_handle wake_device, */ gpe_event_info->flags = (ACPI_GPE_DISPATCH_NOTIFY | ACPI_GPE_LEVEL_TRIGGERED); + } else if (gpe_event_info->flags & ACPI_GPE_AUTO_ENABLED) { + /* + * A reference to this GPE has been added during the GPE block + * initialization, so drop it now to prevent the GPE from being + * permanently enabled and clear its ACPI_GPE_AUTO_ENABLED flag. + */ + (void)acpi_ev_remove_gpe_reference(gpe_event_info); + gpe_event_info->flags &= ~ACPI_GPE_AUTO_ENABLED; } /* diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 2fcbaec8b368..71eddf645566 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -775,7 +775,7 @@ typedef u32 acpi_event_status; * | | | | +-- Type of dispatch:to method, handler, notify, or none * | | | +----- Interrupt type: edge or level triggered * | | +------- Is a Wake GPE - * | +--------- Is GPE masked by the software GPE masking mechanism + * | +--------- Has been enabled automatically at init time * +------------ */ #define ACPI_GPE_DISPATCH_NONE (u8) 0x00 @@ -791,6 +791,7 @@ typedef u32 acpi_event_status; #define ACPI_GPE_XRUPT_TYPE_MASK (u8) 0x08 #define ACPI_GPE_CAN_WAKE (u8) 0x10 +#define ACPI_GPE_AUTO_ENABLED (u8) 0x20 /* * Flags for GPE and Lock interfaces -- cgit v1.2.3-59-g8ed1b