Re: [dpdk-dev] [PATCH 03/16] crypto/cpt/base: add hardware initialization API for CPT

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

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Thursday, June 14, 2018 4:14 AM
> To: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Nithin Dabilpuram <nithin.dabilpuram@cavium.com>;
> Ankur Dwivedi <ankur.dwivedi@cavium.com>; Murthy NSSR
> <Nidadavolu.Murthy@cavium.com>; Narayana Prasad
> <narayanaprasad.athreya@caviumnetworks.com>; Ragothaman Jayaraman
> <Ragothaman.Jayaraman@cavium.com>; Srisivasubramanian Srinivasan
> <Srisivasubramanian.Srinivasan@cavium.com>; dev@dpdk.org
> Subject: Re: [PATCH 03/16] crypto/cpt/base: add hardware initialization API for
> CPT
> 
> -----Original Message-----
> > Date: Fri,  8 Jun 2018 22:15:12 +0530
> > From: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> > To: Akhil Goyal <akhil.goyal@nxp.com>, Pablo de Lara
> > <pablo.de.lara.guarch@intel.com>, Thomas Monjalon
> > <thomas@monjalon.net>
> > Cc: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>, Ankur Dwivedi
> > <ankur.dwivedi@cavium.com>, Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>,  Murthy NSSR
> > <Nidadavolu.Murthy@cavium.com>, Narayana Prasad
> > <narayanaprasad.athreya@caviumnetworks.com>, Ragothaman Jayaraman
> > <Ragothaman.Jayaraman@cavium.com>, Srisivasubramanian Srinivasan
> > <Srisivasubramanian.Srinivasan@cavium.com>, dev@dpdk.org
> > Subject: [PATCH 03/16] crypto/cpt/base: add hardware initialization
> > API for  CPT
> > X-Mailer: git-send-email 2.7.4
> >
> > From: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>
> >
> > Adds hardware device initialization specific api for Cavium CPT device.
> >
> > Signed-off-by: Ankur Dwivedi <ankur.dwivedi@cavium.com>
> > Signed-off-by: Murthy NSSR <Nidadavolu.Murthy@cavium.com>
> > Signed-off-by: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>
> > Signed-off-by: Ragothaman Jayaraman
> <Ragothaman.Jayaraman@cavium.com>
> > Signed-off-by: Srisivasubramanian Srinivasan
> > <Srisivasubramanian.Srinivasan@cavium.com>
> > ---
> >  drivers/crypto/cpt/base/cpt8xxx_device.c | 200 ++++++++++++++++
> > drivers/crypto/cpt/base/cpt8xxx_device.h |  85 +++++++
> >  drivers/crypto/cpt/base/cpt_debug.h      | 231 +++++++++++++++++++
> >  drivers/crypto/cpt/base/cpt_device.c     | 383
> +++++++++++++++++++++++++++++++
> >  drivers/crypto/cpt/base/cpt_device.h     | 162 +++++++++++++
> >  drivers/crypto/cpt/base/cpt_vf_mbox.c    | 176 ++++++++++++++
> >  drivers/crypto/cpt/base/cpt_vf_mbox.h    |  60 +++++
> >  7 files changed, 1297 insertions(+)
> >  create mode 100644 drivers/crypto/cpt/base/cpt8xxx_device.c
> >  create mode 100644 drivers/crypto/cpt/base/cpt8xxx_device.h
> >  create mode 100644 drivers/crypto/cpt/base/cpt_debug.h
> >  create mode 100644 drivers/crypto/cpt/base/cpt_device.c
> >  create mode 100644 drivers/crypto/cpt/base/cpt_device.h
> >  create mode 100644 drivers/crypto/cpt/base/cpt_vf_mbox.c
> >  create mode 100644 drivers/crypto/cpt/base/cpt_vf_mbox.h
> >
> > +#include <assert.h>
> > +#include "cpt_request_mgr.h"
> > +#include <rte_eal_memconfig.h>

List the external to DPDK includes first, then internal to DPDK and then internal to this PMD,
separating these three blocks with a blank line.

> > +
> > +#ifdef CPT_DEBUG
> 
> Remove CPT_DEBUG. No harming in compiling these definitions either case.
> It will avoid the chance of build breakage.
> 

...

> > +int cptvf_deinit_device(struct cpt_vf *dev) {
> > +	struct cpt_vf *cptvf = (struct cpt_vf *)dev;
> > +
> > +	/* Do misc work one last time */
> > +	cptvf_poll_misc(cptvf);
> > +
> > +	/* TODO anything else ?? */
> 
> Remove unclear TODOs
> 
> > +
> > +	return 0;
> > +}
> > +
> > +int cptvf_init_device(struct cpt_vf *cptvf,

Return type should be in the line above the function name,
as per DPDK coding style guidelines.

> > +		      void *pdev,
> > +		      void *reg_base,
> > +		      char *name,
> > +		      uint32_t flags)
> > +{
> > +	(void) flags;
> 
> 
> RTE_SET_USED

Actually, why don't you just remove the parameter?

Thanks,
Pablo