[PATCH v6 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST#

[PATCH v6 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST#

[Sai Krishna Musham]

Add example usage of reset-gpios for PCIe RP PERST#

Add support for PCIe Root Port PERST# signal handling

Sai Krishna Musham (2):
  dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for PCIe
    RP PERST#
  PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

 .../bindings/pci/amd,versal2-mdb-host.yaml    | 22 +++++++
 drivers/pci/controller/dwc/pcie-amd-mdb.c     | 62 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)


base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.44.1

[PATCH v6 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for PCIe RP PERST#

[Sai Krishna Musham]

Update the device tree binding example to include usage of the
`reset-gpios` property in PCIe Root Port (RP) bridge node for PERST#
signal handling.

Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
Changes in v6:
- None

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Remove reset-gpios define as it is already part of pci-bus-common.yaml.

Changes in v3:
- Move reset-gpios to PCI bridge node.

Changes in v2:
- Update commit message

v5 https://lore.kernel.org/all/20250711052357.3859719-1-sai.krishna.musham@amd.com/
v4 https://lore.kernel.org/all/20250626054906.3277029-1-sai.krishna.musham@amd.com/
v3 https://lore.kernel.org/r/20250618080931.2472366-1-sai.krishna.musham@amd.com/
v2 https://lore.kernel.org/r/20250429090046.1512000-1-sai.krishna.musham@amd.com/
v1 https://lore.kernel.org/r/20250326041507.98232-1-sai.krishna.musham@amd.com/
---
 .../bindings/pci/amd,versal2-mdb-host.yaml    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
index 43dc2585c237..421e1116ae7e 100644
--- a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
+++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
@@ -71,6 +71,17 @@ properties:
       - "#address-cells"
       - "#interrupt-cells"
 
+patternProperties:
+  '^pcie@[0-2],0$':
+    type: object
+    $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+    unevaluatedProperties: false
+
 required:
   - reg
   - reg-names
@@ -87,6 +98,7 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
 
     soc {
         #address-cells = <2>;
@@ -112,6 +124,16 @@ examples:
             #size-cells = <2>;
             #interrupt-cells = <1>;
             device_type = "pci";
+
+            pcie@0,0 {
+                device_type = "pci";
+                reg = <0x0 0x0 0x0 0x0 0x0>;
+                reset-gpios = <&tca6416_u37 7 GPIO_ACTIVE_LOW>;
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges;
+            };
+
             pcie_intc_0: interrupt-controller {
                 #address-cells = <0>;
                 #interrupt-cells = <1>;
-- 
2.44.1

[PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

[Sai Krishna Musham]

Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
signal via a GPIO by parsing the new PCIe bridge node to acquire the
reset GPIO. If the bridge node is not found, fall back to acquiring it
from the PCIe node.

As part of this, update the interrupt controller node parsing to use
of_get_child_by_name() instead of of_get_next_child(), since the PCIe
node now has multiple children. This ensures the correct node is
selected during initialization.

Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
---
Changes in v6:
- Simplified error checking condition logic.
- Removed unnecessary fallback message.

Changes in v5:
- Add fall back mechanism to acquire reset GPIO from PCIe node when PCIe Bridge
node is not present.

Changes in v4:
- Resolve kernel test robot warning.
https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-lkp@intel.com/
- Update commit message.

Changes in v3:
- Implement amd_mdb_parse_pcie_port to parse bridge node for reset-gpios property.

Changes in v2:
- Change delay to PCIE_T_PVPERL_MS

v5 https://lore.kernel.org/all/20250711052357.3859719-1-sai.krishna.musham@amd.com/
v4 https://lore.kernel.org/all/20250626054906.3277029-1-sai.krishna.musham@amd.com/
v3 https://lore.kernel.org/r/20250618080931.2472366-1-sai.krishna.musham@amd.com/
v2 https://lore.kernel.org/r/20250429090046.1512000-1-sai.krishna.musham@amd.com/
v1 https://lore.kernel.org/r/20250326041507.98232-1-sai.krishna.musham@amd.com/
---
 drivers/pci/controller/dwc/pcie-amd-mdb.c | 62 ++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-amd-mdb.c b/drivers/pci/controller/dwc/pcie-amd-mdb.c
index 9f7251a16d32..697f5b3fc75e 100644
--- a/drivers/pci/controller/dwc/pcie-amd-mdb.c
+++ b/drivers/pci/controller/dwc/pcie-amd-mdb.c
@@ -18,6 +18,7 @@
 #include <linux/resource.h>
 #include <linux/types.h>
 
+#include "../../pci.h"
 #include "pcie-designware.h"
 
 #define AMD_MDB_TLP_IR_STATUS_MISC		0x4C0
@@ -56,6 +57,7 @@
  * @slcr: MDB System Level Control and Status Register (SLCR) base
  * @intx_domain: INTx IRQ domain pointer
  * @mdb_domain: MDB IRQ domain pointer
+ * @perst_gpio: GPIO descriptor for PERST# signal handling
  * @intx_irq: INTx IRQ interrupt number
  */
 struct amd_mdb_pcie {
@@ -63,6 +65,7 @@ struct amd_mdb_pcie {
 	void __iomem			*slcr;
 	struct irq_domain		*intx_domain;
 	struct irq_domain		*mdb_domain;
+	struct gpio_desc		*perst_gpio;
 	int				intx_irq;
 };
 
@@ -284,7 +287,7 @@ static int amd_mdb_pcie_init_irq_domains(struct amd_mdb_pcie *pcie,
 	struct device_node *pcie_intc_node;
 	int err;
 
-	pcie_intc_node = of_get_next_child(node, NULL);
+	pcie_intc_node = of_get_child_by_name(node, "interrupt-controller");
 	if (!pcie_intc_node) {
 		dev_err(dev, "No PCIe Intc node found\n");
 		return -ENODEV;
@@ -402,6 +405,34 @@ static int amd_mdb_setup_irq(struct amd_mdb_pcie *pcie,
 	return 0;
 }
 
+static int amd_mdb_parse_pcie_port(struct amd_mdb_pcie *pcie)
+{
+	struct device *dev = pcie->pci.dev;
+	struct device_node *pcie_port_node;
+
+	pcie_port_node = of_get_next_child_with_prefix(dev->of_node, NULL, "pcie");
+	if (!pcie_port_node) {
+		dev_err(dev, "No PCIe Bridge node found\n");
+		return -ENODEV;
+	}
+
+	/* Request the GPIO for PCIe reset signal and assert */
+	pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(pcie_port_node),
+						 "reset", GPIOD_OUT_HIGH, NULL);
+	if (IS_ERR(pcie->perst_gpio)) {
+		if (PTR_ERR(pcie->perst_gpio) != -ENOENT) {
+			of_node_put(pcie_port_node);
+			return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
+					     "Failed to request reset GPIO\n");
+		}
+		pcie->perst_gpio = NULL;
+	}
+
+	of_node_put(pcie_port_node);
+
+	return 0;
+}
+
 static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
 				 struct platform_device *pdev)
 {
@@ -426,6 +457,14 @@ static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
 
 	pp->ops = &amd_mdb_pcie_host_ops;
 
+	if (pcie->perst_gpio) {
+		mdelay(PCIE_T_PVPERL_MS);
+
+		/* Deassert the reset signal */
+		gpiod_set_value_cansleep(pcie->perst_gpio, 0);
+		mdelay(PCIE_T_RRS_READY_MS);
+	}
+
 	err = dw_pcie_host_init(pp);
 	if (err) {
 		dev_err(dev, "Failed to initialize host, err=%d\n", err);
@@ -444,6 +483,7 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct amd_mdb_pcie *pcie;
 	struct dw_pcie *pci;
+	int ret;
 
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
@@ -454,6 +494,26 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pcie);
 
+	ret = amd_mdb_parse_pcie_port(pcie);
+
+	/*
+	 * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
+	 * PCIe Bridge node was not found in the device tree. This is not
+	 * considered a fatal error and will trigger a fallback where the
+	 * reset GPIO is acquired directly from the PCIe node.
+	 */
+	if (ret == -ENODEV) {
+
+		/* Request the GPIO for PCIe reset signal and assert */
+		pcie->perst_gpio = devm_gpiod_get_optional(dev, "reset",
+							   GPIOD_OUT_HIGH);
+		if (IS_ERR(pcie->perst_gpio))
+			return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
+					     "Failed to request reset GPIO\n");
+	} else if (ret) {
+		return ret;
+	}
+
 	return amd_mdb_add_pcie_port(pcie, pdev);
 }
 
-- 
2.44.1

Re: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

[Bjorn Helgaas]

On Sat, Jul 19, 2025 at 08:39:51AM +0530, Sai Krishna Musham wrote:
> Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> signal via a GPIO by parsing the new PCIe bridge node to acquire the
> reset GPIO. If the bridge node is not found, fall back to acquiring it
> from the PCIe node.
> 
> As part of this, update the interrupt controller node parsing to use
> of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> node now has multiple children. This ensures the correct node is
> selected during initialization.

> +static int amd_mdb_parse_pcie_port(struct amd_mdb_pcie *pcie)
> +{
> +	struct device *dev = pcie->pci.dev;
> +	struct device_node *pcie_port_node;
> +
> +	pcie_port_node = of_get_next_child_with_prefix(dev->of_node, NULL, "pcie");
> +	if (!pcie_port_node) {
> +		dev_err(dev, "No PCIe Bridge node found\n");
> +		return -ENODEV;
> +	}

Sorry I didn't notice this before.   I don't think we want to emit a
message here either because existing DTs in the field will not have a
Root Port node, and we will just fall back to the 'reset' in the PCIe
node.

There's really nothing wrong in that case, so no need to annoy users
with messages they can't fix.

IIUC, PERST# in the DT is optional anyway (you use
devm_gpiod_get_optional() below).

> +	/* Request the GPIO for PCIe reset signal and assert */
> +	pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(pcie_port_node),
> +						 "reset", GPIOD_OUT_HIGH, NULL);
> +	if (IS_ERR(pcie->perst_gpio)) {
> +		if (PTR_ERR(pcie->perst_gpio) != -ENOENT) {
> +			of_node_put(pcie_port_node);
> +			return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> +					     "Failed to request reset GPIO\n");
> +		}
> +		pcie->perst_gpio = NULL;
> +	}
> +
> +	of_node_put(pcie_port_node);
> +
> +	return 0;
> +}

> @@ -444,6 +483,7 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct amd_mdb_pcie *pcie;
>  	struct dw_pcie *pci;
> +	int ret;
>  
>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>  	if (!pcie)
> @@ -454,6 +494,26 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pcie);
>  
> +	ret = amd_mdb_parse_pcie_port(pcie);
> +
> +	/*
> +	 * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> +	 * PCIe Bridge node was not found in the device tree. This is not
> +	 * considered a fatal error and will trigger a fallback where the
> +	 * reset GPIO is acquired directly from the PCIe node.
> +	 */
> +	if (ret == -ENODEV) {
> +
> +		/* Request the GPIO for PCIe reset signal and assert */
> +		pcie->perst_gpio = devm_gpiod_get_optional(dev, "reset",
> +							   GPIOD_OUT_HIGH);
> +		if (IS_ERR(pcie->perst_gpio))
> +			return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> +					     "Failed to request reset GPIO\n");
> +	} else if (ret) {
> +		return ret;
> +	}
> +
>  	return amd_mdb_add_pcie_port(pcie, pdev);
>  }
>  
> -- 
> 2.44.1
> 

RE: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

[Musham, Sai Krishna]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Bjorn,


> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, July 22, 2025 3:25 AM
> To: Musham, Sai Krishna <sai.krishna.musham@amd.com>
> Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com; mani@kernel.org;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; cassel@kernel.org;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>; Gogada, Bharat
> Kumar <bharat.kumar.gogada@amd.com>; Havalige, Thippeswamy
> <thippeswamy.havalige@amd.com>
> Subject: Re: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
> signal handling
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Sat, Jul 19, 2025 at 08:39:51AM +0530, Sai Krishna Musham wrote:
> > Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> > signal via a GPIO by parsing the new PCIe bridge node to acquire the
> > reset GPIO. If the bridge node is not found, fall back to acquiring it
> > from the PCIe node.
> >
> > As part of this, update the interrupt controller node parsing to use
> > of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> > node now has multiple children. This ensures the correct node is
> > selected during initialization.
>
> > +static int amd_mdb_parse_pcie_port(struct amd_mdb_pcie *pcie)
> > +{
> > +     struct device *dev = pcie->pci.dev;
> > +     struct device_node *pcie_port_node;
> > +
> > +     pcie_port_node = of_get_next_child_with_prefix(dev->of_node, NULL, "pcie");
> > +     if (!pcie_port_node) {
> > +             dev_err(dev, "No PCIe Bridge node found\n");
> > +             return -ENODEV;
> > +     }
>
> Sorry I didn't notice this before.   I don't think we want to emit a
> message here either because existing DTs in the field will not have a
> Root Port node, and we will just fall back to the 'reset' in the PCIe
> node.
>
> There's really nothing wrong in that case, so no need to annoy users
> with messages they can't fix.
>
> IIUC, PERST# in the DT is optional anyway (you use
> devm_gpiod_get_optional() below).
>

Thanks for pointing that out - you're right. There's no need to emit
a message when the PCIe bridge node is missing. I'll remove it
to avoid unnecessary logs during fall back to the 'reset' in the
PCIe node.

Regarding the use of devm_gpiod_get_optional(): since the optional
variant for fwnode isn't available in gpiolib-devres.c, using
devm_gpiod_get_optional() would only check the PCIe node itself,
not its child. That's why I opted for devm_fwnode_gpiod_get() and
handled the optional reset GPIO manually by assigning NULL when
'reset-gpios' is not present in bridge node - please correct me if I'm
missing anything here. Thanks.

> > +     /* Request the GPIO for PCIe reset signal and assert */
> > +     pcie->perst_gpio = devm_fwnode_gpiod_get(dev,
> of_fwnode_handle(pcie_port_node),
> > +                                              "reset", GPIOD_OUT_HIGH, NULL);
> > +     if (IS_ERR(pcie->perst_gpio)) {
> > +             if (PTR_ERR(pcie->perst_gpio) != -ENOENT) {
> > +                     of_node_put(pcie_port_node);
> > +                     return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> > +                                          "Failed to request reset GPIO\n");
> > +             }
> > +             pcie->perst_gpio = NULL;
> > +     }
> > +
> > +     of_node_put(pcie_port_node);
> > +
> > +     return 0;
> > +}
>
> > @@ -444,6 +483,7 @@ static int amd_mdb_pcie_probe(struct platform_device
> *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct amd_mdb_pcie *pcie;
> >       struct dw_pcie *pci;
> > +     int ret;
> >
> >       pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> >       if (!pcie)
> > @@ -454,6 +494,26 @@ static int amd_mdb_pcie_probe(struct platform_device
> *pdev)
> >
> >       platform_set_drvdata(pdev, pcie);
> >
> > +     ret = amd_mdb_parse_pcie_port(pcie);
> > +
> > +     /*
> > +      * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> > +      * PCIe Bridge node was not found in the device tree. This is not
> > +      * considered a fatal error and will trigger a fallback where the
> > +      * reset GPIO is acquired directly from the PCIe node.
> > +      */
> > +     if (ret == -ENODEV) {
> > +
> > +             /* Request the GPIO for PCIe reset signal and assert */
> > +             pcie->perst_gpio = devm_gpiod_get_optional(dev, "reset",
> > +                                                        GPIOD_OUT_HIGH);
> > +             if (IS_ERR(pcie->perst_gpio))
> > +                     return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> > +                                          "Failed to request reset GPIO\n");
> > +     } else if (ret) {
> > +             return ret;
> > +     }
> > +
> >       return amd_mdb_add_pcie_port(pcie, pdev);
> >  }
> >
> > --
> > 2.44.1
> >

Regards,
Sai Krishna

Re: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

[Manivannan Sadhasivam]

On Sat, Jul 19, 2025 at 08:39:51AM GMT, Sai Krishna Musham wrote:
> Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> signal via a GPIO by parsing the new PCIe bridge node to acquire the
> reset GPIO. If the bridge node is not found, fall back to acquiring it
> from the PCIe node.
> 
> As part of this, update the interrupt controller node parsing to use
> of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> node now has multiple children. This ensures the correct node is
> selected during initialization.
> 
> Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
> ---
> Changes in v6:
> - Simplified error checking condition logic.
> - Removed unnecessary fallback message.
> 
> Changes in v5:
> - Add fall back mechanism to acquire reset GPIO from PCIe node when PCIe Bridge
> node is not present.
> 
> Changes in v4:
> - Resolve kernel test robot warning.
> https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-lkp@intel.com/
> - Update commit message.
> 
> Changes in v3:
> - Implement amd_mdb_parse_pcie_port to parse bridge node for reset-gpios property.
> 
> Changes in v2:
> - Change delay to PCIE_T_PVPERL_MS
> 
> v5 https://lore.kernel.org/all/20250711052357.3859719-1-sai.krishna.musham@amd.com/
> v4 https://lore.kernel.org/all/20250626054906.3277029-1-sai.krishna.musham@amd.com/
> v3 https://lore.kernel.org/r/20250618080931.2472366-1-sai.krishna.musham@amd.com/
> v2 https://lore.kernel.org/r/20250429090046.1512000-1-sai.krishna.musham@amd.com/
> v1 https://lore.kernel.org/r/20250326041507.98232-1-sai.krishna.musham@amd.com/
> ---
>  drivers/pci/controller/dwc/pcie-amd-mdb.c | 62 ++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-amd-mdb.c b/drivers/pci/controller/dwc/pcie-amd-mdb.c
> index 9f7251a16d32..697f5b3fc75e 100644
> --- a/drivers/pci/controller/dwc/pcie-amd-mdb.c
> +++ b/drivers/pci/controller/dwc/pcie-amd-mdb.c
> @@ -18,6 +18,7 @@
>  #include <linux/resource.h>
>  #include <linux/types.h>
>  
> +#include "../../pci.h"
>  #include "pcie-designware.h"
>  
>  #define AMD_MDB_TLP_IR_STATUS_MISC		0x4C0
> @@ -56,6 +57,7 @@
>   * @slcr: MDB System Level Control and Status Register (SLCR) base
>   * @intx_domain: INTx IRQ domain pointer
>   * @mdb_domain: MDB IRQ domain pointer
> + * @perst_gpio: GPIO descriptor for PERST# signal handling
>   * @intx_irq: INTx IRQ interrupt number
>   */
>  struct amd_mdb_pcie {
> @@ -63,6 +65,7 @@ struct amd_mdb_pcie {
>  	void __iomem			*slcr;
>  	struct irq_domain		*intx_domain;
>  	struct irq_domain		*mdb_domain;
> +	struct gpio_desc		*perst_gpio;
>  	int				intx_irq;
>  };
>  
> @@ -284,7 +287,7 @@ static int amd_mdb_pcie_init_irq_domains(struct amd_mdb_pcie *pcie,
>  	struct device_node *pcie_intc_node;
>  	int err;
>  
> -	pcie_intc_node = of_get_next_child(node, NULL);
> +	pcie_intc_node = of_get_child_by_name(node, "interrupt-controller");
>  	if (!pcie_intc_node) {
>  		dev_err(dev, "No PCIe Intc node found\n");
>  		return -ENODEV;
> @@ -402,6 +405,34 @@ static int amd_mdb_setup_irq(struct amd_mdb_pcie *pcie,
>  	return 0;
>  }
>  
> +static int amd_mdb_parse_pcie_port(struct amd_mdb_pcie *pcie)
> +{
> +	struct device *dev = pcie->pci.dev;
> +	struct device_node *pcie_port_node;
> +
> +	pcie_port_node = of_get_next_child_with_prefix(dev->of_node, NULL, "pcie");
> +	if (!pcie_port_node) {
> +		dev_err(dev, "No PCIe Bridge node found\n");
> +		return -ENODEV;
> +	}
> +

Please use for_each_child_of_node_with_prefix() and get rid of the above check.
Since this is a scoped variant, you do not need to care about OF node refcount.

If your platform supports only one Root Port, you can add a comment on top that
the loop will execute only once. Maybe you can also add a TODO so that someone
could prepare the driver to handle multi Root Ports in the future.

> +	/* Request the GPIO for PCIe reset signal and assert */

Drop the comment.

> +	pcie->perst_gpio = devm_fwnode_gpiod_get(dev, of_fwnode_handle(pcie_port_node),
> +						 "reset", GPIOD_OUT_HIGH, NULL);
> +	if (IS_ERR(pcie->perst_gpio)) {
> +		if (PTR_ERR(pcie->perst_gpio) != -ENOENT) {
> +			of_node_put(pcie_port_node);
> +			return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> +					     "Failed to request reset GPIO\n");
> +		}
> +		pcie->perst_gpio = NULL;

Not required.

> +	}
> +
> +	of_node_put(pcie_port_node);
> +
> +	return 0;
> +}
> +
>  static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
>  				 struct platform_device *pdev)
>  {
> @@ -426,6 +457,14 @@ static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
>  
>  	pp->ops = &amd_mdb_pcie_host_ops;
>  
> +	if (pcie->perst_gpio) {
> +		mdelay(PCIE_T_PVPERL_MS);
> +
> +		/* Deassert the reset signal */
> +		gpiod_set_value_cansleep(pcie->perst_gpio, 0);
> +		mdelay(PCIE_T_RRS_READY_MS);
> +	}
> +
>  	err = dw_pcie_host_init(pp);
>  	if (err) {
>  		dev_err(dev, "Failed to initialize host, err=%d\n", err);
> @@ -444,6 +483,7 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct amd_mdb_pcie *pcie;
>  	struct dw_pcie *pci;
> +	int ret;
>  
>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>  	if (!pcie)
> @@ -454,6 +494,26 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pcie);
>  
> +	ret = amd_mdb_parse_pcie_port(pcie);
> +

Spurious newline

> +	/*
> +	 * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> +	 * PCIe Bridge node was not found in the device tree. This is not
> +	 * considered a fatal error and will trigger a fallback where the
> +	 * reset GPIO is acquired directly from the PCIe node.

s/PCIe node/PCIe Host Bridge node

> +	 */
> +	if (ret == -ENODEV) {

Please use the below pattern:

	if (ret) {
		if (ret != -ENODEV) {
			dev_err();
			return ret;
		}

		pcie->perst_gpio = devm_gpiod_get_optional()...
	}

> +

Spurious newline

> +		/* Request the GPIO for PCIe reset signal and assert */

Drop the comment

- Mani

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

RE: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

[Musham, Sai Krishna]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Manivannan,

Thanks for the review.

> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: Wednesday, July 23, 2025 7:53 PM
> To: Musham, Sai Krishna <sai.krishna.musham@amd.com>
> Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; cassel@kernel.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> Simek, Michal <michal.simek@amd.com>; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>; Havalige, Thippeswamy
> <thippeswamy.havalige@amd.com>
> Subject: Re: [PATCH v6 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
> signal handling
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Sat, Jul 19, 2025 at 08:39:51AM GMT, Sai Krishna Musham wrote:
> > Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> > signal via a GPIO by parsing the new PCIe bridge node to acquire the
> > reset GPIO. If the bridge node is not found, fall back to acquiring it
> > from the PCIe node.
> >
> > As part of this, update the interrupt controller node parsing to use
> > of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> > node now has multiple children. This ensures the correct node is
> > selected during initialization.
> >
> > Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
> > ---
> > Changes in v6:
> > - Simplified error checking condition logic.
> > - Removed unnecessary fallback message.
> >
> > Changes in v5:
> > - Add fall back mechanism to acquire reset GPIO from PCIe node when PCIe
> Bridge
> > node is not present.
> >
> > Changes in v4:
> > - Resolve kernel test robot warning.
> > https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-lkp@intel.com/
> > - Update commit message.
> >
> > Changes in v3:
> > - Implement amd_mdb_parse_pcie_port to parse bridge node for reset-gpios
> property.
> >
> > Changes in v2:
> > - Change delay to PCIE_T_PVPERL_MS
> >
> > v5 https://lore.kernel.org/all/20250711052357.3859719-1-
> sai.krishna.musham@amd.com/
> > v4 https://lore.kernel.org/all/20250626054906.3277029-1-
> sai.krishna.musham@amd.com/
> > v3 https://lore.kernel.org/r/20250618080931.2472366-1-
> sai.krishna.musham@amd.com/
> > v2 https://lore.kernel.org/r/20250429090046.1512000-1-
> sai.krishna.musham@amd.com/
> > v1 https://lore.kernel.org/r/20250326041507.98232-1-
> sai.krishna.musham@amd.com/
> > ---
> >  drivers/pci/controller/dwc/pcie-amd-mdb.c | 62 ++++++++++++++++++++++-
> >  1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-amd-mdb.c
> b/drivers/pci/controller/dwc/pcie-amd-mdb.c
> > index 9f7251a16d32..697f5b3fc75e 100644
> > --- a/drivers/pci/controller/dwc/pcie-amd-mdb.c
> > +++ b/drivers/pci/controller/dwc/pcie-amd-mdb.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/resource.h>
> >  #include <linux/types.h>
> >
> > +#include "../../pci.h"
> >  #include "pcie-designware.h"
> >
> >  #define AMD_MDB_TLP_IR_STATUS_MISC           0x4C0
> > @@ -56,6 +57,7 @@
> >   * @slcr: MDB System Level Control and Status Register (SLCR) base
> >   * @intx_domain: INTx IRQ domain pointer
> >   * @mdb_domain: MDB IRQ domain pointer
> > + * @perst_gpio: GPIO descriptor for PERST# signal handling
> >   * @intx_irq: INTx IRQ interrupt number
> >   */
> >  struct amd_mdb_pcie {
> > @@ -63,6 +65,7 @@ struct amd_mdb_pcie {
> >       void __iomem                    *slcr;
> >       struct irq_domain               *intx_domain;
> >       struct irq_domain               *mdb_domain;
> > +     struct gpio_desc                *perst_gpio;
> >       int                             intx_irq;
> >  };
> >
> > @@ -284,7 +287,7 @@ static int amd_mdb_pcie_init_irq_domains(struct
> amd_mdb_pcie *pcie,
> >       struct device_node *pcie_intc_node;
> >       int err;
> >
> > -     pcie_intc_node = of_get_next_child(node, NULL);
> > +     pcie_intc_node = of_get_child_by_name(node, "interrupt-controller");
> >       if (!pcie_intc_node) {
> >               dev_err(dev, "No PCIe Intc node found\n");
> >               return -ENODEV;
> > @@ -402,6 +405,34 @@ static int amd_mdb_setup_irq(struct amd_mdb_pcie
> *pcie,
> >       return 0;
> >  }
> >
> > +static int amd_mdb_parse_pcie_port(struct amd_mdb_pcie *pcie)
> > +{
> > +     struct device *dev = pcie->pci.dev;
> > +     struct device_node *pcie_port_node;
> > +
> > +     pcie_port_node = of_get_next_child_with_prefix(dev->of_node, NULL, "pcie");
> > +     if (!pcie_port_node) {
> > +             dev_err(dev, "No PCIe Bridge node found\n");
> > +             return -ENODEV;
> > +     }
> > +
>
> Please use for_each_child_of_node_with_prefix() and get rid of the above check.
> Since this is a scoped variant, you do not need to care about OF node refcount.
>
> If your platform supports only one Root Port, you can add a comment on top that
> the loop will execute only once. Maybe you can also add a TODO so that someone
> could prepare the driver to handle multi Root Ports in the future.
>

Thanks for the review, I have made the changes will send in next patch.

> > +     /* Request the GPIO for PCIe reset signal and assert */
>
> Drop the comment.
>
> > +     pcie->perst_gpio = devm_fwnode_gpiod_get(dev,
> of_fwnode_handle(pcie_port_node),
> > +                                              "reset", GPIOD_OUT_HIGH, NULL);
> > +     if (IS_ERR(pcie->perst_gpio)) {
> > +             if (PTR_ERR(pcie->perst_gpio) != -ENOENT) {
> > +                     of_node_put(pcie_port_node);
> > +                     return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> > +                                          "Failed to request reset GPIO\n");
> > +             }
> > +             pcie->perst_gpio = NULL;
>
> Not required.
>
> > +     }
> > +
> > +     of_node_put(pcie_port_node);
> > +
> > +     return 0;
> > +}
> > +
> >  static int amd_mdb_add_pcie_port(struct amd_mdb_pcie *pcie,
> >                                struct platform_device *pdev)
> >  {
> > @@ -426,6 +457,14 @@ static int amd_mdb_add_pcie_port(struct
> amd_mdb_pcie *pcie,
> >
> >       pp->ops = &amd_mdb_pcie_host_ops;
> >
> > +     if (pcie->perst_gpio) {
> > +             mdelay(PCIE_T_PVPERL_MS);
> > +
> > +             /* Deassert the reset signal */
> > +             gpiod_set_value_cansleep(pcie->perst_gpio, 0);
> > +             mdelay(PCIE_T_RRS_READY_MS);
> > +     }
> > +
> >       err = dw_pcie_host_init(pp);
> >       if (err) {
> >               dev_err(dev, "Failed to initialize host, err=%d\n", err);
> > @@ -444,6 +483,7 @@ static int amd_mdb_pcie_probe(struct platform_device
> *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct amd_mdb_pcie *pcie;
> >       struct dw_pcie *pci;
> > +     int ret;
> >
> >       pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> >       if (!pcie)
> > @@ -454,6 +494,26 @@ static int amd_mdb_pcie_probe(struct platform_device
> *pdev)
> >
> >       platform_set_drvdata(pdev, pcie);
> >
> > +     ret = amd_mdb_parse_pcie_port(pcie);
> > +
>
> Spurious newline
>
> > +     /*
> > +      * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> > +      * PCIe Bridge node was not found in the device tree. This is not
> > +      * considered a fatal error and will trigger a fallback where the
> > +      * reset GPIO is acquired directly from the PCIe node.
>
> s/PCIe node/PCIe Host Bridge node
>
> > +      */
> > +     if (ret == -ENODEV) {
>
> Please use the below pattern:
>
>         if (ret) {
>                 if (ret != -ENODEV) {
>                         dev_err();
>                         return ret;
>                 }
>
>                 pcie->perst_gpio = devm_gpiod_get_optional()...
>         }
>
> > +
>
> Spurious newline
>
> > +             /* Request the GPIO for PCIe reset signal and assert */
>
> Drop the comment
>

Sure, I have followed above instructions, will send in next patch.
Thanks

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

Regards,
Sai Krishna