Re: [dpdk-dev] [PATCH v2 1/6] compress/octeontx: add octeontx zip PMD support

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Monday, July 2, 2018 5:55 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; pathreya@caviumnetworks.com;
> mchalla@caviumnetworks.com; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> <Ashish.Gupta@caviumnetworks.com>
> Subject: [PATCH v2 1/6] compress/octeontx: add octeontx zip PMD support

Remove support from title.

> 
> From: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> 
> Add octeontx zip pmd support in compressdev driver.
> Octeontx ZIP appears as PCI device.
> Add device probe and remove support.
> link zip pmd library in rtp.app.mk
> Update meson.build and Makefile to build octeontx zip pmd

It is better to have an introduction on the PMD here, instead of listing all you are doing in the patch.

> 
> Signed-off-by: Ashish Gupta <Ashish.Gupta@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> ---
>  MAINTAINERS                                        |   5 +
>  config/common_base                                 |   5 +
>  drivers/compress/Makefile                          |   1 +
>  drivers/compress/meson.build                       |   2 +-
>  drivers/compress/octeontx/Makefile                 |  30 +
>  drivers/compress/octeontx/include/zip_regs.h       | 721
> +++++++++++++++++++++
>  drivers/compress/octeontx/meson.build              |  11 +
>  .../compress/octeontx/rte_pmd_octeontx_version.map |   3 +
>  drivers/compress/octeontx/zip_pmd.c                | 120 ++++
>  drivers/compress/octeontx/zipvf.c                  |  63 ++
>  drivers/compress/octeontx/zipvf.h                  | 111 ++++
>  mk/rte.app.mk                                      |   1 +
>  12 files changed, 1072 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc1607844..78708d8af 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -850,6 +850,11 @@ F: drivers/compress/isal/
>  F: doc/guides/compressdevs/isal.rst
>  F: doc/guides/compressdevs/features/isal.ini
> 
> +Cavium OCTEONTX zipvf
> +M: Ashish Gupta <ashish.gupta@cavium.com>
> +F: drivers/compress/octeontx
> +F: doc/guides/compressdevs/octeontx.rst
> +F: doc/guides/compressdevs/features/octeontx.ini

The documentation is not yet in this patch, so add it later.

> 
>  Eventdev Drivers
>  ----------------
> diff --git a/config/common_base b/config/common_base index
> 6541ad5b2..396a09682 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -578,6 +578,11 @@ CONFIG_RTE_COMPRESS_MAX_DEVS=64  #
> CONFIG_RTE_COMPRESSDEV_TEST=n
> 
> +#
> +# Compile PMD for Octeontx ZIPVF compression device #
> +CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF=y
> +
>  #
>  # Compile PMD for ISA-L compression device  # diff --git
> a/drivers/compress/Makefile b/drivers/compress/Makefile index
> 592497f51..25cced348 100644
> --- a/drivers/compress/Makefile
> +++ b/drivers/compress/Makefile
> @@ -4,5 +4,6 @@
>  include $(RTE_SDK)/mk/rte.vars.mk
> 
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_ISAL) += isal
> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_ZIPVF) += octeontx
> 
>  include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/drivers/compress/meson.build b/drivers/compress/meson.build index
> fb136e1b2..a7d6f3a40 100644
> --- a/drivers/compress/meson.build
> +++ b/drivers/compress/meson.build
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2018 Intel Corporation
> 
> -drivers = ['isal']
> +drivers = ['isal', 'octeontx']

There is a compilation error on meson:

drivers/meson.build:108:3: ERROR:  File 
drivers/compress/octeontx/rte_pmd_octeontx_compress_version.map does not exist.

...

> +	return rte_compressdev_pmd_destroy(compressdev);
> +}
> +
> +
> +

Too many blank lines here.

> +static struct rte_pci_id pci_id_octtx_zipvf_table[] = {
> +	{
> +		RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM,
> +

...

> +static void
> +octtx_zip_init_log(void)
> +{
> +	octtx_zip_logtype_driver = rte_log_register("comp_octeontx_zip");

Naming convention for the PMD logtypes  is "pmd.driverType.driverName".
In this case, "pmd.compress.octeonx".

> +	if (octtx_zip_logtype_driver >= 0)
> +		rte_log_set_level(octtx_zip_logtype_driver, RTE_LOG_INFO); }
>

> diff --git a/drivers/compress/octeontx/zipvf.h
> b/drivers/compress/octeontx/zipvf.h
> new file mode 100644
> index 000000000..36c44c8c5
> --- /dev/null
> +++ b/drivers/compress/octeontx/zipvf.h

...

> +
> +#define ZIP_ALIGN_ROUNDUP(x, _align) \
> +	((_align) * (((x) + (_align) - 1) / (_align)))
> +
> +/**< ZIP PMD device name */
> +#define COMPRESSDEV_NAME_ZIP_PMD	compress_octtx_zipvf

The naming convention for PMDs is driverType_driverName, so this should be  compress_octeonx
> +