From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Devicetree schema allows the PERST# GPIO to be present in all PCIe bridge
nodes, not just in Root Port node. But the current logic parses PERST# only
from the Root Port nodes. Though it is not causing any issue on the current
platforms, the upcoming platforms will have PERST# in PCIe switch
downstream ports also. So this requires parsing all the PCIe bridge nodes
for the PERST# GPIO.
Hence, rework the parsing logic to extend to all PCIe bridge nodes starting
from the Root Port node. If the 'reset-gpios' property is found for a PCI
bridge node, the GPIO descriptor will be stored in qcom_pcie_perst::desc
and added to the qcom_pcie_port::perst list.
It should be noted that if more than one bridge node has the same GPIO for
PERST# (shared PERST#), the driver will error out. This is due to the
limitation in the GPIOLIB subsystem that allows only exclusive (non-shared)
access to GPIOs from consumers. But this is soon going to get fixed. Once
that happens, it will get incorporated in this driver.
So for now, PERST# sharing is not supported.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 102 +++++++++++++++++++++++++++------
1 file changed, 85 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6170c86f465f43f980f5b2f88bd8799c3c152e68..ccff01c31898cdbc5634221e7f8ef7e86469f5fd 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -267,10 +267,15 @@ struct qcom_pcie_cfg {
bool no_l0s;
};
+struct qcom_pcie_perst {
+ struct list_head list;
+ struct gpio_desc *desc;
+};
+
struct qcom_pcie_port {
struct list_head list;
- struct gpio_desc *reset;
struct phy *phy;
+ struct list_head perst;
};
struct qcom_pcie {
@@ -292,11 +297,14 @@ struct qcom_pcie {
static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
{
+ struct qcom_pcie_perst *perst;
struct qcom_pcie_port *port;
int val = assert ? 1 : 0;
- list_for_each_entry(port, &pcie->ports, list)
- gpiod_set_value_cansleep(port->reset, val);
+ list_for_each_entry(port, &pcie->ports, list) {
+ list_for_each_entry(perst, &port->perst, list)
+ gpiod_set_value_cansleep(perst->desc, val);
+ }
usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
}
@@ -1670,18 +1678,58 @@ static const struct pci_ecam_ops pci_qcom_ecam_ops = {
}
};
-static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node)
+/* Parse PERST# from all nodes in depth first manner starting from @np */
+static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
+ struct qcom_pcie_port *port,
+ struct device_node *np)
{
struct device *dev = pcie->pci->dev;
- struct qcom_pcie_port *port;
+ struct qcom_pcie_perst *perst;
struct gpio_desc *reset;
- struct phy *phy;
int ret;
- reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(node),
- "reset", GPIOD_OUT_HIGH, "PERST#");
- if (IS_ERR(reset))
+ if (!of_find_property(np, "reset-gpios", NULL))
+ goto parse_child_node;
+
+ reset = devm_fwnode_gpiod_get(dev, of_fwnode_handle(np), "reset",
+ GPIOD_OUT_HIGH, "PERST#");
+ if (IS_ERR(reset)) {
+ /*
+ * FIXME: GPIOLIB currently supports exclusive GPIO access only.
+ * Non exclusive access is broken. But shared PERST# requires
+ * non-exclusive access. So once GPIOLIB properly supports it,
+ * implement it here.
+ */
+ if (PTR_ERR(reset) == -EBUSY)
+ dev_err(dev, "Shared PERST# is not supported\n");
+
return PTR_ERR(reset);
+ }
+
+ perst = devm_kzalloc(dev, sizeof(*perst), GFP_KERNEL);
+ if (!perst)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&perst->list);
+ perst->desc = reset;
+ list_add_tail(&perst->list, &port->perst);
+
+parse_child_node:
+ for_each_available_child_of_node_scoped(np, child) {
+ ret = qcom_pcie_parse_perst(pcie, port, child);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node)
+{
+ struct device *dev = pcie->pci->dev;
+ struct qcom_pcie_port *port;
+ struct phy *phy;
+ int ret;
phy = devm_of_phy_get(dev, node, NULL);
if (IS_ERR(phy))
@@ -1695,7 +1743,12 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
if (ret)
return ret;
- port->reset = reset;
+ INIT_LIST_HEAD(&port->perst);
+
+ ret = qcom_pcie_parse_perst(pcie, port, node);
+ if (ret)
+ return ret;
+
port->phy = phy;
INIT_LIST_HEAD(&port->list);
list_add_tail(&port->list, &pcie->ports);
@@ -1705,9 +1758,10 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
{
+ struct qcom_pcie_perst *perst, *tmp_perst;
+ struct qcom_pcie_port *port, *tmp_port;
struct device *dev = pcie->pci->dev;
- struct qcom_pcie_port *port, *tmp;
- int ret = -ENOENT;
+ int ret = -ENODEV;
for_each_available_child_of_node_scoped(dev->of_node, of_port) {
ret = qcom_pcie_parse_port(pcie, of_port);
@@ -1718,7 +1772,9 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
return ret;
err_port_del:
- list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+ list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
+ list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)
+ list_del(&perst->list);
phy_exit(port->phy);
list_del(&port->list);
}
@@ -1729,6 +1785,7 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
static int qcom_pcie_parse_legacy_binding(struct qcom_pcie *pcie)
{
struct device *dev = pcie->pci->dev;
+ struct qcom_pcie_perst *perst;
struct qcom_pcie_port *port;
struct gpio_desc *reset;
struct phy *phy;
@@ -1750,19 +1807,28 @@ static int qcom_pcie_parse_legacy_binding(struct qcom_pcie *pcie)
if (!port)
return -ENOMEM;
- port->reset = reset;
+ perst = devm_kzalloc(dev, sizeof(*perst), GFP_KERNEL);
+ if (!perst)
+ return -ENOMEM;
+
port->phy = phy;
INIT_LIST_HEAD(&port->list);
list_add_tail(&port->list, &pcie->ports);
+ perst->desc = reset;
+ INIT_LIST_HEAD(&port->perst);
+ INIT_LIST_HEAD(&perst->list);
+ list_add_tail(&perst->list, &port->perst);
+
return 0;
}
static int qcom_pcie_probe(struct platform_device *pdev)
{
+ struct qcom_pcie_perst *perst, *tmp_perst;
+ struct qcom_pcie_port *port, *tmp_port;
const struct qcom_pcie_cfg *pcie_cfg;
unsigned long max_freq = ULONG_MAX;
- struct qcom_pcie_port *port, *tmp;
struct device *dev = &pdev->dev;
struct dev_pm_opp *opp;
struct qcom_pcie *pcie;
@@ -1909,7 +1975,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
ret = qcom_pcie_parse_ports(pcie);
if (ret) {
- if (ret != -ENOENT) {
+ if (ret != -ENODEV) {
dev_err_probe(pci->dev, ret,
"Failed to parse Root Port: %d\n", ret);
goto err_pm_runtime_put;
@@ -1968,7 +2034,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
err_host_deinit:
dw_pcie_host_deinit(pp);
err_phy_exit:
- list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+ list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
+ list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)
+ list_del(&perst->list);
phy_exit(port->phy);
list_del(&port->list);
}
--
2.45.2On Fri, Sep 12, 2025 at 02:05:03PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com> > > Devicetree schema allows the PERST# GPIO to be present in all PCIe bridge > nodes, not just in Root Port node. But the current logic parses PERST# only > from the Root Port nodes. Though it is not causing any issue on the current > platforms, the upcoming platforms will have PERST# in PCIe switch > downstream ports also. So this requires parsing all the PCIe bridge nodes > for the PERST# GPIO. > > Hence, rework the parsing logic to extend to all PCIe bridge nodes starting > from the Root Port node. If the 'reset-gpios' property is found for a PCI > bridge node, the GPIO descriptor will be stored in qcom_pcie_perst::desc > and added to the qcom_pcie_port::perst list. The switch part doesn't seem qcom specific. Aren't we going to end up with lots of drivers reimplementing something like the qcom_pcie_port.perst list?
On Fri, Sep 12, 2025 at 06:28:11PM GMT, Bjorn Helgaas wrote: > On Fri, Sep 12, 2025 at 02:05:03PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com> > > > > Devicetree schema allows the PERST# GPIO to be present in all PCIe bridge > > nodes, not just in Root Port node. But the current logic parses PERST# only > > from the Root Port nodes. Though it is not causing any issue on the current > > platforms, the upcoming platforms will have PERST# in PCIe switch > > downstream ports also. So this requires parsing all the PCIe bridge nodes > > for the PERST# GPIO. > > > > Hence, rework the parsing logic to extend to all PCIe bridge nodes starting > > from the Root Port node. If the 'reset-gpios' property is found for a PCI > > bridge node, the GPIO descriptor will be stored in qcom_pcie_perst::desc > > and added to the qcom_pcie_port::perst list. > > The switch part doesn't seem qcom specific. Aren't we going to end up > with lots of drivers reimplementing something like the > qcom_pcie_port.perst list? If this kind of switch is attached to other platforms, then yes. Right now, Qcom host is the only known DT based host platform that has seen this requirement. If other drivers also start showing this pattern, I may try to create a common code somewhere to avoid code duplication. But now, common code is not warranted. - Mani -- மணிவண்ணன் சதாசிவம்
On Mon, Sep 15, 2025 at 06:23:45PM +0530, Manivannan Sadhasivam wrote: > On Fri, Sep 12, 2025 at 06:28:11PM GMT, Bjorn Helgaas wrote: > > On Fri, Sep 12, 2025 at 02:05:03PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com> > > > > > > Devicetree schema allows the PERST# GPIO to be present in all > > > PCIe bridge nodes, not just in Root Port node. But the current > > > logic parses PERST# only from the Root Port nodes. Though it is > > > not causing any issue on the current platforms, the upcoming > > > platforms will have PERST# in PCIe switch downstream ports also. > > > So this requires parsing all the PCIe bridge nodes for the > > > PERST# GPIO. > > > > > > Hence, rework the parsing logic to extend to all PCIe bridge > > > nodes starting from the Root Port node. If the 'reset-gpios' > > > property is found for a PCI bridge node, the GPIO descriptor > > > will be stored in qcom_pcie_perst::desc and added to the > > > qcom_pcie_port::perst list. > > > > The switch part doesn't seem qcom specific. Aren't we going to > > end up with lots of drivers reimplementing something like the > > qcom_pcie_port.perst list? > > If this kind of switch is attached to other platforms, then yes. > Right now, Qcom host is the only known DT based host platform that > has seen this requirement. So I guess the issue here is that pwrctrl controls power to a slot below a Switch Downstream Port, and we want pwrctrl to also control PERST# to that same slot so that pwrctrl can power up the slot and then deassert PERST# to the slot later, e.g., after a T_PVPERL delay? Seems like whatever parses the devicetree power regulator information for the slot should also parse the PERST# GPIO for the slot. Bjorn
On Tue, Sep 16, 2025 at 02:49:06PM GMT, Bjorn Helgaas wrote: > On Mon, Sep 15, 2025 at 06:23:45PM +0530, Manivannan Sadhasivam wrote: > > On Fri, Sep 12, 2025 at 06:28:11PM GMT, Bjorn Helgaas wrote: > > > On Fri, Sep 12, 2025 at 02:05:03PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com> > > > > > > > > Devicetree schema allows the PERST# GPIO to be present in all > > > > PCIe bridge nodes, not just in Root Port node. But the current > > > > logic parses PERST# only from the Root Port nodes. Though it is > > > > not causing any issue on the current platforms, the upcoming > > > > platforms will have PERST# in PCIe switch downstream ports also. > > > > So this requires parsing all the PCIe bridge nodes for the > > > > PERST# GPIO. > > > > > > > > Hence, rework the parsing logic to extend to all PCIe bridge > > > > nodes starting from the Root Port node. If the 'reset-gpios' > > > > property is found for a PCI bridge node, the GPIO descriptor > > > > will be stored in qcom_pcie_perst::desc and added to the > > > > qcom_pcie_port::perst list. > > > > > > The switch part doesn't seem qcom specific. Aren't we going to > > > end up with lots of drivers reimplementing something like the > > > qcom_pcie_port.perst list? > > > > If this kind of switch is attached to other platforms, then yes. > > Right now, Qcom host is the only known DT based host platform that > > has seen this requirement. > > So I guess the issue here is that pwrctrl controls power to a slot > below a Switch Downstream Port, and we want pwrctrl to also control > PERST# to that same slot so that pwrctrl can power up the slot and > then deassert PERST# to the slot later, e.g., after a T_PVPERL delay? > > Seems like whatever parses the devicetree power regulator information > for the slot should also parse the PERST# GPIO for the slot. > Problem is, the controller driver also asserts PERST# during controller initialization. So even if the power control driver parses PERST#, it still need to share it with the controller driver somehow. I tried it in previous iteration, but it turned out to be too complex. So settled with this version where the controller driver parses PERST# as it used to, but not just from Root Port and then asserts/deasserts PERST# through the callback. - Mani -- மணிவண்ணன் சதாசிவம்
© 2016 - 2025 Red Hat, Inc.