Re: [PATCH 1/2] of_pci_irq: add a check to fallback to standard device tree parsing

[Benjamin Herrenschmidt]

On Tue, 2018-02-06 at 12:31 +0800, Ryder Lee wrote:
> On Tue, 2018-02-06 at 15:05 +1100, Benjamin Herrenschmidt wrote:
> > On Tue, 2018-02-06 at 10:38 +0800, Ryder Lee wrote:
> > > 
> > > I think the code should look at the bridge address <0x0800 ...> we list
> > > in bindings for resolving interrupts in this case, but it seems like it
> > > use the 'pdev->defvn << 8' which is not really we want and will lead to
> > > mismatch.
> > > 
> > > 		interrupt-map-mask = <0xf800 0 0 7>;
> > > 		interrupt-map = <0x0000 0 0 1 ...>,
> > > 				<0x0000 0 0 2 ...>,
> > > 				<0x0000 0 0 3 ...>,
> > > 				<0x0000 0 0 4 ...>,
> > > 
> > > 				 0x0800 0 0 1 ...>,
> > > 				 0x0800 0 0 2 ...>,
> > > 				 0x0800 0 0 3 ...>,
> > > 				 0x0800 0 0 4 ...>;
> > > 		...
> > > 		pcie@1,0 {
> > > 			reg = <0x0800 0 0 0 0>;
> > > 			...
> > > 		};
> > > 
> > > 
> > > Or, alternatively, we could add a interrupt-map property in both child
> > > and root node to solve this. The below example is my original version as
> > > I don't want to change that function either.
> > 
> > The code looks at devfn because it's meant to work for PCI including
> > when the devices dont have a device node in the DT.
> > 
> > What I'm trying to figure out is what is it that your parent and
> > children are representing here. Which is/are the root complex ?
> 
> This is a single root complex with 2 root port (children in DT).
> 
> > What is the actual topology as visible on the PCIe bus (is lspci output
> > basically) and how does that map to your representation ?
> 
> # lspci
> 00:00.0 Class 0604: 14c3:0801 //1st slot - pcie@0,0
> 00:01.0 Class 0604: 14c3:0801 //2nd slot - pcie@1,0
> 
> 01:00.0 Class 0280: 14c3:7603 //A device which is connected to 1st slot
> 02:00.0 Class 0200: 8086:1521 //A 4 func device which is connected to
> 2nd slot
> 02:00.1 Class 0200: 8086:1521
> 02:00.2 Class 0200: 8086:1521
> 02:00.3 Class 0200: 8086:1521

Ok so that's a rather standard setup. The "devfn << 8" of your root
ports should be the exact same thing as their first reg property entry,
I'm not sure why you have a mismatch here.

However, that map only represents the INTA..D lines going to the root
ports, not how these get mapped to children of the root ports.

of_irq_parse_pci() will implement standard swizzling if you don't have
nodes for your devices at all. If you do, however, the code assumes
you have a correct and complete interrupt tree in the device-tree.

That means that you need in each "p2p bridge", including your root
ports, an interrupt-map that will map the children INTA...D of that
bridge to the parent INTA...D of that bridge.

Alternatively, you can make the maps in the root ports point directly
to the parent PIC. If you chose to do that, then the interrupt-map in
your root complex becomes only useful to represent the root ports own
interrutps (hotplug, AER,...) and could be replaced by just having
interrupt-parent & interrupts in those root port nodes.

Cheers,
Ben.