Re: [dpdk-dev] [PATCH 00/10] Introducing the NXP CAAM job ring driver

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Gagan,

On 9/13/2018 11:38 AM, Gagandeep Singh wrote:
> The caam_jr PMD provides poll mode crypto driver
> support for NXP SEC 4.x+ (CAAM) hardware accelerator.
>
> This patch has dependancy on below patches:
> http://patchwork.dpdk.org/patch/43986/
> http://patchwork.dpdk.org/patch/43964/
>
> Hemant Agrawal (10):
>    doc: add caam jr cryptodev details
>    crypto/caam_jr: introduce basic driver
>    crypto/caam_jr: add HW config for job rings
>    crypto/caam_jr: add device configuration routines
>    crypto/caam_jr: add queue config functions
>    crypto/caam_jr: add basic session config routines
>    crypto/caam_jr: add enqueue and dequeue routines
>    crypto/caam_jr: add auth cipher and aead session support
>    crypto/caam_jr: add stats support
>    crypto/caam_jr: add security offload support
>
>   config/common_base                            |    8 +
>   config/common_linuxapp                        |    1 +
>   config/defconfig_arm64-dpaa-linuxapp-gcc      |    4 +
>   doc/guides/cryptodevs/caam_jr.rst             |  159 ++
>   doc/guides/cryptodevs/index.rst               |    1 +
>   drivers/crypto/Makefile                       |    1 +
>   drivers/crypto/caam_jr/Makefile               |   46 +
>   drivers/crypto/caam_jr/caam_jr.c              | 2485 +++++++++++++++++
>   drivers/crypto/caam_jr/caam_jr.h              |  257 ++
>   drivers/crypto/caam_jr/caam_jr_config.h       |  207 ++
>   drivers/crypto/caam_jr/caam_jr_desc.h         |  289 ++
>   drivers/crypto/caam_jr/caam_jr_hw.c           |  365 +++
>   drivers/crypto/caam_jr/caam_jr_hw_specific.h  |  503 ++++
>   drivers/crypto/caam_jr/caam_jr_log.h          |   42 +
>   drivers/crypto/caam_jr/caam_jr_pvt.h          |  288 ++
>   drivers/crypto/caam_jr/caam_jr_uio.c          |  491 ++++
>   drivers/crypto/caam_jr/meson.build            |   14 +
>   .../caam_jr/rte_pmd_caam_jr_version.map       |    4 +
>   drivers/crypto/meson.build                    |    2 +-
>   19 files changed, 5166 insertions(+), 1 deletion(-)
>   create mode 100644 doc/guides/cryptodevs/caam_jr.rst
>   create mode 100644 drivers/crypto/caam_jr/Makefile
>   create mode 100644 drivers/crypto/caam_jr/caam_jr.c
>   create mode 100644 drivers/crypto/caam_jr/caam_jr.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_config.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_desc.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_hw.c
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_hw_specific.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_log.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_pvt.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_uio.c
>   create mode 100644 drivers/crypto/caam_jr/meson.build
>   create mode 100644 drivers/crypto/caam_jr/rte_pmd_caam_jr_version.map
>

I have some generic comments on your patchset:
1. Please add description to the patches. None of them have it.
2. Split of the patches could be better. You can have Hardware specific 
code in one patch and then you can use that in the next patch.
3. SG related stuff can be a separate patch.
4. there are checkpatch issues which can be resolved.
5. I could see extra spaces at many places. Please check.
6. Function definitions are not as per DPDK coding style.

Thanks,
Akhil