Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Rafael J. Wysocki <hidden>
Date: 2017-12-26 00:12:25
Also in:
linux-pci, linux-pm, lkml
On Monday, December 25, 2017 12:47:41 PM CET Jeffy Chen wrote:
quoted hunk ↗ jump to hunk
Add of_pci_setup_wake_irq() and of_pci_teardown_wake_irq() to handle the PCIe WAKE# interrupt. Also use the dedicated wakeirq infrastructure to simplify it. Signed-off-by: Jeffy Chen <redacted> --- Changes in v11: Only support 1-per-device PCIe WAKE# pin as suggested. Changes in v10: Use device_set_wakeup_capable() instead of device_set_wakeup_enable(), since dedicated wakeirq will be lost in device_set_wakeup_enable(false). Changes in v9: Fix check error in .cleanup(). Move dedicated wakeirq setup to setup() callback and use device_set_wakeup_enable() to enable/disable. Changes in v8: Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal. Changes in v7: Move PCIE_WAKE handling into pci core. Changes in v6: Fix device_init_wake error handling, and add some comments. Changes in v5: Rebase. Changes in v3: Fix error handling. Changes in v2: Use dev_pm_set_dedicated_wake_irq. drivers/of/of_pci_irq.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci-driver.c | 10 ++++++++++ include/linux/of_pci.h | 9 +++++++++ 3 files changed, 68 insertions(+)diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c index d39565d5477b..def884c1a37a 100644 --- a/drivers/of/of_pci_irq.c +++ b/drivers/of/of_pci_irq.c@@ -1,8 +1,57 @@ #include <linux/kernel.h> #include <linux/of_pci.h> #include <linux/of_irq.h> +#include <linux/pm_wakeirq.h> #include <linux/export.h> +int of_pci_setup_wake_irq(struct pci_dev *pdev) +{ + struct pci_dev *ppdev; + struct device_node *dn; + int ret, irq; + + /* Get the pci_dev of our parent. Hopefully it's a port. */ + ppdev = pdev->bus->self; + /* Nope, it's a host bridge. */ + if (!ppdev) + return 0; + + dn = pci_device_to_OF_node(ppdev); + if (!dn) + return 0; + + irq = of_irq_get_byname(dn, "wakeup");
Why is this a property of the bridge and not of the device itself?
+ if (irq == -EPROBE_DEFER)
Braces here, please.
+ return irq;
+ /* Ignore other errors, since a missing wakeup is non-fatal. */
+ else if (irq < 0) {
+ dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
+ return 0;
+ }
+
+ device_init_wakeup(&pdev->dev, true);Why do you call this before dev_pm_set_dedicated_wake_irq()?
+
+ ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, irq);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to set wake IRQ: %d\n", ret);
+ device_init_wakeup(&pdev->dev, false);
+ return ret;
+ }
+It would be more straightforward to call device_init_wakeup() here.
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq);
+
+void of_pci_teardown_wake_irq(struct pci_dev *pdev)
+{
+ if (!pdev->dev.power.wakeirq)
+ return;
+
+ dev_pm_clear_wake_irq(&pdev->dev);
+ device_init_wakeup(&pdev->dev, false);
+}
+EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq);
+
/**
* of_irq_parse_pci - Resolve the interrupt for a PCI device
* @pdev: the device whose interrupt is to be resolvedThanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html