[PATHC v1] PPC4xx: Adding PCI(E) MSI support

[Tirumala Marri]

Appreciate your review.
 > +     static int int_no = 0;  /* To remember last used interrupt */

>
> This is a worry. There is nothing AFAIK to stop two drivers (eg. network
> & scsi) calling into here at the same time, which could lead to
> corrupting int_no. If you just want a global counter you need a lock.
>
> But, AFAICS this is broken anyway. You never free the interrupt numbers,
> so you're going to run out. Some of your device tree entries only have 3
> (!!), so ifup/ifdown x 3 will exhaust the supply, won't it?
>
> I realise I never replied to your mail the other week about the bitmap
> code, so perhaps it's my fault :)
>
> [Marri] Good point.I will come up with a global array to see if the
interrupt nos are free
and use them.

 > +     return;

> > +}
> > +
> > +static int ppc4xx_msi_check_device(struct pci_dev *pdev, int nvec, int
> type)
> > +{
> > +     dev_dbg(&pdev->dev, "PCIE-MSI:%s called. vec %x type %d\n",
> > +             __func__, nvec, type);
>
> No constraints at all? What about MSI-X ?
> [marri] That will be another patch as it is only specific to some SoCs
>


> > +     msi_virt = dma_alloc_coherent(&dev->dev, 64, &msi_phys,
> GFP_KERNEL);
> > +     msi->msi_addr_hi = 0x0;
> > +     msi->msi_addr_lo = (u32) msi_phys;
> > +     dev_dbg(&dev->dev, "PCIE-MSI: msi address 0x%x \n",
> msi->msi_addr_lo);
>
> So your MSI is a write to just some arbitrary address?
>
> [Marri] We choose a arbitrary address and pass that to msi APIs which would
write
to endpoint device config space. If endpoint wants to cause an MSI it just
generates
memory transaction to this address, as soon as it is on system bus PCI-E
handler
will snoop this address and cause interrupt to CPU.




> > +     /* Progam the Interrupt handler Termination addr registers */
> > +     out_be32(msi->msi_regs + PEIH_TERMADH, msi->msi_addr_hi);
> > +     out_be32(msi->msi_regs + PEIH_TERMADL, msi->msi_addr_lo);
>
> And the hardware detects it by catching writes to that address? But the
> write still lands?
>
> [marri] write still happens to memory but we do't need need to wait for it
to complete.
So the termination would avoid un-necessary delay in acking the write or
race condition
to that particular address.


>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20101104/b792190a/attachment-0001.html>