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

[王颢 <howard_wang@realsil.com.cn>]

Dear Stephen,

I have modified the code related to braces in the latest patch. Recently, I was reviewing the DPDK Coding Style and found the reason why I initially removed the redundant braces. If symmetrical braces are required, it would be better to update the DPDK Coding Style accordingly.

in https://doc.dpdk.org/guides/contributing/coding_style.html
Braces that are not necessary should be left out.

if (test)
        stmt;
else if (bar) {
        stmt;
        stmt;
} else
        stmt;

Best Regards,
Howard Wang

-----邮件原件-----
发件人: Stephen Hemminger <stephen@networkplumber.org> 
发送时间: 2024年10月23日 13:18
收件人: 王颢 <howard_wang@realsil.com.cn>
抄送: dev@dpdk.org; pro_nic_dpdk@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@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@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.