答复: [PATCH v3 01/18] net/r8169: add PMD driver skeleton

[王颢]

Dear Stephen,

OK, I will fix these issues.
I would like to confirm that each document needs to have a blank line at the end, correct? 
Apologies, but we plan to maintain our current approach regarding stats this time.

Best Regards,
Howard Wang

-----邮件原件-----
发件人: Stephen Hemminger <stephen at networkplumber.org> 
发送时间: 2024年10月23日 13:18
收件人: 王颢 <howard_wang at realsil.com.cn>
抄送: dev at dpdk.org; pro_nic_dpdk at realtek.com
主题: Re: [PATCH v3 01/18] net/r8169: add PMD driver skeleton


External mail.



On Wed, 23 Oct 2024 11:33:11 +0800
Howard Wang <howard_wang at realsil.com.cn> wrote:

> Meson build infrastructure, r8169_ethdev minimal skeleton, header with 
> Realtek NIC device and vendor IDs.
>
> Signed-off-by: Howard Wang <howard_wang at realsil.com.cn>

This version is much better than the last.
Still has some issues:
  - git whitespace
  - braces
  - one spelling error

For example, doing git apply showed.

Applying: net/r8169: add support for hardware operations
/home/shemminger/DPDK/main/.git/worktrees/r8169/rebase-apply/patch:2381: trailing whitespace.
                                          (BIT_15 | BIT_14 | BIT_13 | BIT_12 |
/home/shemminger/DPDK/main/.git/worktrees/r8169/rebase-apply/patch:460: new blank line at EOF.
+
/home/shemminger/DPDK/main/.git/worktrees/r8169/rebase-apply/patch:2052: new blank line at EOF.
+
/home/shemminger/DPDK/main/.git/worktrees/r8169/rebase-apply/patch:2073: new blank line at EOF.
+
/home/shemminger/DPDK/main/.git/worktrees/r8169/rebase-apply/patch:2470: new blank line at EOF.
+
warning: squelched 13 whitespace errors
warning: 18 lines add whitespace errors.


Some minor checkpatch warnings.

CHECK:BRACES: braces {} should be used on all arms of this statement
#268: FILE: drivers/net/r8169/r8169_hw.c:67:
+               if (len <= 4 - val_shift)
[...]
+               else {
[...]

CHECK:BRACES: Unbalanced braces around else statement
#270: FILE: drivers/net/r8169/r8169_hw.c:69:
+               else {

CHECK:BRACES: braces {} should be used on all arms of this statement
#334: FILE: drivers/net/r8169/r8169_hw.c:133:
+               if (len <= 4 - val_shift)
[...]
+               else {
[...]

CHECK:BRACES: Unbalanced braces around else statement
#336: FILE: drivers/net/r8169/r8169_hw.c:135:
+               else {

CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#433: FILE: drivers/net/r8169/r8169_hw.c:232:
+
+               }

WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#978: FILE: drivers/net/r8169/r8169_phy.c:2:
+ * Copyright(c) 2024 Realtek Corporation. All rights reserved */

CHECK:BRACES: braces {} should be used on all arms of this statement
#273: FILE: drivers/net/r8169/r8169_phy.c:536:
+       if (hw->hw_ram_code_ver == hw->sw_ram_code_ver) {
[...]
+       } else
[...]

### [PATCH] net/r8169: impelment MTU configuration

WARNING:TYPO_SPELLING: 'impelment' may be misspelled - perhaps 'implement'?
#4:
Subject: [PATCH] net/r8169: impelment MTU configuration
                            ^^^^^^^^^
Suprised that you have to keep track of packets in SW.
The kernel driver is able to get them from the HW.

The kernel driver gets lots more counters from the HW which could go to xstats.