* [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST#
@ 2025-06-26 5:49 Sai Krishna Musham
2025-06-26 5:49 ` [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for " Sai Krishna Musham
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sai Krishna Musham @ 2025-06-26 5:49 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kw, mani, robh, krzk+dt, conor+dt, cassel
Cc: lkp, linux-pci, devicetree, linux-kernel, michal.simek,
bharat.kumar.gogada, thippeswamy.havalige, 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 | 46 ++++++++++++++++++-
2 files changed, 67 insertions(+), 1 deletion(-)
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
--
2.44.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for PCIe RP PERST#
2025-06-26 5:49 [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Sai Krishna Musham
@ 2025-06-26 5:49 ` Sai Krishna Musham
2025-06-27 21:21 ` Rob Herring (Arm)
2025-06-26 5:49 ` [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling Sai Krishna Musham
2025-07-07 12:17 ` [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Musham, Sai Krishna
2 siblings, 1 reply; 7+ messages in thread
From: Sai Krishna Musham @ 2025-06-26 5:49 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kw, mani, robh, krzk+dt, conor+dt, cassel
Cc: lkp, linux-pci, devicetree, linux-kernel, michal.simek,
bharat.kumar.gogada, thippeswamy.havalige, 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>
---
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
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling
2025-06-26 5:49 [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Sai Krishna Musham
2025-06-26 5:49 ` [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for " Sai Krishna Musham
@ 2025-06-26 5:49 ` Sai Krishna Musham
2025-07-08 23:23 ` Bjorn Helgaas
2025-07-07 12:17 ` [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Musham, Sai Krishna
2 siblings, 1 reply; 7+ messages in thread
From: Sai Krishna Musham @ 2025-06-26 5:49 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kw, mani, robh, krzk+dt, conor+dt, cassel
Cc: lkp, linux-pci, devicetree, linux-kernel, michal.simek,
bharat.kumar.gogada, thippeswamy.havalige, 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.
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.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-lkp@intel.com/
Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
---
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
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 | 46 ++++++++++++++++++++++-
1 file changed, 45 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..f011a83550b9 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,10 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcie);
+ ret = amd_mdb_parse_pcie_port(pcie);
+ if (ret)
+ return ret;
+
return amd_mdb_add_pcie_port(pcie, pdev);
}
--
2.44.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for PCIe RP PERST#
2025-06-26 5:49 ` [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for " Sai Krishna Musham
@ 2025-06-27 21:21 ` Rob Herring (Arm)
0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring (Arm) @ 2025-06-27 21:21 UTC (permalink / raw)
To: Sai Krishna Musham
Cc: mani, thippeswamy.havalige, bharat.kumar.gogada, bhelgaas,
lpieralisi, devicetree, linux-pci, lkp, linux-kernel,
michal.simek, cassel, krzk+dt, kw, conor+dt
On Thu, 26 Jun 2025 11:19:05 +0530, Sai Krishna Musham wrote:
> 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>
> ---
> 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
>
> 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(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST#
2025-06-26 5:49 [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Sai Krishna Musham
2025-06-26 5:49 ` [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for " Sai Krishna Musham
2025-06-26 5:49 ` [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling Sai Krishna Musham
@ 2025-07-07 12:17 ` Musham, Sai Krishna
2 siblings, 0 replies; 7+ messages in thread
From: Musham, Sai Krishna @ 2025-07-07 12:17 UTC (permalink / raw)
To: 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
Cc: lkp@intel.com, linux-pci@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Simek, Michal, Gogada, Bharat Kumar, Havalige, Thippeswamy
[AMD Official Use Only - AMD Internal Distribution Only]
Hi all,
Just a gentle ping on this patch series.
Patch 0001 has received a Reviewed-by from Rob Herring, and I'm waiting for feedback on 0002.
Please let me know if any changes are needed.
Thanks,
Sai Krishna
> -----Original Message-----
> From: Sai Krishna Musham <sai.krishna.musham@amd.com>
> Sent: Thursday, June 26, 2025 11:19 AM
> To: 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
> Cc: lkp@intel.com; 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>; Musham, Sai Krishna
> <sai.krishna.musham@amd.com>
> Subject: [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST#
>
> 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 | 46 ++++++++++++++++++-
> 2 files changed, 67 insertions(+), 1 deletion(-)
>
>
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> --
> 2.44.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling
2025-06-26 5:49 ` [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling Sai Krishna Musham
@ 2025-07-08 23:23 ` Bjorn Helgaas
2025-07-09 13:58 ` Musham, Sai Krishna
0 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2025-07-08 23:23 UTC (permalink / raw)
To: Sai Krishna Musham
Cc: bhelgaas, lpieralisi, kw, mani, robh, krzk+dt, conor+dt, cassel,
lkp, linux-pci, devicetree, linux-kernel, michal.simek,
bharat.kumar.gogada, thippeswamy.havalige
On Thu, Jun 26, 2025 at 11:19:06AM +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.
>
> 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.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-lkp@intel.com/
Omit these tags. This kernel test robot report is basically a code
review comment that doesn't need to be acknowledged here (the robot's
report says:
If you fix the issue in a separate patch/commit (i.e. not just a new
version of the same patch/commit), kindly add following tags ...
IIUC this is just a new version of the same patch, so doesn't need the
tags.
> Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
> ---
> 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
>
> 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 | 46 ++++++++++++++++++++++-
> 1 file changed, 45 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..f011a83550b9 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,10 @@ static int amd_mdb_pcie_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, pcie);
>
> + ret = amd_mdb_parse_pcie_port(pcie);
> + if (ret)
> + return ret;
I'm not a DT expert, but doesn't this break if you run
amd_mdb_parse_pcie_port() on a system with an existing DT that lacks
the pcie@0,0 stanza you added to the binding in patch [1/2]?
I.e., amd_mdb_parse_pcie_port() will return -ENODEV in that case, and
the probe will now fail?
It's good to add new functionality, but if the driver runs with a DT
that doesn't describe the new functionality, it should fall back to
the previous behavior (without the new functionality) instead of
failing completely.
> return amd_mdb_add_pcie_port(pcie, pdev);
> }
>
> --
> 2.44.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling
2025-07-08 23:23 ` Bjorn Helgaas
@ 2025-07-09 13:58 ` Musham, Sai Krishna
0 siblings, 0 replies; 7+ messages in thread
From: Musham, Sai Krishna @ 2025-07-09 13:58 UTC (permalink / raw)
To: Bjorn Helgaas
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, lkp@intel.com,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Simek, Michal, Gogada, Bharat Kumar,
Havalige, Thippeswamy
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Bjorn,
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Wednesday, July 9, 2025 4:53 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;
> lkp@intel.com; 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 v4 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 Thu, Jun 26, 2025 at 11:19:06AM +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.
> >
> > 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.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202506241020.rPD1a2Vr-
> lkp@intel.com/
>
> Omit these tags. This kernel test robot report is basically a code
> review comment that doesn't need to be acknowledged here (the robot's
> report says:
>
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of the same patch/commit), kindly add following tags ...
>
> IIUC this is just a new version of the same patch, so doesn't need the
> tags.
>
Sure, I will omit these tags, Thanks.
> > Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
> > ---
> > 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
> >
> > 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 | 46 ++++++++++++++++++++++-
> > 1 file changed, 45 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..f011a83550b9 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,10 @@ static int amd_mdb_pcie_probe(struct platform_device
> *pdev)
> >
> > platform_set_drvdata(pdev, pcie);
> >
> > + ret = amd_mdb_parse_pcie_port(pcie);
> > + if (ret)
> > + return ret;
>
> I'm not a DT expert, but doesn't this break if you run
> amd_mdb_parse_pcie_port() on a system with an existing DT that lacks
> the pcie@0,0 stanza you added to the binding in patch [1/2]?
>
> I.e., amd_mdb_parse_pcie_port() will return -ENODEV in that case, and
> the probe will now fail?
>
> It's good to add new functionality, but if the driver runs with a DT
> that doesn't describe the new functionality, it should fall back to
> the previous behavior (without the new functionality) instead of
> failing completely.
>
Thanks for the review, I'll make this change for backward compatibility
and include it in the next patch.
> > return amd_mdb_add_pcie_port(pcie, pdev);
> > }
> >
> > --
> > 2.44.1
> >
Regards,
Sai Krishna
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-07-09 13:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-26 5:49 [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Sai Krishna Musham
2025-06-26 5:49 ` [PATCH v4 1/2] dt-bindings: PCI: amd-mdb: Add example usage of reset-gpios for " Sai Krishna Musham
2025-06-27 21:21 ` Rob Herring (Arm)
2025-06-26 5:49 ` [PATCH v4 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling Sai Krishna Musham
2025-07-08 23:23 ` Bjorn Helgaas
2025-07-09 13:58 ` Musham, Sai Krishna
2025-07-07 12:17 ` [PATCH v4 0/2] Add support for AMD Versal Gen 2 MDB PCIe RP PERST# Musham, Sai Krishna
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).