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: Fri, 28 Sep 2018 16:44:20 +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 NAM04-CO1-obe.outbound.protection.outlook.com (mail-eopbgr690040.outbound.protection.outlook.com [40.107.69.40]) by dpdk.org (Postfix) with ESMTP id DA94F1B113 for ; Fri, 28 Sep 2018 13:13:34 +0200 (CEST) In-Reply-To: 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, Did you get a chance to look at this? Thanks, Anoob On 24-09-2018 17:06, Joseph, Anoob wrote: > 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 >> >