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

Yang, Qiming qiming.yang at intel.com
Fri Nov 18 03:10:57 CET 2016


Hi, Thomas

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] 
Sent: Thursday, November 17, 2016 9:37 PM
To: Yang, Qiming <qiming.yang at intel.com>
Cc: dev at dpdk.org; Horton, Remy <remy.horton at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>; Chen, Jing D <jing.d.chen at intel.com>
Subject: Re: [dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-17 17:42, Qiming Yang:
> This patch added API for 'rte_eth_dev_fwver_get'
> 
> void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int 
> fw_length);

Copying some code here doesn't help really help.
Could you describe what we can expect in this string?
How can we compare this version number across different devices of the same driver?
How does it apply to FPGA devices?
What is the potential use?

Qiming: I'll add more describes in the next version.

[...]
>  /**
> + * Retrieve the firmware version of an Ethernet device.

We should stop talking about Ethernet device.
Networking device is more appropriate.
And in this case, "device" is enough.

Qiming: Got it.

> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param fw_version
> + *   A pointer the firmware version of an Ethernet device
> + * @param fw_length
> + *   The size of the firmware version, which should be large enough to store
> + *   the firmware version of the device.

How do we know that the length is too small?

Qiming: The length is a number large enough to store the firmware version. Original I use a fixed number 20, but in order to be more flexible, I replace it into a variable. 

> + */
> +void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int 
> +fw_length);

Why not returning some errors?

Qiming: It's a good advice.

You forgot to remove the deprecation notice in this patch.

PS: the series is broken as some patches are not numbered and this one is not the first one. Please use a cover-letter to introduce such series.

Qiming: It's my fault. Thank you for your reminder, I will pay attention next time.


More information about the dev mailing list