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 6EB32A0588; Thu, 16 Apr 2020 23:45:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C88FF1DE4D; Thu, 16 Apr 2020 23:45:41 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id B30071D9FE for ; Thu, 16 Apr 2020 23:45:40 +0200 (CEST) IronPort-SDR: 3nkCM6TGfacZKmABsu3vLdN2EoMkqjo3D42a68fEUiEiwH0b+9r1H5bJtmUPq5jhS5UI6iDpA6 WuwSP0ARWQug== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Apr 2020 14:45:39 -0700 IronPort-SDR: JpiuHrggdoF62gUbRh0OOjf4N8v8M111UyBObuop9PsMQM6E4vWIqeTCJhfYVhZi1IftBNdu88 1WCNruR7psYw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,392,1580803200"; d="scan'208";a="332989803" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by orsmga001.jf.intel.com with ESMTP; 16 Apr 2020 14:45:39 -0700 Received: from orsmsx123.amr.corp.intel.com (10.22.240.116) by ORSMSX103.amr.corp.intel.com (10.22.225.130) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 16 Apr 2020 14:45:39 -0700 Received: from orsmsx155.amr.corp.intel.com ([169.254.7.107]) by ORSMSX123.amr.corp.intel.com ([169.254.1.39]) with mapi id 14.03.0439.000; Thu, 16 Apr 2020 14:45:38 -0700 From: "Chautru, Nicolas" To: Akhil Goyal , "dev@dpdk.org" CC: "Richardson, Bruce" Thread-Topic: [PATCH v2 10/13] baseband/fpga_5gnr_fec: add configure function Thread-Index: AQHWBia5R0/Gp5PdrkGqhlhqLpHPAqh8s+sA//+q7EA= Date: Thu, 16 Apr 2020 21:45:38 +0000 Message-ID: <1183128033837D43A851F70F33ED5C57893CA2A6@ORSMSX155.amr.corp.intel.com> References: <1585526580-113508-1-git-send-email-nicolas.chautru@intel.com> <1585526580-113508-11-git-send-email-nicolas.chautru@intel.com> In-Reply-To: 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.22.254.140] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 10/13] baseband/fpga_5gnr_fec: add configure function 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 Akhil,=20 > > diff --git a/drivers/baseband/fpga_5gnr_fec/Makefile > > b/drivers/baseband/fpga_5gnr_fec/Makefile > > index 3f5c511..b68a79f 100644 > > --- a/drivers/baseband/fpga_5gnr_fec/Makefile > > +++ b/drivers/baseband/fpga_5gnr_fec/Makefile > > @@ -23,4 +23,7 @@ LIBABIVER :=3D 1 > > # library source files > > SRCS-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC) +=3D > > rte_fpga_5gnr_fec.c > > > > +# export include files > > +SYMLINK-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC)-include > +=3D > > fpga_5gnr_fec.h > > + > > include $(RTE_SDK)/mk/rte.lib.mk > > diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h > > b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h > > new file mode 100644 > > index 0000000..7eebc7d > > --- /dev/null > > +++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h > > @@ -0,0 +1,74 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2020 Intel Corporation */ > > + > > +#ifndef _FPGA_5GNR_FEC_H_ > > +#define _FPGA_5GNR_FEC_H_ > > + > > +#include > > +#include > > + > > +/** > > + * @file fpga_5gnr_fec.h > > + * > > + * Interface for Intel(R) FGPA 5GNR FEC device configuration at the > > +host level, > > + * directly accessible by the application. > > + * Configuration related to 5GNR functionality is done through > > + * librte_bbdev library. > > + * > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice */ >=20 > The exposed PMD header files are normally prefixed as rte_pmd_ >=20 > You should rename your other header file as fpga_5gnr_fec.h And this one = as > rte_pmd_fpga_5gnr_fec.h >=20 OK > BTW what is the need of a pmd API to configure the fpga? > Is it not possible to do that as one of rte_bbdev_ops ? This configuration is done agnostically of bbdev, the application doesn't n= eed to know there is actual HW device requiring to be configured below the = hood. Also this operation would be possible for the VF driver which doesn't have = access to related registers.=20 Typical deployment is within a container environment and this configuration= is done without any need of any dependency on anything from librte_bbdev.= =20 Something we were considering was to push a standalone application (without= dependency on BBDEV or even DPDK) to do that configuration from PF.=20 Currently DPDK community are only able to run the PF driver, since to run f= rom the VF would need a separate application to configure the device to all= ow usage from VF.=20 We need to provide such application externally to DPDK currently (pretty mu= ch that function wrapped up with args and xml parsing).=20 So we were wondering whether this could still be provided with DPDK (even i= f application has no DPDK dependency) or just push it externally on git hub= . I see pros and cons, any view on that or similar concern for other crypto= drivers? >=20 > I can see that the comments in rte_bbdev_ops are not in proper format. > '<' should not be there if comment is before the element Could you please > correct them in a separate patch? Please check other structures as well >=20 Thanks. This was missed all over the place, will make a ticket for fixing i= n separate patch.=20 For now will fix in that new serie today.=20 > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/**< Number of Virtual Functions FGPA 4G FEC supports */ #define > > +FPGA_5GNR_FEC_NUM_VFS 8 > > + > > +/** > > + * Structure to pass FPGA 4G FEC configuration. > > + */ > > +struct fpga_5gnr_fec_conf { > > + /**< 1 if PF is used for dataplane, 0 for VFs */ > > + bool pf_mode_en; > > + /**< Number of UL queues per VF */ > > + uint8_t vf_ul_queues_number[FPGA_5GNR_FEC_NUM_VFS]; > > + /**< Number of DL queues per VF */ > > + uint8_t vf_dl_queues_number[FPGA_5GNR_FEC_NUM_VFS]; > > + /**< UL bandwidth. Needed for schedule algorithm */ > > + uint8_t ul_bandwidth; > > + /**< DL bandwidth. Needed for schedule algorithm */ > > + uint8_t dl_bandwidth; > > + /**< UL Load Balance */ > > + uint8_t ul_load_balance; > > + /**< DL Load Balance */ > > + uint8_t dl_load_balance; > > + /**< FLR timeout value */ > > + uint16_t flr_time_out; >=20 > If you are adding comment before the element, then no need to add '<'