| Message ID | 20250401-wake_irq_support-v1-2-d2e22f4a0efd@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Add support for PCIe wake interrupt | expand |
On Tue, Apr 01, 2025 at 10:12:44AM +0530, Krishna Chaitanya Chundru wrote: > PCIe wake interrupt is needed for bringing back PCIe device state > from D3cold to D0. > > Implement new functions, of_pci_setup_wake_irq() and > of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices > using the Device Tree. > > From the port bus driver call these functions to enable wake support > for bridges. [...] > --- a/drivers/pci/pcie/portdrv.c > +++ b/drivers/pci/pcie/portdrv.c > @@ -695,6 +695,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev, > if (type == PCI_EXP_TYPE_RC_EC) > pcie_link_rcec(dev); > > + status = of_pci_setup_wake_irq(dev); > + if (status) > + return status; > + > status = pcie_port_device_register(dev); > if (status) > return status; > @@ -728,6 +732,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev) > pm_runtime_dont_use_autosuspend(&dev->dev); > } > > + of_pci_teardown_wake_irq(dev); > + > pcie_port_device_remove(dev); > > pci_disable_device(dev); Why doesn't the teardown order mirror the probe order, i.e. why is of_pci_teardown_wake_irq() called *before* pcie_port_device_remove() instead of after? (pcie_port_device_remove() is the opposite of pcie_port_device_register().) Also, why is it safe to bail out of probe on failure of of_pci_setup_wake_irq() without unwinding whatever pcie_link_rcec() has done? I think this needs either an explanation or reordering. Thanks, Lukas
On 4/1/2025 12:31 PM, Lukas Wunner wrote: > On Tue, Apr 01, 2025 at 10:12:44AM +0530, Krishna Chaitanya Chundru wrote: >> PCIe wake interrupt is needed for bringing back PCIe device state >> from D3cold to D0. >> >> Implement new functions, of_pci_setup_wake_irq() and >> of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices >> using the Device Tree. >> >> From the port bus driver call these functions to enable wake support >> for bridges. > [...] >> --- a/drivers/pci/pcie/portdrv.c >> +++ b/drivers/pci/pcie/portdrv.c >> @@ -695,6 +695,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev, >> if (type == PCI_EXP_TYPE_RC_EC) >> pcie_link_rcec(dev); >> >> + status = of_pci_setup_wake_irq(dev); >> + if (status) >> + return status; >> + >> status = pcie_port_device_register(dev); >> if (status) >> return status; >> @@ -728,6 +732,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev) >> pm_runtime_dont_use_autosuspend(&dev->dev); >> } >> >> + of_pci_teardown_wake_irq(dev); >> + >> pcie_port_device_remove(dev); >> >> pci_disable_device(dev); > > Why doesn't the teardown order mirror the probe order, i.e. why is > of_pci_teardown_wake_irq() called *before* pcie_port_device_remove() > instead of after? > ack, in the next patch I will move teardown after pcie_port_device_remove() > (pcie_port_device_remove() is the opposite of pcie_port_device_register().) > > Also, why is it safe to bail out of probe on failure of > of_pci_setup_wake_irq() without unwinding whatever pcie_link_rcec() > has done? I think this needs either an explanation or reordering. > if there is a failure in port_device_register also we are not unwinding so I taught it is already taken care. Looks like it is not. In the next patch I will move of_pci_setup_wake_irq() above pcie_link_rcec() to avoid all this. - Krishna Chaitanya. > Thanks, > > Lukas
On 4/1/2025 10:12 AM, Krishna Chaitanya Chundru wrote: > PCIe wake interrupt is needed for bringing back PCIe device state > from D3cold to D0. > > Implement new functions, of_pci_setup_wake_irq() and > of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices > using the Device Tree. > > From the port bus driver call these functions to enable wake support > for bridges. > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> > --- > drivers/pci/of.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/pci.h | 6 +++++ > drivers/pci/pcie/portdrv.c | 6 +++++ > 3 files changed, 72 insertions(+) > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index 7a806f5c0d201bc322d4a53d6ac47cab2cd28c55..abb0ba001edf604170aaa118f7fdc1a1709c171f 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -7,6 +7,7 @@ > #define pr_fmt(fmt) "PCI: OF: " fmt > > #include <linux/cleanup.h> > +#include <linux/gpio/consumer.h> > #include <linux/irqdomain.h> > #include <linux/kernel.h> > #include <linux/pci.h> > @@ -15,6 +16,7 @@ > #include <linux/of_address.h> > #include <linux/of_pci.h> > #include <linux/platform_device.h> > +#include <linux/pm_wakeirq.h> > #include "pci.h" > > #ifdef CONFIG_PCI > @@ -851,3 +853,61 @@ 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); > + > +/** > + * of_pci_setup_wake_irq - Set up wake interrupt for PCI device > + * @pdev: The PCI device structure > + * > + * This function sets up the wake interrupt for a PCI device by getting the > + * corresponding GPIO pin from the device tree, and configuring it as a > + * dedicated wake interrupt. > + * > + * Return: 0 if the wake gpio is not available or successfully parsed else > + * errno otherwise. > + */ > +int of_pci_setup_wake_irq(struct pci_dev *pdev) > +{ > + struct gpio_desc *wake; > + struct device_node *dn; > + int ret, wake_irq; > + > + dn = pci_device_to_OF_node(pdev); > + if (!dn) > + return 0; > + > + wake = devm_fwnode_gpiod_get(&pdev->dev, of_fwnode_handle(dn), > + "wake", GPIOD_IN, NULL); > + if (IS_ERR(wake)) { > + dev_warn(&pdev->dev, "Cannot get wake GPIO\n"); > + return 0; > + } > + > + wake_irq = gpiod_to_irq(wake); > + device_init_wakeup(&pdev->dev, true); > + > + ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, wake_irq); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret); > + device_init_wakeup(&pdev->dev, false); > + return ret; > + } > + irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq); > + > +/** > + * of_pci_teardown_wake_irq - Teardown wake interrupt setup for PCI device > + * > + * @pdev: The PCI device structure > + * > + * This function tears down the wake interrupt setup for a PCI device, > + * clearing the dedicated wake interrupt and disabling device wake-up. > + */ > +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.h b/drivers/pci/pci.h > index 01e51db8d285af54673db3ea526ceda073c94ec9..6e3d90db4b2505dd3885b482d4c5eafa033714e7 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -820,6 +820,9 @@ void pci_release_of_node(struct pci_dev *dev); > void pci_set_bus_of_node(struct pci_bus *bus); > void pci_release_bus_of_node(struct pci_bus *bus); > > +int of_pci_setup_wake_irq(struct pci_dev *pdev); > +void of_pci_teardown_wake_irq(struct pci_dev *pdev); > + > int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge); > bool of_pci_supply_present(struct device_node *np); > > @@ -863,6 +866,9 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br > return 0; > } > > +static int of_pci_setup_wake_irq(struct pci_dev *pdev) { return 0; } > +static void of_pci_teardown_wake_irq(struct pci_dev *pdev) { } > + > static inline bool of_pci_supply_present(struct device_node *np) > { > return false; > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c > index 02e73099bad0532466fa10f549cc3c5013aa1bbb..fe1da757e9eca0f82ae0d8043c0e4547ac9c30b6 100644 > --- a/drivers/pci/pcie/portdrv.c > +++ b/drivers/pci/pcie/portdrv.c > @@ -695,6 +695,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev, > if (type == PCI_EXP_TYPE_RC_EC) > pcie_link_rcec(dev); > > + status = of_pci_setup_wake_irq(dev); > + if (status) > + return status; > + > status = pcie_port_device_register(dev); > if (status) > return status; > @@ -728,6 +732,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev) > pm_runtime_dont_use_autosuspend(&dev->dev); > } > > + of_pci_teardown_wake_irq(dev); > + > pcie_port_device_remove(dev); > > pci_disable_device(dev); > we need to teardown the wake irq in shutdown also, I will add it in the next patch. - Krishna Chaitanya.
diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 7a806f5c0d201bc322d4a53d6ac47cab2cd28c55..abb0ba001edf604170aaa118f7fdc1a1709c171f 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -7,6 +7,7 @@ #define pr_fmt(fmt) "PCI: OF: " fmt #include <linux/cleanup.h> +#include <linux/gpio/consumer.h> #include <linux/irqdomain.h> #include <linux/kernel.h> #include <linux/pci.h> @@ -15,6 +16,7 @@ #include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/platform_device.h> +#include <linux/pm_wakeirq.h> #include "pci.h" #ifdef CONFIG_PCI @@ -851,3 +853,61 @@ 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); + +/** + * of_pci_setup_wake_irq - Set up wake interrupt for PCI device + * @pdev: The PCI device structure + * + * This function sets up the wake interrupt for a PCI device by getting the + * corresponding GPIO pin from the device tree, and configuring it as a + * dedicated wake interrupt. + * + * Return: 0 if the wake gpio is not available or successfully parsed else + * errno otherwise. + */ +int of_pci_setup_wake_irq(struct pci_dev *pdev) +{ + struct gpio_desc *wake; + struct device_node *dn; + int ret, wake_irq; + + dn = pci_device_to_OF_node(pdev); + if (!dn) + return 0; + + wake = devm_fwnode_gpiod_get(&pdev->dev, of_fwnode_handle(dn), + "wake", GPIOD_IN, NULL); + if (IS_ERR(wake)) { + dev_warn(&pdev->dev, "Cannot get wake GPIO\n"); + return 0; + } + + wake_irq = gpiod_to_irq(wake); + device_init_wakeup(&pdev->dev, true); + + ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, wake_irq); + if (ret < 0) { + dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret); + device_init_wakeup(&pdev->dev, false); + return ret; + } + irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING); + + return 0; +} +EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq); + +/** + * of_pci_teardown_wake_irq - Teardown wake interrupt setup for PCI device + * + * @pdev: The PCI device structure + * + * This function tears down the wake interrupt setup for a PCI device, + * clearing the dedicated wake interrupt and disabling device wake-up. + */ +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.h b/drivers/pci/pci.h index 01e51db8d285af54673db3ea526ceda073c94ec9..6e3d90db4b2505dd3885b482d4c5eafa033714e7 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -820,6 +820,9 @@ void pci_release_of_node(struct pci_dev *dev); void pci_set_bus_of_node(struct pci_bus *bus); void pci_release_bus_of_node(struct pci_bus *bus); +int of_pci_setup_wake_irq(struct pci_dev *pdev); +void of_pci_teardown_wake_irq(struct pci_dev *pdev); + int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge); bool of_pci_supply_present(struct device_node *np); @@ -863,6 +866,9 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br return 0; } +static int of_pci_setup_wake_irq(struct pci_dev *pdev) { return 0; } +static void of_pci_teardown_wake_irq(struct pci_dev *pdev) { } + static inline bool of_pci_supply_present(struct device_node *np) { return false; diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c index 02e73099bad0532466fa10f549cc3c5013aa1bbb..fe1da757e9eca0f82ae0d8043c0e4547ac9c30b6 100644 --- a/drivers/pci/pcie/portdrv.c +++ b/drivers/pci/pcie/portdrv.c @@ -695,6 +695,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev, if (type == PCI_EXP_TYPE_RC_EC) pcie_link_rcec(dev); + status = of_pci_setup_wake_irq(dev); + if (status) + return status; + status = pcie_port_device_register(dev); if (status) return status; @@ -728,6 +732,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev) pm_runtime_dont_use_autosuspend(&dev->dev); } + of_pci_teardown_wake_irq(dev); + pcie_port_device_remove(dev); pci_disable_device(dev);
PCIe wake interrupt is needed for bringing back PCIe device state from D3cold to D0. Implement new functions, of_pci_setup_wake_irq() and of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices using the Device Tree. From the port bus driver call these functions to enable wake support for bridges. Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> --- drivers/pci/of.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.h | 6 +++++ drivers/pci/pcie/portdrv.c | 6 +++++ 3 files changed, 72 insertions(+)