Dear Andrew Murray,

On Tue, 29 Jan 2013 13:22:04 +0000, Andrew Murray wrote:

quoted

+static int __init mvebu_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{

[snip]

quoted

+
+       /*
+        * Build an laddr array that describes the PCI device in a DT
+        * way
+        */
+       laddr[0] = cpu_to_be32(port->devfn << 8);
+       laddr[1] = laddr[2] = 0;
+       intspec = cpu_to_be32(pin);
+
+       ret = of_irq_map_raw(port->dn, &intspec, 1, laddr, &oirq);
+       if (ret) {
+               dev_err(&pcie->pdev->dev,
+                       "%s: of_irq_map_raw() failed, %d\n",
+                       __func__, ret);
+               return ret;
+       }

Are you able to replace the above code with a call to of_irq_map_pci? I'm not
sure which approach is better. The of_irq_map_pci function doesn't require the
pin argument and instead uses the DT and/or performs its own pin swizzling. I
guess this means that if there are PCIe devices in the DT tree that does any
thing strange with pins then it would be reflected in the IRQ you get. I've
found that you will also need to provide an implementation of
pcibios_get_phb_of_node for this to work correctly (see my RFC bios32 patch).

I did try using the of_irq_map_pci() function, but unfortunately, it
didn't work. IIRC, it didn't work because none of the pci_dev in my PCI
tree had any 'struct device_node' associated to them, or at least not
the one that had the right pdev->bus->number and pdev->devfn.

But, I guess that your patch that implements pcibios_get_phb_of_node()
should fix this problem. I'll try this. Thanks!

quoted

+static int mvebu_pcie_init(void)
+{
+       return platform_driver_probe(&mvebu_pcie_driver,
+                                    mvebu_pcie_probe);
+}

If you have multiple 'mvebu-pcie' in your DT then you will end up
with multiple calls to
mvebu_pcie_probe/mvebu_pcie_enable/pci_common_init.

Right. In practice, there will only ever be a single DT node, since all
PCIe interfaces are sub-nodes of the PCI controller node. But I
understand the theoretical problem.

However pci_common_init/pcibios_init_hw assumes it will only ever be called
once, and will thus result in trying to create multiple busses with the same
bus number. (The first root bus it creates is always zero provided you haven't
implemented hw->scan).

I noticed this in Thierry's patch set and posted an RFC patch which overcomes
this issue (patchwork.kernel.org/patch/2001171) and others. Perhaps you would
want to include this in your patchset?

Sure, I'll give it a test, and report if it works for me.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com