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

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Aug 12, 2025 at 02:41:15PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 07, 2025 at 01:10:19PM +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 host bridge 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
> > host bridge node now has multiple children. This ensures the correct
> > node is selected during initialization.
> ...

> >   * @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,28 @@ 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 __maybe_unused;
> > +
> > +	/*
> > +	 * This platform currently supports only one Root Port, so the loop
> > +	 * will execute only once.
> > +	 * TODO: Enhance the driver to handle multiple Root Ports in the future.
> > +	 */
> > +	for_each_child_of_node_with_prefix(dev->of_node, pcie_port_node, "pcie") {
> 
> This is only the second user of for_each_child_of_node_with_prefix()
> in the whole tree.  Also the only use of "__maybe_unused" in
> drivers/pci/controller/.
> 
> Most of the PCI controller drivers use
> for_each_available_child_of_node_scoped(); can we do the same here?

I suppose you used for_each_child_of_node_with_prefix() because, as
you mentioned in the commit log, there may be multiple children
(interrupt controller and Root Port(s))?

I think of_irq_parse_and_map_pci() takes care of much of the INTx
setup based on interrupt-map, so many drivers don't contain any
explicit INTx stuff.  But amd-mdb must be an exception for some
reason.

Most bindings don't include an interrupt-controller child in the pcie@
stanza.  I don't know enough about device tree to understand why
amd-mdb needs it.

Bjorn