From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH v2 08/10] igb_uio: fix IRQ mode handling
Date: Fri, 18 Jul 2014 14:41:21 +0200
Message-ID: <3162526.fhnn2gTx5h@xps13>
References: <20140606235028.189345212@networkplumber.org>
 <20140606235113.600528535@networkplumber.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev-VfR2kkLFssw@public.gmane.org
To: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <20140606235113.600528535-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

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