[dpdk-dev,v4,1/5] ethdev: add firmware version get

Message ID 1483531428-14481-2-git-send-email-qiming.yang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Qiming Yang Jan. 4, 2017, 12:03 p.m. UTC
  This patch adds a new API 'rte_eth_dev_fw_version_get' for
fetching firmware version 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.
v4 changes:
* remove deprecation notice, rename API as rte_eth_dev_fw_version_get
---
---
 doc/guides/nics/features/default.ini   |  1 +
 doc/guides/rel_notes/deprecation.rst   |  4 ----
 doc/guides/rel_notes/release_17_02.rst |  3 +++
 lib/librte_ether/rte_ethdev.c          | 14 ++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 23 +++++++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  1 +
 6 files changed, 42 insertions(+), 4 deletions(-)
  

Comments

Thomas Monjalon Jan. 5, 2017, 1:44 p.m. UTC | #1
2017-01-04 20:03, Qiming Yang:
> This patch adds a new API 'rte_eth_dev_fw_version_get' for
> fetching firmware version related information by a given device.
[...]
>  /**
> + * Retrieve the firmware version of a device.
> + *
> + * @param port_id
> + *   The port identifier of the device.
> + * @param fw_major
> + *   A pointer to store the major firmware version of a device.
> + * @param fw_minor
> + *   A pointer to store the minor firmware version of a device.
> + * @param fw_patch
> + *   A pointer to store the firmware patch number of a device.
> + * @param etrack_id
> + *   A pointer to store the nvm version of a device.
> + */
> +void rte_eth_dev_fw_version_get(uint8_t port_id, uint32_t *fw_major,
> +	uint32_t *fw_minor, uint32_t *fw_patch, uint32_t *etrack_id);

After reading few comments, I think it should just fill a string.
There is no way the firmware version can be generalized or standardized.
If the application wants to do some processing like number comparisons,
it has to be aware of the specific firmware version string format.
If you want to help the application to parse this string, it should be
a PMD specific API.
  
Qiming Yang Jan. 8, 2017, 3:09 a.m. UTC | #2
-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Thursday, January 5, 2017 9:45 PM
To: Yang, Qiming <qiming.yang@intel.com>
Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/5] ethdev: add firmware version get

2017-01-04 20:03, Qiming Yang:
> This patch adds a new API 'rte_eth_dev_fw_version_get' for fetching 
> firmware version related information by a given device.
[...]
>  /**
> + * Retrieve the firmware version of a device.
> + *
> + * @param port_id
> + *   The port identifier of the device.
> + * @param fw_major
> + *   A pointer to store the major firmware version of a device.
> + * @param fw_minor
> + *   A pointer to store the minor firmware version of a device.
> + * @param fw_patch
> + *   A pointer to store the firmware patch number of a device.
> + * @param etrack_id
> + *   A pointer to store the nvm version of a device.
> + */
> +void rte_eth_dev_fw_version_get(uint8_t port_id, uint32_t *fw_major,
> +	uint32_t *fw_minor, uint32_t *fw_patch, uint32_t *etrack_id);

After reading few comments, I think it should just fill a string.
There is no way the firmware version can be generalized or standardized.
If the application wants to do some processing like number comparisons, it has to be aware of the specific firmware version string format.
If you want to help the application to parse this string, it should be a PMD specific API.
Qiming: I agree with you.
  

Patch

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index f1bf9bf..ae40d57 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -50,6 +50,7 @@  Timesync             =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+FW version           =
 EEPROM dump          =
 Registers dump       =
 Multiprocess aware   =
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1438c77..291e03d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -30,10 +30,6 @@  Deprecation Notices
   ``nb_seg_max`` and ``nb_mtu_seg_max`` providing information about number of
   segments limit to be transmitted by device for TSO/non-TSO packets.
 
-* In 17.02 ABI change is planned: the ``rte_eth_dev_info`` structure
-  will be extended with a new member ``fw_version`` in order to store
-  the NIC firmware version.
-
 * ethdev: an API change is planned for 17.02 for the function
   ``_rte_eth_dev_callback_process``. In 17.02 the function will return an ``int``
   instead of ``void`` and a fourth parameter ``void *ret_param`` will be added.
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 180af82..d6958d4 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -52,6 +52,9 @@  New Features
   See the :ref:`Generic flow API <Generic_flow_API>` documentation for more
   information.
 
+* **Added firmware version get API.**
+ Added a new function ``rte_eth_dev_fw_version_get()`` to fetch firmware version
+ related information by a given device.
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 280f0db..a4b20b5 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_version_get(uint8_t port_id, uint32_t *fw_major,
+		uint32_t *fw_minor, uint32_t *fw_patch, uint32_t *etrack_id)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_RET(port_id);
+	dev = &rte_eth_devices[port_id];
+
+	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);
+}
+
+void
 rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index fb51754..9c7efa1 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1150,6 +1150,11 @@  typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
 /**< @internal Check DD bit of specific RX descriptor */
 
+typedef void (*eth_fw_version_get_t)(struct rte_eth_dev *dev,
+		uint32_t *fw_major, uint32_t *fw_minor,
+		uint32_t *fw_patch, uint32_t *etrack_id);
+/**< @internal Get firmware version information of an Ethernet device. */
+
 typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
 	uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
 
@@ -1457,6 +1462,7 @@  struct eth_dev_ops {
 	eth_txq_info_get_t         txq_info_get; /**< retrieve TX queue information. */
 	eth_dev_supported_ptypes_get_t dev_supported_ptypes_get;
 	/**< Get packet types supported and identified by device. */
+	eth_fw_version_get_t       fw_version_get; /**< Get firmware version. */
 
 	vlan_filter_set_t          vlan_filter_set; /**< Filter VLAN Setup. */
 	vlan_tpid_set_t            vlan_tpid_set; /**< Outer/Inner VLAN TPID Setup. */
@@ -2395,6 +2401,23 @@  void rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr);
 void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info);
 
 /**
+ * Retrieve the firmware version of a device.
+ *
+ * @param port_id
+ *   The port identifier of the device.
+ * @param fw_major
+ *   A pointer to store the major firmware version of a device.
+ * @param fw_minor
+ *   A pointer to store the minor firmware version of a device.
+ * @param fw_patch
+ *   A pointer to store the firmware patch number of a device.
+ * @param etrack_id
+ *   A pointer to store the nvm version of a device.
+ */
+void rte_eth_dev_fw_version_get(uint8_t port_id, uint32_t *fw_major,
+	uint32_t *fw_minor, uint32_t *fw_patch, uint32_t *etrack_id);
+
+/**
  * Retrieve the supported packet types of an Ethernet device.
  *
  * When a packet type is announced as supported, it *must* be recognized by
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index a021781..0cf94ed 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -151,6 +151,7 @@  DPDK_17.02 {
 	global:
 
 	_rte_eth_dev_reset;
+	rte_eth_dev_fw_version_get;
 	rte_flow_create;
 	rte_flow_destroy;
 	rte_flow_flush;