diff mbox

[v2,1/2] PCI/portdrv: add support for different MSI interrupts for PCIe port services

Message ID 1495098098-1984-2-git-send-email-gabriele.paoloni@huawei.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Gabriele Paoloni May 18, 2017, 9:01 a.m. UTC 
Currently PCIe port services are assigned with different interrutps
only if MSI-x are supported by calling pcie_port_enable_msix().
If a root port supports MSI instead of MSI-x currently we fall back
to use a single shared interrupt for all the services.
This patch renames and extends pcie_port_enable_msix() to use MSI in
case MSI-x allocation fails.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 drivers/pci/pcie/portdrv.h      |  8 ++++---
 drivers/pci/pcie/portdrv_core.c | 46 +++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 21 deletions(-)

Comments

Christoph Hellwig May 18, 2017, 9:26 a.m. UTC | #1 
> - * pcie_port_enable_msix - try to set up MSI-X as interrupt mode for given port
> + * pcie_port_enable_msix_or_msi - try to set up MSI-X or MSI as interrupt mode
> + * for given port

just rename it to pcie_port_enable_vecs to avoid the overly long line..

> +		 * pci_irq_vector() below is able to handle entry differently
> +		 * depending on MSI vs MSI-x case

As mentioned in the review for the other patch:  no need to repeat
this for every user of pci_irq_vector.

Otherwise this looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Gabriele Paoloni May 18, 2017, 10:08 a.m. UTC | #2 
Hi Christoph

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Christoph Hellwig
> Sent: 18 May 2017 10:27
> To: Gabriele Paoloni
> Cc: bhelgaas@google.com; helgaas@kernel.org; Linuxarm; linux-
> pci@vger.kernel.org; lukas@wunner.de; linux-kernel@vger.kernel.org;
> mika.westerberg@linux.intel.com; hch@infradead.org; liudongdong (C)
> Subject: Re: [PATCH v2 1/2] PCI/portdrv: add support for different MSI
> interrupts for PCIe port services
> 
> > - * pcie_port_enable_msix - try to set up MSI-X as interrupt mode for
> given port
> > + * pcie_port_enable_msix_or_msi - try to set up MSI-X or MSI as
> interrupt mode
> > + * for given port
> 
> just rename it to pcie_port_enable_vecs to avoid the overly long line..

The function is actually renamed but I missed to update the comment, my bad.

> 
> > +		 * pci_irq_vector() below is able to handle entry
> differently
> > +		 * depending on MSI vs MSI-x case
> 
> As mentioned in the review for the other patch:  no need to repeat
> this for every user of pci_irq_vector.

I also missed to remove this comment

> 
> Otherwise this looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Many thanks
I'll fix comments and send out v3 straight

Cheers
Gab
diff mbox

Patch

diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 587aef3..1993e2c 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -13,10 +13,12 @@ 
 
 #define PCIE_PORT_DEVICE_MAXSERVICES   5
 /*
- * According to the PCI Express Base Specification 2.0, the indices of
- * the MSI-X table entries used by port services must not exceed 31
+ * According to the PCI Express Base Specification REV. 3.1 and according
+ * to the PCI Local Bus Specification REV. 3.0 respectively, the indices of
+ * the MSI-X table entries or the max number of MSI vectors used by port
+ * services must not exceed 31
  */
-#define PCIE_PORT_MAX_MSIX_ENTRIES	32
+#define PCIE_PORT_MAX_MSI_ENTRIES	32
 
 #define get_descriptor_id(type, service) (((type - 4) << 8) | service)
 
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index cea504f..254bc73 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -44,14 +44,15 @@  static void release_pcie_device(struct device *dev)
 }
 
 /**
- * pcie_port_enable_msix - try to set up MSI-X as interrupt mode for given port
+ * pcie_port_enable_msix_or_msi - try to set up MSI-X or MSI as interrupt mode
+ * for given port
  * @dev: PCI Express port to handle
  * @irqs: Array of interrupt vectors to populate
  * @mask: Bitmask of port capabilities returned by get_port_device_capability()
  *
  * Return value: 0 on success, error code on failure
  */
-static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
+static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask)
 {
 	int nr_entries, entry, nvec = 0;
 
@@ -61,8 +62,8 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
 	 * equal to the number of entries this port actually uses, we'll happily
 	 * go through without any tricks.
 	 */
-	nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSIX_ENTRIES,
-			PCI_IRQ_MSIX);
+	nr_entries = pci_alloc_irq_vectors(dev, 1, PCIE_PORT_MAX_MSI_ENTRIES,
+			PCI_IRQ_MSIX | PCI_IRQ_MSI);
 	if (nr_entries < 0)
 		return nr_entries;
 
@@ -77,7 +78,13 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
 		 * Number field in the PCI Express Capabilities register", where
 		 * according to Section 7.8.2 of the specification "For MSI-X,
 		 * the value in this field indicates which MSI-X Table entry is
-		 * used to generate the interrupt message."
+		 * used to generate the interrupt message." and "For MSI, the
+		 * value in this field indicates the offset between the base
+		 * Message Data and the interrupt message that is generated."
+		 *
+		 * pci_irq_vector() below is able to handle entry differently
+		 * depending on MSI vs MSI-x case
+		 *
 		 */
 		pcie_capability_read_word(dev, PCI_EXP_FLAGS, &reg16);
 		entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9;
@@ -100,7 +107,13 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
 		 * MSI/MSI-X vectors assigned to the port is going to be used
 		 * for AER, where "For MSI-X, the value in this register
 		 * indicates which MSI-X Table entry is used to generate the
-		 * interrupt message."
+		 * interrupt message."  and "For MSI, the value
+		 * in this field indicates the offset between the base Message
+		 * Data and the interrupt message that is generated."
+		 *
+		 * pci_irq_vector() below is able to handle entry differently
+		 * depending on MSI vs MSI-x case
+		 *
 		 */
 		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 		pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &reg32);
@@ -124,7 +137,7 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
 
 		/* Now allocate the MSI-X vectors for real */
 		nr_entries = pci_alloc_irq_vectors(dev, nvec, nvec,
-				PCI_IRQ_MSIX);
+				PCI_IRQ_MSIX | PCI_IRQ_MSI);
 		if (nr_entries < 0)
 			return nr_entries;
 	}
@@ -146,26 +159,23 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *irqs, int mask)
  */
 static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 {
-	unsigned flags = PCI_IRQ_LEGACY | PCI_IRQ_MSI;
 	int ret, i;
 
 	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
 		irqs[i] = -1;
 
 	/*
-	 * If MSI cannot be used for PCIe PME or hotplug, we have to use
-	 * INTx or other interrupts, e.g. system shared interrupt.
+	 * Make sure MSI can be used for PCIe PME or hotplug. otherwise we have
+	 * to use INTx or other interrupts, e.g. system shared interrupt.
 	 */
-	if (((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi()) ||
-	    ((mask & PCIE_PORT_SERVICE_HP) && pciehp_no_msi())) {
-		flags &= ~PCI_IRQ_MSI;
-	} else {
-		/* Try to use MSI-X if supported */
-		if (!pcie_port_enable_msix(dev, irqs, mask))
+	if (!((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi()) &&
+	    !((mask & PCIE_PORT_SERVICE_HP) && pciehp_no_msi()))
+		/* Try to use MSI-X or MSI if supported */
+		if (!pcie_port_enable_irq_vec(dev, irqs, mask))
 			return 0;
-	}
 
-	ret = pci_alloc_irq_vectors(dev, 1, 1, flags);
+	/* fall back to legacy IRQ */
+	ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_LEGACY);
 	if (ret < 0)
 		return -ENODEV;