From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v4 0/7] e1000: add rx interrupt support
Date: Thu, 29 Oct 2015 19:18:32 +0100
Message-ID: <1603993.a4RWodNe3S@xps13>
References: <1446134187-4573-1-git-send-email-yong.liu@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: Yong Liu <yong.liu@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com
 [209.85.212.179]) by dpdk.org (Postfix) with ESMTP id E28CD5A69
 for <dev@dpdk.org>; Thu, 29 Oct 2015 19:19:43 +0100 (CET)
Received: by wikq8 with SMTP id q8so292662284wik.1
 for <dev@dpdk.org>; Thu, 29 Oct 2015 11:19:43 -0700 (PDT)
In-Reply-To: <1446134187-4573-1-git-send-email-yong.liu@intel.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2015-10-29 23:56, Yong Liu:
> This patch set will enable interrup for physical and emulated e1000 device.
> Rx queue interrupt will work with uio driver or vfio driver with msi mode.
> l3fwd-power will disable interrupt immediately when wake-up for that e1000 not
> support interrupt auto clear.
> LSC and rxq interrupt will be seperated for e1000 can only support one
> interrupt cause in the same time.

Don't you think it should be explained in a doc?
	doc/guides/nics/e1000em.rst
I'm especially confused by the need of changing an example for this PMD.
Does it mean the API behaviour must be changed?

[...]
> Marvin Liu (7):
>   e1000: add rx interrupt support
>   e1000: separate lsc and rxq interrupt disable function
>   e1000: add ethdev rxq enable and disable function
>   e1000: add rxq interrupt handler
>   e1000: check lsc and rxq not enable in the same time
>   e1000: lsc interrupt setup function only enable itself
>   l3fwd-power: disable interrupt when wake up from sleep

Announcing a support in first patch and making it work later
is a strange logic to review.

You forgot to keep the Acked-by: Cunming Liang <cunming.liang@intel.com>

Applied in this order:
	e1000: restrict link interrupt setup scope
	e1000: separate link and Rx interrupt disabling
	e1000: support Rx interrupt setup
	e1000: add Rx interrupt handler
	l3fwd-power: disable Rx interrupt when waking up
Thanks