<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div dir="ltr">
<div dir="ltr">I like the separation. I can include it in V4 and see, it would be helpful to know if it’s more or less confusing that way. </div>
<div dir="ltr"><br>
</div>
<div dir="ltr">For the prompt before each list, can we say something like “Avoid doing the following” and “Suggested actions” or something a little better grammatically. We could also just say “Avoid”.  </div>
<div id="ms-outlook-mobile-signature" dir="ltr"></div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Ferruh Yigit <ferruh.yigit@amd.com><br>
<b>Sent:</b> Thursday, September 12, 2024 1:13:33 AM<br>
<b>To:</b> Nandini Persad <nandinipersad361@gmail.com><br>
<b>Cc:</b> dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>; Stephen Hemminger <stephen@networkplumber.org><br>
<b>Subject:</b> Re: [PATCH v3] doc: add new driver guidelines</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On 9/11/2024 5:04 PM, Nandini Persad wrote:<br>
> Hi Ferruh,<br>
> <br>
> I will work with Stephen on this. For the tone of the list, I believe we<br>
> can try different ways to make the tone more friendly, while still being<br>
> concise.<br>
> <br>
> What about something like this:<br>
> # Avoid including unused headers (process-iwyu.py).<br>
> # Keep compiler warnings enabled in the build file.<br>
> # Instead of using #ifdef with driver-specific macros, opt for runtime<br>
> configuration.<br>
> # Document device parameters in the driver guide.<br>
> # Make device operations structs 'const'.<br>
> # Use dynamic logging.<br>
> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.<br>
> # Add SPDX license tags and copyright notices on all sides.<br>
> # Don’t introduce public APIs directly from the driver.<br>
> <br>
> It's slightly more friendly.<br>
> <br>
> Let me know what you think, I'm open to trying another way.<br>
> <br>
<br>
I think above is better.<br>
<br>
Another option can be separating it as "Do" and "Do Not" list, as<br>
following, do you think does it help, or makes it harder to understand?<br>
<br>
Avoid doing:<br>
- Using PMD specific macros when DPDK ones exist<br>
- Including unused headers<br>
- Disable compiler warnings for driver<br>
- #ifdef with driver-defined macros<br>
- DPDK version checks (via RTE_VERSION_NUM) in the upstream code<br>
- Public APIs directly from the driver<br>
<br>
Suggested to do:<br>
- Runtime configuration when applicable<br>
- Document device parameters in the driver guide<br>
- Make device operations struct 'const'<br>
- Dynamic logging<br>
- SPDX license tags and copyright notice on each file<br>
<br>
<br>
> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com<br>
> <<a href="mailto:ferruh.yigit@amd.com">mailto:ferruh.yigit@amd.com</a>>> wrote:<br>
> <br>
>     On 9/10/2024 3:58 PM, Nandini Persad wrote:<br>
>     > This document was created to assist contributors<br>
>     > in creating DPDK drivers, providing suggestions<br>
>     > and guidelines for how to upstream effectively.<br>
>     ><br>
> <br>
>     There are minor differences in this v3 and v2, isn't this version on top<br>
>     of v2, can those changes be from Stephen?<br>
> <br>
>     <...><br>
> <br>
>     > +<br>
>     > +Additional Suggestions<br>
>     > +----------------------<br>
>     > +<br>
>     > +* We recommend using DPDK macros instead of inventing new ones in<br>
>     the PMD.<br>
>     > +* Do not include unused headers (process-iwyu.py).<br>
>     > +* Do not disable compiler warning in the build file.<br>
>     > +* Do not use #ifdef with driver-defined macros, instead prefer<br>
>     runtime configuration.<br>
>     > +* Document device parameters in the driver guide.<br>
>     > +* Make device operations struct 'const'.<br>
>     > +* Use dynamic logging.<br>
>     > +* Do not use DPDK version checks (via RTE_VERSION_NUM) in the<br>
>     upstream code.<br>
>     > +* Be sure to have SPDX license tags and copyright notice on each<br>
>     side.<br>
>     > +* Do not introduce public Apis directly from the driver.<br>
>     ><br>
> <br>
>     API (Application Programming Interface) is an acronym and should be all<br>
>     uppercase, like 'APIs'.<br>
> <br>
>     Overall the language in this list is imperative, I think it helps to<br>
>     make it simple, but I am not sure about the tone, I wonder if we can do<br>
>     better, do you have any suggestions?<br>
> <br>
> <br>
>     > +<br>
>     > +<br>
>     > +Dependencies<br>
>     > +------------<br>
>     > +<br>
>     > +At times, drivers may have dependencies to external software.<br>
>     > +For driver dependencies, same DPDK rules for dependencies applies.<br>
>     > +Dependencies should be publicly and freely available to<br>
>     > +upstream the driver.<br>
>     > +<br>
>     > +<br>
>     > +Test Tools<br>
>     > +----------<br>
>     > +<br>
>     > +Per patch in a patch series, be sure to use the proper test tools.<br>
>     > +<br>
>     > +* checkpatches.sh<br>
>     > +* check-git-log.sh<br>
>     > +* check-meson.py<br>
>     > +* check-doc-vs-code.sh<br>
>     > +* check-spdx-tag.sh<br>
>     ><br>
> <br>
>     `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am<br>
>     for keeping it here, as "additional suggestions" are more things to take<br>
>     into consideration during design/development, above are actual scripts<br>
>     that we can use to verify code.<br>
> <br>
>     And long term intention was to move this "tools to run list" to a more<br>
>     generic documentation, as these are not really specific to new PMD<br>
>     guide, but "additional suggestions" will stay in this document.<br>
> <br>
<br>
</div>
</span></font></div>
</body>
</html>