| Message ID | 1470924665-25860-3-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Aug 11, 2016 at 07:11:05AM -0700, Christoph Hellwig wrote: > ahci currently insists on an explicit call to pci_intx before falling back > from MSI or MSI-X to legacy irqs. As pci_intx is a no-op if the command > register already contains the right value is seems safe and useful to add > this call to pci_alloc_irq_vectors so that ahci can just use > pci_alloc_irq_vectors. Looking at ahci_init_interrupts() (and probably at commit d684a90d ("ahci: per-port msix support")) it looks like pci_alloc_irq_vectors() is able to preserve the current AHCI logic? > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/pci/msi.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index 9233e7f..593698e 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -1200,8 +1200,11 @@ int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > } > > /* use legacy irq if allowed */ > - if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) > + if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) { > + pci_intx(dev, 1); It would rather called pci_intx_for_msi() here. But because it is a generic code I am not sure what implications it has for all drivers out there. > return 1; > + } > + > return vecs; > } > EXPORT_SYMBOL(pci_alloc_irq_vectors); > -- > 2.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 18, 2016 at 11:20:07AM +0200, Alexander Gordeev wrote: > On Thu, Aug 11, 2016 at 07:11:05AM -0700, Christoph Hellwig wrote: > > ahci currently insists on an explicit call to pci_intx before falling back > > from MSI or MSI-X to legacy irqs. As pci_intx is a no-op if the command > > register already contains the right value is seems safe and useful to add > > this call to pci_alloc_irq_vectors so that ahci can just use > > pci_alloc_irq_vectors. > > Looking at ahci_init_interrupts() (and probably at commit d684a90d > ("ahci: per-port msix support")) it looks like pci_alloc_irq_vectors() > is able to preserve the current AHCI logic? ^^^^ is *not* able (sorry) > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > drivers/pci/msi.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > > index 9233e7f..593698e 100644 > > --- a/drivers/pci/msi.c > > +++ b/drivers/pci/msi.c > > @@ -1200,8 +1200,11 @@ int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > } > > > > /* use legacy irq if allowed */ > > - if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) > > + if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) { > > + pci_intx(dev, 1); > > It would rather called pci_intx_for_msi() here. But because it is > a generic code I am not sure what implications it has for all > drivers out there. > > > return 1; > > + } > > + > > return vecs; > > } > > EXPORT_SYMBOL(pci_alloc_irq_vectors); > > -- > > 2.1.4 > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 18, 2016 at 11:20:07AM +0200, Alexander Gordeev wrote: > On Thu, Aug 11, 2016 at 07:11:05AM -0700, Christoph Hellwig wrote: > > ahci currently insists on an explicit call to pci_intx before falling back > > from MSI or MSI-X to legacy irqs. As pci_intx is a no-op if the command > > register already contains the right value is seems safe and useful to add > > this call to pci_alloc_irq_vectors so that ahci can just use > > pci_alloc_irq_vectors. > > Looking at ahci_init_interrupts() (and probably at commit d684a90d > ("ahci: per-port msix support")) it looks like pci_alloc_irq_vectors() > is able to preserve the current AHCI logic? Not quite. For the currentl logic we need 3 calls to pci_alloc_irq_vectors, and I have a patch to implement that. From looking at the changelogs and intentions I think we can consolidate that down to two calls (per-port vectors and single vectors) and I will propose that as an RFC on top of the base which, which already is a huge simplification of the driver. > > @@ -1200,8 +1200,11 @@ int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > } > > > > /* use legacy irq if allowed */ > > - if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) > > + if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) { > > + pci_intx(dev, 1); > > It would rather called pci_intx_for_msi() here. But because it is > a generic code I am not sure what implications it has for all > drivers out there. It probably should be pci_intx_for_msi. For now I'm not touching drivers that need the quirk, so how about getting the intx in now so that the conversion can start, and I'll send a follow on to convert to pci_intx_for_msi with Cc to all the relevant parties for the quirk as a follow on? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 18, 2016 at 05:26:49PM +0200, Christoph Hellwig wrote: > It probably should be pci_intx_for_msi. For now I'm not touching > drivers that need the quirk, so how about getting the intx in > now so that the conversion can start, and I'll send a follow on > to convert to pci_intx_for_msi with Cc to all the relevant parties > for the quirk as a follow on? Probably should work. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 9233e7f..593698e 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -1200,8 +1200,11 @@ int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, } /* use legacy irq if allowed */ - if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) + if ((flags & PCI_IRQ_LEGACY) && min_vecs == 1) { + pci_intx(dev, 1); return 1; + } + return vecs; } EXPORT_SYMBOL(pci_alloc_irq_vectors);
ahci currently insists on an explicit call to pci_intx before falling back from MSI or MSI-X to legacy irqs. As pci_intx is a no-op if the command register already contains the right value is seems safe and useful to add this call to pci_alloc_irq_vectors so that ahci can just use pci_alloc_irq_vectors. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/pci/msi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)