From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Joseph, Anoob" Subject: Re: [PATCH v2 09/33] crypto/octeontx: adds symmetric capabilities Date: Mon, 24 Sep 2018 17:06:54 +0530 Message-ID: References: <1528476325-15585-1-git-send-email-anoob.joseph@caviumnetworks.com> <1536033560-21541-1-git-send-email-ajoseph@caviumnetworks.com> <1536033560-21541-10-git-send-email-ajoseph@caviumnetworks.com> <0b2cecb1-f055-61d6-3c10-802d0d647b96@nxp.com> <21787cc6-1151-53e9-a86b-aa008b421fb1@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: Akhil Goyal , Anoob Joseph , Pablo de Lara , Thomas Monjalon , Murthy NSSR , Jerin Jacob , Narayana Prasad , dev@dpdk.org, Ankur Dwivedi , Nithin Dabilpuram , Ragothaman Jayaraman , Srisivasubramanian S , Tejasree Kondoj To: Fiona Trahe Return-path: Received: from NAM05-DM3-obe.outbound.protection.outlook.com (mail-eopbgr730059.outbound.protection.outlook.com [40.107.73.59]) by dpdk.org (Postfix) with ESMTP id 57EFD2B8C for ; Mon, 24 Sep 2018 13:38:32 +0200 (CEST) In-Reply-To: <21787cc6-1151-53e9-a86b-aa008b421fb1@caviumnetworks.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Fiona, Can you please comment on this? We are adding all capabilities of octeontx-crypto PMD as a macro in otx_cryptodev_capabilites.h file and then we are using it from otx_cryptodev_ops.c. This is the approach followed by QAT crypto PMD. As per my understanding, this is to ensure that cryptodev_ops file remains simple. For other PMDs with fewer number of capabilities, the structure can be populated in the .c file itself without the size of the file coming into the picture. But this would cause checkpatch to report error. Akhil's suggestion is to move the entire definition to a header and include it from the .c file. I believe, the QAT approach was to avoid variable definition in the header. What do you think would be a better approach here? Thanks, Anoob On 17-09-2018 18:05, Joseph, Anoob wrote: > Hi Akhil, > > On 17-09-2018 17:31, Akhil Goyal wrote: >> External Email >> >>> diff --git a/drivers/crypto/octeontx/otx_cryptodev_ops.c >>> b/drivers/crypto/octeontx/otx_cryptodev_ops.c >>> index d25f9c1..cc0030e 100644 >>> --- a/drivers/crypto/octeontx/otx_cryptodev_ops.c >>> +++ b/drivers/crypto/octeontx/otx_cryptodev_ops.c >>> @@ -10,9 +10,15 @@ >>>   #include "cpt_pmd_logs.h" >>> >>>   #include "otx_cryptodev.h" >>> +#include "otx_cryptodev_capabilities.h" >>>   #include "otx_cryptodev_hw_access.h" >>>   #include "otx_cryptodev_ops.h" >>> >>> +static const struct rte_cryptodev_capabilities otx_capabilities[] = { >>> +     OTX_SYM_CAPABILITIES, >>> +     RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST() >>> +}; >>> + >> >> better to have otx_capabilities structure defined in the >> otx_cryptodev_capabilities.h >> >> I don't see any value addition of creating a macro in one file using >> in a separate structure in another file >> >> which doesn't have anything new in that structure. It would also give >> checkpatch error. >> >> You can directly have a capability structure without the #define. > This was the convention followed in qat driver. > > https://git.dpdk.org/dpdk/tree/drivers/crypto/qat/qat_sym_capabilities.h > > I guess it was to avoid variable definition in header. May be Pablo > too can comment on this. I'll make the change accordingly. > > Thanks, > Anoob >