PCI / PM: Fix native PME handling during system suspend/resume
Commit76cde7e495(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 commit76cde7e495and restore the previous (and correct) behavior of pcie_pme_work_fn(). Fixes:76cde7e495(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>
This commit is contained in:
		
							parent
							
								
									0ce3fcaff9
								
							
						
					
					
						commit
						c7b5a4e6e8
					
				| @ -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; | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user