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 98F68A04B6; Mon, 12 Oct 2020 10:37:26 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6DD701D67B; Mon, 12 Oct 2020 10:37:25 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 155EE1D679 for ; Mon, 12 Oct 2020 10:37:23 +0200 (CEST) IronPort-SDR: ISbmPCM8O7QGJurMqqgDoCKelTD1aIJPwzo/ywLYey8ZCWXMr2rvo9kmZn/TYZzwRNP1YKqq/p gz1vUER/KkIA== X-IronPort-AV: E=McAfee;i="6000,8403,9771"; a="145572528" X-IronPort-AV: E=Sophos;i="5.77,366,1596524400"; d="scan'208";a="145572528" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Oct 2020 01:37:22 -0700 IronPort-SDR: HRnlNZzu9fDuapxTSAqYgcL2jo8M7P+DE+x8v9ExS/bt5XuS3Ia/T/u/ITHe01YBCyw3743zf+ ffCTNW6XneQw== X-IronPort-AV: E=Sophos;i="5.77,366,1596524400"; d="scan'208";a="520615865" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.244.119]) ([10.213.244.119]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Oct 2020 01:37:21 -0700 To: Jiawen Wu , dev@dpdk.org 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> <000001d69eea$1f4e7660$5deb6320$@trustnetic.com> From: Ferruh Yigit Message-ID: Date: Mon, 12 Oct 2020 09:37:20 +0100 MIME-Version: 1.0 In-Reply-To: <000001d69eea$1f4e7660$5deb6320$@trustnetic.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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/10/2020 10:45 AM, Jiawen Wu wrote: > 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. >>> >> >> [Please don't top post, it makes archives un-readable] >> >> 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. >> >> 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? >> >> The code for the DPDK should follow the DPDK coding convention [1] and should >> have as less checkpatch warnings/errors as possible. >> >> [1] https://doc.dpdk.org/guides/contributing/coding_style.html >> >> Thanks, >> ferruh >> >> > > 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 = (x) + (y); \ > (y) += 2; \ > } while(0) > > It seems to reuse argument, too. > Should I fix this 'check', or treat it as a false positive? > That is a check, please double check if the usage is safe, and if it is you can ignore it. > >>> -----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 >>> >>> >>> >>> > > > >