| Message ID | 1413981115-22045-1-git-send-email-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wednesday, October 22, 2014 02:31:55 PM Lucas Stach wrote: > If the irqchip handling the PCIe PME interrupt is not able > to enable interrupt wakeup we should properly reflect this > in the PME suspend status. > > This fixes a kernel warning on resume, where it would try > to disable the irq wakeup that failed to be activated while > suspending. The issue was introduced with 76cde7e49590 > (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle). > > Reported-by: Richard Zhu <richard.zhu@freescale.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Tested-by: Richard Zhu <richard.zhu@freescale.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Bjorn, do you want me to handle this or will you take it? > --- > Trimmed warning on resume looks like this: > [ 109.292736] WARNING: CPU: 0 PID: 609 at kernel/irq/manage.c:536 irq_set_irq_wake+0xc0/0xf8() > [ 109.301193] Unbalanced IRQ 384 wake disable > [ 109.305392] Modules linked in: > [ 109.308502] CPU: 0 PID: 609 Comm: kworker/u2:9 Tainted: G W 3.18.0-rc1-00009-g820df3d-dirty #268 > [ 109.318368] Workqueue: events_unbound async_run_entry_fn > [ 109.323718] Backtrace: > [ 109.326233] [<80012460>] (dump_backtrace) from [<80012744>] (show_stack+0x18/0x1c) > [ 109.339616] [<8001272c>] (show_stack) from [<806d8dc8>] (dump_stack+0x8c/0xa4) > [ 109.346885] [<806d8d3c>] (dump_stack) from [<8002a88c>] (warn_slowpath_common+0x70/0x94) > [ 109.360773] [<8002a81c>] (warn_slowpath_common) from [<8002a8e8>] (warn_slowpath_fmt+0x38/0x40) > [ 109.376334] [<8002a8b4>] (warn_slowpath_fmt) from [<8006c2a8>] (irq_set_irq_wake+0xc0/0xf8) > [ 109.388351] [<8006c1e8>] (irq_set_irq_wake) from [<802f22cc>] (pcie_pme_resume+0x34/0x64) > [ 109.402328] [<802f2298>] (pcie_pme_resume) from [<802f1590>] (resume_iter+0x44/0x50) > [ 109.413742] [<802f154c>] (resume_iter) from [<803784d4>] (device_for_each_child+0x4c/0x78) > [ 109.422039] [<80378488>] (device_for_each_child) from [<802f196c>] (pcie_port_device_resume+0x18/0x20) > [ 109.436085] [<802f1954>] (pcie_port_device_resume) from [<802e6f40>] (pci_pm_resume+0x7c/0x10c) > [ 109.444829] [<802e6ec4>] (pci_pm_resume) from [<8038502c>] (dpm_run_callback.isra.12+0x34/0x7c) > [ 109.459323] [<80384ff8>] (dpm_run_callback.isra.12) from [<8038543c>] (device_resume+0xb8/0x194) > [ 109.474961] [<80385384>] (device_resume) from [<80385538>] (async_resume+0x20/0x4c) > [ 109.490523] [<80385518>] (async_resume) from [<800471a0>] (async_run_entry_fn+0x48/0x11c) > [ 109.502371] [<80047158>] (async_run_entry_fn) from [<8003f46c>] (process_one_work+0x1ac/0x414) > [ 109.515711] [<8003f2c0>] (process_one_work) from [<8003ff50>] (worker_thread+0x148/0x4e0) > [ 109.534446] [<8003fe08>] (worker_thread) from [<80044704>] (kthread+0xdc/0xf8) > [ 109.552225] [<80044628>] (kthread) from [<8000ec08>] (ret_from_fork+0x14/0x2c) > --- > drivers/pci/pcie/pme.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c > index a9f9c46e5022..63fc63911295 100644 > --- a/drivers/pci/pcie/pme.c > +++ b/drivers/pci/pcie/pme.c > @@ -397,6 +397,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; > + int ret; > > if (device_may_wakeup(&port->dev)) { > wakeup = true; > @@ -407,9 +408,10 @@ static int pcie_pme_suspend(struct pcie_device *srv) > } > spin_lock_irq(&data->lock); > if (wakeup) { > - enable_irq_wake(srv->irq); > + ret = enable_irq_wake(srv->irq); > data->suspend_level = PME_SUSPEND_WAKEUP; > - } else { > + } > + if (!wakeup || ret) { > struct pci_dev *port = srv->port; > > pcie_pme_interrupt_enable(port, false); >
On Wed, Oct 22, 2014 at 03:34:56PM +0200, Rafael J. Wysocki wrote: > On Wednesday, October 22, 2014 02:31:55 PM Lucas Stach wrote: > > If the irqchip handling the PCIe PME interrupt is not able > > to enable interrupt wakeup we should properly reflect this > > in the PME suspend status. > > > > This fixes a kernel warning on resume, where it would try > > to disable the irq wakeup that failed to be activated while > > suspending. The issue was introduced with 76cde7e49590 > > (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle). > > > > Reported-by: Richard Zhu <richard.zhu@freescale.com> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > Tested-by: Richard Zhu <richard.zhu@freescale.com> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Bjorn, do you want me to handle this or will you take it? It fixes 76cde7e49590, which it looks like you merged for v3.18-rc1, so probably this should be merged as a fix before v3.18, right? If you agree, you can go ahead and merge it since you merged the original. It would be nice to have the actual warning, e.g., just the "Unbalanced IRQ 384 wake disable" part, in the changelog to help correlate this with the place where the problem is detected. Bjorn > > --- > > Trimmed warning on resume looks like this: > > [ 109.292736] WARNING: CPU: 0 PID: 609 at kernel/irq/manage.c:536 irq_set_irq_wake+0xc0/0xf8() > > [ 109.301193] Unbalanced IRQ 384 wake disable > > [ 109.305392] Modules linked in: > > [ 109.308502] CPU: 0 PID: 609 Comm: kworker/u2:9 Tainted: G W 3.18.0-rc1-00009-g820df3d-dirty #268 > > [ 109.318368] Workqueue: events_unbound async_run_entry_fn > > [ 109.323718] Backtrace: > > [ 109.326233] [<80012460>] (dump_backtrace) from [<80012744>] (show_stack+0x18/0x1c) > > [ 109.339616] [<8001272c>] (show_stack) from [<806d8dc8>] (dump_stack+0x8c/0xa4) > > [ 109.346885] [<806d8d3c>] (dump_stack) from [<8002a88c>] (warn_slowpath_common+0x70/0x94) > > [ 109.360773] [<8002a81c>] (warn_slowpath_common) from [<8002a8e8>] (warn_slowpath_fmt+0x38/0x40) > > [ 109.376334] [<8002a8b4>] (warn_slowpath_fmt) from [<8006c2a8>] (irq_set_irq_wake+0xc0/0xf8) > > [ 109.388351] [<8006c1e8>] (irq_set_irq_wake) from [<802f22cc>] (pcie_pme_resume+0x34/0x64) > > [ 109.402328] [<802f2298>] (pcie_pme_resume) from [<802f1590>] (resume_iter+0x44/0x50) > > [ 109.413742] [<802f154c>] (resume_iter) from [<803784d4>] (device_for_each_child+0x4c/0x78) > > [ 109.422039] [<80378488>] (device_for_each_child) from [<802f196c>] (pcie_port_device_resume+0x18/0x20) > > [ 109.436085] [<802f1954>] (pcie_port_device_resume) from [<802e6f40>] (pci_pm_resume+0x7c/0x10c) > > [ 109.444829] [<802e6ec4>] (pci_pm_resume) from [<8038502c>] (dpm_run_callback.isra.12+0x34/0x7c) > > [ 109.459323] [<80384ff8>] (dpm_run_callback.isra.12) from [<8038543c>] (device_resume+0xb8/0x194) > > [ 109.474961] [<80385384>] (device_resume) from [<80385538>] (async_resume+0x20/0x4c) > > [ 109.490523] [<80385518>] (async_resume) from [<800471a0>] (async_run_entry_fn+0x48/0x11c) > > [ 109.502371] [<80047158>] (async_run_entry_fn) from [<8003f46c>] (process_one_work+0x1ac/0x414) > > [ 109.515711] [<8003f2c0>] (process_one_work) from [<8003ff50>] (worker_thread+0x148/0x4e0) > > [ 109.534446] [<8003fe08>] (worker_thread) from [<80044704>] (kthread+0xdc/0xf8) > > [ 109.552225] [<80044628>] (kthread) from [<8000ec08>] (ret_from_fork+0x14/0x2c) > > --- > > drivers/pci/pcie/pme.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c > > index a9f9c46e5022..63fc63911295 100644 > > --- a/drivers/pci/pcie/pme.c > > +++ b/drivers/pci/pcie/pme.c > > @@ -397,6 +397,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; > > + int ret; > > > > if (device_may_wakeup(&port->dev)) { > > wakeup = true; > > @@ -407,9 +408,10 @@ static int pcie_pme_suspend(struct pcie_device *srv) > > } > > spin_lock_irq(&data->lock); > > if (wakeup) { > > - enable_irq_wake(srv->irq); > > + ret = enable_irq_wake(srv->irq); > > data->suspend_level = PME_SUSPEND_WAKEUP; > > - } else { > > + } > > + if (!wakeup || ret) { > > struct pci_dev *port = srv->port; > > > > pcie_pme_interrupt_enable(port, false); > > > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, October 23, 2014 12:11:18 PM Bjorn Helgaas wrote: > On Wed, Oct 22, 2014 at 03:34:56PM +0200, Rafael J. Wysocki wrote: > > On Wednesday, October 22, 2014 02:31:55 PM Lucas Stach wrote: > > > If the irqchip handling the PCIe PME interrupt is not able > > > to enable interrupt wakeup we should properly reflect this > > > in the PME suspend status. > > > > > > This fixes a kernel warning on resume, where it would try > > > to disable the irq wakeup that failed to be activated while > > > suspending. The issue was introduced with 76cde7e49590 > > > (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle). > > > > > > Reported-by: Richard Zhu <richard.zhu@freescale.com> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > Tested-by: Richard Zhu <richard.zhu@freescale.com> > > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Bjorn, do you want me to handle this or will you take it? > > It fixes 76cde7e49590, which it looks like you merged for v3.18-rc1, so > probably this should be merged as a fix before v3.18, right? If you agree, > you can go ahead and merge it since you merged the original. OK, I've queued it up for my next pull request (which most likely will be sent tomorrow). > It would be nice to have the actual warning, e.g., just the "Unbalanced IRQ > 384 wake disable" part, in the changelog to help correlate this with the > place where the problem is detected. Done. Rafael > > > --- > > > Trimmed warning on resume looks like this: > > > [ 109.292736] WARNING: CPU: 0 PID: 609 at kernel/irq/manage.c:536 irq_set_irq_wake+0xc0/0xf8() > > > [ 109.301193] Unbalanced IRQ 384 wake disable > > > [ 109.305392] Modules linked in: > > > [ 109.308502] CPU: 0 PID: 609 Comm: kworker/u2:9 Tainted: G W 3.18.0-rc1-00009-g820df3d-dirty #268 > > > [ 109.318368] Workqueue: events_unbound async_run_entry_fn > > > [ 109.323718] Backtrace: > > > [ 109.326233] [<80012460>] (dump_backtrace) from [<80012744>] (show_stack+0x18/0x1c) > > > [ 109.339616] [<8001272c>] (show_stack) from [<806d8dc8>] (dump_stack+0x8c/0xa4) > > > [ 109.346885] [<806d8d3c>] (dump_stack) from [<8002a88c>] (warn_slowpath_common+0x70/0x94) > > > [ 109.360773] [<8002a81c>] (warn_slowpath_common) from [<8002a8e8>] (warn_slowpath_fmt+0x38/0x40) > > > [ 109.376334] [<8002a8b4>] (warn_slowpath_fmt) from [<8006c2a8>] (irq_set_irq_wake+0xc0/0xf8) > > > [ 109.388351] [<8006c1e8>] (irq_set_irq_wake) from [<802f22cc>] (pcie_pme_resume+0x34/0x64) > > > [ 109.402328] [<802f2298>] (pcie_pme_resume) from [<802f1590>] (resume_iter+0x44/0x50) > > > [ 109.413742] [<802f154c>] (resume_iter) from [<803784d4>] (device_for_each_child+0x4c/0x78) > > > [ 109.422039] [<80378488>] (device_for_each_child) from [<802f196c>] (pcie_port_device_resume+0x18/0x20) > > > [ 109.436085] [<802f1954>] (pcie_port_device_resume) from [<802e6f40>] (pci_pm_resume+0x7c/0x10c) > > > [ 109.444829] [<802e6ec4>] (pci_pm_resume) from [<8038502c>] (dpm_run_callback.isra.12+0x34/0x7c) > > > [ 109.459323] [<80384ff8>] (dpm_run_callback.isra.12) from [<8038543c>] (device_resume+0xb8/0x194) > > > [ 109.474961] [<80385384>] (device_resume) from [<80385538>] (async_resume+0x20/0x4c) > > > [ 109.490523] [<80385518>] (async_resume) from [<800471a0>] (async_run_entry_fn+0x48/0x11c) > > > [ 109.502371] [<80047158>] (async_run_entry_fn) from [<8003f46c>] (process_one_work+0x1ac/0x414) > > > [ 109.515711] [<8003f2c0>] (process_one_work) from [<8003ff50>] (worker_thread+0x148/0x4e0) > > > [ 109.534446] [<8003fe08>] (worker_thread) from [<80044704>] (kthread+0xdc/0xf8) > > > [ 109.552225] [<80044628>] (kthread) from [<8000ec08>] (ret_from_fork+0x14/0x2c) > > > --- > > > drivers/pci/pcie/pme.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c > > > index a9f9c46e5022..63fc63911295 100644 > > > --- a/drivers/pci/pcie/pme.c > > > +++ b/drivers/pci/pcie/pme.c > > > @@ -397,6 +397,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; > > > + int ret; > > > > > > if (device_may_wakeup(&port->dev)) { > > > wakeup = true; > > > @@ -407,9 +408,10 @@ static int pcie_pme_suspend(struct pcie_device *srv) > > > } > > > spin_lock_irq(&data->lock); > > > if (wakeup) { > > > - enable_irq_wake(srv->irq); > > > + ret = enable_irq_wake(srv->irq); > > > data->suspend_level = PME_SUSPEND_WAKEUP; > > > - } else { > > > + } > > > + if (!wakeup || ret) { > > > struct pci_dev *port = srv->port; > > > > > > pcie_pme_interrupt_enable(port, false); > > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index a9f9c46e5022..63fc63911295 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -397,6 +397,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; + int ret; if (device_may_wakeup(&port->dev)) { wakeup = true; @@ -407,9 +408,10 @@ static int pcie_pme_suspend(struct pcie_device *srv) } spin_lock_irq(&data->lock); if (wakeup) { - enable_irq_wake(srv->irq); + ret = enable_irq_wake(srv->irq); data->suspend_level = PME_SUSPEND_WAKEUP; - } else { + } + if (!wakeup || ret) { struct pci_dev *port = srv->port; pcie_pme_interrupt_enable(port, false);