DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 05/10] Subjec: igb_uio: msix cleanups
Date: Fri, 18 Jul 2014 13:05:23 +0200	[thread overview]
Message-ID: <1912261.SMQix2CgIZ@xps13> (raw)
In-Reply-To: <20140606235108.480879979@networkplumber.org>

Hi Stephen,

2014-06-06 16:50, Stephen Hemminger:
> Since only one MSI-X entry is ever defined, there is no need to
> put it as an array in the driver private data structure. One msix_entry
> can just be put on the stack and initialized there.

When merging this patch, I realized it's not complete:
an occurence of the msix_entries array is remaining.
See the regarding part of your patch and my proposal below
to be merged in this patch.

> @@ -67,8 +52,6 @@
>  	struct pci_dev *pdev;
>  	spinlock_t lock; /* spinlock for accessing PCI config space or msix data
> in multi tasks/isr */ enum igbuio_intr_mode mode;
> -	struct msix_entry \
> -		msix_entries[IGBUIO_NUM_MSI_VECTORS]; /* pointer to the msix vectors to
> be allocated later */ };
> 
>  static char *intr_mode;
> @@ -526,17 +509,16 @@
> 
>  	/* check if it need to try msix first */
>  	if (igbuio_intr_mode_preferred == IGBUIO_MSIX_INTR_MODE) {
> -		int vector;
> -
> -		for (vector = 0; vector < IGBUIO_NUM_MSI_VECTORS; vector ++)
> -			udev->msix_entries[vector].entry = vector;
> +		/* only one MSIX vector needed */
> +		struct msix_entry msix_entry = {
> +			.entry = 0,
> +		};
> 
> -		if (pci_enable_msix(udev->pdev, udev->msix_entries, IGBUIO_NUM_MSI_VECTORS) == 0) {
> +		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
>  			udev->mode = IGBUIO_MSIX_INTR_MODE;
> -		}
> -		else {
> -			pci_disable_msix(udev->pdev);
> -			pr_info("fail to enable pci msix, or not enough msix entries\n");
> +		} else {
> +			pr_err("failed to enable pci msix, or not enough msix entries\n");
> +			udev->mode = IGBUIO_LEGACY_INTR_MODE;
>  		}
>  	}
>  	switch (udev->mode) {


Proposed changes:
- udev->info.irq need to be set with msix_entry
- udev->mode is already the legacy one by default


 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			udev->mode = RTE_INTR_MODE_MSIX;
-		} else {
+			udev->info.irq = msix_entry.vector;
+			udev->info.irq_flags = 0;
+		} else
 			pr_err("failed to enable pci msix, or not enough msix entries\n");
-			udev->mode = IGBUIO_LEGACY_INTR_MODE;
-		}
 	}
-	switch (udev->mode) {
-	case RTE_INTR_MODE_MSIX:
-		udev->info.irq_flags = 0;
-		udev->info.irq = udev->msix_entries[0].vector;
-		break;
-	case RTE_INTR_MODE_MSI:
-		break;
-	case RTE_INTR_MODE_LEGACY:
+	if (udev->mode == RTE_INTR_MODE_LEGACY) {
 		udev->info.irq_flags = IRQF_SHARED;
 		udev->info.irq = dev->irq;
-		break;
-	default:
-		break;
 	}


Please confirm it's ok for you.
-- 
Thomas

  reply	other threads:[~2014-07-18 11:04 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 23:50 [dpdk-dev] [PATCH v2 00/10] igb_uio patches Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 01/10] igb_uio: use kernel standard log message Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 02/10] igb_uio: use standard uio naming Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 03/10] igb_uio: fix checkpatch warnings Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 04/10] igb_uio: dont wrap pci_num_vf function needlessly Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 05/10] Subjec: igb_uio: msix cleanups Stephen Hemminger
2014-07-18 11:05   ` Thomas Monjalon [this message]
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 06/10] igb_uio: propogate error numbers in probe code Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 07/10] igb_uio: make irq mode param read-only Stephen Hemminger
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 08/10] igb_uio: fix IRQ mode handling Stephen Hemminger
2014-07-18 11:49   ` Thomas Monjalon
2014-07-18 12:41   ` Thomas Monjalon
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 09/10] igbuio: show irq mode in sysfs Stephen Hemminger
2014-06-08 15:37   ` Neil Horman
2014-06-11 18:27     ` Carew, Alan
2014-06-11 20:08       ` Stephen Hemminger
2014-06-16  8:03         ` Carew, Alan
2014-06-13  0:28       ` Neil Horman
2014-06-06 23:50 ` [dpdk-dev] [PATCH v2 10/10] igbuio: use mode string for module param Stephen Hemminger
2014-06-13 16:14 ` [dpdk-dev] [PATCH v2 00/10] igb_uio patches Thomas Monjalon
2014-06-13 17:24   ` Stephen Hemminger
2014-06-13 17:51     ` [dpdk-dev] [PATCH] igb_uio: cap max VFs at 7 to reserve one for PF Chris Wright
2014-06-13 18:02       ` Richardson, Bruce
2014-06-13 18:14         ` Chris Wright
2014-06-13 19:22           ` Richardson, Bruce
2014-06-13 19:48             ` Chris Wright
2014-06-16 14:13             ` Ananyev, Konstantin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1912261.SMQix2CgIZ@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).