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 260CFA058E; Thu, 26 Mar 2020 03:53:54 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 20D731C037; Thu, 26 Mar 2020 03:53:53 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5E7761C036 for ; Thu, 26 Mar 2020 03:53:50 +0100 (CET) IronPort-SDR: 97DD+f9R7hc3Soabs9bh3KT6E7SXHO5G4P8D7rYmxxJKMyXgkfoWMI7Im0S+AFDuimUW9MLzP8 EjqVpOAdp15A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 19:53:50 -0700 IronPort-SDR: rS+taSCF0kbfRMWmQHImUddm2B++0AspnF5V4fPmFPo3xv2IHKyav+GyGuoHmuSdLfdVeZ2EUD w5uUrPIMywLg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="240552287" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga008.fm.intel.com with ESMTP; 25 Mar 2020 19:53:49 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 25 Mar 2020 19:53:49 -0700 Received: from FMSMSX109.amr.corp.intel.com ([169.254.15.183]) by FMSMSX157.amr.corp.intel.com ([169.254.14.85]) with mapi id 14.03.0439.000; Wed, 25 Mar 2020 19:53:49 -0700 From: "Chautru, Nicolas" To: Akhil Goyal , "thomas@monjalon.net" , "dev@dpdk.org" CC: "Yigit, Ferruh" Thread-Topic: [PATCH v3 06/14] test-bbdev: support HARQ validation Thread-Index: AQHV8lZzFMiR8T6qLU2EX308+3DyaahZ4c6AgABsO8A= Date: Thu, 26 Mar 2020 02:53:48 +0000 Message-ID: <1183128033837D43A851F70F33ED5C576EFD26C2@FMSMSX109.amr.corp.intel.com> References: <1582778348-113547-15-git-send-email-nicolas.chautru@intel.com> <1583348102-13253-1-git-send-email-nicolas.chautru@intel.com> <1583348102-13253-7-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.1.200.106] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 06/14] test-bbdev: support HARQ validation 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" From: Akhil Goyal =20 >> + /* ignore parity mismatch false alarms for long iterations */ >> + { >> + if (!(expected_status & (1 << RTE_BBDEV_SYNDROME_ERROR)) > >What is the need of these extra braces. > Missing check added now on new patchset. Thanks.=20 >> + >> +/* Compute K0 for a given configuration for HARQ output length=20 >> +computation >> */ >> +static inline uint16_t >> +get_k0(uint16_t n_cb, uint16_t z_c, uint8_t basegraph, uint8_t=20 >> +rv_index) { >> + if (rv_index =3D=3D 0) >> + return 0; >> + uint16_t n =3D (basegraph =3D=3D 1 ? 66 : 50) * z_c; >> + if (n_cb =3D=3D n) { >> + if (rv_index =3D=3D 1) >> + return (basegraph =3D=3D 1 ? 17 : 13) * z_c; >> + else if (rv_index =3D=3D 2) >> + return (basegraph =3D=3D 1 ? 33 : 25) * z_c; >> + else >> + return (basegraph =3D=3D 1 ? 56 : 43) * z_c; >> + } >> + /* LBRM case - includes a division by N */ >> + if (rv_index =3D=3D 1) >> + return (((basegraph =3D=3D 1 ? 17 : 13) * n_cb) >> + / n) * z_c; >> + else if (rv_index =3D=3D 2) >> + return (((basegraph =3D=3D 1 ? 33 : 25) * n_cb) >> + / n) * z_c; >> + else >> + return (((basegraph =3D=3D 1 ? 56 : 43) * n_cb) >> + / n) * z_c; >> +} > >Please add comments for the logic behind these calculations.=20 >It would be better to remove these hard codings and define some macros. >There are some other hard codings in the patch. Please fix those as well. >> + Changed that function and added reference to related 3GPP table. Thanks.=20 >> +/* HARQ output length including the Filler bits */ static inline=20 >> +uint16_t compute_Harq_Len(struct rte_bbdev_op_ldpc_dec *ops_ld) { > >Camel Casing? > Thanks. > >One generic comment on the patches. I can see that the name of MACROS Is q= uite big. It would be better to rename them with a shorter logical name. > Are you refering to the RTE_BBDEV_LDPC_... ones? These come from the API an= d are not defined in this patchset, I would not want to change them now as = these are exposed out for some time now. Thanks Nic