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

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

[Krishna Chaitanya Chundru]

PCIe WAKE# interrupt is needed for bringing back PCIe device state from
D3cold to D0.

This is pending from long time, there was two attempts done previously to
add WAKE# support[1], [2]. Those series tried to add support for legacy
interrupts along with WAKE#. Legacy interrupts are already available in
the latest kernel and we can ignore them. For the wake IRQ the series is
trying to use interrupts property define in the device tree.

This series is using gpio property instead of interrupts, from
gpio desc driver will allocate the dedicate IRQ.

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# GPIO 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 patch.
https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/

[1]: https://lore.kernel.org/all/b2b91240-95fe-145d-502c-d52225497a34@nvidia.com/T/
[2]: https://lore.kernel.org/all/20171226023646.17722-1-jeffy.chen@rock-chips.com/

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v4:
- Move wake from portdrv to core framework to endpoint (Bjorn).
- Added support for multiple WAKE# case (Bjorn). But traverse from
  endpoint upstream port to root port till you get WAKE#. And use
  IRQF_SHARED flag for requesting interrupts.
- Link to v3: https://lore.kernel.org/r/20250605-wake_irq_support-v3-0-7ba56dc909a5@oss.qualcomm.com

Changes in v3:
- Update the commit messages, function names etc as suggested by Mani.
- return wake_irq if returns error (Neil).
- Link to v2: https://lore.kernel.org/r/20250419-wake_irq_support-v2-0-06baed9a87a1@oss.qualcomm.com

Changes in v2:
- Move the wake irq teardown after pcie_port_device_remove
  and move of_pci_setup_wake_irq before pcie_link_rcec (Lukas)
- teardown wake irq in shutdown also.
- Link to v1: https://lore.kernel.org/r/20250401-wake_irq_support-v1-0-d2e22f4a0efd@oss.qualcomm.com

---
Krishna Chaitanya Chundru (3):
      arm64: dts: qcom: sc7280: Add wake GPIO
      PM: sleep: wakeirq: Add support for custom IRQ flags in dedicated wake IRQ setup
      PCI: Add support for PCIe WAKE# interrupt

 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts   |  1 +
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       |  1 +
 drivers/base/power/wakeirq.c                   | 43 +++++++++++++++--
 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 +
 include/linux/pm_wakeirq.h                     |  6 +++
 11 files changed, 138 insertions(+), 5 deletions(-)
---
base-commit: 5f10a4bfd256d0ff64ef13baf7af7b1adf00740c
change-id: 20250329-wake_irq_support-79772fc8cd44

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH v4 1/3] arm64: dts: qcom: sc7280: Add wake GPIO

[Krishna Chaitanya Chundru]

Add WAKE# gpio which is needed to bring PCIe device state
from D3cold to D0.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts   | 1 +
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 10c152ac03c874df5f1dc386d9079d3db1c55362..a4d85772f86955ad061433b138581fa9d81110a4 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -810,6 +810,7 @@ &mdss_edp_phy {
 
 &pcieport1 {
 	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
 };
 
 &pcie1 {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index 60b3cf50ea1d61dd5e8b573b5f1c6faa1c291eee..5e73060771329cade097bf1a71056a456a7937d7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -477,6 +477,7 @@ &pcie1 {
 
 &pcieport1 {
 	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
 };
 
 &pm8350c_pwm {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 0b0212b670797a364d7f0e7a458fc73245fff8db..240513774612fb2bfcdb951e5a5a77c49f49eb82 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -418,6 +418,7 @@ &lpass_va_macro {
 
 &pcieport1 {
 	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
 };
 
 &pcie1 {

-- 
2.34.1

[PATCH v4 2/3] PM: sleep: wakeirq: Add support for custom IRQ flags in dedicated wake IRQ setup

[Krishna Chaitanya Chundru]

Some devices require more flexibility when configuring their dedicated
wake-up interrupts, such as support for IRQF_SHARED or other IRQ flags.
This is particularly useful in PCIe systems where multiple endpoints
(e.g., Wi-Fi and Bluetooth controllers) share a common WAKE# signal
line for waking the device from D3cold to D0. In such cases, drivers
can use this API with IRQF_SHARED to register a shared wake IRQ handler.

Update the internal helper __dev_pm_set_dedicated_wake_irq() to accept an
irq_flags argument. Modify the existing dev_pm_set_dedicated_wake_irq()
and dev_pm_set_dedicated_wake_irq_reverse() to preserve current behavior
by passing default flags (IRQF_ONESHOT | IRQF_NO_AUTOEN).

Introduce a new API, dev_pm_set_dedicated_wake_irq_flags(), to allow
callers to specify custom IRQ flags. If IRQF_SHARED is used, remove
IRQF_NO_AUTOEN and disable the IRQ after setup to prevent spurious wakeups.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/base/power/wakeirq.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/pm_wakeirq.h   |  6 ++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
index 8aa28c08b2891f3af490175362cc1a759069bd50..655c28d5fc6850f50fc2ed74c5fbc066a21ae7b3 100644
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -168,7 +168,8 @@ static irqreturn_t handle_threaded_wake_irq(int irq, void *_wirq)
 	return IRQ_HANDLED;
 }
 
-static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag)
+static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag,
+					   unsigned int irq_flags)
 {
 	struct wake_irq *wirq;
 	int err;
@@ -197,8 +198,7 @@ static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned
 	 * so we use a threaded irq.
 	 */
 	err = request_threaded_irq(irq, NULL, handle_threaded_wake_irq,
-				   IRQF_ONESHOT | IRQF_NO_AUTOEN,
-				   wirq->name, wirq);
+				   irq_flags, wirq->name, wirq);
 	if (err)
 		goto err_free_name;
 
@@ -234,7 +234,7 @@ static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned
  */
 int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 {
-	return __dev_pm_set_dedicated_wake_irq(dev, irq, 0);
+	return __dev_pm_set_dedicated_wake_irq(dev, irq, 0, IRQF_ONESHOT | IRQF_NO_AUTOEN);
 }
 EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
 
@@ -255,10 +255,43 @@ EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
  */
 int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
 {
-	return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE);
+	return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE,
+					       IRQF_ONESHOT | IRQF_NO_AUTOEN);
 }
 EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_reverse);
 
+/**
+ * dev_pm_set_dedicated_wake_irq_flags - Request a dedicated wake-up interrupt
+ *                                       with custom flags
+ * @dev: Device entry
+ * @irq: Device wake-up interrupt
+ * @flags: IRQ flags (e.g., IRQF_SHARED)
+ *
+ * This API sets up a threaded interrupt handler for a device that has
+ * a dedicated wake-up interrupt in addition to the device IO interrupt,
+ * allowing the caller to specify custom IRQ flags such as IRQF_SHARED.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int dev_pm_set_dedicated_wake_irq_flags(struct device *dev, int irq, unsigned long flags)
+{
+	struct wake_irq *wirq;
+	int ret;
+
+	flags |= IRQF_ONESHOT;
+	if (!(flags & IRQF_SHARED))
+		flags |= IRQF_NO_AUTOEN;
+
+	ret =  __dev_pm_set_dedicated_wake_irq(dev, irq, 0, flags);
+	if (!ret && (flags & IRQF_SHARED)) {
+		wirq = dev->power.wakeirq;
+		disable_irq_nosync(wirq->irq);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_flags);
+
 /**
  * dev_pm_enable_wake_irq_check - Checks and enables wake-up interrupt
  * @dev: Device
diff --git a/include/linux/pm_wakeirq.h b/include/linux/pm_wakeirq.h
index 25b63ed51b765c2c6919f259668a12675330835e..14950616efe34f4fa5408ca54cd8eeb1f7f0ff13 100644
--- a/include/linux/pm_wakeirq.h
+++ b/include/linux/pm_wakeirq.h
@@ -11,6 +11,7 @@ extern int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq);
 extern int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq);
 extern void dev_pm_clear_wake_irq(struct device *dev);
 extern int devm_pm_set_wake_irq(struct device *dev, int irq);
+extern int dev_pm_set_dedicated_wake_irq_flags(struct device *dev, int irq, unsigned long flags);
 
 #else	/* !CONFIG_PM */
 
@@ -38,5 +39,10 @@ static inline int devm_pm_set_wake_irq(struct device *dev, int irq)
 	return 0;
 }
 
+static inline int dev_pm_set_dedicated_wake_irq_flags(struct device *dev,
+						      int irq, unsigned long flags)
+{
+	return 0;
+}
 #endif	/* CONFIG_PM */
 #endif	/* _LINUX_PM_WAKEIRQ_H */

-- 
2.34.1

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

[Krishna Chaitanya Chundru]

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

[Bjorn Helgaas]

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 2/3] PM: sleep: wakeirq: Add support for custom IRQ flags in dedicated wake IRQ setup

[Bjorn Helgaas]

On Fri, Aug 01, 2025 at 04:29:43PM +0530, Krishna Chaitanya Chundru wrote:
> Some devices require more flexibility when configuring their dedicated
> wake-up interrupts, such as support for IRQF_SHARED or other IRQ flags.

I guess the "dedicated" in dev_pm_set_dedicated_wake_irq() means "an
interrupt that only signals wakeup requests, possibly used by several
devices," not "an interrupt only used by this device"?

> This is particularly useful in PCIe systems where multiple endpoints
> (e.g., Wi-Fi and Bluetooth controllers) share a common WAKE# signal
> line for waking the device from D3cold to D0. In such cases, drivers
> can use this API with IRQF_SHARED to register a shared wake IRQ handler.

Nit: WAKE# does not itself wakeup any devices.  It only tells software
that the device *requested* a wakeup, and it's up to software to
actually perform it.

> Update the internal helper __dev_pm_set_dedicated_wake_irq() to accept an
> irq_flags argument. Modify the existing dev_pm_set_dedicated_wake_irq()
> and dev_pm_set_dedicated_wake_irq_reverse() to preserve current behavior
> by passing default flags (IRQF_ONESHOT | IRQF_NO_AUTOEN).
> 
> Introduce a new API, dev_pm_set_dedicated_wake_irq_flags(), to allow
> callers to specify custom IRQ flags. If IRQF_SHARED is used, remove
> IRQF_NO_AUTOEN and disable the IRQ after setup to prevent spurious wakeups.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/base/power/wakeirq.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  include/linux/pm_wakeirq.h   |  6 ++++++
>  2 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
> index 8aa28c08b2891f3af490175362cc1a759069bd50..655c28d5fc6850f50fc2ed74c5fbc066a21ae7b3 100644
> --- a/drivers/base/power/wakeirq.c
> +++ b/drivers/base/power/wakeirq.c
> @@ -168,7 +168,8 @@ static irqreturn_t handle_threaded_wake_irq(int irq, void *_wirq)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag)
> +static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned int flag,
> +					   unsigned int irq_flags)
>  {
>  	struct wake_irq *wirq;
>  	int err;
> @@ -197,8 +198,7 @@ static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned
>  	 * so we use a threaded irq.
>  	 */
>  	err = request_threaded_irq(irq, NULL, handle_threaded_wake_irq,
> -				   IRQF_ONESHOT | IRQF_NO_AUTOEN,
> -				   wirq->name, wirq);
> +				   irq_flags, wirq->name, wirq);
>  	if (err)
>  		goto err_free_name;
>  
> @@ -234,7 +234,7 @@ static int __dev_pm_set_dedicated_wake_irq(struct device *dev, int irq, unsigned
>   */
>  int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
>  {
> -	return __dev_pm_set_dedicated_wake_irq(dev, irq, 0);
> +	return __dev_pm_set_dedicated_wake_irq(dev, irq, 0, IRQF_ONESHOT | IRQF_NO_AUTOEN);
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
>  
> @@ -255,10 +255,43 @@ EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq);
>   */
>  int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq)
>  {
> -	return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE);
> +	return __dev_pm_set_dedicated_wake_irq(dev, irq, WAKE_IRQ_DEDICATED_REVERSE,
> +					       IRQF_ONESHOT | IRQF_NO_AUTOEN);
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_reverse);
>  
> +/**
> + * dev_pm_set_dedicated_wake_irq_flags - Request a dedicated wake-up interrupt
> + *                                       with custom flags
> + * @dev: Device entry
> + * @irq: Device wake-up interrupt
> + * @flags: IRQ flags (e.g., IRQF_SHARED)
> + *
> + * This API sets up a threaded interrupt handler for a device that has
> + * a dedicated wake-up interrupt in addition to the device IO interrupt,
> + * allowing the caller to specify custom IRQ flags such as IRQF_SHARED.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int dev_pm_set_dedicated_wake_irq_flags(struct device *dev, int irq, unsigned long flags)
> +{
> +	struct wake_irq *wirq;
> +	int ret;
> +
> +	flags |= IRQF_ONESHOT;
> +	if (!(flags & IRQF_SHARED))
> +		flags |= IRQF_NO_AUTOEN;
> +
> +	ret =  __dev_pm_set_dedicated_wake_irq(dev, irq, 0, flags);
> +	if (!ret && (flags & IRQF_SHARED)) {
> +		wirq = dev->power.wakeirq;
> +		disable_irq_nosync(wirq->irq);
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_set_dedicated_wake_irq_flags);
> +
>  /**
>   * dev_pm_enable_wake_irq_check - Checks and enables wake-up interrupt
>   * @dev: Device
> diff --git a/include/linux/pm_wakeirq.h b/include/linux/pm_wakeirq.h
> index 25b63ed51b765c2c6919f259668a12675330835e..14950616efe34f4fa5408ca54cd8eeb1f7f0ff13 100644
> --- a/include/linux/pm_wakeirq.h
> +++ b/include/linux/pm_wakeirq.h
> @@ -11,6 +11,7 @@ extern int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq);
>  extern int dev_pm_set_dedicated_wake_irq_reverse(struct device *dev, int irq);
>  extern void dev_pm_clear_wake_irq(struct device *dev);
>  extern int devm_pm_set_wake_irq(struct device *dev, int irq);
> +extern int dev_pm_set_dedicated_wake_irq_flags(struct device *dev, int irq, unsigned long flags);
>  
>  #else	/* !CONFIG_PM */
>  
> @@ -38,5 +39,10 @@ static inline int devm_pm_set_wake_irq(struct device *dev, int irq)
>  	return 0;
>  }
>  
> +static inline int dev_pm_set_dedicated_wake_irq_flags(struct device *dev,
> +						      int irq, unsigned long flags)
> +{
> +	return 0;
> +}
>  #endif	/* CONFIG_PM */
>  #endif	/* _LINUX_PM_WAKEIRQ_H */
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 2/3] PM: sleep: wakeirq: Add support for custom IRQ flags in dedicated wake IRQ setup

[Rafael J. Wysocki]

On Fri, Aug 1, 2025 at 11:31 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Aug 01, 2025 at 04:29:43PM +0530, Krishna Chaitanya Chundru wrote:
> > Some devices require more flexibility when configuring their dedicated
> > wake-up interrupts, such as support for IRQF_SHARED or other IRQ flags.
>
> I guess the "dedicated" in dev_pm_set_dedicated_wake_irq() means "an
> interrupt that only signals wakeup requests, possibly used by several
> devices," not "an interrupt only used by this device"?

Yes, it is supposed to mean "dedicated for signaling wakeup".

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

[Manivannan Sadhasivam]

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

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

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

[Manivannan Sadhasivam]

On Fri, Aug 01, 2025 at 04:29:41PM GMT, Krishna Chaitanya Chundru wrote:
> PCIe WAKE# interrupt is needed for bringing back PCIe device state from
> D3cold to D0.
> 
> This is pending from long time, there was two attempts done previously to
> add WAKE# support[1], [2]. Those series tried to add support for legacy
> interrupts along with WAKE#. Legacy interrupts are already available in
> the latest kernel and we can ignore them. For the wake IRQ the series is
> trying to use interrupts property define in the device tree.
> 
> This series is using gpio property instead of interrupts, from
> gpio desc driver will allocate the dedicate IRQ.
> 
> 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# GPIO 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.
> 

I think you should stop saying 'endpoint' here and switch to 'slot' as that's
the terminology the PCIe spec uses while defining WAKE#.

> 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.
> 

I don't think we should walk the hierarchy all the way up to RP. If the slot
supports WAKE#, it should be defined in the immediate bridge node of the
endpoint (as DT uses bridge node to described the slot). Otherwise, if the slot
doesn't use WAKE#, walking up till RP may falsely assign wake IRQ to the
endpoint.

- Mani

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

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

[Bjorn Helgaas]

On Mon, Aug 04, 2025 at 03:45:05PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Aug 01, 2025 at 04:29:41PM GMT, Krishna Chaitanya Chundru wrote:
> > PCIe WAKE# interrupt is needed for bringing back PCIe device state
> > from D3cold to D0.
> > 
> > This is pending from long time, there was two attempts done
> > previously to add WAKE# support[1], [2]. Those series tried to add
> > support for legacy interrupts along with WAKE#. Legacy interrupts
> > are already available in the latest kernel and we can ignore them.
> > For the wake IRQ the series is trying to use interrupts property
> > define in the device tree.
> > 
> > This series is using gpio property instead of interrupts, from
> > gpio desc driver will allocate the dedicate IRQ.
> > 
> > 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# GPIO 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.
> 
> I think you should stop saying 'endpoint' here and switch to 'slot'
> as that's the terminology the PCIe spec uses while defining WAKE#.

I think the main question is where WAKE# is terminated.  It's asserted
by an "add-in card" (PCIe CEM r6.0, sec 2.3) or a "component" or
"Function" (PCIe Base r7.0, sec 5.3.3.2).  A slot can provide a WAKE#
wire, and we need to know what the other end is connected to.

AFAICS, WAKE# routing is unrelated to the PCIe topology *except* that
in "applications where Beacon is used on some Ports of the Switch and
WAKE# is used for other Ports," WAKE# must be connected to the Switch
so it can translate it to Beacon (PCIe r7.0, sec 5.3.3.2).

So we can't assume WAKE# is connected to the Port leading to the
component that asserts WAKE#.

> > 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.
> 
> I don't think we should walk the hierarchy all the way up to RP. If
> the slot supports WAKE#, it should be defined in the immediate
> bridge node of the endpoint (as DT uses bridge node to described the
> slot). Otherwise, if the slot doesn't use WAKE#, walking up till RP
> may falsely assign wake IRQ to the endpoint.

I don't think we can walk the PCIe hierarchy because in general WAKE#
routing is not related to that hierarchy.

Bjorn

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add wake GPIO

[Bjorn Andersson]

On Fri, Aug 01, 2025 at 04:29:42PM +0530, Krishna Chaitanya Chundru wrote:
> Add WAKE# gpio which is needed to bring PCIe device state
> from D3cold to D0.
> 

What tree did you base this on? None of these boards has pcieport1
defined in the upstream kernel.

Regards,
Bjorn

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts   | 1 +
>  arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 10c152ac03c874df5f1dc386d9079d3db1c55362..a4d85772f86955ad061433b138581fa9d81110a4 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -810,6 +810,7 @@ &mdss_edp_phy {
>  
>  &pcieport1 {
>  	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>  };
>  
>  &pcie1 {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index 60b3cf50ea1d61dd5e8b573b5f1c6faa1c291eee..5e73060771329cade097bf1a71056a456a7937d7 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -477,6 +477,7 @@ &pcie1 {
>  
>  &pcieport1 {
>  	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>  };
>  
>  &pm8350c_pwm {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 0b0212b670797a364d7f0e7a458fc73245fff8db..240513774612fb2bfcdb951e5a5a77c49f49eb82 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -418,6 +418,7 @@ &lpass_va_macro {
>  
>  &pcieport1 {
>  	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>  };
>  
>  &pcie1 {
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add wake GPIO

[Krishna Chaitanya Chundru]

On 8/11/2025 10:06 PM, Bjorn Andersson wrote:
> On Fri, Aug 01, 2025 at 04:29:42PM +0530, Krishna Chaitanya Chundru wrote:
>> Add WAKE# gpio which is needed to bring PCIe device state
>> from D3cold to D0.
>>
> 
> What tree did you base this on? None of these boards has pcieport1
> defined in the upstream kernel.
> 
Sorry I forgot to add dependencies to dependencies to one more series.
I will add the dependencies in the next series.

- Krishna Chaitanya.
> Regards,
> Bjorn
> 
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts   | 1 +
>>   arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 1 +
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 1 +
>>   3 files changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> index 10c152ac03c874df5f1dc386d9079d3db1c55362..a4d85772f86955ad061433b138581fa9d81110a4 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
>> @@ -810,6 +810,7 @@ &mdss_edp_phy {
>>   
>>   &pcieport1 {
>>   	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>>   };
>>   
>>   &pcie1 {
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> index 60b3cf50ea1d61dd5e8b573b5f1c6faa1c291eee..5e73060771329cade097bf1a71056a456a7937d7 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> @@ -477,6 +477,7 @@ &pcie1 {
>>   
>>   &pcieport1 {
>>   	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>>   };
>>   
>>   &pm8350c_pwm {
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 0b0212b670797a364d7f0e7a458fc73245fff8db..240513774612fb2bfcdb951e5a5a77c49f49eb82 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -418,6 +418,7 @@ &lpass_va_macro {
>>   
>>   &pcieport1 {
>>   	reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
>>   };
>>   
>>   &pcie1 {
>>
>> -- 
>> 2.34.1
>>

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

[Manivannan Sadhasivam]

On Mon, Aug 04, 2025 at 10:50:23AM GMT, Bjorn Helgaas wrote:
> On Mon, Aug 04, 2025 at 03:45:05PM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Aug 01, 2025 at 04:29:41PM GMT, Krishna Chaitanya Chundru wrote:
> > > PCIe WAKE# interrupt is needed for bringing back PCIe device state
> > > from D3cold to D0.
> > > 
> > > This is pending from long time, there was two attempts done
> > > previously to add WAKE# support[1], [2]. Those series tried to add
> > > support for legacy interrupts along with WAKE#. Legacy interrupts
> > > are already available in the latest kernel and we can ignore them.
> > > For the wake IRQ the series is trying to use interrupts property
> > > define in the device tree.
> > > 
> > > This series is using gpio property instead of interrupts, from
> > > gpio desc driver will allocate the dedicate IRQ.
> > > 
> > > 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# GPIO 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.
> > 
> > I think you should stop saying 'endpoint' here and switch to 'slot'
> > as that's the terminology the PCIe spec uses while defining WAKE#.
> 
> I think the main question is where WAKE# is terminated.  It's asserted
> by an "add-in card" (PCIe CEM r6.0, sec 2.3) or a "component" or
> "Function" (PCIe Base r7.0, sec 5.3.3.2).  A slot can provide a WAKE#
> wire, and we need to know what the other end is connected to.
> 
> AFAICS, WAKE# routing is unrelated to the PCIe topology *except* that
> in "applications where Beacon is used on some Ports of the Switch and
> WAKE# is used for other Ports," WAKE# must be connected to the Switch
> so it can translate it to Beacon (PCIe r7.0, sec 5.3.3.2).
> 
> So we can't assume WAKE# is connected to the Port leading to the
> component that asserts WAKE#.
> 

I've submitted a PR to add wake-gpios to the endpoint node:
https://github.com/devicetree-org/dt-schema/pull/170

- Mani

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