[dpdk-dev] [PATCH v2 01/33] config: add Cavium OcteonTX crypto PMD skeleton

[Akhil Goyal]

On 9/18/2018 6:10 PM, Joseph at dpdk.org wrote:
> Hi Akhil,
>
> On 18-09-2018 18:01, Akhil Goyal wrote:
>> External Email
>>
>> Hi Anoob,
>>
>> On 9/17/2018 7:43 PM, Joseph, Anoob wrote:
>>> Hi Akhil,
>>>
>>>
>>> On 17-09-2018 17:50, Akhil Goyal wrote:
>>>> External Email
>>>>
>>>> On 9/17/2018 5:12 PM, Joseph, Anoob wrote:
>>>>
>>>>> Hi Akhil,
>>>>> On 17-09-2018 16:07, Akhil Goyal wrote:
>>>>>> External Email
>>>>>>>> I think it would be better to squash the makefile related
>>>>>>>> changes in
>>>>>>>> the 3/33 patch as the code
>>>>>>>> is actually added in that and here the code is not getting compiled
>>>>>>>> here.
>>>>>>> So the changes in the following files has to be moved to patch 3/33?
>>>>>>>     drivers/crypto/Makefile
>>>>>>>     drivers/crypto/meson.build
>>>>>>>     drivers/crypto/octeontx/Makefile
>>>>>>>     drivers/crypto/octeontx/meson.build
>>>>>>>     mk/rte.app.mk
>>>>>>> I think this patch will just have MAINTAINER edit (even that
>>>>>>> might be
>>>>>>> required to be moved to 3/33?) & changes to config/common_base,
>>>>>>> after
>>>>>>> that. Is that fine?
>>>>>> In my opinion, you do not need this patch as separate one.
>>>>>> config/common_base can also be added in the 3/33.
>>>>> In that case 02/33 patch would become the first patch right? The same
>>>>> problem would be there too, I guess. The macros added in that patch
>>>>> gets
>>>>> used only in 03/33 patch. Is that fine?
>>>> I think that would be fine. Better to have a 03/33 patch before 02/33
>>>> if it doesn't have dependencies.
>>> 03/33 patch is dependent on 02/33 patch. Shall I proceed with merging
>>> 01/33 to 03/33 and make 02/33 the first patch?
>>>>> The first patch would be a shell patch for most PMD additions. That's
>>>>> the reason we started this way. If you want it changed, will do so.
>>>>> Please do let me know what will be the right approach.
>>>> For the makefiles, you would be compiling the empty files which does
>>>> not have any code. That does not make any sense to me.
>>> With 01/33 there won't be any files compiled. We are just adding the
>>> library (which would be empty)
>>>> Normally, when we submit a new PMD, we add the basic PMD probe/remove
>>>> in the first patch and add it into build system. Maintainers is also
>>>> updated for the new PMD.
>>>>
>>>> Further ops are added later in the patchset.
>>>>
>>>> Hardware specific header files/ functions are added before they are
>>>> used in the driver in a single/multiple logical patches.
>>>>
>>>> In the end, documentation is added along with release note and
>>>> MAINTAINERS update for documentation files.
>>> I shall proceed with merging 01/33 to 03/33, if you can confirm making
>>> 02/33 the first patch is fine. Or please do let me know if you have
>>> any other suggestions.
>>>
>> I see that the 2/33 is adding the logging macros. I believe that can
>> also be merged in the 03/33.
> ./devtools/check-git-log.sh was giving me issues when one patch was
> having edits in both drivers/common/cpt & drivers/crypto/octeontx.
> That's the reason it was separated out. Many patches had to be divided
> because of this.
Oh,  my bad... I missed that this is a separate driver. I thought it was 
for the same driver.
It would be fine to have 02/33 as your first patch as Thomas also suggested.
>> Also I missed one comment on the documentation patch.. Please add your
>> pmd doc entry in index.rst as well.
> Will fix this in v3.
>
> Thanks,
> Anoob