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 65BF6A0577; Wed, 15 Apr 2020 08:14:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 26C591C235; Wed, 15 Apr 2020 08:14:02 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id A5C701C232 for ; Wed, 15 Apr 2020 08:14:00 +0200 (CEST) IronPort-SDR: R/KCojbL0p4HSwLrhSbMAJaF8ahQRtYPGuW2HAQ+dq74Nfs3aCxal+YDLVycdtaKnxad/3qpSs 6HbY6ZyFGfJw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2020 23:13:59 -0700 IronPort-SDR: MHBCRJbYjZPIsHErQKvHOd7Syq0rvYT84oWhAqJOUb0QlGrKspxfGOBrl45OLAOKBqbTlpf/Xr N0+wSBD1qJ+Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,385,1580803200"; d="scan'208";a="245611540" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga008.fm.intel.com with ESMTP; 14 Apr 2020 23:13:59 -0700 Received: from fmsmsx102.amr.corp.intel.com (10.18.124.200) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 23:13:59 -0700 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX102.amr.corp.intel.com (10.18.124.200) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 23:13:58 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.225]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.89]) with mapi id 14.03.0439.000; Wed, 15 Apr 2020 14:13:55 +0800 From: "Xu, Rosen" To: "Chautru, Nicolas" , "dev@dpdk.org" , "akhil.goyal@nxp.com" CC: "Richardson, Bruce" , "O'Hare, Cathal" Thread-Topic: [dpdk-dev] [PATCH v2 06/13] baseband/fpga_5gnr_fec: add queue configuration Thread-Index: AQHWBicJq64l3B4afUSIi+X+hr/JtKhzUQAggAQAcACAAntZoA== Date: Wed, 15 Apr 2020 06:13:54 +0000 Message-ID: <0E78D399C70DA940A335608C6ED296D73AF109BE@SHSMSX104.ccr.corp.intel.com> References: <1585526580-113508-1-git-send-email-nicolas.chautru@intel.com> <1585526580-113508-7-git-send-email-nicolas.chautru@intel.com> <0E78D399C70DA940A335608C6ED296D73AEF8ECE@SHSMSX104.ccr.corp.intel.com> <1183128033837D43A851F70F33ED5C576EFEAB6B@FMSMSX109.amr.corp.intel.com> In-Reply-To: <1183128033837D43A851F70F33ED5C576EFEAB6B@FMSMSX109.amr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZWFjZTg1YjgtOGRlOC00OWMzLThkYTItMmNhNDY2ODcxODJhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYjJ5XC9nNWRna3VXR3ZCd1VSYm1zNTlHbTl4aFNrYXBiakhmbXVtMTQzZVpcL3Bia0EyTG81ZFFMcFcyRXBTNFlMIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 06/13] baseband/fpga_5gnr_fec: add queue configuration 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" Hi, > -----Original Message----- > From: Chautru, Nicolas > Sent: Tuesday, April 14, 2020 8:16 > To: Xu, Rosen ; dev@dpdk.org; akhil.goyal@nxp.com > Cc: Richardson, Bruce > Subject: RE: [dpdk-dev] [PATCH v2 06/13] baseband/fpga_5gnr_fec: add > queue configuration >=20 > Thanks Rosen for your thorough code review. Some comments in-line below. >=20 > > From: Xu, Rosen > > > > Hi, > > > > Could you prefix all functions name with the FPGA IP name? FPGA is a > > very common device name. > > >=20 > I don't see such a guideline being used across all other PMDs. > Unsure it always help notably as this would create many long function nam= es > with fpga_5gnr_fec_ added each time, and these names are not exposed > outside of this .c file. > Also some of the fpga_ prefixes would apply for either fpga_lte_fec or > fpga_5gnr_fec PMD. > If this becomes of a new guide lines we can rename every single function = in > each existing baseband PMD in future release (not just this new PMD). What I mentioned is that, let's take FVL for example, in our PMD, we name i= t's PMD functions with I40e_xxx, i40e is Intel NIC name, but for your design it named with fpga_5g= nr_xxx, fpga is a common device, That means not Intel only provide FPGA, no sure if any other developer summ= it other FPGA based 5G acceleration IP, is it ok? > > > /* Read a register of FPGA 5GNR FEC device */ static uint32_t > > > fpga_reg_read_32(void *mmio_base, uint32_t offset) @@ -31,9 > +106,115 > > > @@ > > > return rte_le_to_cpu_32(ret); > > > } > > > > Why you didn't use rte_writeXXX() API of DPDK rte library? > > >=20 > rte_write32 includes some memory barriers which are not required here. > Also we handle the byte endianness in these internal implementations. >=20 > Thanks, > Nic