DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: liwencheng <liwencheng@phytium.com.cn>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2 1/3] net/macb: add new driver
Date: Mon, 2 Dec 2024 12:31:03 -0800	[thread overview]
Message-ID: <20241202123103.70e7af2e@hermes.local> (raw)
In-Reply-To: <1730863931-1309539-1-git-send-email-liwencheng@phytium.com.cn>

On Wed,  6 Nov 2024 03:32:11 +0000
liwencheng <liwencheng@phytium.com.cn> wrote:

> add Phytium NIC MACB ethdev PMD driver.
> 
> Signed-off-by: liwencheng <liwencheng@phytium.com.cn>
> ---
>  drivers/net/macb/base/generic_phy.c |  276 +++++
>  drivers/net/macb/base/generic_phy.h |  198 ++++
>  drivers/net/macb/base/macb_common.c |  667 +++++++++++++
>  drivers/net/macb/base/macb_common.h |  253 +++++
>  drivers/net/macb/base/macb_errno.h  |   54 +
>  drivers/net/macb/base/macb_hw.h     | 1136 +++++++++++++++++++++
>  drivers/net/macb/base/macb_type.h   |   23 +
>  drivers/net/macb/base/macb_uio.c    |  351 +++++++
>  drivers/net/macb/base/macb_uio.h    |   50 +
>  drivers/net/macb/base/meson.build   |   26 +
>  drivers/net/macb/macb_ethdev.c      | 1885 +++++++++++++++++++++++++++++++++++
>  drivers/net/macb/macb_ethdev.h      |   91 ++
>  drivers/net/macb/macb_log.h         |   19 +
>  drivers/net/macb/macb_rxtx.c        | 1380 +++++++++++++++++++++++++
>  drivers/net/macb/macb_rxtx.h        |  325 ++++++
>  drivers/net/macb/meson.build        |   18 +
>  drivers/net/meson.build             |    1 +
>  17 files changed, 6753 insertions(+)
>  create mode 100644 drivers/net/macb/base/generic_phy.c
>  create mode 100644 drivers/net/macb/base/generic_phy.h
>  create mode 100644 drivers/net/macb/base/macb_common.c
>  create mode 100644 drivers/net/macb/base/macb_common.h
>  create mode 100644 drivers/net/macb/base/macb_errno.h
>  create mode 100644 drivers/net/macb/base/macb_hw.h
>  create mode 100644 drivers/net/macb/base/macb_type.h
>  create mode 100644 drivers/net/macb/base/macb_uio.c
>  create mode 100644 drivers/net/macb/base/macb_uio.h
>  create mode 100644 drivers/net/macb/base/meson.build
>  create mode 100644 drivers/net/macb/macb_ethdev.c
>  create mode 100644 drivers/net/macb/macb_ethdev.h
>  create mode 100644 drivers/net/macb/macb_log.h
>  create mode 100644 drivers/net/macb/macb_rxtx.c
>  create mode 100644 drivers/net/macb/macb_rxtx.h
>  create mode 100644 drivers/net/macb/meson.build

Please send v3 addressing my previous comments:

For better review, it is preferred that drivers be broken into smaller
chunks when submitting.

  https://doc.dpdk.org/guides/contributing/new_driver.html


1. Missing documentation
   all new drivers need to have documentation in doc/guides/nics
   to describe hardware and features.

2. Missing license tag.
   These files are missing required SPDX license tags:
drivers/net/macb/base/generic_phy.h
drivers/net/macb/base/macb_errno.h
drivers/net/macb/base/macb_hw.h

3. Missing mailmap entry

4. Missing update to MAINTAINERS

5. Build fails with lastest GCC. The problem is you chose too long a name for some of the
   xstats. Recommend choosing same name as other drivers if possible.


   Warnings found:

[1307/3230] Compiling C object drivers/libtmp_rte_net_macb.a.p/net_macb_macb_ethdev.c.o
In file included from ../drivers/net/macb/base/macb_common.h:36,
                 from ../drivers/net/macb/macb_ethdev.h:10,
                 from ../drivers/net/macb/macb_rxtx.h:8,
                 from ../drivers/net/macb/macb_ethdev.c:11:
../drivers/net/macb/base/macb_hw.h:1063:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1063:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1088:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1088:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
[1326/3230] Compiling C object drivers/libtmp_rte_net_macb.a.p/net_macb_macb_rxtx.c.o
In file included from ../drivers/net/macb/base/macb_common.h:36,
                 from ../drivers/net/macb/macb_ethdev.h:10,
                 from ../drivers/net/macb/macb_rxtx.h:8,
                 from ../drivers/net/macb/macb_rxtx.c:24:
../drivers/net/macb/base/macb_hw.h:1063:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1063:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1088:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1088:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
[2338/3230] Compiling C object drivers/net/macb/base/libmacb_base.a.p/generic_phy.c.o
In file included from ../drivers/net/macb/base/macb_common.h:36,
                 from ../drivers/net/macb/base/generic_phy.h:4,
                 from ../drivers/net/macb/base/generic_phy.c:5:
../drivers/net/macb/base/macb_hw.h:1063:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1063:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1088:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1088:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
[2343/3230] Compiling C object drivers/net/macb/base/libmacb_base.a.p/macb_uio.c.o
In file included from ../drivers/net/macb/base/macb_common.h:36,
                 from ../drivers/net/macb/base/macb_uio.h:4,
                 from ../drivers/net/macb/base/macb_uio.c:6:
../drivers/net/macb/base/macb_hw.h:1063:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1063:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1088:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1088:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
[2352/3230] Compiling C object drivers/net/macb/base/libmacb_base.a.p/macb_common.c.o
In file included from ../drivers/net/macb/base/macb_common.h:36,
                 from ../drivers/net/macb/base/macb_uio.h:4,
                 from ../drivers/net/macb/base/macb_common.c:7:
../drivers/net/macb/base/macb_hw.h:1063:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1063:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1063 |         GEM_STAT_TITLE(TX1519CNT, "tx_greater_than_1518_byte_frames"),
      |         ^~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1088:35: warning: initializer-string for array of ‘char’ is too long [-Wunterminated-string-initialization]
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/macb/base/macb_hw.h:1043:24: note: in definition of macro ‘GEM_STAT_TITLE_BITS’
 1043 |         .stat_string = title,                           \
      |                        ^~~~~
../drivers/net/macb/base/macb_hw.h:1088:9: note: in expansion of macro ‘GEM_STAT_TITLE’
 1088 |         GEM_STAT_TITLE(RX1519CNT, "rx_greater_than_1518_byte_frames"),


  parent reply	other threads:[~2024-12-02 20:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-01 10:07 [PATCH v1 1/2] " liwencheng
2024-11-01 10:07 ` [PATCH v1 2/2] /usertools/dpdk-devbind:add the binding and unbinding of platform device liwencheng
2024-11-05  8:43   ` [PATCH v2 3/3] usertools/dpdk-devbind: add platform device bind/unbind liwencheng
2024-11-06  3:34     ` [PATCH v2 3/3] usertools/dpdk-devbind: add bind/unbind for platform device liwencheng
2024-11-11  7:43       ` liwencheng
2024-12-03  8:15         ` [PATCH][v2, " Wencheng Li
2024-11-06  3:55   ` [PATCH v1 2/2] /usertools/dpdk-devbind:add the binding and unbinding of " Stephen Hemminger
2024-11-12  1:08     ` liwencheng
2024-11-01 16:13 ` [PATCH v1 1/2] net/macb: add new driver Stephen Hemminger
2024-11-01 17:42 ` Stephen Hemminger
2024-11-02  5:43 ` Stephen Hemminger
2024-11-05  8:41 ` [PATCH v2 1/3] " liwencheng
2024-11-05  8:41   ` [PATCH v2 2/3] net/macb: add NEON vectorized Rx/Tx liwencheng
2024-11-06  3:33     ` liwencheng
2024-11-11  7:42       ` liwencheng
2024-12-06  8:08         ` [PATCH v3 " Wencheng Li
2024-12-10  7:05         ` [PATCH v3 2/6] " Wencheng Li
2024-12-10  7:07         ` [PATCH v3 3/6] net/macb: fix logic error in macb_rxq_rearm function Wencheng Li
2024-11-06  3:32   ` [PATCH v2 1/3] net/macb: add new driver liwencheng
2024-11-11  7:42     ` liwencheng
2024-12-06  8:06       ` [PATCH v3 " Wencheng Li
2024-12-10  7:04       ` [PATCH v3 1/6] " Wencheng Li
2024-12-10  7:07       ` [PATCH v3 4/6] net/macb: zero ol_flags in each recv function Wencheng Li
2024-12-10  7:08       ` [PATCH v3 5/6] net/macb: fix tab errors in meson.build file Wencheng Li
2024-12-10 19:46         ` Stephen Hemminger
2024-12-02 20:31     ` Stephen Hemminger [this message]
2024-11-22 17:55   ` [PATCH v2 1/3] net/macb: add new driver Stephen Hemminger
2024-11-22 20:38   ` Stephen Hemminger
2024-11-22 20:44   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241202123103.70e7af2e@hermes.local \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=liwencheng@phytium.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).