From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Qiming" Subject: Re: [PATCH v3 2/4] net/e1000: add firmware version get Date: Wed, 4 Jan 2017 03:14:11 +0000 Message-ID: References: <1481008582-69416-1-git-send-email-qiming.yang@intel.com> <1482841816-54143-1-git-send-email-qiming.yang@intel.com> <1482841816-54143-3-git-send-email-qiming.yang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "Horton, Remy" To: "Yigit, Ferruh" , "dev@dpdk.org" , "thomas.monjalon@6wind.com" Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 752142BB9 for ; Wed, 4 Jan 2017 04:14:19 +0100 (CET) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" See the reply below. -----Original Message----- From: Yigit, Ferruh=20 Sent: Tuesday, January 3, 2017 11:03 PM To: Yang, Qiming ; dev@dpdk.org; thomas.monjalon@6wi= nd.com Cc: Horton, Remy Subject: Re: [PATCH v3 2/4] net/e1000: add firmware version get On 12/27/2016 12:30 PM, Qiming Yang wrote: > This patch adds a new function eth_igb_fw_version_get. >=20 > Signed-off-by: Qiming Yang > --- > v3 changes: > * use eth_igb_fw_version_get(struct rte_eth_dev *dev, u32 *fw_major, > u32 *fw_minor, u32 *fw_minor, u32 *fw_patch, u32 *etrack_id) instead > of eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version, > int fw_length). Add statusment in /doc/guides/nics/features/igb.ini. > --- > --- > doc/guides/nics/features/igb.ini | 1 + > drivers/net/e1000/igb_ethdev.c | 43 ++++++++++++++++++++++++++++++++++= ++++++ > 2 files changed, 44 insertions(+) >=20 > diff --git a/doc/guides/nics/features/igb.ini=20 > b/doc/guides/nics/features/igb.ini > index 9fafe72..ffd87ba 100644 > --- a/doc/guides/nics/features/igb.ini > +++ b/doc/guides/nics/features/igb.ini > @@ -39,6 +39,7 @@ EEPROM dump =3D Y > Registers dump =3D Y > BSD nic_uio =3D Y > Linux UIO =3D Y > +FW version =3D Y Please keep same location with default.ini file. Why you are putting this j= ust into middle of the uio and vfio? Qiming: It's a clerical error, I want to add this line at the end of this f= ile. > Linux VFIO =3D Y > x86-32 =3D Y > x86-64 =3D Y > diff --git a/drivers/net/e1000/igb_ethdev.c=20 > b/drivers/net/e1000/igb_ethdev.c index 4a15447..25344b7 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -120,6 +120,8 @@ static int eth_igb_xstats_get_names(struct rte_eth_de= v *dev, > unsigned limit); > static void eth_igb_stats_reset(struct rte_eth_dev *dev); static=20 > void eth_igb_xstats_reset(struct rte_eth_dev *dev); > +static void eth_igb_fw_version_get(struct rte_eth_dev *dev, u32 *fw_majo= r, > + u32 *fw_minor, u32 *fw_patch, u32 *etrack_id); I think you can use a struct as parameter here. But beware, that struct sho= uld NOT be a public struct. Qiming: I think only add a private struct for igb is unnecessary. Keep the = arguments consistent with rte_eth_dev_fw_info_get is better. What do you think? > static void eth_igb_infos_get(struct rte_eth_dev *dev, > struct rte_eth_dev_info *dev_info); static const uint32_t=20 > *eth_igb_supported_ptypes_get(struct rte_eth_dev *dev); @@ -389,6=20 > +391,7 @@ static const struct eth_dev_ops eth_igb_ops =3D { > .xstats_get_names =3D eth_igb_xstats_get_names, > .stats_reset =3D eth_igb_stats_reset, > .xstats_reset =3D eth_igb_xstats_reset, > + .fw_version_get =3D eth_igb_fw_version_get, > .dev_infos_get =3D eth_igb_infos_get, > .dev_supported_ptypes_get =3D eth_igb_supported_ptypes_get, > .mtu_set =3D eth_igb_mtu_set, > @@ -1981,6 +1984,46 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev) =20 > } > =20 <...>