Re: [dpdk-dev] [PATCH 1/3] net/mlx5: add missing function documentation

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

I hate to sound pedantic but...

On Thu, Feb 15, 2018 at 10:29:25AM +0100, Nelio Laranjeiro wrote:
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_trigger.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
> index f5711a998..a70b13d52 100644
> --- a/drivers/net/mlx5/mlx5_trigger.c
> +++ b/drivers/net/mlx5/mlx5_trigger.c
> @@ -14,6 +14,12 @@
>  #include "mlx5_rxtx.h"
>  #include "mlx5_utils.h"
>  
> +/**
> + * Stop traffic on Tx queues.
> + *
> + * @param  priv

...there's an extra space between @param and priv :)

> + *   Pointer to private structure.
> + */
>  static void
>  priv_txq_stop(struct priv *priv)
>  {
> @@ -23,6 +29,15 @@ priv_txq_stop(struct priv *priv)
>  		mlx5_priv_txq_release(priv, i);
>  }
>  
> +/**
> + * Start traffic on Tx queues.
> + *
> + * @param  priv

Ditto.

> + *   Pointer to private structure.
> + *
> + * @return
> + *   0 on success, errno on error.

This should more accurately read "0 on success, positive errno value
otherwise".

I would suggest to document these functions only after the errno mess is
sorted (patch 2/3 should come first) for a smaller amount of changes.

> + */
>  static int
>  priv_txq_start(struct priv *priv)
>  {
> @@ -58,6 +73,12 @@ priv_txq_start(struct priv *priv)
>  	return ret;
>  }
>  
> +/**
> + * Stop traffic on Rx queues.
> + *
> + * @param  priv

Extra space here also.

> + *   Pointer to private structure.
> + */
>  static void
>  priv_rxq_stop(struct priv *priv)
>  {
> @@ -67,6 +88,15 @@ priv_rxq_stop(struct priv *priv)
>  		mlx5_priv_rxq_release(priv, i);
>  }
>  
> +/**
> + * Start traffic on Rx queues.
> + *
> + * @param  priv

Again.

> + *   Pointer to private structure.
> + *
> + * @return
> + *   0 on success, errno on error.

Ditto regarding errno sign.

> + */
>  static int
>  priv_rxq_start(struct priv *priv)
>  {
> -- 
> 2.11.0
> 

There are other functions are not properly documented either, you might want
to add them for consistency:

mlx5_ethdev.c:
- mlx5_dev_supported_ptypes_get (no documentation)
- mlx5_link_update_unlocked_gset (return value)
- mlx5_link_update_unlocked_gs (return value)
- priv_link_update (return value)
- mlx5_link_update (return value)

mlx5_flow.c:
- mlx5_flow_create_eth (return value)
- mlx5_flow_create_vlan (return value)
- mlx5_flow_create_ipv4 (return value)
- mlx5_flow_create_ipv6 (return value)
- mlx5_flow_create_udp (return value)
- mlx5_flow_create_tcp (return value)
- mlx5_flow_create_vxlan (return value)
- mlx5_flow_create_flag_mark (return value)
- priv_flow_create_drop_queue (return in case of error)
- priv_flow_verify (inconsistent @return syntax)

mlx5_glue.c:
- mlx5_glue_* (too many of them and those are basic wrappers, don't bother)

mlx5_mr.c:
- priv_mr_verify (inconsistent @return syntax)

mlx5_rxq.c:
- mlx5_priv_rxq_ibv_verify (ditto)
- mlx5_priv_rxq_verify (ditto)
- mlx5_priv_ind_table_ibv_verify (ditto)
- mlx5_priv_hrxq_ibv_verify (ditto)

mlx5_rxtx.c:
- mlx5_tx_burst_raw_vec
- mlx5_tx_burst_vec
- mlx5_rx_burst_vec
- priv_check_raw_vec_tx_support
- priv_check_vec_tx_support
- rxq_check_vec_support
- priv_check_vec_rx_support

mlx5_trigger.c:
- Those are taken care of by the current patch.

mlx5_txq.c:
- mlx5_priv_txq_ibv_verify (inconsistent @return syntax)
- mlx5_priv_txq_verify (ditto)

mlx5_vlan.c
- mlx5_vlan_offload_set (return value)

I think that's all.

-- 
Adrien Mazarguil
6WIND