From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bruce.richardson@intel.com>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 7CFD637B2
 for <dev@dpdk.org>; Thu, 26 May 2016 15:16:02 +0200 (CEST)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga101.fm.intel.com with ESMTP; 26 May 2016 06:16:01 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.26,367,1459839600"; d="scan'208";a="974995276"
Received: from bricha3-mobl3.ger.corp.intel.com ([10.252.31.37])
 by fmsmga001.fm.intel.com with SMTP; 26 May 2016 06:15:58 -0700
Received: by  (sSMTP sendmail emulation); Thu, 26 May 2016 14:15:57 +0025
Date: Thu, 26 May 2016 14:15:57 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hurd <stephen.hurd@broadcom.com>
Cc: dev@dpdk.org
Message-ID: <20160526131557.GG7708@bricha3-MOBL3>
References: <CAJ9nmBYsstDhTCB4H1V5zGFD7ffTeXd=L+v+pdCjqB5KJ6AEvQ@mail.gmail.com>
 <1463179589-82681-1-git-send-email-stephen.hurd@broadcom.com>
 <1463179589-82681-26-git-send-email-stephen.hurd@broadcom.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1463179589-82681-26-git-send-email-stephen.hurd@broadcom.com>
Organization: Intel Shannon Ltd.
User-Agent: Mutt/1.5.23 (2014-03-12)
Subject: Re: [dpdk-dev] [PATCH v2 26/40] bnxt: add HWRM stat context free
 function
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: Thu, 26 May 2016 13:16:02 -0000

On Fri, May 13, 2016 at 03:46:15PM -0700, Stephen Hurd wrote:
> Add function and associated structures and definitions as well as
> some convenienct functions for manipulating the state of the entire
> function.
> 
Again, I think more explanation is needed in the commit message. The commit
title refers to freeing stat contexts, but the patch itself contains functions
working on filters. Either the filter functions belong in a different patch,
or we need more explanation as to why they would belong in this one.

I'd also question if the ordering of the patches should be changed. In other
cases you have a single patch adding allocation and free functions together, but
for these stats contexts there are two patches which are separated in the series.
Logically, if they are not merged, it would be good if they could be at least
sequential commits.

Thanks,
/Bruce