PCI / PM: Fix native PME handling during system suspend/resume

Commit 76cde7e49590 (PCI / PM: Make PCIe PME interrupts wake up from
suspend-to-idle) went too far with preventing pcie_pme_work_fn() from
clearing the root port's PME Status and re-enabling the PME interrupt
which should be done for PMEs to work correctly after system resume.

The failing scenario is as follows:

 1. pcie_pme_suspend() finds that the PME IRQ should be designated
    for system wakeup, so it calls enable_irq_wake() and then sets
    data->suspend_level to PME_SUSPEND_WAKEUP.

 2. PME interrupt happens at this point.

 3. pcie_pme_irq() runs, disables the PME interrupt and queues up
    the execution of pcie_pme_work_fn().

 4. pcie_pme_work_fn() runs before pcie_pme_resume() and breaks out
    of the loop right away, because data->suspend_level is not
    PME_SUSPEND_NONE, and it doesn't re-enable the PME interrupt
    for the same reason.

 5. pcie_pme_resume() runs and simply calls disable_irq_wake()
    without re-enabling the PME interrupt (because data->suspend_level
    is not PME_SUSPEND_NONE), so the PME interrupt remains disabled
    and the PME Status remains set.

To fix this notice that there is no reason why pcie_pme_work_fn()
should behave in a special way during system resume if the PME
interrupt is not disabled by pcie_pme_suspend() and partially revert
commit 76cde7e49590 and restore the previous (and correct) behavior
of pcie_pme_work_fn().

Fixes: 76cde7e49590 (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle)
Reported-and-tested-by: Naresh Solanki <naresh.solanki@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 80e58d25006d04b86e6efe82f581db80c81fe85c..fafdb165dd2ed663779f77bc59b3da3fb32af5c4 100644 (file)
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -40,17 +40,11 @@ static int __init pcie_pme_setup(char *str)
 }
 __setup("pcie_pme=", pcie_pme_setup);
 
-enum pme_suspend_level {
-       PME_SUSPEND_NONE = 0,
-       PME_SUSPEND_WAKEUP,
-       PME_SUSPEND_NOIRQ,
-};
-
 struct pcie_pme_service_data {
        spinlock_t lock;
        struct pcie_device *srv;
        struct work_struct work;
-       enum pme_suspend_level suspend_level;
+       bool noirq; /* If set, keep the PME interrupt disabled. */
 };
 
 /**
@@ -228,7 +222,7 @@ static void pcie_pme_work_fn(struct work_struct *work)
        spin_lock_irq(&data->lock);
 
        for (;;) {
-               if (data->suspend_level != PME_SUSPEND_NONE)
+               if (data->noirq)
                        break;
 
                pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
@@ -255,7 +249,7 @@ static void pcie_pme_work_fn(struct work_struct *work)
                spin_lock_irq(&data->lock);
        }
 
-       if (data->suspend_level == PME_SUSPEND_NONE)
+       if (!data->noirq)
                pcie_pme_interrupt_enable(port, true);
 
        spin_unlock_irq(&data->lock);
@@ -378,7 +372,7 @@ static int pcie_pme_suspend(struct pcie_device *srv)
 {
        struct pcie_pme_service_data *data = get_service_data(srv);
        struct pci_dev *port = srv->port;
-       bool wakeup, wake_irq_enabled = false;
+       bool wakeup;
        int ret;
 
        if (device_may_wakeup(&port->dev)) {
@@ -388,19 +382,16 @@ static int pcie_pme_suspend(struct pcie_device *srv)
                wakeup = pcie_pme_check_wakeup(port->subordinate);
                up_read(&pci_bus_sem);
        }
-       spin_lock_irq(&data->lock);
        if (wakeup) {
                ret = enable_irq_wake(srv->irq);
-               if (ret == 0) {
-                       data->suspend_level = PME_SUSPEND_WAKEUP;
-                       wake_irq_enabled = true;
-               }
-       }
-       if (!wake_irq_enabled) {
-               pcie_pme_interrupt_enable(port, false);
-               pcie_clear_root_pme_status(port);
-               data->suspend_level = PME_SUSPEND_NOIRQ;
+               if (!ret)
+                       return 0;
        }
+
+       spin_lock_irq(&data->lock);
+       pcie_pme_interrupt_enable(port, false);
+       pcie_clear_root_pme_status(port);
+       data->noirq = true;
        spin_unlock_irq(&data->lock);
 
        synchronize_irq(srv->irq);
@@ -417,15 +408,15 @@ static int pcie_pme_resume(struct pcie_device *srv)
        struct pcie_pme_service_data *data = get_service_data(srv);
 
        spin_lock_irq(&data->lock);
-       if (data->suspend_level == PME_SUSPEND_NOIRQ) {
+       if (data->noirq) {
                struct pci_dev *port = srv->port;
 
                pcie_clear_root_pme_status(port);
                pcie_pme_interrupt_enable(port, true);
+               data->noirq = false;
        } else {
                disable_irq_wake(srv->irq);
        }
-       data->suspend_level = PME_SUSPEND_NONE;
        spin_unlock_irq(&data->lock);
 
        return 0;