Re: [PATCH v6 2/2] PCI: altera: support dt binding update

[matthew . gerlach]

On Tue, 11 Jun 2024, Bjorn Helgaas wrote:

> On Tue, Jun 11, 2024 at 11:35:25AM -0500, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
> > From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >
> > Add support for the device tree binding update. As part of
> > converting the binding document from text to yaml, with schema
> > validation, a device tree subnode was added to properly map
> > legacy interrupts. Maintain backward compatibility with previous binding.
>
> If something was *added* to the binding, I think it would be helpful
> to split that into two patches: (1) convert to YAML with zero
> functional changes, (2) add the new stuff.  Adding something at the
> same time as changing the format makes it hard to review.

Thanks for feedback. It was during the conversion to YAML that a problem 
with the original binding was discovered. As Rob Herring pointed out in
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240513205913.313592-1-matthew.gerlach@xxxxxxxxxxxxxxx/

"Making the PCI host the interrupt parent didn't even work in the kernel
 until somewhat recently (maybe a few years now). That's why a bunch of PCI
 hosts have an interrupt-controller child node."

This was an attempt to fix the problem. I can resubmit a conversion to YAML 
with zero functional changes.

Matthew Gerlach


> Then we could have a more specific subject and commit log for *this*
> patch.
>
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/pci/controller/pcie-altera.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
> > index a9536dc4bf96..88511fa2f078 100644
> > --- a/drivers/pci/controller/pcie-altera.c
> > +++ b/drivers/pci/controller/pcie-altera.c
> > @@ -667,11 +667,20 @@ static void altera_pcie_isr(struct irq_desc *desc)
> >  static int altera_pcie_init_irq_domain(struct altera_pcie *pcie)
> >  {
> >  	struct device *dev = &pcie->pdev->dev;
> > -	struct device_node *node = dev->of_node;
> > +	struct device_node *node, *child;
> >
> >  	/* Setup INTx */
> > +	child = of_get_next_child(dev->of_node, NULL);
> > +	if (child)
> > +		node = child;
> > +	else
> > +		node = dev->of_node;
> > +
> >  	pcie->irq_domain = irq_domain_add_linear(node, PCI_NUM_INTX,
> > -					&intx_domain_ops, pcie);
> > +						 &intx_domain_ops, pcie);
> > +	if (child)
> > +		of_node_put(child);
> > +
> >  	if (!pcie->irq_domain) {
> >  		dev_err(dev, "Failed to get a INTx IRQ domain\n");
> >  		return -ENOMEM;
> > --
> > 2.34.1