Skip to content

Commit 7482c5c

Browse files
committed
PM: ACPI: PCI: Drop acpi_pm_set_bridge_wakeup()
The idea behind acpi_pm_set_bridge_wakeup() was to allow bridges to be reference counted for wakeup enabling, because they may be enabled to signal wakeup on behalf of their subordinate devices and that may happen for multiple times in a row, whereas for the other devices it only makes sense to enable wakeup signaling once. However, this becomes problematic if the bridge itself is suspended, because it is treated as a "regular" device in that case and the reference counting doesn't work. For instance, suppose that there are two devices below a bridge and they both can signal wakeup. Every time one of them is suspended, wakeup signaling is enabled for the bridge, so when they both have been suspended, the bridge's wakeup reference counter value is 2. Say that the bridge is suspended subsequently and acpi_pci_wakeup() is called for it. Because the bridge can signal wakeup, that function will invoke acpi_pm_set_device_wakeup() to configure it and __acpi_pm_set_device_wakeup() will be called with the last argument equal to 1. This causes __acpi_device_wakeup_enable() invoked by it to omit the reference counting, because the reference counter of the target device (the bridge) is 2 at that time. Now say that the bridge resumes and one of the device below it resumes too, so the bridge's reference counter becomes 0 and wakeup signaling is disabled for it, but there is still the other suspended device which may need the bridge to signal wakeup on its behalf and that is not going to work. To address this scenario, use wakeup enable reference counting for all devices, not just for bridges, so drop the last argument from __acpi_device_wakeup_enable() and __acpi_pm_set_device_wakeup(), which causes acpi_pm_set_device_wakeup() and acpi_pm_set_bridge_wakeup() to become identical, so drop the latter and use the former instead of it everywhere. Fixes: 1ba51a7 ("ACPI / PCI / PM: Rework acpi_pci_propagate_wakeup()") Signed-off-by: Rafael J. Wysocki <[email protected]> Reviewed-by: Mika Westerberg <[email protected]> Acked-by: Bjorn Helgaas <[email protected]> Cc: 4.14+ <[email protected]> # 4.14+
1 parent d60cd06 commit 7482c5c

File tree

3 files changed

+14
-36
lines changed

3 files changed

+14
-36
lines changed

drivers/acpi/device_pm.c

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -749,17 +749,18 @@ static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
749749
static DEFINE_MUTEX(acpi_wakeup_lock);
750750

751751
static int __acpi_device_wakeup_enable(struct acpi_device *adev,
752-
u32 target_state, int max_count)
752+
u32 target_state)
753753
{
754754
struct acpi_device_wakeup *wakeup = &adev->wakeup;
755755
acpi_status status;
756756
int error = 0;
757757

758758
mutex_lock(&acpi_wakeup_lock);
759759

760-
if (wakeup->enable_count >= max_count)
760+
if (wakeup->enable_count >= INT_MAX) {
761+
acpi_handle_info(adev->handle, "Wakeup enable count out of bounds!\n");
761762
goto out;
762-
763+
}
763764
if (wakeup->enable_count > 0)
764765
goto inc;
765766

@@ -799,7 +800,7 @@ static int __acpi_device_wakeup_enable(struct acpi_device *adev,
799800
*/
800801
static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
801802
{
802-
return __acpi_device_wakeup_enable(adev, target_state, 1);
803+
return __acpi_device_wakeup_enable(adev, target_state);
803804
}
804805

805806
/**
@@ -829,8 +830,12 @@ static void acpi_device_wakeup_disable(struct acpi_device *adev)
829830
mutex_unlock(&acpi_wakeup_lock);
830831
}
831832

832-
static int __acpi_pm_set_device_wakeup(struct device *dev, bool enable,
833-
int max_count)
833+
/**
834+
* acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device.
835+
* @dev: Device to enable/disable to generate wakeup events.
836+
* @enable: Whether to enable or disable the wakeup functionality.
837+
*/
838+
int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
834839
{
835840
struct acpi_device *adev;
836841
int error;
@@ -850,36 +855,14 @@ static int __acpi_pm_set_device_wakeup(struct device *dev, bool enable,
850855
return 0;
851856
}
852857

853-
error = __acpi_device_wakeup_enable(adev, acpi_target_system_state(),
854-
max_count);
858+
error = __acpi_device_wakeup_enable(adev, acpi_target_system_state());
855859
if (!error)
856860
dev_dbg(dev, "Wakeup enabled by ACPI\n");
857861

858862
return error;
859863
}
860-
861-
/**
862-
* acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device.
863-
* @dev: Device to enable/disable to generate wakeup events.
864-
* @enable: Whether to enable or disable the wakeup functionality.
865-
*/
866-
int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
867-
{
868-
return __acpi_pm_set_device_wakeup(dev, enable, 1);
869-
}
870864
EXPORT_SYMBOL_GPL(acpi_pm_set_device_wakeup);
871865

872-
/**
873-
* acpi_pm_set_bridge_wakeup - Enable/disable remote wakeup for given bridge.
874-
* @dev: Bridge device to enable/disable to generate wakeup events.
875-
* @enable: Whether to enable or disable the wakeup functionality.
876-
*/
877-
int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
878-
{
879-
return __acpi_pm_set_device_wakeup(dev, enable, INT_MAX);
880-
}
881-
EXPORT_SYMBOL_GPL(acpi_pm_set_bridge_wakeup);
882-
883866
/**
884867
* acpi_dev_pm_low_power - Put ACPI device into a low-power state.
885868
* @dev: Device to put into a low-power state.

drivers/pci/pci-acpi.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,15 +1060,15 @@ static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable)
10601060
{
10611061
while (bus->parent) {
10621062
if (acpi_pm_device_can_wakeup(&bus->self->dev))
1063-
return acpi_pm_set_bridge_wakeup(&bus->self->dev, enable);
1063+
return acpi_pm_set_device_wakeup(&bus->self->dev, enable);
10641064

10651065
bus = bus->parent;
10661066
}
10671067

10681068
/* We have reached the root bus. */
10691069
if (bus->bridge) {
10701070
if (acpi_pm_device_can_wakeup(bus->bridge))
1071-
return acpi_pm_set_bridge_wakeup(bus->bridge, enable);
1071+
return acpi_pm_set_device_wakeup(bus->bridge, enable);
10721072
}
10731073
return 0;
10741074
}

include/acpi/acpi_bus.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,6 @@ acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
620620
bool acpi_pm_device_can_wakeup(struct device *dev);
621621
int acpi_pm_device_sleep_state(struct device *, int *, int);
622622
int acpi_pm_set_device_wakeup(struct device *dev, bool enable);
623-
int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable);
624623
#else
625624
static inline void acpi_pm_wakeup_event(struct device *dev)
626625
{
@@ -651,10 +650,6 @@ static inline int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
651650
{
652651
return -ENODEV;
653652
}
654-
static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
655-
{
656-
return -ENODEV;
657-
}
658653
#endif
659654

660655
#ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT

0 commit comments

Comments
 (0)