| Message ID | 20230208111645.3863534-4-mmaddireddy@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add DT based PCIe wake support in PCI core driver | expand |
On Wed, Feb 08, 2023 at 04:46:43PM +0530, Manikanta Maddireddy wrote: > From: Jeffy Chen <jeffy.chen@rock-chips.com> > > Add of_pci_setup_wake_irq() to parse the PCIe WAKE# interrupt from the > device tree and set the wake irq. Add of_pci_teardown_wake_irq() to clear > the wake irq. > > Call of_pci_setup_wake_irq() in pci_device_probe() to setup PCIe WAKE# > interrupt during PCIe Endpoint enumeration. > > Enable or disable PCIe WAKE# interrupt in platform_pci_set_wakeup(). > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> > --- > > Changes in v14: > pci_platform_pm_ops structure is removed in latest kernel, so dropped > pci-of driver. Instead, enable wake in platform_pci_set_wakeup(). > > Changes in v13: > Fix compiler error reported by kbuild test robot <fengguang.wu@intel.com> > > Changes in v12: > Enable the wake irq in noirq stage to avoid possible irq storm. > > 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/pci/of.c | 49 ++++++++++++++++++++++++++++++++++++++++ > drivers/pci/pci-driver.c | 10 ++++++++ > drivers/pci/pci.c | 7 ++++++ > drivers/pci/pci.h | 8 +++++++ > 4 files changed, 74 insertions(+) > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index ff897c40ed71..1c348e63f175 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -13,6 +13,7 @@ > #include <linux/of_irq.h> > #include <linux/of_address.h> > #include <linux/of_pci.h> > +#include <linux/pm_wakeirq.h> > #include "pci.h" > > #ifdef CONFIG_PCI > @@ -705,3 +706,51 @@ u32 of_pci_get_slot_power_limit(struct device_node *node, > return slot_power_limit_mw; > } > EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit); > + > +int of_pci_setup_wake_irq(struct pci_dev *pdev) > +{ > + struct pci_dev *ppdev; Perhaps "parent" since that's what it is referring to? ppdev is a bit vague. > + 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"); > + if (irq == -EPROBE_DEFER) { > + return irq; > + } else if (irq < 0) { > + /* Ignore other errors, since a missing wakeup is non-fatal. */ > + dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq); dev_dbg() maybe? As it is this would add an annoying info message for basically every PCI controller on every DT-based board out there. Thierry
On 2/8/2023 5:20 PM, Thierry Reding wrote: > On Wed, Feb 08, 2023 at 04:46:43PM +0530, Manikanta Maddireddy wrote: >> From: Jeffy Chen <jeffy.chen@rock-chips.com> >> >> Add of_pci_setup_wake_irq() to parse the PCIe WAKE# interrupt from the >> device tree and set the wake irq. Add of_pci_teardown_wake_irq() to clear >> the wake irq. >> >> Call of_pci_setup_wake_irq() in pci_device_probe() to setup PCIe WAKE# >> interrupt during PCIe Endpoint enumeration. >> >> Enable or disable PCIe WAKE# interrupt in platform_pci_set_wakeup(). >> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >> --- >> >> Changes in v14: >> pci_platform_pm_ops structure is removed in latest kernel, so dropped >> pci-of driver. Instead, enable wake in platform_pci_set_wakeup(). >> >> Changes in v13: >> Fix compiler error reported by kbuild test robot <fengguang.wu@intel.com> >> >> Changes in v12: >> Enable the wake irq in noirq stage to avoid possible irq storm. >> >> 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/pci/of.c | 49 ++++++++++++++++++++++++++++++++++++++++ >> drivers/pci/pci-driver.c | 10 ++++++++ >> drivers/pci/pci.c | 7 ++++++ >> drivers/pci/pci.h | 8 +++++++ >> 4 files changed, 74 insertions(+) >> >> diff --git a/drivers/pci/of.c b/drivers/pci/of.c >> index ff897c40ed71..1c348e63f175 100644 >> --- a/drivers/pci/of.c >> +++ b/drivers/pci/of.c >> @@ -13,6 +13,7 @@ >> #include <linux/of_irq.h> >> #include <linux/of_address.h> >> #include <linux/of_pci.h> >> +#include <linux/pm_wakeirq.h> >> #include "pci.h" >> >> #ifdef CONFIG_PCI >> @@ -705,3 +706,51 @@ u32 of_pci_get_slot_power_limit(struct device_node *node, >> return slot_power_limit_mw; >> } >> EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit); >> + >> +int of_pci_setup_wake_irq(struct pci_dev *pdev) >> +{ >> + struct pci_dev *ppdev; > Perhaps "parent" since that's what it is referring to? ppdev is a bit > vague. ppdev is already used in of_irq_parse_pci(), I think it mean parent pci_dev. I see that parent is mostly used for pci_bus parent. Let me know if it is fine to leave it as ppdev or need to rename it. > >> + 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"); >> + if (irq == -EPROBE_DEFER) { >> + return irq; >> + } else if (irq < 0) { >> + /* Ignore other errors, since a missing wakeup is non-fatal. */ >> + dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq); > dev_dbg() maybe? As it is this would add an annoying info message for > basically every PCI controller on every DT-based board out there. > > Thierry Ack. I will wait for few days for other review this series before sending new patch set. Manikanta
diff --git a/drivers/pci/of.c b/drivers/pci/of.c index ff897c40ed71..1c348e63f175 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -13,6 +13,7 @@ #include <linux/of_irq.h> #include <linux/of_address.h> #include <linux/of_pci.h> +#include <linux/pm_wakeirq.h> #include "pci.h" #ifdef CONFIG_PCI @@ -705,3 +706,51 @@ u32 of_pci_get_slot_power_limit(struct device_node *node, return slot_power_limit_mw; } EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit); + +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"); + if (irq == -EPROBE_DEFER) { + return irq; + } else if (irq < 0) { + /* Ignore other errors, since a missing wakeup is non-fatal. */ + dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq); + return 0; + } + + device_init_wakeup(&pdev->dev, true); + + 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; + } + + /* Start out disabled to avoid irq storm */ + dev_pm_disable_wake_irq(&pdev->dev); + + return 0; +} +EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq); + +void of_pci_teardown_wake_irq(struct pci_dev *pdev) +{ + dev_pm_clear_wake_irq(&pdev->dev); + device_init_wakeup(&pdev->dev, false); +} +EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq); diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index d934c27491c4..fca966137fac 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -14,6 +14,7 @@ #include <linux/sched.h> #include <linux/sched/isolation.h> #include <linux/cpu.h> +#include <linux/of_pci.h> #include <linux/pm_runtime.h> #include <linux/suspend.h> #include <linux/kexec.h> @@ -456,10 +457,17 @@ static int pci_device_probe(struct device *dev) if (error < 0) return error; + error = of_pci_setup_wake_irq(pci_dev); + if (error < 0) { + pcibios_free_irq(pci_dev); + return error; + } + pci_dev_get(pci_dev); error = __pci_device_probe(drv, pci_dev); if (error) { pcibios_free_irq(pci_dev); + of_pci_teardown_wake_irq(pci_dev); pci_dev_put(pci_dev); } @@ -471,6 +479,8 @@ static void pci_device_remove(struct device *dev) struct pci_dev *pci_dev = to_pci_dev(dev); struct pci_driver *drv = pci_dev->driver; + of_pci_teardown_wake_irq(pci_dev); + if (drv->remove) { pm_runtime_get_sync(dev); drv->remove(pci_dev); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index fba95486caaf..332a0b98b6c3 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -27,6 +27,7 @@ #include <linux/interrupt.h> #include <linux/device.h> #include <linux/pm_runtime.h> +#include <linux/pm_wakeirq.h> #include <linux/pci_hotplug.h> #include <linux/vmalloc.h> #include <asm/dma.h> @@ -1052,6 +1053,12 @@ static inline int platform_pci_set_wakeup(struct pci_dev *dev, bool enable) if (pci_use_mid_pm()) return PCI_POWER_ERROR; + /* Enable or disable wakeirq if set via device tree. */ + if (enable) + dev_pm_enable_wake_irq(&dev->dev); + else + dev_pm_disable_wake_irq(&dev->dev); + return acpi_pci_wakeup(dev, enable); } diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 9ed3b5550043..83a2af148631 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -631,6 +631,8 @@ int of_pci_get_max_link_speed(struct device_node *node); u32 of_pci_get_slot_power_limit(struct device_node *node, u8 *slot_power_limit_value, u8 *slot_power_limit_scale); +int of_pci_setup_wake_irq(struct pci_dev *pdev); +void of_pci_teardown_wake_irq(struct pci_dev *pdev); void pci_set_of_node(struct pci_dev *dev); void pci_release_of_node(struct pci_dev *dev); void pci_set_bus_of_node(struct pci_bus *bus); @@ -669,6 +671,12 @@ of_pci_get_slot_power_limit(struct device_node *node, return 0; } +static inline int of_pci_setup_wake_irq(struct pci_dev *pdev) +{ + return 0; +} + +static inline void of_pci_teardown_wake_irq(struct pci_dev *pdev) { } static inline void pci_set_of_node(struct pci_dev *dev) { } static inline void pci_release_of_node(struct pci_dev *dev) { } static inline void pci_set_bus_of_node(struct pci_bus *bus) { }