From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-we0-f171.google.com (mail-we0-f171.google.com
[74.125.82.171]) by dpdk.org (Postfix) with ESMTP id 56B69959
for <dev@dpdk.org>; Fri, 18 Jul 2014 13:04:35 +0200 (CEST)
Received: by mail-we0-f171.google.com with SMTP id p10so4440402wes.2
for <dev@dpdk.org>; Fri, 18 Jul 2014 04:05:33 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20130820;
h=x-gm-message-state:from:to:cc:subject:date:message-id:organization
:user-agent:in-reply-to:references:mime-version
:content-transfer-encoding:content-type;
bh=txFalO10/g0HYGZX/UYqS3YNKMWuTSBzPwEe6oLMKd0=;
b=En3kElyN1QpjLDcqSKK8gDM3q4q/aCZ+vdDZ/44umA1l4RUkY8EovdMIXGSkp7jYHz
HKd1G4dxIiUltZIG9vAqzCqQmsyHJgJtrR1fdF7oVNRZHiSqM7dwoRyb92BjDpAUKmDA
ctdUZLVFu604OkMzFHqXbAt5GGtpau+RVbap9iDWnPafnjjw40UC3bmhvq1kErmwzQxm
y39GBs/cPW20ph5JYBnRg+dfGSoCE9i/1DM+S/ZXAvHnZI+yW/NAoPtYRV1uaeqB5Zeq
kxkkGH6HtbRazJUMc2NtD7A8PEvvp9nqgjdxSb7rn/dzAn7OWYrJhXnuT84soQTU/2Ka
vpXQ==
X-Gm-Message-State: ALoCoQnDNRFrGP2OlbLa9OB+YCd58R7g5m/yxoWSDcAXZZXuxduwI3SXsjvWg4lXniSP4w48ai7y
X-Received: by 10.180.13.47 with SMTP id e15mr31989478wic.28.1405681532032;
Fri, 18 Jul 2014 04:05:32 -0700 (PDT)
Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])
by mx.google.com with ESMTPSA id cj8sm13413666wjb.5.2014.07.18.04.05.29
for <multiple recipients>
(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Fri, 18 Jul 2014 04:05:30 -0700 (PDT)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 18 Jul 2014 13:05:23 +0200
Message-ID: <1912261.SMQix2CgIZ@xps13>
Organization: 6WIND
User-Agent: KMail/4.13.2 (Linux/3.15.5-1-ARCH; KDE/4.13.2; x86_64; ; )
In-Reply-To: <20140606235108.480879979@networkplumber.org>
References: <20140606235028.189345212@networkplumber.org>
<20140606235108.480879979@networkplumber.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 05/10] Subjec: igb_uio: msix cleanups
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 18 Jul 2014 11:04:35 -0000
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