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

[Bjorn Helgaas <helgaas@kernel.org>]

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
>