Re: [PATCH v2 5/9] raw/dpaa2_cmdif: introduce DPAA2 command interface driver

[Shreyansh Jain <shreyansh.jain@nxp.com>]

> -----Original Message-----
> From: Nipun Gupta
> Sent: Saturday, April 7, 2018 8:04 PM
> To: thomas@monjalon.net; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org; Nipun Gupta <nipun.gupta@nxp.com>
> Subject: [PATCH v2 5/9] raw/dpaa2_cmdif: introduce DPAA2 command
> interface driver
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
>  MAINTAINERS                                        |   8 ++
>  config/common_base                                 |   1 +
>  config/common_linuxapp                             |   1 +
>  drivers/raw/Makefile                               |   3 +
>  drivers/raw/dpaa2_cmdif/Makefile                   |  33 +++++
>  drivers/raw/dpaa2_cmdif/dpaa2_cmdif.c              | 139
> +++++++++++++++++++++
>  drivers/raw/dpaa2_cmdif/dpaa2_cmdif_logs.h         |  35 ++++++
>  .../dpaa2_cmdif/rte_pmd_dpaa2_cmdif_version.map    |   4 +
>  mk/rte.app.mk                                      |   1 +
>  9 files changed, 225 insertions(+)
>  create mode 100644 drivers/raw/dpaa2_cmdif/Makefile
>  create mode 100644 drivers/raw/dpaa2_cmdif/dpaa2_cmdif.c
>  create mode 100644 drivers/raw/dpaa2_cmdif/dpaa2_cmdif_logs.h
>  create mode 100644
> drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif_version.map
> 

[...]

> diff --git a/drivers/raw/dpaa2_cmdif/dpaa2_cmdif_logs.h
> b/drivers/raw/dpaa2_cmdif/dpaa2_cmdif_logs.h
> new file mode 100644
> index 0000000..5eb0885
> --- /dev/null
> +++ b/drivers/raw/dpaa2_cmdif/dpaa2_cmdif_logs.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2018 NXP
> + */
> +
> +#ifndef __DPAA2_CMDIF_LOGS_H__
> +#define __DPAA2_CMDIF_LOGS_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <rte_rawdev.h>
> +
> +extern int dpaa2_cmdif_logtype;
> +
> +#define DPAA2_CMDIF_LOG(level, fmt, args...) \
> +	rte_log(RTE_LOG_ ## level, dpaa2_cmdif_logtype, "%s(): " fmt "\n",
> \
> +		__func__, ##args)

Trivial comment: Would it be better to have two variants - one which prints the function names and which doesn't. ERR/WARNING with function name should be avoided.

> +
> +#define DPAA2_CMDIF_FUNC_TRACE() DPAA2_CMDIF_LOG(DEBUG, ">>")
> +
> +#define DPAA2_CMDIF_DEBUG(fmt, args...) \
> +	DPAA2_CMDIF_LOG(DEBUG, fmt, ## args)
> +#define DPAA2_CMDIF_INFO(fmt, args...) \
> +	DPAA2_CMDIF_LOG(INFO, fmt, ## args)
> +#define DPAA2_CMDIF_ERR(fmt, args...) \
> +	DPAA2_CMDIF_LOG(ERR, fmt, ## args)
> +#define DPAA2_CMDIF_WARN(fmt, args...) \
> +	DPAA2_CMDIF_LOG(WARNING, fmt, ## args)
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __DPAA2_CMDIF_LOGS_H__ */

[...]