diff mbox

[1/5] net: thunderx: switch to pci_alloc_irq_vectors

Message ID 20170327082936.6830-2-hch@lst.de (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Christoph Hellwig March 27, 2017, 8:29 a.m. UTC 
Remove the deprecated pci_enable_msix API in favour of it's successor.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/net/ethernet/cavium/thunder/nic_main.c | 75 ++++++--------------------
 1 file changed, 15 insertions(+), 60 deletions(-)

Comments

Sunil Kovvuri March 27, 2017, 4:03 p.m. UTC | #1 
On Mon, Mar 27, 2017 at 1:59 PM, Christoph Hellwig <hch@lst.de> wrote:
> Remove the deprecated pci_enable_msix API in favour of it's successor.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/net/ethernet/cavium/thunder/nic_main.c | 75 ++++++--------------------
>  1 file changed, 15 insertions(+), 60 deletions(-)
> -       nic->num_vec = pci_msix_vec_count(nic->pdev);
> -
> -       ret = pci_enable_msix(nic->pdev, nic->msix_entries, nic->num_vec);
> -       if (ret) {
> +       /* Enable MSI-X */
> +       ret = pci_alloc_irq_vectors(nic->pdev, nic->num_vec, nic->num_vec,
> +                       PCI_IRQ_MSIX);
nic->num_vec will always be zero here.

> +       while (--i >= 0)
> +               free_irq(pci_irq_vector(nic->pdev, i), nic);
> +       pci_free_irq_vectors(nic->pdev);
>         return ret;
IRQ handlers are not registered for all vectors to free them unconditionally

> +       int i;
> +
> +       for (i = 0; i < nic->num_vec; i++)
> +               free_irq(pci_irq_vector(nic->pdev, i), nic);
> +       pci_free_irq_vectors(nic->pdev);
>  }
Same as here.

How urgent is this ?
Can we comeup with a proper patch to fix this in couple of weeks, instead of
going with these untested patches ?
Christoph Hellwig March 27, 2017, 5:12 p.m. UTC | #2 
On Mon, Mar 27, 2017 at 09:33:32PM +0530, Sunil Kovvuri wrote:
> How urgent is this ?

I would like to get this into 4.12 to stop people from adding new users of
pci_enable_msix.

> Can we comeup with a proper patch to fix this in couple of weeks, instead of
> going with these untested patches ?

Sure.
diff mbox

Patch

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 767234e2e8f9..205569db9093 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -65,10 +65,7 @@  struct nicpf {
 	bool			mbx_lock[MAX_NUM_VFS_SUPPORTED];
 
 	/* MSI-X */
-	bool			msix_enabled;
 	u8			num_vec;
-	struct msix_entry	*msix_entries;
-	bool			irq_allocated[NIC_PF_MSIX_VECTORS];
 	char			irq_name[NIC_PF_MSIX_VECTORS][20];
 };
 
@@ -1088,7 +1085,7 @@  static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
 	u64 intr;
 	u8  vf, vf_per_mbx_reg = 64;
 
-	if (irq == nic->msix_entries[NIC_PF_INTR_ID_MBOX0].vector)
+	if (irq == pci_irq_vector(nic->pdev, NIC_PF_INTR_ID_MBOX0))
 		mbx = 0;
 	else
 		mbx = 1;
@@ -1107,76 +1104,30 @@  static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
 	return IRQ_HANDLED;
 }
 
-static int nic_enable_msix(struct nicpf *nic)
+static int nic_register_interrupts(struct nicpf *nic)
 {
 	int i, ret;
 
-	nic->num_vec = pci_msix_vec_count(nic->pdev);
-
-	nic->msix_entries = kmalloc_array(nic->num_vec,
-					  sizeof(struct msix_entry),
-					  GFP_KERNEL);
-	if (!nic->msix_entries)
-		return -ENOMEM;
-
-	for (i = 0; i < nic->num_vec; i++)
-		nic->msix_entries[i].entry = i;
-
-	ret = pci_enable_msix(nic->pdev, nic->msix_entries, nic->num_vec);
-	if (ret) {
+	/* Enable MSI-X */
+	ret = pci_alloc_irq_vectors(nic->pdev, nic->num_vec, nic->num_vec,
+			PCI_IRQ_MSIX);
+	if (ret < 0) {
 		dev_err(&nic->pdev->dev,
 			"Request for #%d msix vectors failed, returned %d\n",
 			   nic->num_vec, ret);
-		kfree(nic->msix_entries);
 		return ret;
 	}
 
-	nic->msix_enabled = 1;
-	return 0;
-}
-
-static void nic_disable_msix(struct nicpf *nic)
-{
-	if (nic->msix_enabled) {
-		pci_disable_msix(nic->pdev);
-		kfree(nic->msix_entries);
-		nic->msix_enabled = 0;
-		nic->num_vec = 0;
-	}
-}
-
-static void nic_free_all_interrupts(struct nicpf *nic)
-{
-	int irq;
-
-	for (irq = 0; irq < nic->num_vec; irq++) {
-		if (nic->irq_allocated[irq])
-			free_irq(nic->msix_entries[irq].vector, nic);
-		nic->irq_allocated[irq] = false;
-	}
-}
-
-static int nic_register_interrupts(struct nicpf *nic)
-{
-	int i, ret;
-
-	/* Enable MSI-X */
-	ret = nic_enable_msix(nic);
-	if (ret)
-		return ret;
-
 	/* Register mailbox interrupt handler */
 	for (i = NIC_PF_INTR_ID_MBOX0; i < nic->num_vec; i++) {
 		sprintf(nic->irq_name[i],
 			"NICPF Mbox%d", (i - NIC_PF_INTR_ID_MBOX0));
 
-		ret = request_irq(nic->msix_entries[i].vector,
+		ret = request_irq(pci_irq_vector(nic->pdev, i),
 				  nic_mbx_intr_handler, 0,
 				  nic->irq_name[i], nic);
 		if (ret)
 			goto fail;
-
-		nic->irq_allocated[i] = true;
 	}
 
 	/* Enable mailbox interrupt */
@@ -1185,15 +1136,19 @@  static int nic_register_interrupts(struct nicpf *nic)
 
 fail:
 	dev_err(&nic->pdev->dev, "Request irq failed\n");
-	nic_free_all_interrupts(nic);
-	nic_disable_msix(nic);
+	while (--i >= 0)
+		free_irq(pci_irq_vector(nic->pdev, i), nic);
+	pci_free_irq_vectors(nic->pdev);
 	return ret;
 }
 
 static void nic_unregister_interrupts(struct nicpf *nic)
 {
-	nic_free_all_interrupts(nic);
-	nic_disable_msix(nic);
+	int i;
+
+	for (i = 0; i < nic->num_vec; i++)
+		free_irq(pci_irq_vector(nic->pdev, i), nic);
+	pci_free_irq_vectors(nic->pdev);
 }
 
 static int nic_num_sqs_en(struct nicpf *nic, int vf_en)