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@xxxxxxx> > --- > 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@xxxxxxxxx/ > - 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@xxxxxxx/ > v4 https://lore.kernel.org/all/20250626054906.3277029-1-sai.krishna.musham@xxxxxxx/ > v3 https://lore.kernel.org/r/20250618080931.2472366-1-sai.krishna.musham@xxxxxxx/ > v2 https://lore.kernel.org/r/20250429090046.1512000-1-sai.krishna.musham@xxxxxxx/ > v1 https://lore.kernel.org/r/20250326041507.98232-1-sai.krishna.musham@xxxxxxx/ > --- > 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 -- மணிவண்ணன் சதாசிவம்
