Skip to content

Commit 1ba51a7

Browse files
committed
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 <rafael.j.wysocki@intel.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
1 parent 99d8845 commit 1ba51a7

File tree

3 files changed

+57
-24
lines changed

3 files changed

+57
-24
lines changed

drivers/acpi/device_pm.c

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -684,29 +684,21 @@ static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
684684

685685
static DEFINE_MUTEX(acpi_wakeup_lock);
686686

687-
/**
688-
* acpi_device_wakeup_enable - Enable wakeup functionality for device.
689-
* @adev: ACPI device to enable wakeup functionality for.
690-
* @target_state: State the system is transitioning into.
691-
*
692-
* Enable the GPE associated with @adev so that it can generate wakeup signals
693-
* for the device in response to external (remote) events and enable wakeup
694-
* power for it.
695-
*
696-
* Callers must ensure that @adev is a valid ACPI device node before executing
697-
* this function.
698-
*/
699-
static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
687+
static int __acpi_device_wakeup_enable(struct acpi_device *adev,
688+
u32 target_state, int max_count)
700689
{
701690
struct acpi_device_wakeup *wakeup = &adev->wakeup;
702691
acpi_status status;
703692
int error = 0;
704693

705694
mutex_lock(&acpi_wakeup_lock);
706695

707-
if (wakeup->enable_count > 0)
696+
if (wakeup->enable_count >= max_count)
708697
goto out;
709698

699+
if (wakeup->enable_count > 0)
700+
goto inc;
701+
710702
error = acpi_enable_wakeup_device_power(adev, target_state);
711703
if (error)
712704
goto out;
@@ -718,13 +710,31 @@ static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
718710
goto out;
719711
}
720712

713+
inc:
721714
wakeup->enable_count++;
722715

723716
out:
724717
mutex_unlock(&acpi_wakeup_lock);
725718
return error;
726719
}
727720

721+
/**
722+
* acpi_device_wakeup_enable - Enable wakeup functionality for device.
723+
* @adev: ACPI device to enable wakeup functionality for.
724+
* @target_state: State the system is transitioning into.
725+
*
726+
* Enable the GPE associated with @adev so that it can generate wakeup signals
727+
* for the device in response to external (remote) events and enable wakeup
728+
* power for it.
729+
*
730+
* Callers must ensure that @adev is a valid ACPI device node before executing
731+
* this function.
732+
*/
733+
static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
734+
{
735+
return __acpi_device_wakeup_enable(adev, target_state, 1);
736+
}
737+
728738
/**
729739
* acpi_device_wakeup_disable - Disable wakeup functionality for device.
730740
* @adev: ACPI device to disable wakeup functionality for.
@@ -752,12 +762,8 @@ static void acpi_device_wakeup_disable(struct acpi_device *adev)
752762
mutex_unlock(&acpi_wakeup_lock);
753763
}
754764

755-
/**
756-
* acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device.
757-
* @dev: Device to enable/disable to generate wakeup events.
758-
* @enable: Whether to enable or disable the wakeup functionality.
759-
*/
760-
int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
765+
static int __acpi_pm_set_device_wakeup(struct device *dev, bool enable,
766+
int max_count)
761767
{
762768
struct acpi_device *adev;
763769
int error;
@@ -777,13 +783,35 @@ int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
777783
return 0;
778784
}
779785

780-
error = acpi_device_wakeup_enable(adev, acpi_target_system_state());
786+
error = __acpi_device_wakeup_enable(adev, acpi_target_system_state(),
787+
max_count);
781788
if (!error)
782789
dev_dbg(dev, "Wakeup enabled by ACPI\n");
783790

784791
return error;
785792
}
786-
EXPORT_SYMBOL(acpi_pm_set_device_wakeup);
793+
794+
/**
795+
* acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device.
796+
* @dev: Device to enable/disable to generate wakeup events.
797+
* @enable: Whether to enable or disable the wakeup functionality.
798+
*/
799+
int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
800+
{
801+
return __acpi_pm_set_device_wakeup(dev, enable, 1);
802+
}
803+
EXPORT_SYMBOL_GPL(acpi_pm_set_device_wakeup);
804+
805+
/**
806+
* acpi_pm_set_bridge_wakeup - Enable/disable remote wakeup for given bridge.
807+
* @dev: Bridge device to enable/disable to generate wakeup events.
808+
* @enable: Whether to enable or disable the wakeup functionality.
809+
*/
810+
int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
811+
{
812+
return __acpi_pm_set_device_wakeup(dev, enable, INT_MAX);
813+
}
814+
EXPORT_SYMBOL_GPL(acpi_pm_set_bridge_wakeup);
787815

788816
/**
789817
* acpi_dev_pm_low_power - Put ACPI device into a low-power state.

drivers/pci/pci-acpi.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,15 +573,15 @@ static int acpi_pci_propagate_wakeup(struct pci_bus *bus, bool enable)
573573
{
574574
while (bus->parent) {
575575
if (acpi_pm_device_can_wakeup(&bus->self->dev))
576-
return acpi_pm_set_device_wakeup(&bus->self->dev, enable);
576+
return acpi_pm_set_bridge_wakeup(&bus->self->dev, enable);
577577

578578
bus = bus->parent;
579579
}
580580

581581
/* We have reached the root bus. */
582582
if (bus->bridge) {
583583
if (acpi_pm_device_can_wakeup(bus->bridge))
584-
return acpi_pm_set_device_wakeup(bus->bridge, enable);
584+
return acpi_pm_set_bridge_wakeup(bus->bridge, enable);
585585
}
586586
return 0;
587587
}

include/acpi/acpi_bus.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,7 @@ acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
606606
bool acpi_pm_device_can_wakeup(struct device *dev);
607607
int acpi_pm_device_sleep_state(struct device *, int *, int);
608608
int acpi_pm_set_device_wakeup(struct device *dev, bool enable);
609+
int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable);
609610
#else
610611
static inline void acpi_pm_wakeup_event(struct device *dev)
611612
{
@@ -636,6 +637,10 @@ static inline int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
636637
{
637638
return -ENODEV;
638639
}
640+
static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
641+
{
642+
return -ENODEV;
643+
}
639644
#endif
640645

641646
#ifdef CONFIG_ACPI_SLEEP

0 commit comments

Comments
 (0)