From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AB69EA00C5; Wed, 14 Sep 2022 15:46:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 53D624021D; Wed, 14 Sep 2022 15:46:19 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id 256E140156 for ; Wed, 14 Sep 2022 15:46:16 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 28EDM7ZH006953; Wed, 14 Sep 2022 06:44:08 -0700 Received: from nam11-bn8-obe.outbound.protection.outlook.com (mail-bn8nam11lp2168.outbound.protection.outlook.com [104.47.58.168]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3jk69923m9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 14 Sep 2022 06:44:07 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cCS8TKDWAgkV1Ym2Uefbg/Fz81oPFQXLJWarJpRTIMS1XhF1Y0KS0i6OGjUmiyxqdPWRT78Xn0y+FQIYM+9bggPrVgRQkP5HIIZ6QN1K+lNfqxb/ujhvkc3M1eNDtj7uygWWzakw+OoM3UxUTuqvMmfPIuG4x1wc/UJLahjQHSRLDk7kCnRYfYLKttMFuTVXmfeCdd8YpquCVyG6fKZJ3qn+wOiTo5dyh44u6wWbgbJkM6tfgOVmY3Kt57WMJah21r34bEXqN3DeNgqPSW6nxyAzgnjyAiQ0B8PEoXWcFzQIK5Ewn3mHsRSPNns/L/e8gwD0/D6/N4aPScxm3CHU4A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=pLc+dUtwNTgMzJM5aPa+MEDt6SWPEHi8d2GSliJFtfY=; b=nIRyzRm4wPMTfmgmaNeeeZgmoQcxtU/RTFr9krrgfAB7+ZrJ0/YjEILlgg0oo5zHPMYro65JBePxYzgcS6MD70k5uo2aAn+5hUg9+wGYWgUJu9oHViBhqnVoOiQtchqT4uh54m0LxdTU4d6KRXFftgCutmlYWvKzX4fP7Ydv43Wl6gFlIn/njG8FJz0cAyYEYXr50fl5X60QbsaJyFatkLYgL7FXq3vL2ePFzv8YdbXCDsKrhdq1D+Y8LBNphXjyyIMD56Spr0/MU5mxnwC6Ej4AUqm8Djr/GRecTE3NDhcDJBXPYZ4GFhZr0jPQR3gg8g7fd+54e3niFLoAPUcLxQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pLc+dUtwNTgMzJM5aPa+MEDt6SWPEHi8d2GSliJFtfY=; b=aD+/qMaMlcJoCDlGqYirKhhNC3kTVrpxQkabe8aZau5Wd3KhLDskBFd7hVgC9L2sji3++OUrgF8CCBBdv8Q6xZRK7Q3bgt48O/ZPLGYY1qhHp3fyLgYIMNGg/l3tDwZ61wgUB/5DT/lJSMmxP5Z/XUO4oDWe9zsLNh5lhIgR8Oo= Received: from CO6PR18MB4484.namprd18.prod.outlook.com (2603:10b6:5:359::9) by CH0PR18MB4148.namprd18.prod.outlook.com (2603:10b6:610:e3::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5612.22; Wed, 14 Sep 2022 13:44:05 +0000 Received: from CO6PR18MB4484.namprd18.prod.outlook.com ([fe80::6d7f:3f2f:935d:7319]) by CO6PR18MB4484.namprd18.prod.outlook.com ([fe80::6d7f:3f2f:935d:7319%8]) with mapi id 15.20.5612.022; Wed, 14 Sep 2022 13:44:05 +0000 From: Akhil Goyal To: Bruce Richardson , Maxime Coquelin CC: Thomas Monjalon , "Chautru, Nicolas" , "dev@dpdk.org" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" , Tom Rix , "mdr@ashroe.eu" , "david.marchand@redhat.com" , "stephen@networkplumber.org" Subject: RE: [EXT] Re: [PATCH v1 00/10] baseband/acc200 Thread-Topic: [EXT] Re: [PATCH v1 00/10] baseband/acc200 Thread-Index: AQHYvkIzwAe+aJa23E6MyuiHzlTScK3SYkwAgAxsjoCAABTkgIAAGPsAgAABRuA= Date: Wed, 14 Sep 2022 13:44:04 +0000 Message-ID: References: <1657238503-143836-1-git-send-email-nicolas.chautru@intel.com> <16306674.geO5KgaWL5@thomas> <95130dc6-69ca-9be4-ccda-fabbc6c6c88a@redhat.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CO6PR18MB4484:EE_|CH0PR18MB4148:EE_ x-ms-office365-filtering-correlation-id: 0f6ba9ba-fce4-4f9e-b504-08da965730af x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: v22iTrWYMXKUkbahUnLQVClwb90ZQAXkIntHJPEFiCnT5ccy4frIcpbrWkgzeyI44PAJseslATnclf8yf3g/JqFrOpU7Ymo6CZZ1/64wTBm0zGz/5BVF7TxkwBABbHSmTHWiNhRklfiOXAGCSIY6Fx2NxSdlLgaTlcQ43gE6yxdxUZLQiVB2utX1Of29CjPrgWAJihVGu4Ud8yWy8Izw4JjW8MkqVx4g1vnt+TNSvMKB+RmQHCI/nWCqcJ9VXidFTjgpCHTHbP0lLamBlfMVUeP8TiCQd6IkyJzQRCGE7ZnWwUgUmzkzHBTIi3NsTmRCZir0v69d6eBeMOmZzqzM8DpKHcTxXXw2dznh3q6ffooxRLInblUAn6kZraHxq4P571SxrvyLvTpytw183qUqxJTMFzjv+xCdwjdSdbpKeKNdTha5bleHuXIy7MxkNnxf1/oes4esCEzHwmsvzprvO4OIh2kQ7YHgU51tbtPnBoAfeEmIUSnItsLjmJ9Lex3+kNNMN0hoy7dhM4twZgiAiMIDv2nykf0CEUnLfIbMLtKaRHdoVKX7AlAwHig40JAxGrovqQZGMJGVXOHhh9xeQW2emwVvzLe8I1eosK7sI+xUsERXoNZ+RwvkL71n5zx9DWFR3xntssypnQI8pxVCKWXvCcHTeXM0JLP1jPeBLW/nWEvcfRusgGjAhc4uRHudI8DsrlV7e5JscrLTWxLb7nhuWRzMMf8ZXkXbQ/tzX7vj1j8+IAQDrF9BUwso75oQtGd+/s1v2yhTl5rUm4g1ZjVqcKnY9wHkYQMg4tsfp0Q= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CO6PR18MB4484.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(346002)(376002)(396003)(366004)(136003)(39860400002)(451199015)(66476007)(83380400001)(186003)(38100700002)(38070700005)(122000001)(5660300002)(30864003)(66446008)(8936002)(7416002)(8676002)(4326008)(52536014)(64756008)(55016003)(2906002)(66946007)(66556008)(71200400001)(19627235002)(478600001)(53546011)(55236004)(41300700001)(9686003)(26005)(6506007)(966005)(7696005)(316002)(33656002)(110136005)(54906003)(76116006)(86362001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?1DMGywK+y34QDfMyWSykuJB58DtNiXOb0pc1AAKb0P8A8eKxpYq1JMuDUSP7?= =?us-ascii?Q?Gsf/VKS108vxwdD+YJaF8VlCyoG2oMkcufow4lvXP3xi76/k+m9hAUbM3OYb?= =?us-ascii?Q?uONL1469CpW2RKDsMPTj5wiks6HHCjM9YyI1am7sMDdA6pvqyVuoOMNFo6fT?= =?us-ascii?Q?+qydM+5j8fLwPFtOXGlZFgtruPRUk/5It4IeIndo0X96s1YCg6OJKhB85XfH?= =?us-ascii?Q?1NweafEtq8rItfdhStE3rEB2N9PAfVZI32ochpUbVG+AIsqgaB9owHmfLEDy?= =?us-ascii?Q?foCcoRaRPpe2K4KSfye08yaXlj0JE1kX1QsPRN+TfGZS2Fh9RujQ7ZPbt9sX?= =?us-ascii?Q?2cMvq5UOqvuNBM/GkDpAb8x2wRR4aqOfEsqz8rf9TL9dtLqamHrnNYrFxsWq?= =?us-ascii?Q?hdFNopJhUTj0wE683xkMLIAGwWv/wmbSrl8Pdo7gLJfHQWCTakYQLqyouUhs?= =?us-ascii?Q?01/H8GHYQ+fHjmy4KGvZZvW0lBkJzHY8j0p5N4sNSaWB6N4dE84DUBHtxRkZ?= =?us-ascii?Q?T5md5WY4AtZE8hI84zaHAci32PbxD91MqqC6kPQef9eh4fMx0EC1/NILftrd?= =?us-ascii?Q?6m6POHIWy3WHksMowHfwmO4DzdNM7fu+fDBk716WWLyNlJUS+LWBS4K/lyWH?= =?us-ascii?Q?6gvzeF4bdkrdo8FxTY/NLqU5yZ8y5E9K6jtCKdZevjjBzflDHiqej7O7uO2X?= =?us-ascii?Q?Obr6Ter0CYj9lcoAknmnYrC09/1HNa0oQxAHxzs9Utj/O4mxm+4HOZhfyb4J?= =?us-ascii?Q?mjL+3CAnq1MCyo7BeF9Yh39hW4j93TLQuwj1R0+pHgPMbcBbySjp2EVD7iFe?= =?us-ascii?Q?E+hQncoTbweuw7Bsyc1HcTYZrLNljL6t/vaKXtahi709oB46BSW2BNOGFU1V?= =?us-ascii?Q?FBx15ayUI9nuFLFnM4wMtVv2TdfwmHNfP2h7yvyMsRC3xeGaxjSeQPPm6w4I?= =?us-ascii?Q?d/Sb43xjNOza/xg2ommK9CPc8X/CT21TkJdzh8X9C7B/yZjtnCl5UzMkAeHY?= =?us-ascii?Q?ztX5iJzCPZKJfr4kl0T34Zj9n7lpMPepP7gcfY6QMJsDjt2RA09oTjYecIV4?= =?us-ascii?Q?ZAsDCZDFbgJO5ZHf6bGNtjyDFzR8Lf2wLt61Wm0uaIl/bCeCEFXE8AbB+f05?= =?us-ascii?Q?jPI5YfLCTO1BKyTeDmJeSitzjvrhid6RNg6XqRJ3LS0EJ/K/yBoxT4yS40OT?= =?us-ascii?Q?45T+xGqVd2pjaU4yhX+41FFQKGGzkyuDf15PKx4Rn62o/9q2o6uQPfpa+Yaq?= =?us-ascii?Q?AK0SG1dfOLYmI1K7kaJ5pfOJtwKt81PFiThYcfq2vuj5i4lxfChIKWbN0W4x?= =?us-ascii?Q?F5LQEmu4nLAzgYenMTgS7aUk0y9/Y5bwVvbytqk3i9dfsAcpa0Fgc963wWlp?= =?us-ascii?Q?HHitbqXeeKhOU1GTEnJZIC1ZTNpMwAzqXYDn2hBN4ZEobXbxNYn8co9Zdgga?= =?us-ascii?Q?USxV9RL7cwhJ26DHjZwPMbrzhPiFPDQGRJgTE9ahFxvbJalmpVMvyF5OIfFd?= =?us-ascii?Q?HRRCuq7QAhbIzATHiJaXKQhOz7WgyBVwvshux2oFafS22XLumYtrgAKUclfD?= =?us-ascii?Q?i2vA90x9pzZhMRZN8xJ70lGlxtlS01N3chwEBbOp?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: marvell.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4484.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0f6ba9ba-fce4-4f9e-b504-08da965730af X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Sep 2022 13:44:04.8864 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: lSMBr9XmrX5RN8CJT69GPlCnw8n9OkZmcex/nY7/nC/9eV2PgRzg10UGqjG/qjaj4qfhn94s8RKGpejSfYo3Lw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR18MB4148 X-Proofpoint-ORIG-GUID: rXG0kShVGZHKQbz_QQuUSarMZkKNtCCB X-Proofpoint-GUID: rXG0kShVGZHKQbz_QQuUSarMZkKNtCCB X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1 definitions=2022-09-14_05,2022-09-14_04,2022-06-22_01 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > > > > On 9/14/22 12:35, Thomas Monjalon wrote: > > > 06/09/2022 14:51, Tom Rix: > > > > On 9/1/22 1:34 PM, Chautru, Nicolas wrote: > > > > > From: Tom Rix > > > > > > On 8/31/22 6:26 PM, Chautru, Nicolas wrote: > > > > > > > From: Tom Rix > > > > > > > > On 8/31/22 3:37 PM, Chautru, Nicolas wrote: > > > > > > > > > > > > > Comparing ACC200 & ACC100 header files, I > > > > > > > > > > > > > understand ACC200 is an evolution of the ACC10x > > > > > > > > > > > > > family. The FEC bits are really close, ACC200 mai= n > > > > > > > > > > > > > addition seems to be FFT acceleration which could > > > > > > > > > > > > > be handled in ACC10x driver based on device ID. > > > > > > > > > > > > > > > > > > > > > > > > > > I think both drivers have to be merged in order t= o > > > > > > > > > > > > > avoid code duplication. That's how other families > > > > > > > > > > > > > of devices (e.g. i40e) are handled. > > > > > > > > > > > > I haven't seen your reply on this point. Do you > > > > > > > > > > > > confirm you are working on a single driver for ACC > > > > > > > > > > > > family in order to avoid code duplication? > > > > > > > > > > > > > > > > > > > > > > > The implementation is based on distinct ACC100 and > > > > > > > > > > > ACC200 drivers. The 2 > > > > > > > > > > devices are fundamentally different generation, process= es > > > > > > > > > > and IP. > > > > > > > > > > > MountBryce is an eASIC device over PCIe while ACC200 = is > > > > > > > > > > > an integrated > > > > > > > > > > accelerator on Xeon CPU. > > > > > > > > > > > The actual implementation are not the same, underlyin= g > > > > > > > > > > > IP are all distinct > > > > > > > > > > even if many of the descriptor format have similarities= . > > > > > > > > > > > The actual capabilities of the acceleration are > > > > > > > > > > > different and/or new. The workaround and silicon > > > > > > > > > > > errata are also different causing different > > > > > > > > > > limitation and implementation in the driver (see the > > > > > > > > > > serie with ongoing changes for ACC100 in parallel). > > > > > > > > > > > This is fundamentally distinct from ACC101 which was = a > > > > > > > > > > > derivative product > > > > > > > > > > from ACC100 and where it made sense to share > > > > > > > > > > implementation > > > > > > between > > > > > > > > > > ACC100 and ACC101. > > > > > > > > > > > So in a nutshell these 2 devices and drivers are 2 > > > > > > > > > > > different beasts and the > > > > > > > > > > intention is to keep them intentionally separate as in > > > > > > > > > > the serie. > > > > > > > > > > > Let me know if unclear, thanks! > > > > > > > > > > Nic, > > > > > > > > > > > > > > > > > > > > I used a similarity checker to compare acc100 and acc20= 0 > > > > > > > > > > > > > > > > > > > > https://urldefense.proofpoint.com/v2/url?u=3Dhttps- > 3A__dickgrune.com_Programs_similarity- > 5Ftester_&d=3DDwIBAg&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3DDnL7Si2wl_PRwpZ9TW > ey3eu68gBzn7DkPwuqhd6WNyo&m=3Dm846AfMSeaddoC0hcxp1mHU4LV3jRUpw > P-Ie_41buNb9nACOR5La8n8LEFBCnn4t&s=3D9dgMzk9UMFLA- > b1EJbi4lK3GG6mNMgOXZRyDZqe00TU&e=3D > > > > > > > > > > > > > > > > > > > > l=3Dsimum.log if [ -f $l ]; then rm $l fi > > > > > > > > > > > > > > > > > > > > sim_c -s -R -o$l -R -p -P -a . > > > > > > > > > > > > > > > > > > > > There results are > > > > > > > > > > > > > > > > > > > > ./acc200/acc200_pf_enum.h consists for 100 % of > > > > > > > > > > ./acc100/acc100_pf_enum.h material > > > > > > > > > > ./acc100/acc100_pf_enum.h consists for 98 % of > > > > > > > > > > ./acc200/acc200_pf_enum.h material > > > > > > > > > > ./acc100/rte_acc100_pmd.h consists for 98 % of > > > > > > > > > > ./acc200/acc200_pmd.h material ./acc200/acc200_vf_enum.= h > > > > > > > > > > consists for 95 % of ./acc100/acc100_pf_enum.h material > > > > > > > > > > ./acc200/acc200_pmd.h consists for 92 % of > > > > > > > > > > ./acc100/rte_acc100_pmd.h material > > > > > > > > > > ./acc200/rte_acc200_cfg.h consists for 92 % of > > > > > > > > > > ./acc100/rte_acc100_cfg.h material > > > > > > > > > > ./acc100/rte_acc100_pmd.c consists for 87 % of > > > > > > > > > > ./acc200/rte_acc200_pmd.c material > > > > > > > > > > ./acc100/acc100_vf_enum.h consists for 80 % of > > > > > > > > > > ./acc200/acc200_pf_enum.h material > > > > > > > > > > ./acc200/rte_acc200_pmd.c consists for 78 % of > > > > > > > > > > ./acc100/rte_acc100_pmd.c material > > > > > > > > > > ./acc100/rte_acc100_cfg.h consists for 75 % of > > > > > > > > > > ./acc200/rte_acc200_cfg.h material > > > > > > > > > > > > > > > > > > > > Spot checking the first *pf_enum.h at 100%, these are t= he > > > > > > > > > > devices' registers, they are the same. > > > > > > > > > > > > > > > > > > > > I raised this similarity issue with 100 vs 101. > > > > > > > > > > > > > > > > > > > > Having multiple copies is difficult to support and shou= ld > > > > > > > > > > be avoided. > > > > > > > > > > > > > > > > > > > > For the end user, they should have to use only one > > > > > > > > > > driver. > > > > > > > > > > > > > > > > > > > There are really different IP and do not have the same > > > > > > > > > interface (PCIe/DDR vs > > > > > > > > integrated) and there is big serie of changes which are > > > > > > > > specific to ACC100 coming in parallel. Any workaround, > > > > > > > > optimization would be > > > > > > different. > > > > > > > > > I agree that for the coming serie of integrated accelerat= or > > > > > > > > > we will use a > > > > > > > > unified driver approach but for that very case that would b= e > > > > > > > > quite messy to artificially put them within the same PMD. > > > > > > > > > > > > > > > > How is the IP different when 100% of the registers are the > > > > > > > > same ? > > > > > > > > > > > > > > > These are 2 different HW aspects. The base toplevel > > > > > > > configuration registers > > > > > > are kept similar on purpose but the underlying IP are totally > > > > > > different design and implementation. > > > > > > > Even the registers have differences but not visible here, the > > > > > > > actual RDL file > > > > > > would define more specifically these registers bitfields and > > > > > > implementation including which ones are not implemented (but th= at > > > > > > is proprietary information), and at bbdev level the interface i= s > > > > > > not some much register based than processing based on data from > > > > > > DMA. > > > > > > > Basically even if there was a common driver, all these would = be > > > > > > > duplicated > > > > > > and they are indeed different IP (including different vendors).= . > > > > > > > But I agree with the general intent and to have a common driv= er > > > > > > > for the > > > > > > integrated driver serie (ACC200, ACC300...) now that we are > > > > > > moving away from PCIe/DDR lookaside acceleration and eASIC/FPGA > > > > > > implementation (ACC100/AC101). > > > > > > > > > > > > Looking a little deeper, at how the driver is lays out some of > > > > > > its bitfields and private data by reviewing the > > > > > > > > > > > > ./acc200/acc200_pmd.h consists for 92 % of > > > > > > ./acc100/rte_acc100_pmd.h > > > > > > > > > > > > There are some minor changes to existing reserved bitfields. A > > > > > > new structure for fft. The acc200_device, the private data for > > > > > > the driver, is an exact copy of acc100_device. > > > > > > > > > > > > acc200_pmd.h is the superset and could be used with little > > > > > > changes as a common acc_pmd.h. acc200 is doing everything the > > > > > > acc100 did in a very similar if not exact way, adding the fft > > > > > > feature. > > > > > > > > > > > > Can you point to some portion of this patchset that is so uniqu= e > > > > > > that it could not be abstracted to an if-check or function and = so > > > > > > requiring this separate, nearly identical driver ? > > > > > > > > > > > You used a similarity checker really, there are actually way more > > > > > relevent differences than what you imply here. With regards to t= he > > > > > 2 pf_enum.h file, there are many registers that have same or > > > > > similar names but have now different values being mapped hence yo= u > > > > > just cannot use one for the other. Saying that > > > > > "./acc200/acc200_pmd.h consists for 92 % of > > > > > ./acc100/rte_acc100_pmd.h" is just not correct and really > > > > > irrelevant. Just do a diff side by side please and check, that > > > > > should be extremely obvious, that metrics tells more about the > > > > > similarity checker limitation than anything else. Even when usin= g > > > > > a common driver for ACC200/300 they will have distinct register > > > > > enum files being auto-generated and coming from distinct RDL. > > > > > Again just do a diff of these 2 files. I believe you will agree > > > > > that is not relevant for these files to try to artificially merge= d > > > > > these together. > > > > > > > > > > With regards to the pmd.h, some structure/defines are indeed comm= on > > > > > and could be moved to a common file (for instance turboencoder an= d > > > > > LDPC encoder which are more vanilla and unlikely to change for > > > > > future product unlike the decoders which have different feature s= et > > > > > and behaviour; or some 3GPP constant that can be defined once). = We > > > > > can definitely change these to put together shared > > > > > structures/defines, but not intending to try to artificially put > > > > > things together with spaghetti code. We would like to keep 3 > > > > > parallel versions of these PMD for 3 different product lines whic= h > > > > > are indeed fundamentally different designs (including different > > > > > workaround required as can be seen on the parallel ACC100 serie > > > > > under review). - one version for FPGA implementation (support fo= r > > > > > N3000, N6000, ...) - one version for eASIC lookaside card > > > > > implementation (ACC100, ACC101, ...) - one version for the > > > > > integrated Xeon accelerators (ACC200, ACC300, ...) > > > > > > > > Some suggestions on refactoring, > > > > > > > > For the registers, have a common file. > > > > > > > > For the shared functionality, ex/ ldpc encoder, break these out to > > > > its own shared file. > > > > > > > > The public interface, see my earlier comments on the documentation, > > > > should be have the same interfaces and the few differences > > > > highlighted. > > > > > > +1 to have common files, and all in a single directory > > > drivers/baseband/acc100/ > > > > Jus to be sure we are aligned, do you mean to have both drivers in the > > same directory, which will share some common files? That's the way I > > would go. > > >=20 > I think the expectation is that the two drivers will diverge in future, s= o > having separate directories should be ok, even with common files placed i= n > one directory are shared with another. With meson include paths its prett= y > trivial to manage if it's just header files, and even if there are common= C > files, there is always the option of using drivers/common if we want to > split them out. As I understand it, right now it's only headers inluding > functions which can be static inline, so simple sharing via include paths > should work fine. >=20 It can be ok to have 2 separate directories, but=20 - is it not possible to have them in same directory say 'acc' for all affi= liated devices. Similar to other vendors' devices (cnxk, i40e, mlx). - Can both the devices - acc100 and acc200 coexist? If not, same directory = is good enough. - there can be multiple files or directories in 'acc' which can be named ap= propriately to denote the actual device(acc100/200). Having cross dependency across different drivers of same type looks a kind = of hacking the meson. This was a reason we moved to have a drivers/common/ for some of the driver= s. Also including "../acc100/abc.h" does not look appropriate to me. IMO, we should not add unnecessary directories when the code is common and = can be managed in a single one. However, technically it is also ok to have 2 separate directories. But, agr= eeing on this will set a precedence for future next generation devices from the same vendors. It may= be a topic of discussion in techboard. -Akhil