From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 75D0EA0577;
	Tue, 14 Apr 2020 02:16:27 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 57FA91BFE8;
	Tue, 14 Apr 2020 02:16:26 +0200 (CEST)
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id 0E4CB1BFE6
 for <dev@dpdk.org>; Tue, 14 Apr 2020 02:16:24 +0200 (CEST)
IronPort-SDR: bQzrDf//wMGsStk2j+F2rsm7LBjAzlDFWCluXKZaPdULyQwMrFkiUm4hYheP0gOTVlxPZRYns0
 zLeFN/W7YpIg==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga005.jf.intel.com ([10.7.209.41])
 by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 13 Apr 2020 17:16:23 -0700
IronPort-SDR: sXYRlVf0k/sw5UrJBoBmSh7gnpBhbieISqY4k+UWLaeQL1HYhuteM/TCmsiKu7aXOdJjS0zXHJ
 9r677hIxVIVw==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.72,380,1580803200"; d="scan'208";a="426872336"
Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204])
 by orsmga005.jf.intel.com with ESMTP; 13 Apr 2020 17:16:23 -0700
Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by
 FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS)
 id 14.3.439.0; Mon, 13 Apr 2020 17:16:23 -0700
Received: from FMSMSX109.amr.corp.intel.com ([169.254.15.183]) by
 FMSMSX151.amr.corp.intel.com ([169.254.7.87]) with mapi id 14.03.0439.000;
 Mon, 13 Apr 2020 17:16:22 -0700
From: "Chautru, Nicolas" <nicolas.chautru@intel.com>
To: "Xu, Rosen" <rosen.xu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "akhil.goyal@nxp.com" <akhil.goyal@nxp.com>
CC: "Richardson, Bruce" <bruce.richardson@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v2 06/13] baseband/fpga_5gnr_fec: add queue
 configuration
Thread-Index: AQHWD68mVleArXJ7Ck61eeUar/1m4qh3v4vw
Date: Tue, 14 Apr 2020 00:16:22 +0000
Message-ID: <1183128033837D43A851F70F33ED5C576EFEAB6B@FMSMSX109.amr.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>
In-Reply-To: <0E78D399C70DA940A335608C6ED296D73AEF8ECE@SHSMSX104.ccr.corp.intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
dlp-product: dlpe-windows
dlp-version: 11.2.0.6
dlp-reaction: no-action
x-originating-ip: [10.1.200.107]
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Thanks Rosen for your thorough code review. Some comments in-line below.=20

> From: Xu, Rosen <rosen.xu@intel.com>
>=20
> Hi,
>=20
> 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.=20
Unsure it always help notably as this would create many long function names=
 with fpga_5gnr_fec_ added each time, and these names are not exposed outsi=
de of this .c file.
Also some of the fpga_ prefixes would apply for either fpga_lte_fec or fpga=
_5gnr_fec PMD.=20
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).=20

> >  /* 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);
> >  }
>=20
> 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,=20
Nic