[PATCH v4 3/3] PCI: Add support for PCIe WAKE# interrupt

Krishna Chaitanya Chundru posted 3 patches 2 months ago
[PATCH v4 3/3] PCI: Add support for PCIe WAKE# interrupt
Posted by Krishna Chaitanya Chundru 2 months ago
According to the PCIe specification 6, sec 5.3.3.2, there are two defined
wakeup mechanisms: Beacon and WAKE# for the Link wakeup mechanisms to
provide a means of signaling the platform to re-establish power and
reference clocks to the components within its domain. Adding WAKE#
support in PCI framework.

According to the PCIe specification, multiple WAKE# signals can exist in a
system. In configurations involving a PCIe switch, each downstream port
(DSP) of the switch may be connected to a separate WAKE# line, allowing
each endpoint to signal WAKE# independently. To support this, the WAKE#
should be described in the device tree node of the upstream bridge to which
the endpoint is connected. For example, in a switch-based topology, the
WAKE# can be defined in the DSP of the switch. In a direct connection
scenario, the WAKE# can be defined in the root port. If all endpoints share
a single WAKE# line, the GPIO should be defined in the root port.

During endpoint probe, the driver searches for the WAKE# in its immediate
upstream bridge. If not found, it continues walking up the hierarchy until
it either finds a WAKE# or reaches the root port. Once found, the driver
registers the wake IRQ in shared mode, as the WAKE# may be shared among
multiple endpoints.

When the IRQ is asserted, the wake handler triggers a pm_runtime_resume().
The PM framework ensures that the parent device is resumed before the
child i.e controller driver which can bring back link to D0.

WAKE# is added in dts schema and merged based on this link.

Link: https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/of.c         | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c | 10 ++++++++
 drivers/pci/pci.h        | 10 ++++++++
 drivers/pci/probe.c      |  2 ++
 drivers/pci/remove.c     |  1 +
 include/linux/pci.h      |  2 ++
 6 files changed, 91 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f119845637e163d9051437c89662762f8..5a832bbf2dd5da728080f83220f47c3578cb6b5a 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
@@ -586,6 +588,29 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 	return irq_create_of_mapping(&oirq);
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_and_map_pci);
+
+void pci_parse_of_wake_gpio(struct pci_dev *dev)
+{
+	struct device_node *dn __free(device_node) = pci_device_to_OF_node(dev);
+	struct gpio_desc *gpio;
+
+	if (!dn)
+		return;
+
+	gpio = fwnode_gpiod_get_index(of_fwnode_handle(no_free_ptr(dn)),
+				      "wake", 0, GPIOD_IN, NULL);
+	if (!IS_ERR(gpio))
+		dev->wake = gpio;
+}
+
+void pci_remove_of_wake_gpio(struct pci_dev *dev)
+{
+	if (!dev->wake)
+		return;
+
+	gpiod_put(dev->wake);
+	dev->wake = NULL;
+}
 #endif	/* CONFIG_OF_IRQ */
 
 static int pci_parse_request_of_pci_ranges(struct device *dev,
@@ -1010,3 +1035,44 @@ int of_pci_get_equalization_presets(struct device *dev,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
+
+int pci_configure_wake_irq(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge = pdev;
+	struct gpio_desc *wake;
+	int ret, wake_irq;
+
+	while (bridge) {
+		wake = bridge->wake;
+		if (wake)
+			break;
+		bridge = pci_upstream_bridge(bridge);  // Move to upstream bridge
+	}
+
+	if (!wake)
+		return 0;
+
+	wake_irq = gpiod_to_irq(wake);
+	if (wake_irq < 0) {
+		dev_err(&pdev->dev, "Failed to get wake irq: %d\n", wake_irq);
+		return wake_irq;
+	}
+
+	device_init_wakeup(&pdev->dev, true);
+
+	ret = dev_pm_set_dedicated_wake_irq_flags(&pdev->dev, wake_irq,
+						  IRQF_SHARED | IRQ_TYPE_EDGE_FALLING);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret);
+		device_init_wakeup(&pdev->dev, false);
+		return ret;
+	}
+
+	return 0;
+}
+
+void pci_remove_wake_irq(struct pci_dev *pdev)
+{
+	dev_pm_clear_wake_irq(&pdev->dev);
+	device_init_wakeup(&pdev->dev, false);
+}
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index b853585cb1f87216981bde2a7782b8ed9c337636..2a1dca1d19b914d21b300ea78be0e0dce418cc88 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -447,10 +447,19 @@ static int pci_device_probe(struct device *dev)
 	if (error < 0)
 		return error;
 
+	if (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ENDPOINT) {
+		error =  pci_configure_wake_irq(pci_dev);
+		if (error) {
+			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);
+		pci_remove_wake_irq(pci_dev);
 		pci_dev_put(pci_dev);
 	}
 
@@ -475,6 +484,7 @@ static void pci_device_remove(struct device *dev)
 		pm_runtime_put_noidle(dev);
 	}
 	pcibios_free_irq(pci_dev);
+	pci_remove_wake_irq(pci_dev);
 	pci_dev->driver = NULL;
 	pci_iov_remove(pci_dev);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 12215ee72afb682b669c0e3a582b5379828e70c4..c8cf0b404a4f31b271f187dddd75a007c7566982 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -920,6 +920,11 @@ 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);
 
+void pci_parse_of_wake_gpio(struct pci_dev *dev);
+void pci_remove_of_wake_gpio(struct pci_dev *dev);
+int pci_configure_wake_irq(struct pci_dev *pdev);
+void pci_remove_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);
 int of_pci_get_equalization_presets(struct device *dev,
@@ -965,6 +970,11 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline void pci_parse_of_wake_gpio(struct pci_dev *dev) { }
+static inline void pci_remove_of_wake_gpio(struct pci_dev *dev) { }
+static inline int pci_configure_wake_irq(struct pci_dev *pdev) { return 0; }
+static inline void pci_remove_wake_irq(struct pci_dev *pdev) { }
+
 static inline bool of_pci_supply_present(struct device_node *np)
 {
 	return false;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e6a34db778266862564532becc2a30aec09bab22..4fb9d8df19bc41cb84dcd1886546076bcc867a43 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2717,6 +2717,8 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
+	pci_parse_of_wake_gpio(dev);
+
 	/* Notifier could use PCI capabilities */
 	ret = device_add(&dev->dev);
 	WARN_ON(ret < 0);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 445afdfa6498edc88f1ef89df279af1419025495..1910f7c18b8f9b11c8136fea970788aaf834c97f 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -52,6 +52,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	if (pci_dev_test_and_set_removed(dev))
 		return;
 
+	pci_remove_of_wake_gpio(dev);
 	pci_doe_sysfs_teardown(dev);
 	pci_npem_remove(dev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 05e68f35f39238f8b9ce08df97b384d1c1e89bbe..8f861298e41d2f0d2dd0fc3f5778fe0e77a93511 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -548,6 +548,8 @@ struct pci_dev {
 	/* These methods index pci_reset_fn_methods[] */
 	u8 reset_methods[PCI_NUM_RESET_METHODS]; /* In priority order */
 
+	struct gpio_desc *wake; /* Holds WAKE# gpio */
+
 #ifdef CONFIG_PCIE_TPH
 	u16		tph_cap;	/* TPH capability offset */
 	u8		tph_mode;	/* TPH mode */

-- 
2.34.1
Re: [PATCH v4 3/3] PCI: Add support for PCIe WAKE# interrupt
Posted by Bjorn Helgaas 2 months ago
On Fri, Aug 01, 2025 at 04:29:44PM +0530, Krishna Chaitanya Chundru wrote:
> According to the PCIe specification 6, sec 5.3.3.2, there are two defined
> wakeup mechanisms: Beacon and WAKE# for the Link wakeup mechanisms to
> provide a means of signaling the platform to re-establish power and
> reference clocks to the components within its domain. Adding WAKE#
> support in PCI framework.

I think Beacon is a hardware mechanism invisible to software (PCIe
r7.0, sec 4.2.7.8.1).

> According to the PCIe specification, multiple WAKE# signals can exist in a
> system. In configurations involving a PCIe switch, each downstream port
> (DSP) of the switch may be connected to a separate WAKE# line, allowing
> each endpoint to signal WAKE# independently. To support this, the WAKE#
> should be described in the device tree node of the upstream bridge to which
> the endpoint is connected.

I think this says a bit more than we know.  AFAICS, the PCIe spec does
not require any particular WAKE# routing.  WAKE# *could* be routed to
an upstream bridge (as shown in the 5.3.3.2 implementation note), but
it doesn't have to be.  I think we need to allow WAKE# to be described
by an Endpoint directly (which I think this patch does).

I'm not sure about searching upstream PCI bridges.  I don't think
there's anything in the PCIe spec about a connection between WAKE#
routing and the PCI topology.  Maybe we need to search enclosing DT
scopes?  I'm not really sure how DT works in this respect.  WAKE#
could be routed to some GPIO completely unrelated to the PCI host
bridge.

I don't see anything that would prevent a Switch Port from asserting a
WAKE# interrupt, so I'm not sure we should restrict it to Endpoints.

> For example, in a switch-based topology, the
> WAKE# can be defined in the DSP of the switch. In a direct connection
> scenario, the WAKE# can be defined in the root port. If all endpoints share
> a single WAKE# line, the GPIO should be defined in the root port.
> 
> During endpoint probe, the driver searches for the WAKE# in its immediate
> upstream bridge. If not found, it continues walking up the hierarchy until
> it either finds a WAKE# or reaches the root port. Once found, the driver
> registers the wake IRQ in shared mode, as the WAKE# may be shared among
> multiple endpoints.
> 
> When the IRQ is asserted, the wake handler triggers a pm_runtime_resume().

I guess "wake handler" refers to handle_threaded_wake_irq()?  If so,
just use the name directly to make it easier for people to follow
this.

> The PM framework ensures that the parent device is resumed before the
> child i.e controller driver which can bring back link to D0.

Nit: a *device* can be in D0.  Links would be in L0, etc.

> WAKE# is added in dts schema and merged based on this link.
> 
> Link: https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ...

> +void pci_parse_of_wake_gpio(struct pci_dev *dev)
> +{
> +	struct device_node *dn __free(device_node) = pci_device_to_OF_node(dev);

I'm still trying to wrap my head around __free().  Why are we using
__free() and no_free_ptr() here?  AFAICS we're not allocating anything
here.

> +	struct gpio_desc *gpio;
> +
> +	if (!dn)
> +		return;
> +
> +	gpio = fwnode_gpiod_get_index(of_fwnode_handle(no_free_ptr(dn)),
> +				      "wake", 0, GPIOD_IN, NULL);
> +	if (!IS_ERR(gpio))
> +		dev->wake = gpio;
> +}
> +
> +void pci_remove_of_wake_gpio(struct pci_dev *dev)
> +{
> +	if (!dev->wake)
> +		return;
> +
> +	gpiod_put(dev->wake);
> +	dev->wake = NULL;
> +}
>  #endif	/* CONFIG_OF_IRQ */
>  
>  static int pci_parse_request_of_pci_ranges(struct device *dev,
> @@ -1010,3 +1035,44 @@ int of_pci_get_equalization_presets(struct device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> +
> +int pci_configure_wake_irq(struct pci_dev *pdev)
> +{
> +	struct pci_dev *bridge = pdev;
> +	struct gpio_desc *wake;
> +	int ret, wake_irq;
> +
> +	while (bridge) {
> +		wake = bridge->wake;
> +		if (wake)
> +			break;
> +		bridge = pci_upstream_bridge(bridge);  // Move to upstream bridge

If we need to search more scopes, I think we should be searching DT
scopes, not PCI bridges.

> +	}
> +
> +	if (!wake)
> +		return 0;
> +
> +	wake_irq = gpiod_to_irq(wake);
> +	if (wake_irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get wake irq: %d\n", wake_irq);
> +		return wake_irq;
> +	}
> +
> +	device_init_wakeup(&pdev->dev, true);
> +
> +	ret = dev_pm_set_dedicated_wake_irq_flags(&pdev->dev, wake_irq,
> +						  IRQF_SHARED | IRQ_TYPE_EDGE_FALLING);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret);
> +		device_init_wakeup(&pdev->dev, false);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void pci_remove_wake_irq(struct pci_dev *pdev)
> +{
> +	dev_pm_clear_wake_irq(&pdev->dev);
> +	device_init_wakeup(&pdev->dev, false);
> +}
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index b853585cb1f87216981bde2a7782b8ed9c337636..2a1dca1d19b914d21b300ea78be0e0dce418cc88 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -447,10 +447,19 @@ static int pci_device_probe(struct device *dev)
>  	if (error < 0)
>  		return error;
>  
> +	if (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ENDPOINT) {

I guess there's a policy question here: configuring this in
pci_device_probe() implies that we only pay attention to WAKE# when a
driver is bound to the device.  Or, since WAKE# may be shared, I guess
we pay attention to it if any device sharing this WAKE# IRQ has a
driver?

And since we check for Endpoint, we ignore any potential WAKE# IRQs
from Switches?

ACPI has corresponding wakeup mechanisms.  Are they limited to devices
with drivers or to Endpoints?  Seems like this OF-based mechanism
should work similarly if possible.

> +		error =  pci_configure_wake_irq(pci_dev);
> +		if (error) {
> +			pcibios_free_irq(pci_dev);

As far as I can tell, pcibios_free_irq() is a no-op and I should have
removed it completely at the time of 6c777e8799a9 ("Revert "PCI, x86:
Implement pcibios_alloc_irq() and pcibios_free_irq()"").

I think we should remove it before this series rather than add new
calls.

> +			return error;
> +		}
> +	}
> +
>  	pci_dev_get(pci_dev);
>  	error = __pci_device_probe(drv, pci_dev);
>  	if (error) {
>  		pcibios_free_irq(pci_dev);
> +		pci_remove_wake_irq(pci_dev);
>  		pci_dev_put(pci_dev);
>  	}
>  
> @@ -475,6 +484,7 @@ static void pci_device_remove(struct device *dev)
>  		pm_runtime_put_noidle(dev);
>  	}
>  	pcibios_free_irq(pci_dev);
> +	pci_remove_wake_irq(pci_dev);
>  	pci_dev->driver = NULL;
>  	pci_iov_remove(pci_dev);
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 12215ee72afb682b669c0e3a582b5379828e70c4..c8cf0b404a4f31b271f187dddd75a007c7566982 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -920,6 +920,11 @@ 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);
>  
> +void pci_parse_of_wake_gpio(struct pci_dev *dev);
> +void pci_remove_of_wake_gpio(struct pci_dev *dev);
> +int pci_configure_wake_irq(struct pci_dev *pdev);
> +void pci_remove_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);
>  int of_pci_get_equalization_presets(struct device *dev,
> @@ -965,6 +970,11 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
>  	return 0;
>  }
>  
> +static inline void pci_parse_of_wake_gpio(struct pci_dev *dev) { }
> +static inline void pci_remove_of_wake_gpio(struct pci_dev *dev) { }
> +static inline int pci_configure_wake_irq(struct pci_dev *pdev) { return 0; }
> +static inline void pci_remove_wake_irq(struct pci_dev *pdev) { }
> +
>  static inline bool of_pci_supply_present(struct device_node *np)
>  {
>  	return false;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index e6a34db778266862564532becc2a30aec09bab22..4fb9d8df19bc41cb84dcd1886546076bcc867a43 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2717,6 +2717,8 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Set up MSI IRQ domain */
>  	pci_set_msi_domain(dev);
>  
> +	pci_parse_of_wake_gpio(dev);
> +
>  	/* Notifier could use PCI capabilities */
>  	ret = device_add(&dev->dev);
>  	WARN_ON(ret < 0);
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 445afdfa6498edc88f1ef89df279af1419025495..1910f7c18b8f9b11c8136fea970788aaf834c97f 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -52,6 +52,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>  	if (pci_dev_test_and_set_removed(dev))
>  		return;
>  
> +	pci_remove_of_wake_gpio(dev);
>  	pci_doe_sysfs_teardown(dev);
>  	pci_npem_remove(dev);
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 05e68f35f39238f8b9ce08df97b384d1c1e89bbe..8f861298e41d2f0d2dd0fc3f5778fe0e77a93511 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -548,6 +548,8 @@ struct pci_dev {
>  	/* These methods index pci_reset_fn_methods[] */
>  	u8 reset_methods[PCI_NUM_RESET_METHODS]; /* In priority order */
>  
> +	struct gpio_desc *wake; /* Holds WAKE# gpio */
> +
>  #ifdef CONFIG_PCIE_TPH
>  	u16		tph_cap;	/* TPH capability offset */
>  	u8		tph_mode;	/* TPH mode */
> 
> -- 
> 2.34.1
>
Re: [PATCH v4 3/3] PCI: Add support for PCIe WAKE# interrupt
Posted by Manivannan Sadhasivam 2 months ago
On Fri, Aug 01, 2025 at 04:27:25PM GMT, Bjorn Helgaas wrote:
> On Fri, Aug 01, 2025 at 04:29:44PM +0530, Krishna Chaitanya Chundru wrote:
> > According to the PCIe specification 6, sec 5.3.3.2, there are two defined
> > wakeup mechanisms: Beacon and WAKE# for the Link wakeup mechanisms to
> > provide a means of signaling the platform to re-establish power and
> > reference clocks to the components within its domain. Adding WAKE#
> > support in PCI framework.
> 
> I think Beacon is a hardware mechanism invisible to software (PCIe
> r7.0, sec 4.2.7.8.1).
> 
> > According to the PCIe specification, multiple WAKE# signals can exist in a
> > system. In configurations involving a PCIe switch, each downstream port
> > (DSP) of the switch may be connected to a separate WAKE# line, allowing
> > each endpoint to signal WAKE# independently. To support this, the WAKE#
> > should be described in the device tree node of the upstream bridge to which
> > the endpoint is connected.
> 
> I think this says a bit more than we know.  AFAICS, the PCIe spec does
> not require any particular WAKE# routing.  WAKE# *could* be routed to
> an upstream bridge (as shown in the 5.3.3.2 implementation note), but
> it doesn't have to be.  I think we need to allow WAKE# to be described
> by an Endpoint directly (which I think this patch does).
> 
> I'm not sure about searching upstream PCI bridges.  I don't think
> there's anything in the PCIe spec about a connection between WAKE#
> routing and the PCI topology.  Maybe we need to search enclosing DT
> scopes?  I'm not really sure how DT works in this respect.  WAKE#
> could be routed to some GPIO completely unrelated to the PCI host
> bridge.
> 

PCIe spec r5, fig 5-4 describes that WAKE# is supposed to be connected to a
*slot*, not directly to the endpointi, though endpoint is the one toggling it.
And in devicetree, we describe the slot using the bridge node. So it makes sense
to add the 'wake-gpios' property to the bridge node only. It is what allowed by
the dtschema today.

> I don't see anything that would prevent a Switch Port from asserting a
> WAKE# interrupt, so I'm not sure we should restrict it to Endpoints.
> 

Atleast fig 5-4 describes that the switch has to generate Beacon to RC for WAKE#
asserted by the endpoints connected to the downstream slots.

> > For example, in a switch-based topology, the
> > WAKE# can be defined in the DSP of the switch. In a direct connection
> > scenario, the WAKE# can be defined in the root port. If all endpoints share
> > a single WAKE# line, the GPIO should be defined in the root port.
> > 
> > During endpoint probe, the driver searches for the WAKE# in its immediate
> > upstream bridge. If not found, it continues walking up the hierarchy until
> > it either finds a WAKE# or reaches the root port. Once found, the driver
> > registers the wake IRQ in shared mode, as the WAKE# may be shared among
> > multiple endpoints.
> > 
> > When the IRQ is asserted, the wake handler triggers a pm_runtime_resume().
> 
> I guess "wake handler" refers to handle_threaded_wake_irq()?  If so,
> just use the name directly to make it easier for people to follow
> this.
> 
> > The PM framework ensures that the parent device is resumed before the
> > child i.e controller driver which can bring back link to D0.
> 
> Nit: a *device* can be in D0.  Links would be in L0, etc.
> 
> > WAKE# is added in dts schema and merged based on this link.
> > 
> > Link: https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ...
> 
> > +void pci_parse_of_wake_gpio(struct pci_dev *dev)
> > +{
> > +	struct device_node *dn __free(device_node) = pci_device_to_OF_node(dev);
> 
> I'm still trying to wrap my head around __free().  Why are we using
> __free() and no_free_ptr() here?  AFAICS we're not allocating anything
> here.
> 
> > +	struct gpio_desc *gpio;
> > +
> > +	if (!dn)
> > +		return;
> > +
> > +	gpio = fwnode_gpiod_get_index(of_fwnode_handle(no_free_ptr(dn)),
> > +				      "wake", 0, GPIOD_IN, NULL);
> > +	if (!IS_ERR(gpio))
> > +		dev->wake = gpio;
> > +}
> > +
> > +void pci_remove_of_wake_gpio(struct pci_dev *dev)
> > +{
> > +	if (!dev->wake)
> > +		return;
> > +
> > +	gpiod_put(dev->wake);
> > +	dev->wake = NULL;
> > +}
> >  #endif	/* CONFIG_OF_IRQ */
> >  
> >  static int pci_parse_request_of_pci_ranges(struct device *dev,
> > @@ -1010,3 +1035,44 @@ int of_pci_get_equalization_presets(struct device *dev,
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > +
> > +int pci_configure_wake_irq(struct pci_dev *pdev)
> > +{
> > +	struct pci_dev *bridge = pdev;
> > +	struct gpio_desc *wake;
> > +	int ret, wake_irq;
> > +
> > +	while (bridge) {
> > +		wake = bridge->wake;
> > +		if (wake)
> > +			break;
> > +		bridge = pci_upstream_bridge(bridge);  // Move to upstream bridge
> 
> If we need to search more scopes, I think we should be searching DT
> scopes, not PCI bridges.
> 
> > +	}
> > +
> > +	if (!wake)
> > +		return 0;
> > +
> > +	wake_irq = gpiod_to_irq(wake);
> > +	if (wake_irq < 0) {
> > +		dev_err(&pdev->dev, "Failed to get wake irq: %d\n", wake_irq);
> > +		return wake_irq;
> > +	}
> > +
> > +	device_init_wakeup(&pdev->dev, true);
> > +
> > +	ret = dev_pm_set_dedicated_wake_irq_flags(&pdev->dev, wake_irq,
> > +						  IRQF_SHARED | IRQ_TYPE_EDGE_FALLING);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret);
> > +		device_init_wakeup(&pdev->dev, false);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +void pci_remove_wake_irq(struct pci_dev *pdev)
> > +{
> > +	dev_pm_clear_wake_irq(&pdev->dev);
> > +	device_init_wakeup(&pdev->dev, false);
> > +}
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index b853585cb1f87216981bde2a7782b8ed9c337636..2a1dca1d19b914d21b300ea78be0e0dce418cc88 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -447,10 +447,19 @@ static int pci_device_probe(struct device *dev)
> >  	if (error < 0)
> >  		return error;
> >  
> > +	if (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ENDPOINT) {
> 
> I guess there's a policy question here: configuring this in
> pci_device_probe() implies that we only pay attention to WAKE# when a
> driver is bound to the device.  Or, since WAKE# may be shared, I guess
> we pay attention to it if any device sharing this WAKE# IRQ has a
> driver?
> 

I don't think we check if the driver is bind to the device before putting it
into D3Cold. If the device was previously enabled from sysfs, then it cannot
ask host to wake it up.

- Mani

-- 
மணிவண்ணன் சதாசிவம்