| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7b91b725-6b47-bf8f-e6e5-e4584f274ec4@oss.qualcomm.com>
Date: Mon, 9 Jun 2025 11:27:49 +0530
From: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
cros-qcom-dts-watchers@...omium.org,
Bjorn Helgaas <bhelgaas@...gle.com>, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, quic_vbadigan@...cinc.com,
quic_mrana@...cinc.com, Sherry Sun <sherry.sun@....com>
Subject: Re: [PATCH v3 2/2] PCI/portdrv: Add support for PCIe wake interrupt
On 6/6/2025 1:56 AM, Bjorn Helgaas wrote:
> On Thu, Jun 05, 2025 at 10:54:45AM +0530, Krishna Chaitanya Chundru wrote:
>> PCIe wake interrupt is needed for bringing back PCIe device state
>> from D3cold to D0.
>
> Does this refer to the WAKE# signal or Beacon or both? I guess the
> comments in the patch suggest WAKE#. Is there any spec section we can
> cite here?
>
we are referring only WAKE# signal, I will add the PCIe spec r6.0, sec
5.3.3.2 in next patch version.
>> Implement new functions, of_pci_setup_wake_irq() and
>> of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices
>> using the Device Tree.
>>
>> From the port bus driver call these functions to enable wake support
>> for bridges.
>
> What is the connection to bridges and portdrv? WAKE# is described in
> PCIe r6.0, sec 5.3.3.2, and PCIe CEM r6.0, sec 2.3, but AFAICS neither
> restricts it to bridges.
>
The wake# is used by the endpoints to wake host to bring PCIe device
state to D0 again, in direct attach root port wake# will be connected
to the root port and in switch cases the wake# will connected to the
switch Downstream ports and switch will consolidate wake# from different
downstream ports and sends to the root port. The wake# will be used by
root port bridges only. portdrv is the driver for root port.
> Unless there's some related functionality in a Root Port, RCEC, or
> Switch Port, maybe this code should be elsewhere, like
> set_pcie_port_type(), so we could set this up for any PCIe device that
> has a WAKE# description?
>
As this is only used by root port, I am ok to change it to there to
enable this only in case of rootport.
But we need to make some changes in the flow as of node is not assigned
yet pci dev and also if wake registration fails we can't stop the driver
from probing as driver is not yet in the picture yet.
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
>> Tested-by: Sherry Sun <sherry.sun@....com>
>> ---
>> drivers/pci/of.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/pci/pci.h | 6 +++++
>> drivers/pci/pcie/portdrv.c | 12 ++++++++-
>> 3 files changed, 84 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index ab7a8252bf4137a17971c3eb8ab70ce78ca70969..3487cd62d81f0a66e7408e286475e8d91c2e410a 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
>> @@ -966,3 +968,68 @@ u32 of_pci_get_slot_power_limit(struct device_node *node,
>> return slot_power_limit_mw;
>> }
>> EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
>> +
>> +/**
>> + * of_pci_slot_setup_wake_irq - Set up wake interrupt for PCI device
>> + * @pdev: The PCI device structure
>> + *
>> + * This function sets up the wake interrupt for a PCI device by getting the
>> + * corresponding WAKE# gpio from the device tree, and configuring it as a
>> + * dedicated wake interrupt.
>> + *
>> + * Return: 0 if the WAKE# gpio is not available or successfully parsed else
>> + * errno otherwise.
>> + */
>> +int of_pci_slot_setup_wake_irq(struct pci_dev *pdev)
>> +{
>> + struct gpio_desc *wake;
>> + struct device_node *dn;
>> + int ret, wake_irq;
>> +
>> + dn = pci_device_to_OF_node(pdev);
>> + if (!dn)
>> + return 0;
>> +
>> + wake = devm_fwnode_gpiod_get(&pdev->dev, of_fwnode_handle(dn),
>> + "wake", GPIOD_IN, NULL);
>
> I guess this finds "wake-gpio" or "wake-gpios", as used in
> Documentation/devicetree/bindings/pci/qcom,pcie.yaml,
> qcom,pcie-sa8775p.yaml, etc? Are these names specified in any generic
> place, e.g.,
> https://github.com/devicetree-org/dt-schema/tree/main/dtschema/schemas/pci?
>
I created a patch to add them in common schemas:
https://lore.kernel.org/all/20250515090517.3506772-1-krishna.chundru@oss.qualcomm.com/
- Krishna Chaitanya.
>> + if (IS_ERR(wake) && PTR_ERR(wake) != -ENOENT) {
>> + dev_err(&pdev->dev, "Failed to get wake GPIO: %ld\n", PTR_ERR(wake));
>> + return PTR_ERR(wake);
>> + }
>> + if (IS_ERR(wake))
>> + return 0;
>> +
>> + wake_irq = gpiod_to_irq(wake);
>> + if (wake_irq < 0) {
>> + dev_err(&pdev->dev, "Dailed to get wake irq: %d\n", wake_irq);
>
> s/Dailed/Failed/
>
>> + return wake_irq;
>> + }
>> +
>> + device_init_wakeup(&pdev->dev, true);
>> +
>> + ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, wake_irq);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Failed to set wake IRQ: %d\n", ret);
>> + device_init_wakeup(&pdev->dev, false);
>> + return ret;
>> + }
>> + irq_set_irq_type(wake_irq, IRQ_TYPE_EDGE_FALLING);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(of_pci_slot_setup_wake_irq);
>> +
>> +/**
>> + * of_pci_slot_teardown_wake_irq - Teardown wake interrupt setup for PCI device
>> + *
>> + * @pdev: The PCI device structure
>> + *
>> + * This function tears down the wake interrupt setup for a PCI device,
>> + * clearing the dedicated wake interrupt and disabling device wake-up.
>> + */
>> +void of_pci_slot_teardown_wake_irq(struct pci_dev *pdev)
>> +{
>> + dev_pm_clear_wake_irq(&pdev->dev);
>> + device_init_wakeup(&pdev->dev, false);
>> +}
>> +EXPORT_SYMBOL_GPL(of_pci_slot_teardown_wake_irq);
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 39f368d2f26de872f6484c6cb4e12752abfff7bc..dd7a4da1225bbdb1dff82707b580e7e0a95a5abf 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -888,6 +888,9 @@ void pci_release_of_node(struct pci_dev *dev);
>> void pci_set_bus_of_node(struct pci_bus *bus);
>> void pci_release_bus_of_node(struct pci_bus *bus);
>>
>> +int of_pci_slot_setup_wake_irq(struct pci_dev *pdev);
>> +void of_pci_slot_teardown_wake_irq(struct pci_dev *pdev);
>> +
>> int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
>> bool of_pci_supply_present(struct device_node *np);
>>
>> @@ -931,6 +934,9 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
>> return 0;
>> }
>>
>> +static int of_pci_slot_setup_wake_irq(struct pci_dev *pdev) { return 0; }
>> +static void of_pci_slot_teardown_wake_irq(struct pci_dev *pdev) { }
>> +
>> static inline bool of_pci_supply_present(struct device_node *np)
>> {
>> return false;
>> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
>> index e8318fd5f6ed537a1b236a3a0f054161d5710abd..9a6beec87e4523a33ecace684109cd44e025c97b 100644
>> --- a/drivers/pci/pcie/portdrv.c
>> +++ b/drivers/pci/pcie/portdrv.c
>> @@ -694,12 +694,18 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
>> (type != PCI_EXP_TYPE_RC_EC)))
>> return -ENODEV;
>>
>> + status = of_pci_slot_setup_wake_irq(dev);
>> + if (status)
>> + return status;
>> +
>> if (type == PCI_EXP_TYPE_RC_EC)
>> pcie_link_rcec(dev);
>>
>> status = pcie_port_device_register(dev);
>> - if (status)
>> + if (status) {
>> + of_pci_slot_teardown_wake_irq(dev);
>> return status;
>> + }
>>
>> pci_save_state(dev);
>>
>> @@ -732,6 +738,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev)
>>
>> pcie_port_device_remove(dev);
>>
>> + of_pci_slot_teardown_wake_irq(dev);
>> +
>> pci_disable_device(dev);
>> }
>>
>> @@ -744,6 +752,8 @@ static void pcie_portdrv_shutdown(struct pci_dev *dev)
>> }
>>
>> pcie_port_device_remove(dev);
>> +
>> + of_pci_slot_teardown_wake_irq(dev);
>> }
>>
>> static pci_ers_result_t pcie_portdrv_error_detected(struct pci_dev *dev,
>>
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists