From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
by dpdk.org (Postfix) with ESMTP id 8A5BB5592
for <dev@dpdk.org>; Tue, 3 Jan 2017 15:59:01 +0100 (CET)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
by orsmga105.jf.intel.com with ESMTP; 03 Jan 2017 06:59:00 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.33,455,1477983600"; d="scan'208";a="1078727520"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38])
([10.237.220.38])
by orsmga001.jf.intel.com with ESMTP; 03 Jan 2017 06:58:59 -0800
To: Qiming Yang <qiming.yang@intel.com>, dev@dpdk.org,
thomas.monjalon@6wind.com
References: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>
<1482841816-54143-1-git-send-email-qiming.yang@intel.com>
<1482841816-54143-2-git-send-email-qiming.yang@intel.com>
Cc: remy.horton@intel.com
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <fd0d2aa7-9329-e48e-d741-a2a970b468b2@intel.com>
Date: Tue, 3 Jan 2017 14:58:58 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
Thunderbird/45.5.1
MIME-Version: 1.0
In-Reply-To: <1482841816-54143-2-git-send-email-qiming.yang@intel.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add firmware information get
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <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: Tue, 03 Jan 2017 14:59:02 -0000
On 12/27/2016 12:30 PM, Qiming Yang wrote:
> This patch adds a new API 'rte_eth_dev_fw_info_get' for fetching
> firmware related information by a given device.
>
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>
> Acked-by: Remy Horton <remy.horton@intel.com>
> ---
> v2 changes:
> * modified some comment statements.
> v3 changes:
> * change API, use rte_eth_dev_fw_info_get(uint8_t port_id,
> uint32_t *fw_major, uint32_t *fw_minor, uint32_t *fw_patch,
> uint32_t *etrack_id) instead of rte_eth_dev_fwver_get(uint8_t port_id,
> char *fw_version, int fw_length).
> Add statusment in /doc/guides/nics/features/default.ini and
> release_17_02.rst.
> ---
> ---
> doc/guides/nics/features/default.ini | 1 +
> doc/guides/rel_notes/release_17_02.rst | 4 ++++
> lib/librte_ether/rte_ethdev.c | 14 ++++++++++++++
> lib/librte_ether/rte_ethdev.h | 23 +++++++++++++++++++++++
> lib/librte_ether/rte_ether_version.map | 1 +
> 5 files changed, 43 insertions(+)
This patch also should remove deprecation notice.
Item 3 of the requested changes.
>
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index f1bf9bf..8237ee4 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -66,3 +66,4 @@ x86-64 =
> Usage doc =
> Design doc =
> Perf doc =
> +FW version =
I am not sure about this location, I think it can be before "EEPROM
dump", what do you think?
> diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
> index 180af82..f6dc6c0 100644
> --- a/doc/guides/rel_notes/release_17_02.rst
> +++ b/doc/guides/rel_notes/release_17_02.rst
> @@ -52,6 +52,10 @@ New Features
> See the :ref:`Generic flow API <Generic_flow_API>` documentation for more
> information.
>
> +* **Added firmware information get API.**
> + Added a new function ``rte_eth_dev_fw_info_get()`` to fetch firmware related
> + information by a given device. Information include major firmware version,
> + minor firmware version, patch number and etrack id.
>
> Resolved Issues
> ---------------
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 280f0db..f399f09 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1586,6 +1586,20 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint8_t port_id, uint16_t rx_queue_id,
> }
>
> void
> +rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t *fw_major, uint32_t *fw_minor,
> + uint32_t *fw_patch, uint32_t *etrack_id)
I am for rte_eth_dev_fw_version_get(), to limit the scope of the API.
And API name and eth_dev_ops name should match..
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_RET(port_id);
> + dev = &rte_eth_devices[port_id];
> +
What do you think setting all arguments to zero here?
> + RTE_FUNC_PTR_OR_RET(*dev->dev_ops->fw_version_get);
> + (*dev->dev_ops->fw_version_get)(dev, fw_major, fw_minor,
> + fw_patch, etrack_id);
> +}
> +
<...>
> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -156,5 +156,6 @@ DPDK_17.02 {
> rte_flow_flush;
> rte_flow_query;
> rte_flow_validate;
> + rte_eth_dev_fw_info_get;
Please add this alphabetically sorted.
>
> } DPDK_16.11;
>