mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>, Pavel Machek <pavel@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Danilo Krummrich <dakr@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Linus Walleij <linusw@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, linux-gpio@vger.kernel.org,
	quic_vbadigan@quicinc.com, quic_mrana@quicinc.com,
	sherry.sun@nxp.com, driver-core@lists.linux.dev
Subject: Re: [PATCH v7 3/3] PCI: Add support for PCIe WAKE# interrupt
Date: Wed, 25 Feb 2026 18:02:52 +0530	[thread overview]
Message-ID: <8b6e1772-6659-4c80-a3fd-ed624218d7be@oss.qualcomm.com> (raw)
In-Reply-To: <pjeee3vwggsrvp4kxarc46vjx2igk53xd6eoxjqh7ri5rf6fhi@5d3ccirqnxu6>



On 2/23/2026 7:37 PM, Manivannan Sadhasivam wrote:
> On Wed, Feb 18, 2026 at 01:42:25PM +0530, Krishna Chaitanya Chundru wrote:
>> According to the PCI Express specification (PCIe r7.0, Section 5.3.3.2),
>> two link wakeup mechanisms are defined: Beacon and WAKE#. Beacon is a
>> hardware-only mechanism and is invisible to software (PCIe r7.0,
>> Section 4.2.7.8.1). This change adds support for the WAKE# mechanism in
>> the PCI core.
>>
>> According to the PCIe specification, multiple WAKE# signals can exist in
>> a system or each component in the hierarchy could share a single WAKE#
>> signal. 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. From figure 5.4 in sec
>> 5.3.3.2, WAKE# can also be terminated at the switch itself. To support
>> this, the WAKE# should be described in the device tree node of the
>> endpoint/bridge. If all endpoints share a single WAKE# line, then each
>> endpoint node should describe the same WAKE# signal or a single WAKE# in
>> the Root Port node.
>>
>> In pci_device_add(), PCI framework will search for the WAKE# in device
>> node, If not found, it searches in its upstream port only if upstream port
>> is Root Port. Once found, register for the wake IRQ in shared mode, as the
>> WAKE# may be shared among multiple endpoints.
>>
>> When a device asserts WAKE#, PM core will wakeup the system, resume the
>> device and its parent(s) in the hierarchy, which will cause the restoration
>> of power and refclk to the device.
>>
>> WAKE# is added in dts schema and merged based on below links.
>>
>> Link: https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/
>> Link: https://github.com/devicetree-org/dt-schema/pull/170
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/of.c     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/pci/pci.c    |  9 +++++++++
>>   drivers/pci/pci.h    |  8 ++++++++
>>   drivers/pci/probe.c  |  2 ++
>>   drivers/pci/remove.c |  1 +
>>   include/linux/pci.h  |  2 ++
>>   6 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 9bb5f258759be3f1e23496f083353600a4ef6743..23248900253faafaf9509d87c531b454fca41798 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,59 @@ 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);
>> +
>> +static void pci_configure_wake_irq(struct pci_dev *pdev, struct gpio_desc *wake)
>> +{
>> +	int ret, wake_irq;
>> +
>> +	wake_irq = gpiod_to_irq(wake);
>> +	if (wake_irq < 0) {
>> +		pci_err(pdev, "Failed to get wake irq: %d\n", wake_irq);
>> +		return;
>> +	}
>> +
>> +	device_init_wakeup(&pdev->dev, true);
>> +
>> +	ret = dev_pm_set_dedicated_shared_wake_irq(&pdev->dev, wake_irq,
>> +						   IRQ_TYPE_EDGE_FALLING);
>> +	if (ret < 0) {
>> +		pci_err(pdev, "Failed to set wake IRQ: %d\n", ret);
>> +		device_init_wakeup(&pdev->dev, false);
>> +	}
>> +}
>> +
>> +void pci_configure_of_wake_gpio(struct pci_dev *dev)
>> +{
>> +	struct device_node *dn = pci_device_to_OF_node(dev);
>> +	struct pci_dev *upstream;
>> +	struct gpio_desc *gpio;
>> +
>> +	if (!dn)
>> +		return;
>> +
>> +	gpio = fwnode_gpiod_get(of_fwnode_handle(dn), "wake",
>> +				GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE, NULL);
>> +	if (IS_ERR(gpio)) {
>> +		/*
>> +		 * In case the entire topology shares a single WAKE# signal, look for it
>> +		 * in the upstream bridge node. But if it is not Root Port, then skip it.
>> +		 */
> This comment need to be a bit more verbose. Something like,
>
> "The devices in a hierarchy expose wakeup capability through the 'wake-gpios'
> property defined either in the device node or in the Slot node. So first check
> for the property in device node and if not available, check in the Slot node."
>
> Also, move it above fwnode_gpiod_get().
ack.
>> +		upstream = pci_upstream_bridge(dev);
>> +		if (upstream && pci_is_root_bus(upstream->bus) && upstream->wake)
> Check directly in the Root Port (Slot) node.
>
>> +			pci_configure_wake_irq(dev, upstream->wake);
> I don't think we need to request an IRQ in the fallback case. Let's assume that
> there is a single device in hierarchy and the wake-gpios property is defined in
> the Slot node. So here, we should just let the PCI/PM core know that this device
> supports wakeup and PME_EN needs to be set (if the device is also capable of
> generating PME#).
>
> And then we should call pci_configure_wake_irq() from platform_pci_set_wakeup()
> to setup the IRQ handler only for the device which has the wake-gpios property.
>
> So the flow would be:
>
> pci_device_add()->platform_pci_init_wakeup()->of_pci_init_wakeup():
> Request fwnode_gpiod_get() and call device_init_wakeup()
> and set 'dev->wake' if WAKE# is available in the device node. If not, then check
> the Root Port (Slot) node and if WAKE# is available, then just call
> device_init_wakeup() to indicate the PCI and PM cores that this device *may*
> support wakeup.
>
> pci_enable_wake()->platform_pci_set_wakeup()->of_pci_set_wakeup():
> Request WAKE# IRQ only if the device has the wake-gpios property ie., dev->wake
> set. If only the Slot supports wakeup, then the IRQ should be requested for the
> Root Port device. This will allow us to keep both the ACPI and OF platform code
> in sync to some extent.
>
> Then once the WAKE# is asserted, the platform will wakeup with the help of the
> above wakeup event and the host controller driver will apply power to the
> hierarchy.
>
> But you do not seem to call pci_pme_wakeup_bus() after resume...
when we call dev_pm_set_dedicated_shared_wake_irq(), this will request 
an IRQ and by default it will be disabled. The IRQ will be enabled only 
in case of system suspend & after runtime suspend and only if wakeup is 
enabled for the device. I will add this info as a comment, so that it 
will be more clear and also in the commit text. The IRQ handler which 
will be triggered as part of WAKE# IRQ will trigger pm_runtime_resume() 
as per[1]. This will resume the devcie from D3cold to D0. once the 
device is in D0, the endpoint which requests the wake will send PME 
event and that is handled by PCI core already either through polling in 
pci core [2] or pme service in porbus driver[3] [1] wakeirq.c - 
drivers/base/power/wakeirq.c - Linux source code v6.19-rc5 - Bootlin 
Elixir Cross Referencer 
<https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/base/power/wakeirq.c#L163>
[2] pci.c - drivers/pci/pci.c - Linux source code v6.19-rc5 - Bootlin 
Elixir Cross Referencer 
<https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/pci/pci.c#L2378>
[3] pme.c - drivers/pci/pcie/pme.c - Linux source code v6.19-rc5 - 
Bootlin Elixir Cross Referencer 
<https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/pci/pcie/pme.c#L82>

- Krishna Chaitanya.
>> +	} else {
>> +		dev->wake = gpio;
>> +		pci_configure_wake_irq(dev, gpio);
>> +	}
>> +}
>> +
>> +void pci_remove_of_wake_gpio(struct pci_dev *dev)
>> +{
> Missing of_node check.
>
>> +	dev_pm_clear_wake_irq(&dev->dev);
>> +	device_init_wakeup(&dev->dev, false);
>> +	gpiod_put(dev->wake);
>> +	dev->wake = NULL;
>> +}
>>   #endif	/* CONFIG_OF_IRQ */
>>   
>>   static int pci_parse_request_of_pci_ranges(struct device *dev,
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index f3244630bfd05b15d52f866d80a015ed21f98f49..225cb861b3425700fc0d9d4805f5d9efcaab6f56 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -1123,6 +1123,15 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
>>   	return acpi_pci_bridge_d3(dev);
>>   }
>>   
>> +void platform_pci_configure_wake(struct pci_dev *dev)
>> +{
>> +	return pci_configure_of_wake_gpio(dev);
>> +}
>> +
>> +void platform_pci_remove_wake(struct pci_dev *dev)
>> +{
>> +	return pci_remove_of_wake_gpio(dev);
>> +}
> As mentioned above, these should go away and be part of
> platform_pci_set_wakeup().
>
>>   /**
>>    * pci_update_current_state - Read power state of given device and cache it
>>    * @dev: PCI device to handle.
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 13d998fbacce6698514d92500dfea03cc562cdc2..22709573e41caf0ed45b20ee7ded5963f55aa9fe 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -282,6 +282,8 @@ void pci_msix_init(struct pci_dev *dev);
>>   bool pci_bridge_d3_possible(struct pci_dev *dev);
>>   void pci_bridge_d3_update(struct pci_dev *dev);
>>   int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
>> +void platform_pci_configure_wake(struct pci_dev *dev);
>> +void platform_pci_remove_wake(struct pci_dev *dev);
>>   
>>   static inline bool pci_bus_rrs_vendor_id(u32 l)
>>   {
>> @@ -1195,6 +1197,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);
>>   
>> +void pci_configure_of_wake_gpio(struct pci_dev *dev);
>> +void pci_remove_of_wake_gpio(struct pci_dev *dev);
>> +
>>   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,
>> @@ -1240,6 +1245,9 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
>>   	return 0;
>>   }
>>   
>> +static inline void pci_configure_of_wake_gpio(struct pci_dev *dev) { }
>> +static inline void pci_remove_of_wake_gpio(struct pci_dev *dev) { }
>> +
> You've added function definitions under CONFIG_OF_IRQ, but stubs under
> CONFIG_OF. This is why kernel robot also barks at you.
>
> Move the stubs to include/linux/of_pci.h under CONFIG_OF_IRQ guard.
>
> - Mani
>
>>   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 2975974f35e88b5025701d2b721df8386419de8d..7f5132c0c8de36a6ec2775468a3d4e5156a046d0 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2771,6 +2771,8 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>>   	/* Establish pdev->tsm for newly added (e.g. new SR-IOV VFs) */
>>   	pci_tsm_init(dev);
>>   
>> +	platform_pci_configure_wake(dev);
>> +
>>   	pci_npem_create(dev);
>>   
>>   	pci_doe_sysfs_init(dev);
>> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
>> index e9d519993853f92f1810d3eff9f44ca7e3e1abd9..d781b41e57c4444077075690cec926a9fe15334f 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -35,6 +35,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>>   	if (pci_dev_test_and_set_removed(dev))
>>   		return;
>>   
>> +	platform_pci_remove_wake(dev);
>>   	pci_doe_sysfs_teardown(dev);
>>   	pci_npem_remove(dev);
>>   
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..d1e08df8a8deaa87780589f23242767fdcdba541 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -586,6 +586,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
>>
>>


  reply	other threads:[~2026-02-25 12:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-18  8:12 [PATCH v7 0/3] " Krishna Chaitanya Chundru
2026-02-18  8:12 ` [PATCH v7 1/3] PM: sleep: wakeirq: Add support for dedicated shared wake IRQ setup Krishna Chaitanya Chundru
2026-02-18  8:12 ` [PATCH v7 2/3] gpio: Add fwnode_gpiod_get() helper Krishna Chaitanya Chundru
2026-02-19 17:45   ` Linus Walleij
2026-02-23 12:53   ` Manivannan Sadhasivam
2026-02-18  8:12 ` [PATCH v7 3/3] PCI: Add support for PCIe WAKE# interrupt Krishna Chaitanya Chundru
2026-02-18 13:49   ` kernel test robot
2026-02-18 16:58   ` kernel test robot
2026-02-23 14:07   ` Manivannan Sadhasivam
2026-02-25 12:32     ` Krishna Chaitanya Chundru [this message]
2026-02-18 10:01 ` [PATCH v7 0/3] " Bartosz Golaszewski
2026-02-24  8:13   ` Chen-Yu Tsai
2026-03-13  7:03   ` Krishna Chaitanya Chundru

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b6e1772-6659-4c80-a3fd-ed624218d7be@oss.qualcomm.com \
    --to=krishna.chundru@oss.qualcomm.com \
    --cc=bhelgaas@google.com \
    --cc=brgl@bgdev.pl \
    --cc=brgl@kernel.org \
    --cc=dakr@kernel.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=lenb@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=pavel@kernel.org \
    --cc=quic_mrana@quicinc.com \
    --cc=quic_vbadigan@quicinc.com \
    --cc=rafael@kernel.org \
    --cc=sherry.sun@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®