Dear Andrew Murray,
On Tue, 29 Jan 2013 13:22:04 +0000, Andrew Murray wrote:
> > +static int __init mvebu_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> > +{
>
> [snip]
>
> > +
> > + /*
> > + * 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!
> > +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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
