23/10/2018 10:48, Joseph, Anoob:
> From: Thomas Monjalon <tho...@monjalon.net>
> > 22/10/2018 05:49, Joseph, Anoob:
> > > Hi Fiona,
> > >
> > > I do agree that your solution seems to be a neat way for organizing
> > capabilities. But Akhil & Thomas were against that idea and we had to come
> > up
> > with one array with all capabilities. This would not scale well when we
> > start
> > supporting devices with varying capabilities.
> > >
> > > If your plan is to follow the same approach for asym support, maybe we
> > > will
> > also follow suit and submit the required patches.
> > >
> > > @Akhil, Thomas, thoughts?
> >
> > Generally speaking, macros are bad.
> >
> > Why cannot you just write functions?
> > I don't understand your issue.
>
> I had replaced macro with functions when I revised the patch. But when more
> devices (with varying capabilities) need to be supported, this can get
> complicated. Macro approach would be simpler in that case. Right now QAT has
> macros and we would like to stick to what is being followed in the community.
>
> Following would be the use case for macros,
>
> #define QAT_BASE_GEN1_SYM_CAPABILITIES, \
> { CAPABILITES, \
> ... \
> }
>
> #define QAT_EXTRA_GEN2_SYM_CAPABILITIES \
> { CAPABILITES, \
> ... \
> }
>
> static const struct rte_cryptodev_capabilities qat_gen1_sym_capabilities[] = {
> QAT_BASE_GEN1_SYM_CAPABILITIES,
> RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> };
>
> static const struct rte_cryptodev_capabilities qat_gen2_sym_capabilities[] = {
> QAT_BASE_GEN1_SYM_CAPABILITIES,
> QAT_EXTRA_GEN2_SYM_CAPABILITIES,
> RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> };
>
> Without the macros, we will be required to populate the array there itself
> and would mean repetition of GEN1 capabilities. Either approach is fine for
> us, but this could complicate things when we add support for ASYM. Since QAT
> is already doing this, is it fine to move to that approach as we add support
> for ASYM? Or if QAT is to follow any other scheme, I'm fine with adopting
> that as well. Whatever is simple and uniform would work.
You can use simple assignments in functions and avoid repetition.
There is a warning in checkpatch about macros.
I think we should be more strict with this warning.
