All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
@ 2021-08-13 16:02 Rob Herring
  2021-08-13 17:19 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2021-08-13 16:02 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-pci, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
buses fails resulting in the following warnings:

WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c

The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
the passed in bus even if it's not the host bridge's bus. Based on the
name of the function, that's clearly not what we want. Fix this by
walking the bus parents to the root bus.

Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
Compile tested only. Mauro, Can you see if this fixes your issue.

 drivers/pci/of.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a143b02b2dcd..ea70aede054c 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
 	if (!bus->dev.of_node)
 		return NULL;
 
+	/* Find the host bridge bus */
+	while (!pci_is_root_bus(bus))
+		bus = bus->parent;
+
 	/* Start looking for a phandle to an MSI controller. */
 	d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
 	if (d)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-13 16:02 [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes Rob Herring
@ 2021-08-13 17:19 ` Mauro Carvalho Chehab
  2021-08-14 17:32   ` Rob Herring
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-13 17:19 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-pci, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

Em Fri, 13 Aug 2021 11:02:57 -0500
Rob Herring <robh@kernel.org> escreveu:

> When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> buses fails resulting in the following warnings:
> 
> WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> 
> The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> the passed in bus even if it's not the host bridge's bus. Based on the
> name of the function, that's clearly not what we want. Fix this by
> walking the bus parents to the root bus.
> 
> Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Compile tested only. Mauro, Can you see if this fixes your issue.
> 
>  drivers/pci/of.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a143b02b2dcd..ea70aede054c 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
>  	if (!bus->dev.of_node)
>  		return NULL;
>  
> +	/* Find the host bridge bus */
> +	while (!pci_is_root_bus(bus))
> +		bus = bus->parent;
> +
>  	/* Start looking for a phandle to an MSI controller. */
>  	d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
>  	if (d)

Nope, it didn't solve the issue.


Thanks,
Mauro

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-13 17:19 ` Mauro Carvalho Chehab
@ 2021-08-14 17:32   ` Rob Herring
  2021-08-16  8:42     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2021-08-14 17:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: PCI, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

On Fri, Aug 13, 2021 at 12:19 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Fri, 13 Aug 2021 11:02:57 -0500
> Rob Herring <robh@kernel.org> escreveu:
>
> > When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> > buses fails resulting in the following warnings:
> >
> > WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> >
> > The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> > the passed in bus even if it's not the host bridge's bus. Based on the
> > name of the function, that's clearly not what we want. Fix this by
> > walking the bus parents to the root bus.
> >
> > Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > Compile tested only. Mauro, Can you see if this fixes your issue.
> >
> >  drivers/pci/of.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index a143b02b2dcd..ea70aede054c 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
> >       if (!bus->dev.of_node)
> >               return NULL;
> >
> > +     /* Find the host bridge bus */
> > +     while (!pci_is_root_bus(bus))
> > +             bus = bus->parent;
> > +
> >       /* Start looking for a phandle to an MSI controller. */
> >       d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
> >       if (d)
>
> Nope, it didn't solve the issue.

Can you try adding some prints of the domain, pci dev, and DT node to
pci_set_bus_msi_domain(). Comparing those when having child nodes or
not would be helpful.

Rob

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-14 17:32   ` Rob Herring
@ 2021-08-16  8:42     ` Mauro Carvalho Chehab
  2021-08-16 13:36       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-16  8:42 UTC (permalink / raw)
  To: Rob Herring; +Cc: PCI, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

Em Sat, 14 Aug 2021 12:32:56 -0500
Rob Herring <robh@kernel.org> escreveu:

> On Fri, Aug 13, 2021 at 12:19 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Fri, 13 Aug 2021 11:02:57 -0500
> > Rob Herring <robh@kernel.org> escreveu:
> >  
> > > When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> > > buses fails resulting in the following warnings:
> > >
> > > WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> > >
> > > The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> > > the passed in bus even if it's not the host bridge's bus. Based on the
> > > name of the function, that's clearly not what we want. Fix this by
> > > walking the bus parents to the root bus.
> > >
> > > Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > > Compile tested only. Mauro, Can you see if this fixes your issue.
> > >
> > >  drivers/pci/of.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > index a143b02b2dcd..ea70aede054c 100644
> > > --- a/drivers/pci/of.c
> > > +++ b/drivers/pci/of.c
> > > @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
> > >       if (!bus->dev.of_node)
> > >               return NULL;
> > >
> > > +     /* Find the host bridge bus */
> > > +     while (!pci_is_root_bus(bus))
> > > +             bus = bus->parent;
> > > +
> > >       /* Start looking for a phandle to an MSI controller. */
> > >       d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
> > >       if (d)  
> >
> > Nope, it didn't solve the issue.  
> 
> Can you try adding some prints of the domain, pci dev, and DT node to
> pci_set_bus_msi_domain(). Comparing those when having child nodes or
> not would be helpful.

Debug patch enclosed.

This is what happens when msi-parent is at /soc/pcie@f4000000/pcie@0,0:

[    4.305247] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: ffff000104a2f6c0
[    4.442212] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: ffff000104a2f6c0
[    4.688145] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: ffff000104a2f6c0
[    4.800613] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
[    4.856254] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: ffff000104a2f6c0
[    4.922117] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: ffff000104a2f6c0
[    5.032708] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0

And this is what happens when msi-parent is at /soc/pcie@f4000000
(either with or without your patch applied):

[    4.120328] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.214541] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.226054] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.478858] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: 0000000000000000
[    4.491218] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.502793] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.588196] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
[    4.597161] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.608747] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.658892] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: 0000000000000000
[    4.671241] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.682869] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.732938] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: 0000000000000000
[    4.745305] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.756880] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
[    4.850363] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
[    4.859322] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
[    4.870895] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000

Btw, despite lspci works on both cases, the Ethernet adapter stops
working when msi-parent is at /soc/pcie@f4000000. I didn't notice it
before, as (up to last week) I was using WiFi to connect to this board.

Thanks,
Mauro

[PATCH] PCI: probe: Add a debug printk at pci_set_bus_msi_domain

That helps to discover problems when trying to get the MSI IRQ
domain.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c5dfc1afb1d3..f73bd81922b3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -866,6 +866,8 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
        for (b = bus, d = NULL; !d && !pci_is_root_bus(b); b = b->parent) {
                if (b->self)
                        d = dev_get_msi_domain(&b->self->dev);
+               dev_dbg(&b->dev, "%s: of_node %pOF: IRQ domain: %px\n",
+                       __func__, b->dev.of_node, d);
        }
 
        if (!d)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-16  8:42     ` Mauro Carvalho Chehab
@ 2021-08-16 13:36       ` Mauro Carvalho Chehab
  2021-08-16 19:01         ` Rob Herring
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-16 13:36 UTC (permalink / raw)
  To: Rob Herring; +Cc: PCI, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

Em Mon, 16 Aug 2021 10:42:52 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Sat, 14 Aug 2021 12:32:56 -0500
> Rob Herring <robh@kernel.org> escreveu:
> 
> > On Fri, Aug 13, 2021 at 12:19 PM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:  
> > >
> > > Em Fri, 13 Aug 2021 11:02:57 -0500
> > > Rob Herring <robh@kernel.org> escreveu:
> > >    
> > > > When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> > > > buses fails resulting in the following warnings:
> > > >
> > > > WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> > > >
> > > > The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> > > > the passed in bus even if it's not the host bridge's bus. Based on the
> > > > name of the function, that's clearly not what we want. Fix this by
> > > > walking the bus parents to the root bus.
> > > >
> > > > Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > > Compile tested only. Mauro, Can you see if this fixes your issue.
> > > >
> > > >  drivers/pci/of.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > index a143b02b2dcd..ea70aede054c 100644
> > > > --- a/drivers/pci/of.c
> > > > +++ b/drivers/pci/of.c
> > > > @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
> > > >       if (!bus->dev.of_node)
> > > >               return NULL;
> > > >
> > > > +     /* Find the host bridge bus */
> > > > +     while (!pci_is_root_bus(bus))
> > > > +             bus = bus->parent;
> > > > +
> > > >       /* Start looking for a phandle to an MSI controller. */
> > > >       d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
> > > >       if (d)    
> > >
> > > Nope, it didn't solve the issue.    
> > 
> > Can you try adding some prints of the domain, pci dev, and DT node to
> > pci_set_bus_msi_domain(). Comparing those when having child nodes or
> > not would be helpful.  
> 
> Debug patch enclosed.
> 
> This is what happens when msi-parent is at /soc/pcie@f4000000/pcie@0,0:
> 
> [    4.305247] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: ffff000104a2f6c0
> [    4.442212] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: ffff000104a2f6c0
> [    4.688145] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: ffff000104a2f6c0
> [    4.800613] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> [    4.856254] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: ffff000104a2f6c0
> [    4.922117] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: ffff000104a2f6c0
> [    5.032708] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> 
> And this is what happens when msi-parent is at /soc/pcie@f4000000
> (either with or without your patch applied):
> 
> [    4.120328] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.214541] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.226054] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.478858] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: 0000000000000000
> [    4.491218] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.502793] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.588196] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> [    4.597161] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.608747] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.658892] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: 0000000000000000
> [    4.671241] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.682869] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.732938] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: 0000000000000000
> [    4.745305] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.756880] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> [    4.850363] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> [    4.859322] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> [    4.870895] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> 
> Btw, despite lspci works on both cases, the Ethernet adapter stops
> working when msi-parent is at /soc/pcie@f4000000. I didn't notice it
> before, as (up to last week) I was using WiFi to connect to this board.
> 
> Thanks,
> Mauro
> 
> [PATCH] PCI: probe: Add a debug printk at pci_set_bus_msi_domain
> 
> That helps to discover problems when trying to get the MSI IRQ
> domain.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c5dfc1afb1d3..f73bd81922b3 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -866,6 +866,8 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
>         for (b = bus, d = NULL; !d && !pci_is_root_bus(b); b = b->parent) {
>                 if (b->self)
>                         d = dev_get_msi_domain(&b->self->dev);
> +               dev_dbg(&b->dev, "%s: of_node %pOF: IRQ domain: %px\n",
> +                       __func__, b->dev.of_node, d);
>         }
>  
>         if (!d)
> 

I added a lot of other debug stuff (see attached).

When msi-parent is at:

	pcie@f4000000 {
		msi-parent = <&its_pcie>;
	};

It produces:

	 (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	 (null): pci_host_bridge_of_msi_domain: of_node /soc/pcie@f4000000
	irq: irq_find_matching_fwspec: of_node: /soc/interrupt-controller@f5100000: 0000000000000000
	 (null): of_msi_get_domain: (1) of_node /soc/pcie@f4000000 domain: 0000000000000000
	 (null): pci_host_bridge_of_msi_domain: of_msi_get_domain(): of_node /soc/pcie@f4000000 domain: 0000000000000000
	irq: irq_find_matching_fwspec: of_node: /soc/pcie@f4000000: 0000000000000000
	 (null): pci_host_bridge_of_msi_domain: irq_find_matching_host(): of_node /soc/pcie@f4000000 domain: 0000000000000000
	irq: irq_find_matching_fwspec: of_node: /soc/pcie@f4000000: 0000000000000000
	irq: irq_find_matching_fwspec: of_node: /soc/pcie@f4000000: 0000000000000000

When msi-parent is at:

	pcie@f4000000 {
		pcie@0,0 {
			msi-parent = <&its_pcie>;
		};
	};

It produces:
	 (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	 (null): pci_host_bridge_of_msi_domain: of_node /soc/pcie@f4000000
	 (null): pci_host_bridge_of_msi_domain: of_msi_get_domain(): of_node /soc/pcie@f4000000 domain: 0000000000000000
	irq: irq_find_matching_fwspec: of_node: /soc/pcie@f4000000: ffff000107e57600
	 (null): pci_host_bridge_of_msi_domain: irq_find_matching_host(): of_node /soc/pcie@f4000000 domain: ffff000107e57600

I'm starting to suspect that the problem is somewhere inside the 
OF-specific code which handles MSI IRQs (drivers/of/irq.c?), but I didn't
find yet the root cause.

Thanks,
Mauro

---

Debug patch:

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 352e14b007e7..1b710513ecb3 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -623,10 +623,16 @@ u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32 id_in)
 struct irq_domain *of_msi_map_get_device_domain(struct device *dev, u32 id,
 						u32 bus_token)
 {
+	struct irq_domain *d;
 	struct device_node *np = NULL;
 
 	__of_msi_map_id(dev, &np, id);
-	return irq_find_matching_host(np, bus_token);
+	d = irq_find_matching_host(np, bus_token);
+
+	dev_info(dev, "%s: of_node %pOF domain: %px\n",
+		__func__, np, d);
+
+	return d;
 }
 
 /**
@@ -649,8 +655,13 @@ struct irq_domain *of_msi_get_domain(struct device *dev,
 
 	/* Check for a single msi-parent property */
 	msi_np = of_parse_phandle(np, "msi-parent", 0);
+	dev_info(dev, "%s: msi parent: %pOF, token %i\n",
+		__func__, msi_np, token);
 	if (msi_np && !of_property_read_bool(msi_np, "#msi-cells")) {
 		d = irq_find_matching_host(msi_np, token);
+		dev_info(dev, "%s: (1) of_node %pOF domain: %px\n",
+			__func__, np, d);
+
 		if (!d)
 			of_node_put(msi_np);
 		return d;
@@ -665,6 +676,8 @@ struct irq_domain *of_msi_get_domain(struct device *dev,
 						   "#msi-cells",
 						   index, &args)) {
 			d = irq_find_matching_host(args.np, token);
+			dev_info(dev, "%s: (2) of_node %pOF domain: %px\n",
+				__func__, np, d);
 			if (d)
 				return d;
 
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 9232255c8515..19fbe50a6f82 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1552,6 +1552,8 @@ struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev)
 	if (!dom)
 		dom = iort_get_device_domain(&pdev->dev, rid,
 					     DOMAIN_BUS_PCI_MSI);
+	dev_dbg(&pdev->dev, "%s: of_node %pOF domain: %px\n",
+		__func__, pdev->dev.of_node, dom);
 	return dom;
 }
 
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index f5fc9e29a725..33b9b06ebb5d 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -20,7 +20,7 @@ void pci_set_of_node(struct pci_dev *dev)
 {
 	if (!dev->bus->dev.of_node) {
 		dev_dbg(&dev->dev, "%s: BUS of_node is null\n",
-			__func__, dev->bus->dev.of_node);
+			__func__);
 		return;
 	}
 	dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
@@ -88,11 +88,22 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
 #ifdef CONFIG_IRQ_DOMAIN
 	struct irq_domain *d;
 
+dev_dbg(&bus->dev, "%s: of_node %pOF\n", __func__, bus->dev.of_node);
 	if (!bus->dev.of_node)
 		return NULL;
 
+	/* Find the host bridge bus */
+//	while (bus->bridge->parent) {
+	while (!pci_is_root_bus(bus)) {
+		dev_dbg(&bus->dev, "%s: of_node %pOF is not the root bus\n",
+			__func__, bus->dev.of_node);
+		bus = bus->parent;
+	}
+
 	/* Start looking for a phandle to an MSI controller. */
 	d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
+	dev_dbg(&bus->dev, "%s: of_msi_get_domain(): of_node %pOF domain: %px\n",
+		__func__, bus->dev.of_node, d);
 	if (d)
 		return d;
 
@@ -101,6 +112,8 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
 	 * directly attached to the host bridge.
 	 */
 	d = irq_find_matching_host(bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
+	dev_dbg(&bus->dev, "%s: irq_find_matching_host(): of_node %pOF domain: %px\n",
+		__func__, bus->dev.of_node, d);
 	if (d)
 		return d;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c5dfc1afb1d3..3488ed6c9624 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -866,6 +866,8 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
 	for (b = bus, d = NULL; !d && !pci_is_root_bus(b); b = b->parent) {
 		if (b->self)
 			d = dev_get_msi_domain(&b->self->dev);
+		dev_dbg(&b->dev, "%s: of_node %pOF: IRQ domain: %px\n",
+			__func__, b->dev.of_node, d);
 	}
 
 	if (!d)
@@ -2447,17 +2449,18 @@ static struct irq_domain *pci_dev_msi_domain(struct pci_dev *dev)
 	 */
 	d = dev_get_msi_domain(&dev->dev);
 	if (d)
-		return d;
+		goto ret;
 
 	/*
 	 * Let's see if we have a firmware interface able to provide
 	 * the domain.
 	 */
 	d = pci_msi_get_device_domain(dev);
-	if (d)
-		return d;
+ret:
+	dev_dbg(&dev->dev, "%s: of_node %pOF domain: %px\n",
+		__func__, dev->dev.of_node, d);
 
-	return NULL;
+	return d;
 }
 
 static void pci_set_msi_domain(struct pci_dev *dev)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-16 13:36       ` Mauro Carvalho Chehab
@ 2021-08-16 19:01         ` Rob Herring
  2021-08-16 19:47           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2021-08-16 19:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: PCI, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

On Mon, Aug 16, 2021 at 8:36 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Mon, 16 Aug 2021 10:42:52 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
>
> > Em Sat, 14 Aug 2021 12:32:56 -0500
> > Rob Herring <robh@kernel.org> escreveu:
> >
> > > On Fri, Aug 13, 2021 at 12:19 PM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:
> > > >
> > > > Em Fri, 13 Aug 2021 11:02:57 -0500
> > > > Rob Herring <robh@kernel.org> escreveu:
> > > >
> > > > > When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> > > > > buses fails resulting in the following warnings:
> > > > >
> > > > > WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> > > > >
> > > > > The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> > > > > the passed in bus even if it's not the host bridge's bus. Based on the
> > > > > name of the function, that's clearly not what we want. Fix this by
> > > > > walking the bus parents to the root bus.
> > > > >
> > > > > Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > > > Compile tested only. Mauro, Can you see if this fixes your issue.
> > > > >
> > > > >  drivers/pci/of.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > > index a143b02b2dcd..ea70aede054c 100644
> > > > > --- a/drivers/pci/of.c
> > > > > +++ b/drivers/pci/of.c
> > > > > @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
> > > > >       if (!bus->dev.of_node)
> > > > >               return NULL;
> > > > >
> > > > > +     /* Find the host bridge bus */
> > > > > +     while (!pci_is_root_bus(bus))
> > > > > +             bus = bus->parent;
> > > > > +
> > > > >       /* Start looking for a phandle to an MSI controller. */
> > > > >       d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
> > > > >       if (d)
> > > >
> > > > Nope, it didn't solve the issue.
> > >
> > > Can you try adding some prints of the domain, pci dev, and DT node to
> > > pci_set_bus_msi_domain(). Comparing those when having child nodes or
> > > not would be helpful.
> >
> > Debug patch enclosed.
> >
> > This is what happens when msi-parent is at /soc/pcie@f4000000/pcie@0,0:
> >
> > [    4.305247] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: ffff000104a2f6c0
> > [    4.442212] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: ffff000104a2f6c0
> > [    4.688145] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: ffff000104a2f6c0
> > [    4.800613] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> > [    4.856254] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: ffff000104a2f6c0
> > [    4.922117] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: ffff000104a2f6c0
> > [    5.032708] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> >
> > And this is what happens when msi-parent is at /soc/pcie@f4000000
> > (either with or without your patch applied):
> >
> > [    4.120328] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.214541] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.226054] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.478858] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: 0000000000000000
> > [    4.491218] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.502793] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.588196] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> > [    4.597161] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.608747] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.658892] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: 0000000000000000
> > [    4.671241] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.682869] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.732938] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: 0000000000000000
> > [    4.745305] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.756880] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.850363] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> > [    4.859322] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > [    4.870895] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> >
> > Btw, despite lspci works on both cases, the Ethernet adapter stops
> > working when msi-parent is at /soc/pcie@f4000000. I didn't notice it
> > before, as (up to last week) I was using WiFi to connect to this board.
> >
> > Thanks,
> > Mauro
> >
> > [PATCH] PCI: probe: Add a debug printk at pci_set_bus_msi_domain
> >
> > That helps to discover problems when trying to get the MSI IRQ
> > domain.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index c5dfc1afb1d3..f73bd81922b3 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -866,6 +866,8 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
> >         for (b = bus, d = NULL; !d && !pci_is_root_bus(b); b = b->parent) {
> >                 if (b->self)
> >                         d = dev_get_msi_domain(&b->self->dev);
> > +               dev_dbg(&b->dev, "%s: of_node %pOF: IRQ domain: %px\n",
> > +                       __func__, b->dev.of_node, d);
> >         }
> >
> >         if (!d)
> >
>
> I added a lot of other debug stuff (see attached).
>
> When msi-parent is at:
>
>         pcie@f4000000 {
>                 msi-parent = <&its_pcie>;
>         };

Are you sure the IRQ domain got created for its_pcie? From your dts
changes, you have a GICv3-ITS without a GICv3. I don't think that's
right.

The DWC driver will skip setting up it's own MSI controller if
'msi-parent' is present in the DWC node. So if you move it out of that
node, then the DWC MSI controller will be found and used. Try removing
'msi-parent' and the its_pcie node.

Rob

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes
  2021-08-16 19:01         ` Rob Herring
@ 2021-08-16 19:47           ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-16 19:47 UTC (permalink / raw)
  To: Rob Herring; +Cc: PCI, Marc Zyngier, Bjorn Helgaas, Lorenzo Pieralisi

Em Mon, 16 Aug 2021 14:01:06 -0500
Rob Herring <robh@kernel.org> escreveu:

> On Mon, Aug 16, 2021 at 8:36 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Mon, 16 Aug 2021 10:42:52 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> >  
> > > Em Sat, 14 Aug 2021 12:32:56 -0500
> > > Rob Herring <robh@kernel.org> escreveu:
> > >  
> > > > On Fri, Aug 13, 2021 at 12:19 PM Mauro Carvalho Chehab
> > > > <mchehab+huawei@kernel.org> wrote:  
> > > > >
> > > > > Em Fri, 13 Aug 2021 11:02:57 -0500
> > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > >  
> > > > > > When a DT contains PCI child bus nodes, lookup of the MSI domain on PCI
> > > > > > buses fails resulting in the following warnings:
> > > > > >
> > > > > > WARNING: CPU: 4 PID: 7 at include/linux/msi.h:256 __pci_enable_msi_range+0x398/0x59c
> > > > > >
> > > > > > The issue is that pci_host_bridge_of_msi_domain() will check the DT node of
> > > > > > the passed in bus even if it's not the host bridge's bus. Based on the
> > > > > > name of the function, that's clearly not what we want. Fix this by
> > > > > > walking the bus parents to the root bus.
> > > > > >
> > > > > > Reported-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > > ---
> > > > > > Compile tested only. Mauro, Can you see if this fixes your issue.
> > > > > >
> > > > > >  drivers/pci/of.c | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > > > index a143b02b2dcd..ea70aede054c 100644
> > > > > > --- a/drivers/pci/of.c
> > > > > > +++ b/drivers/pci/of.c
> > > > > > @@ -84,6 +84,10 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
> > > > > >       if (!bus->dev.of_node)
> > > > > >               return NULL;
> > > > > >
> > > > > > +     /* Find the host bridge bus */
> > > > > > +     while (!pci_is_root_bus(bus))
> > > > > > +             bus = bus->parent;
> > > > > > +
> > > > > >       /* Start looking for a phandle to an MSI controller. */
> > > > > >       d = of_msi_get_domain(&bus->dev, bus->dev.of_node, DOMAIN_BUS_PCI_MSI);
> > > > > >       if (d)  
> > > > >
> > > > > Nope, it didn't solve the issue.  
> > > >
> > > > Can you try adding some prints of the domain, pci dev, and DT node to
> > > > pci_set_bus_msi_domain(). Comparing those when having child nodes or
> > > > not would be helpful.  
> > >
> > > Debug patch enclosed.
> > >
> > > This is what happens when msi-parent is at /soc/pcie@f4000000/pcie@0,0:
> > >
> > > [    4.305247] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: ffff000104a2f6c0
> > > [    4.442212] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: ffff000104a2f6c0
> > > [    4.688145] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: ffff000104a2f6c0
> > > [    4.800613] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> > > [    4.856254] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: ffff000104a2f6c0
> > > [    4.922117] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: ffff000104a2f6c0
> > > [    5.032708] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: ffff000104a2f6c0
> > >
> > > And this is what happens when msi-parent is at /soc/pcie@f4000000
> > > (either with or without your patch applied):
> > >
> > > [    4.120328] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.214541] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.226054] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.478858] pci_bus 0000:03: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0: IRQ domain: 0000000000000000
> > > [    4.491218] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.502793] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.588196] pci_bus 0000:04: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> > > [    4.597161] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.608747] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.658892] pci_bus 0000:05: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0: IRQ domain: 0000000000000000
> > > [    4.671241] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.682869] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.732938] pci_bus 0000:06: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0: IRQ domain: 0000000000000000
> > > [    4.745305] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.756880] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.850363] pci_bus 0000:07: pci_set_bus_msi_domain: of_node (null): IRQ domain: 0000000000000000
> > > [    4.859322] pci_bus 0000:02: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0/pcie@0,0: IRQ domain: 0000000000000000
> > > [    4.870895] pci_bus 0000:01: pci_set_bus_msi_domain: of_node /soc/pcie@f4000000/pcie@0,0: IRQ domain: 0000000000000000
> > >
> > > Btw, despite lspci works on both cases, the Ethernet adapter stops
> > > working when msi-parent is at /soc/pcie@f4000000. I didn't notice it
> > > before, as (up to last week) I was using WiFi to connect to this board.
> > >
> > > Thanks,
> > > Mauro
> > >
> > > [PATCH] PCI: probe: Add a debug printk at pci_set_bus_msi_domain
> > >
> > > That helps to discover problems when trying to get the MSI IRQ
> > > domain.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > >
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index c5dfc1afb1d3..f73bd81922b3 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -866,6 +866,8 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
> > >         for (b = bus, d = NULL; !d && !pci_is_root_bus(b); b = b->parent) {
> > >                 if (b->self)
> > >                         d = dev_get_msi_domain(&b->self->dev);
> > > +               dev_dbg(&b->dev, "%s: of_node %pOF: IRQ domain: %px\n",
> > > +                       __func__, b->dev.of_node, d);
> > >         }
> > >
> > >         if (!d)
> > >  
> >
> > I added a lot of other debug stuff (see attached).
> >
> > When msi-parent is at:
> >
> >         pcie@f4000000 {
> >                 msi-parent = <&its_pcie>;
> >         };  
> 
> Are you sure the IRQ domain got created for its_pcie? From your dts
> changes, you have a GICv3-ITS without a GICv3. I don't think that's
> right.
> 
> The DWC driver will skip setting up it's own MSI controller if
> 'msi-parent' is present in the DWC node. So if you move it out of that
> node, then the DWC MSI controller will be found and used. Try removing
> 'msi-parent' and the its_pcie node.

You're right, thanks!

The enclosed diff is enough for it to cleanup the PCIe logs, while keeping 
the hardware working.

I'll append it to the DTS patch and re-send you the updated DT schema
patches.


diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 89d3b07456c3..98972938390d 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -666,12 +666,6 @@ gpio28: gpio@fff1d000 {
 			clock-names = "apb_pclk";
 		};
 
-		its_pcie: interrupt-controller@f5100000 {
-			reg = <0x0 0xf5100000 0x0 0x100000>;
-			compatible = "arm,gic-v3-its";
-			msi-controller;
-		};
-
 		pcie_phy: pcie-phy@fc000000 {
 			compatible = "hisilicon,hi970-pcie-phy";
 			reg = <0x0 0xfc000000 0x0 0x80000>;
@@ -731,7 +725,6 @@ pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
-				msi-parent = <&its_pcie>;
 
 				pcie@0,0 { // Lane 0: upstream
 					reg = <0 0 0 0 0>;





Thanks,
Mauro

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-16 19:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 16:02 [PATCH] PCI: of: Fix MSI domain lookup with child bus nodes Rob Herring
2021-08-13 17:19 ` Mauro Carvalho Chehab
2021-08-14 17:32   ` Rob Herring
2021-08-16  8:42     ` Mauro Carvalho Chehab
2021-08-16 13:36       ` Mauro Carvalho Chehab
2021-08-16 19:01         ` Rob Herring
2021-08-16 19:47           ` Mauro Carvalho Chehab

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.