From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 6432CA04DB;
Thu, 15 Oct 2020 20:57:16 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 2ABC71D568;
Thu, 15 Oct 2020 20:57:13 +0200 (CEST)
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id EB2061C1F8;
Thu, 15 Oct 2020 20:57:09 +0200 (CEST)
IronPort-SDR: fXCyX3zeT61xx4w2wKyX3ub7+XESPbMVCMdrmKsC4j6wuSCg58NU0GjmOXAonw796qKiol0orL
YYAAaSI6a/xQ==
X-IronPort-AV: E=McAfee;i="6000,8403,9775"; a="165652462"
X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="165652462"
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
15 Oct 2020 11:56:54 -0700
IronPort-SDR: 8Geu0QW2SAqKNCWxqs1zoz1gj7EBKjQEnpU8umOk89h685Gkum/i3XLriYz+Iuyfxp9JbrlIrX
exO0EcJHPhNA==
X-IronPort-AV: E=Sophos;i="5.77,380,1596524400"; d="scan'208";a="464405907"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.251.84.112])
([10.251.84.112])
by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
15 Oct 2020 11:56:52 -0700
To: Tianfei zhang <tianfei.zhang@intel.com>, dev@dpdk.org,
rosen.xu@intel.com, wei.huang@intel.com
Cc: stable@dpdk.org
References: <1600846213-18093-1-git-send-email-tianfei.zhang@intel.com>
<1601257218-6606-1-git-send-email-tianfei.zhang@intel.com>
<1601257218-6606-2-git-send-email-tianfei.zhang@intel.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <aec40a31-8140-6d73-c9d2-54ec4935b63a@intel.com>
Date: Thu, 15 Oct 2020 19:56:48 +0100
MIME-Version: 1.0
In-Reply-To: <1601257218-6606-2-git-send-email-tianfei.zhang@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ
functions
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
On 9/28/2020 2:40 AM, Tianfei zhang wrote:
> From: Wei Huang <wei.huang@intel.com>
>
> Using a pointer instead of using a structure and point to
> ifpga_irq_handle[] in register and unregister interrupt
> functions.
> Treat positive return value of ifpga_unregister_msix_irq()
> as successful.
>
> Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
I suggest commit log as following:
raw/ifpga/base: fix interrupt handler instance usage
Interrupt handler copied to the local 'intr_handle' variable by value
before passing it to IRQ functions.
This leads IRQ functions update the local variable instead of
'ifpga_irq_handle'.
Instead, using 'intr_handle' local variable as pointer to
'ifpga_irq_handle' as intended.
Also handle unsupported interrupt type requests properly, on unsupported
interrupt case:
'ifpga_unregister_msix_irq()' returns success
'ifpga_register_msix_irq()' return failure.
Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org
The "Also" part highlights that patch addressed two different issues, for next
time please split different fixes to the different patches.
Title "fix bug" doesn't give much information, better to give some context.
And for the following part in the original commit log:
"
Treat positive return value of ifpga_unregister_msix_irq()
as successful.
"
It is missing in the patch, I see that part is in the next patch :)
+1 to update, since 'rte_intr_callback_unregister()' can return positive, but
perhaps better to move the change too into its own patch.