From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-wg0-f49.google.com (mail-wg0-f49.google.com [74.125.82.49])
 by dpdk.org (Postfix) with ESMTP id 43B1A5916
 for <dev@dpdk.org>; Fri, 18 Jul 2014 14:40:33 +0200 (CEST)
Received: by mail-wg0-f49.google.com with SMTP id k14so3514970wgh.32
 for <dev@dpdk.org>; Fri, 18 Jul 2014 05:41:31 -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=22gCXHXMSpjzsIgHIoiG23Q2LNf/fO5ioI5wlI8R2Lw=;
 b=K60fDLTnf0/8xMrTIwDvNMLEO9UB4JkSuI0KPeI3SH3Xg+rdrSHw+MS3BRrJxka0lW
 1XVe7zKwdaATyM5kTkem94wELbeEsllmJKY6el7pXQB5VSg/R5NkznVLg9WQr0FGFo7T
 JAzzF8pwcAyuXrde1WWDZ8V+qkpvngBSWlR67u6+WtybRK+A8F/0sft6z5+dAjejloJz
 XPh6O7Drqf2bO1tvN1MLBlxHNxJCVY7jfeu7k9obsRaBrpqh9STUj6+aMuyYCyMDTk7i
 lhc7Dugl/Ye09fnIDKCXxBctTvBpHKqchTxTnBIZ30wPGZcWN4KGybPpqfGlXOt9ScMf
 0/Ig==
X-Gm-Message-State: ALoCoQlwBFphzoEXMjMgU87m2Zy5pFoX0lmDld5zPiRPXo1B/8s+ZXTQvJS/Gn5tniE1c8rM0l5A
X-Received: by 10.180.97.67 with SMTP id dy3mr7647619wib.16.1405687291266;
 Fri, 18 Jul 2014 05:41:31 -0700 (PDT)
Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])
 by mx.google.com with ESMTPSA id ev18sm2165351wid.1.2014.07.18.05.41.29
 for <multiple recipients>
 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Fri, 18 Jul 2014 05:41:30 -0700 (PDT)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 18 Jul 2014 14:41:21 +0200
Message-ID: <3162526.fhnn2gTx5h@xps13>
Organization: 6WIND
User-Agent: KMail/4.13.2 (Linux/3.15.5-1-ARCH; KDE/4.13.2; x86_64; ; )
In-Reply-To: <20140606235113.600528535@networkplumber.org>
References: <20140606235028.189345212@networkplumber.org>
 <20140606235113.600528535@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 08/10] igb_uio: fix IRQ mode handling
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 12:40:33 -0000

Hi Stephen,

I have other (inlined) comments on this patch.

>  	udev->info.version = "0.1";
>  	udev->info.handler = igbuio_pci_irqhandler;
>  	udev->info.irqcontrol = igbuio_pci_irqcontrol;
> +	udev->info.irq = dev->irq;

[...]

> +		/* fall back to MSI */
>  	case IGBUIO_MSI_INTR_MODE:
> -		break;
> +		if (pci_enable_msi(dev) == 0) {
> +			dev_dbg(&dev->dev, "using MSI");
> +			udev->info.irq = dev->irq;

I think we can remove this line: info.irq is already set to the right value.

> +			udev->mode = IGBUIO_MSI_INTR_MODE;
> +			break;
> +		}

There is no default case in this switch statement. It's now required for the 
enum completeness. So I suggest to add these lines:

+	default:
+		dev_err(&dev->dev, "unknown interrupt mode\n");
+		err = -EINVAL;
+		goto fail_release_iomem;

-- 
Thomas