From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D75EFA04B1; Sat, 10 Oct 2020 11:45:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B74511D8E6; Sat, 10 Oct 2020 11:45:51 +0200 (CEST) Received: from smtpbgeu2.qq.com (smtpbgeu2.qq.com [18.194.254.142]) by dpdk.org (Postfix) with ESMTP id 58B591D8E5 for ; Sat, 10 Oct 2020 11:45:49 +0200 (CEST) X-QQ-mid: bizesmtp10t1602323143tpcenji3 Received: from jiawenwu (unknown [183.129.236.74]) by esmtp6.qq.com (ESMTP) with id ; Sat, 10 Oct 2020 17:45:43 +0800 (CST) X-QQ-SSF: 01400000000000C0C000B00A0000000 X-QQ-FEAT: X1hxMK9mv5Mrmk1ZlXTb5NB7LvMyaaKW2CccNKxwUjRMnAZ8+EjMfuSEF0zf0 4DLLlMJDSirEedZ1/sXOldZlJaGd1M5x3aqJwlVsHRvIJvLd3+kqog8aPjL5CJ9I011BJ78 ZDU/YaC0bWpGUYGN4EDTu1Cg9QdTJyDqw5SqhG4yghQQvzQpFvMnwWbaeV7edueU+DV+8zh tM4OT+JgMQ9NWtvKHbJt+HgjrapfRAgUhGVrxK2DD25O+TxF4Lg0OOkOnVgr3qbuaGnslRp 4SI/IWwDxnXnwtvBcCcaTjEeBomssPgRdDQ5cN6P3LgR7flTxvToU4m2kYkhHIAfyViFUuY OhYjVkY0nmxQ8Dr76Y= X-QQ-GoodBg: 2 From: "Jiawen Wu" To: "'Ferruh Yigit'" , References: <20201005120910.189343-1-jiawenwu@trustnetic.com> <9111b3e0-45b7-922f-00d6-08ac0fbeedd7@intel.com> <000701d69de8$bc4880d0$34d98270$@trustnetic.com> <9520ab39-ab11-af1d-f68d-6cc5a9cc7fc6@intel.com> In-Reply-To: <9520ab39-ab11-af1d-f68d-6cc5a9cc7fc6@intel.com> Date: Sat, 10 Oct 2020 17:45:42 +0800 Message-ID: <000001d69eea$1f4e7660$5deb6320$@trustnetic.com>+1DB687AAF179A210 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHJlj4aCK0Iq8YnxyZPtDQE+bqN9QK1MPplAhLFIEcBQWzEeal6VjCw Content-Language: zh-cn X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign6 X-QQ-Bgrelay: 1 Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/9/2020 5:47 PM, Ferruh Yigit wrote: > On 10/9/2020 4:03 AM, jiawenwu@trustnetic.com wrote: > > Hi Ferruh, > > > > For the syntax/style check issue, should I fix all the errors and = warnings or > just fix the errors? > > It seems to be a lot of warnings. > > >=20 > [Please don't top post, it makes archives un-readable] >=20 > Please fix all, but beware that there may be false positive in the = checkpatch > warnings, so you need to process the output first. > This is a new PMD, if the syntax is not put correct at first place, = very unlikely > that it will be fixed later, so lets try to fix them as much as = possible. >=20 > For some drivers, the base code is shared in multiple platforms, like = Linux, > FreeBSD, Windows etc..., for them we are more flexible and we allow to = keep > the original syntax of that shared code, *as long as it is consistent = within itself*. > Do you have similar case in the base folder files? >=20 > The code for the DPDK should follow the DPDK coding convention [1] and = should > have as less checkpatch warnings/errors as possible. >=20 > [1] https://doc.dpdk.org/guides/contributing/coding_style.html >=20 > Thanks, > ferruh >=20 >=20 There are some 'checks' show that macro argument reused will possible = take side-effects. Like this: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'y' - possible side-effects? #56: FILE: drivers/net/txgbe/base/txgbe_regs.h:35: +#define ROUND_UP(x, y) (((x) + (y) - 1) / (y) * (y)) But the example given in the DPDK coding convention is: #define MACRO(x, y) do { \ variable =3D (x) + (y); \ (y) +=3D 2; \ } while(0) It seems to reuse argument, too. Should I fix this 'check', or treat it as a false positive? > > -----Original Message----- > > From: Ferruh Yigit > > Sent: Tuesday, October 6, 2020 7:03 PM > > To: Jiawen Wu ; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD > > > > On 10/5/2020 1:08 PM, Jiawen Wu wrote: > >> v2: re-order patches and fix some known problems > >> v1: introduce txgbe PMD > >> > >> jiawenwu (56): > >> net/txgbe: add build and doc infrastructure > >> net/txgbe: add ethdev probe and remove > >> net/txgbe: add device init and uninit > >> net/txgbe: add error types and registers > >> net/txgbe: add mac type and bus lan id > >> net/txgbe: add HW infrastructure and dummy function > >> net/txgbe: add EEPROM functions > >> net/txgbe: add HW init and reset operation > >> net/txgbe: add PHY init > >> net/txgbe: add module identify > >> net/txgbe: add PHY reset > >> net/txgbe: add info get operation > >> net/txgbe: add interrupt operation > >> net/txgbe: add device configure operation > >> net/txgbe: add link status change > >> net/txgbe: add multi-speed link setup > >> net/txgbe: add autoc read and write > >> net/txgbe: add MAC address operations > >> net/txgbe: add unicast hash bitmap > >> net/txgbe: add RX and TX init > >> net/txgbe: add RX and TX queues setup and release > >> net/txgbe: add RX and TX start and stop > >> net/txgbe: add packet type > >> net/txgbe: fill simple transmit function > >> net/txgbe: fill transmit function with hardware offload > >> net/txgbe: fill TX prepare funtion > >> net/txgbe: fill receive functions > >> net/txgbe: add device start operation > >> net/txgbe: add RX and TX data path start and stop > >> net/txgbe: add device stop and close operations > >> net/txgbe: support RX interrupt > >> net/txgbe: add RX and TX queue info get > >> net/txgbe: add device stats get > >> net/txgbe: add device xstats get > >> net/txgbe: add queue stats mapping > >> net/txgbe: add VLAN handle support > >> net/txgbe: add SWFW semaphore and lock > >> net/txgbe: add PF module init and uninit for SRIOV > >> net/txgbe: add process mailbox operation > >> net/txgbe: add PF module configure for SRIOV > >> net/txgbe: add VMDq configure > >> net/txgbe: add RSS support > >> net/txgbe: add DCB support > >> net/txgbe: add flow control support > >> net/txgbe: add FC auto negotiation support > >> net/txgbe: add priority flow control support > >> net/txgbe: add device promiscuous and allmulticast mode > >> net/txgbe: add MTU set operation > >> net/txgbe: add FW version get operation > >> net/txgbe: add EEPROM info get operation > >> net/txgbe: add register dump support > >> net/txgbe: support device LED on and off > >> net/txgbe: add mirror rule operations > >> net/txgbe: add PTP support > >> net/txgbe: add DCB info get operation > >> net/txgbe: add Rx and Tx descriptor status > >> > > > > Hi Jiawen, > > > > Before going into more detailed reviews, the patchset conflicts with = some > recent changes in the main repo [1], can you please rebase on top of = the latest > head of the repo? > > > > Also DPDK syntax/style check scripts are giving errors, can you = please fix > them too? You should run following to check: > > ./devtools/checkpatches.sh -n56 > > ./devtools/check-git-log.sh -n56 > > (This one needs codespell package to show spelling errors) > > > > > > > > [1] mainly the list is: > > 1) PMD close behavior change, > > - .dev_close changes > > - RTE_ETH_DEV_CLOSE_REMOVE flag removed > > > > 2) Some dev_ops moved to ethdev struct > > - .rx_queue_count > > - .rx_descriptor_done > > - .rx_descriptor_status > > - .tx_descriptor_status > > > > > > > >