From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Fri, 18 Jul 2014 13:04:35 +0200 (CEST) Received: by mail-we0-f171.google.com with SMTP id p10so4440402wes.2 for ; 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 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Jul 2014 04:05:30 -0700 (PDT) From: Thomas Monjalon To: Stephen Hemminger 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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