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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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?

[...]
>  /**
> + * 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.

> + *
> + * @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?

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

Why not returning some errors?

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.