Re: [PATCH v3 01/17] net/ntnic: Add registers for NapaTech SmartNiC

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 6/7/2024 2:03 PM, Serhii Iliushyk wrote:

Moved down. Please don't top post.

> 
> ------------------------------------------------------------------------
> *From:* Mykola Kostenok <mko-plv@napatech.com>
> *Sent:* 04 June 2024 13:29
> *To:* Serhii Iliushyk <sil-plv@napatech.com>; dev@dpdk.org <dev@dpdk.org>
> *Cc:* Christian Koue Muf <ckm@napatech.com>;
> andrew.rybchenko@oktetlabs.ru <andrew.rybchenko@oktetlabs.ru>;
> ferruh.yigit@amd.com <ferruh.yigit@amd.com>
> *Subject:* RE: [PATCH v3 01/17] net/ntnic: Add registers for NapaTech
> SmartNiC
>  
> Recheck-request: iol-unit-amd64-testing, iol-compile-amd64-testing,
> iol-unit-arm64-testing, iol-compile-arm64-testing
> 
>> -----Original Message-----
>> From: Serhii Iliushyk <sil-plv@napatech.com>
>> Sent: Monday, June 3, 2024 7:18 PM
>> To: dev@dpdk.org
>> Cc: Mykola Kostenok <mko-plv@napatech.com>; Christian Koue Muf
>> <ckm@napatech.com>; andrew.rybchenko@oktetlabs.ru;
>> ferruh.yigit@amd.com
>> Subject: [PATCH v3 01/17] net/ntnic: Add registers for NapaTech SmartNiC
>>
>> The NTNIC PMD does not rely on a kernel space Napatech driver,
>> thus all defines related to the register layout is part of the PMD
>> code, which will be added in later commits.
>>
>> Signed-off-by: Serhii Iliushyk <sil-plv@napatech.com>
>> ---
>>  .../supported/nthw_fpga_9563_055_039_0000.c   | 3488
>
> Hi Ferruh,
>
> We have sent a patch set which includes base PMD driver for NapaTech
> smartNiC for DPDK v24.07.
>
> This time, driver has limited basic functionality(Link up only) and we
> have made refactoring to split it into smaller patches.
> Extended functionality is planned for upstream to the next release DPDK
> 24.11.
>
> Patches have passed the CI but there are several code style warnings
> which we assumed as false-positive.
> Could you please take a look at these patches and suggest us how we can
> procced with it?
>
>

Hi Serhii,

Thanks for the rework.

Still there are some big chunk of code added without being used.
Some auto generated code and register definition files are
understandable, but there are lots of .c files as well adding lots of
new functions as a bunch, out of any context.
It is not possible for anyone to detect any unused code or review them
without knowing where/how they are actually used.
(To put with a simple analogy, you are staring to build an apartment by
putting windows and doors in place first.)

I will progress with the series, as it is more pragmatic to have it
merged than keeping out of tree.

There will be more comments on patches, but in high-level, primary
comment is please don't include anything not used/necessary for this
initial functionality.
Everything can be merged later when they are actually needed/used.


Btw, I can see some of the checkpatch warnings are valid, can you please
go through them?

> 
> Disclaimer: This email and any files transmitted with it may contain
> confidential information intended for the addressee(s) only. The
> information is not to be surrendered or copied to unauthorized persons.
> If you have received this communication in error, please notify the
> sender immediately and delete this e-mail from your system.
>

And it would be nice if you can get rid of above disclaimer, it doesn't
make sense in an open-source contribution.