Re: [PATCH 04/24] thunderbolt: Add MSI-X support

From: Mika Westerberg
Date: Mon May 22 2017 - 07:07:58 EST


On Mon, May 22, 2017 at 10:35:08AM +0000, Bernat, Yehezkel wrote:
>
>
> > -----Original Message-----
> > From: Mika Westerberg [mailto:mika.westerberg@xxxxxxxxxxxxxxx]
> > Sent: Monday, May 22, 2017 11:52
> > To: Andreas Noever <andreas.noever@xxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jamet, Michael
> > <michael.jamet@xxxxxxxxx>; Bernat, Yehezkel
> > <yehezkel.bernat@xxxxxxxxx>; Lukas Wunner <lukas@xxxxxxxxx>; Levy,
> > Amir (Jer) <amir.jer.levy@xxxxxxxxx>; Andy Lutomirski <luto@xxxxxxxxxx>;
> > Mario.Limonciello@xxxxxxxx; Jared.Dominguez@xxxxxxxx; Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH 04/24] thunderbolt: Add MSI-X support
> >
> > On Sun, May 21, 2017 at 07:51:09PM +0200, Andreas Noever wrote:
> > > On Thu, May 18, 2017 at 4:38 PM, Mika Westerberg
> > > <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> > > > Intel Thunderbolt controllers support up to 16 MSI-X vectors. Using
> > > Is that true for all generations? If so can we remove the legacy path?
> >
> > Yes, I think it has been the case from the beginning. I have here one
> > Mac Mini with Light Ridge and even it has working MSI-X.
> >
> > Yehezkel, Michael, Amir, do you know if that's the case for all other
> > controller as well?
>
> We are checking it, but what if we don't get the MSI-X we need?
> Don't we have to support MSI as a fallback option or maybe we are sure
> this will never happen in modern kernels?

Indeed, I think it makes sense to keep the fallback code just to be sure
it still works if for some reason we don't get the MSI-X vectors. I
think there is even a command line option to turn MSI-(X) off
completely.

Regarding this, I see that I'm missing a flag in the second call:

pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);

should probably be

pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_LEGACY);

instead to make it work with legacy IRQs as well.