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 1D006A00C5; Wed, 14 Sep 2022 15:19:53 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B30504021D; Wed, 14 Sep 2022 15:19:52 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id E7EC640156 for ; Wed, 14 Sep 2022 15:19:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663161591; x=1694697591; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=9HXyPw7gQlUNenDuhRHqIX+0sAmlr4e5zkJi8z+ns4w=; b=IUNp+K5eclcquIQ3KBlbSz/10m7gsxkwV5RhfnfCH+7AhAEZWP/kCzxr jgvd7Ln2X82Xhd9jWW4Kv6J6UfJfRzsy+8XTCXb+l5s2nhiUqLzmFx4We 4WE8Nn2aEFXzgwuVL9g+4k21YEk9bIjlCX1kpDuBZrPxXIce/Bt+gC/NW 4+4xWFi75PmExZPXyBlWNqgHN7aoyfFOi5cMeoOc8TFOON/lUzS3KXjMs sXyRQlDSGdejdQd0Tiju6X2NuQJFSVtaJHLSUsiTAtAsogUjcIyT2pHDM 6CatqsIQyol3bGZDp/c9Jvx/siZlRF/noXDq1orciAIuYzgip01Xu9Nvn Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10470"; a="299239597" X-IronPort-AV: E=Sophos;i="5.93,315,1654585200"; d="scan'208";a="299239597" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Sep 2022 06:19:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,315,1654585200"; d="scan'208";a="679043412" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga008.fm.intel.com with ESMTP; 14 Sep 2022 06:19:42 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 14 Sep 2022 06:19:41 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 14 Sep 2022 06:19:41 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31 via Frontend Transport; Wed, 14 Sep 2022 06:19:41 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.174) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.31; Wed, 14 Sep 2022 06:19:40 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MwwBiu1VilUBzVR5Ytuab0WmIsQ67BNcNPysklbwW1AjRlWkQqMTL469L4F4gjEFPzjNL4ROA+9CEaOPw2/ZoBVDGuNM/JfoL+aYAQC3ssLKgDsioNL2cM0LB0DeKHKD6VG1NnMYu3rrQbo6OWphp0BQs5g/xI0bOIr4dfRIGW7GozjMTp+aeMs4mWC98APxiBuTpRKcmeWJY6U4qQnkOiq5sVUJGMqcwwgzYWJu6iazUK9uqbm4ewrreqRRv6LwYOIz7kjXscZOe0zgfdE4aObxEOzDixjddfUlej/w4CU/kxen6LsD5yS0/WyGPUwkacGVGXMHVoN8LDK8JLQo3g== 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=HCumcq5AxJxjiDZypev1cHVHD75mjJtrEhhZPOycUlc=; b=MW2mVeRe/ppaw1+b8fNanAn/T+Wgf3qq2abMmxYnVLI7gIU4QsTc5OWcSIGXOYRekCSGDJkfmsKCMzVsbk0AjXQyFbNLpp+rdhufUNigsOaj4j46aJgWMEBRDRVkaVDkeo8iBa6pwguHTbQJ5uqJU8oblhpKbwDKeAIRaOJP1xpjkSbksf83/OogBi5/uRaeUaTMTs26e7AQYR03A57R5qVazrvVehpxTGt0HPd8xXQU47RwJt5tl6mRifdKph8ugSnJli3DoVjZ5grF7dRvq5LnFcJRA5gNiJ+T2EYDWN9JaA8FVee1xVXCqQl20OkZYfMDDAp3vPepVpEgJrWCPg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) by MN2PR11MB4565.namprd11.prod.outlook.com (2603:10b6:208:26a::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5612.16; Wed, 14 Sep 2022 13:19:38 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::13c:8120:d994:16d2]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::13c:8120:d994:16d2%6]) with mapi id 15.20.5612.022; Wed, 14 Sep 2022 13:19:38 +0000 Date: Wed, 14 Sep 2022 14:19:30 +0100 From: Bruce Richardson To: Maxime Coquelin CC: Thomas Monjalon , "Chautru, Nicolas" , "dev@dpdk.org" , "gakhil@marvell.com" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" , Tom Rix , "mdr@ashroe.eu" , "david.marchand@redhat.com" , "stephen@networkplumber.org" Subject: Re: [PATCH v1 00/10] baseband/acc200 Message-ID: References: <1657238503-143836-1-git-send-email-nicolas.chautru@intel.com> <16306674.geO5KgaWL5@thomas> <95130dc6-69ca-9be4-ccda-fabbc6c6c88a@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <95130dc6-69ca-9be4-ccda-fabbc6c6c88a@redhat.com> X-ClientProxiedBy: LO2P265CA0073.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:8::13) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|MN2PR11MB4565:EE_ X-MS-Office365-Filtering-Correlation-Id: fa2eeb43-ed28-4116-c777-08da9653c647 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: zmc4pw71fyeJ+mfhXCAHtxnNNJTxmro4YOIxdhYLOJRbwrVNzK4AkvHRxuZdwk7LM14k6sHdnb5w0siHBCtYA05+AlKplnWF0TtWi7k6v2wQgjzywxdqBomg/V6LmVMPXCdkyzx2S6OEDOTx0dmZwNBQ6VSUY88jJwVHdkd8Iy0r0+kMe6OW5Ouxg2sRidHzDj7uTYU9qjPZqyBoJjdjqGWOSoBMFykGjs1bJOeWVK0ZwKTE8G1xL9oM2Nu7pAer4mWJhiynLOYFnFhtQEP4UYvpylWi80zS++fd/ybQkKMV180zAgNRf3MDTGfnO7+MzK4Iqit1yUYuFDlJApciadxL2jYkeaJK6lxHX8cxXNCnrQ1N2absx+uLlj9Ryi2U/KzFpFlYPpERTXkMWtR3+b9ANFav7RgVm+M/8CO335JnWecp7LyrM0L31iZsjkjt9TIgvNPwA6nnO3InnkGvJ+7BgS4cDpW6+q5BYIZ43QJ4VnCW9JanIJAkqA7q9PXlr07f/shXvPQYg/i1cX9T0Bes49EPoIG0nDzJbkUVaWHnyzqMCt9Uae+E6bG6FZmSaM3o9510undLn+u2MeCV6WjxokZv5WOAcg8EA94FKDNUlsy518wOhnUvJW/t9c0eR3Q1FCNfyKNR8X0LDo4tFs2j5syRbttFXp9SWWl9J2sHaWjTd0Uz11taXw4ESZVNd7AF3tildiRrSV1ubyalGgDpln7XKs4GB37vZnsQ7FtvMzBza05iC0OKiedx/2MW X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1629.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(366004)(136003)(396003)(376002)(346002)(39860400002)(451199015)(6506007)(66476007)(8676002)(6486002)(2906002)(53546011)(6512007)(54906003)(186003)(86362001)(8936002)(30864003)(83380400001)(5660300002)(38100700002)(6666004)(6916009)(82960400001)(66556008)(44832011)(316002)(66946007)(41300700001)(26005)(478600001)(966005)(4326008); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?71K6xL7v66Azg15+XgsUztejjl4kEO8bVoalP93yEHOWqrOZaERREvEYyh5q?= =?us-ascii?Q?7OLIKdWRbDW0N2kf4XGGRiCyIqTS+3FW1aFpz2ftLL47XEUSYhTVanJXXUZA?= =?us-ascii?Q?1RcbHNXxaiTiyysin9QmZlUkdu5hQyx+wS2d4GzT+dNk318GFfgv44bWaWql?= =?us-ascii?Q?Lw8sYDdPDZT2Cd0nwbvdI+TK+eHB7gbHih1Q5pmvat1COJkqFxb9YElAXPvQ?= =?us-ascii?Q?TCanNJ7c+aIPEi1hRz4/fd7pqSpPN5AUF7a9Z0KMpgTFIzb68p9jFJFDzwFi?= =?us-ascii?Q?tCzrK04VRkHFckMhbRD092wjLRvsCgl7pWUHWYM6kLQlUI4bEfslm18McKVO?= =?us-ascii?Q?UShsg0PILpFRs57fv6JvdxonUYqiJoMB/lK97grF7SvrsNuBjOG/fFZXWBHs?= =?us-ascii?Q?lWoK51/aiMkuL/EEXo0jgNeX8t0/5ukZoGlu/sq9YSMgLjYpv4TXCS7HUJpo?= =?us-ascii?Q?LTjdu725Iik49IXrCWGWTBXhh+2/51HI1rR6Vdt0VNZeYUzU4j0wGKJdXjvX?= =?us-ascii?Q?qQaubmXSU1enzNE9XqZptX3z2uR10rYF4V/5vBJNOwrAiDXSww9WY0l7LpO/?= =?us-ascii?Q?tz22Liv3tDCULU9G+WZJPBcenQziPWOM0GTUmbYb+BOh71NqQeho61yacZca?= =?us-ascii?Q?4WyI7pcLnso67N4WcjUg50DZKbBV5pCHPqW1f1Cx2Wlr1286Vk6K5Y9/TSJ0?= =?us-ascii?Q?ANHxODs4irxAgf3YmR7QRu7RWsm1AI5LpN5HKadgCa2l92M+TK6sZAXMG+AX?= =?us-ascii?Q?uEy1adjJF9c3QHGn4lmfArp63aowdYsy/Sr5HviI+HHfjtstdv7HHUNgkIP8?= =?us-ascii?Q?9yVVMsQuRN6JNBy7kLmxjBVJMFZC2JseMdB2cDHQND6CFFjIkV/Kt3+iLIg/?= =?us-ascii?Q?tt6VMN2Xv6NanDaW4Pu+LMs63ssesk4a7zWgUdcVbQflkV1HPNAuTH3I1kXr?= =?us-ascii?Q?jO2vDVOCz+2mIoQqyFov5AfMl1LkbD1ELiWlEABEp2bnCnJb+Hq8LSna7kgL?= =?us-ascii?Q?LaUfmumD1wK03aPwCgxye8gyEKqbXQKCylS9fWzdPqa/Uith6/tIVfw55uRe?= =?us-ascii?Q?WWOplDIEuSMAEjHwbQZ4PxeWiA/GyxPDNMMYWwj8T7cMn5OYMI0p8Mw4fv5o?= =?us-ascii?Q?712G684i/PoR5ScQa6JSUw5iDe3G08JZ0H2NJQStJfCcmXyon2Db6S3/bxfN?= =?us-ascii?Q?ocAWAZOVhr1orwA2DrfN7Iyu11DqP/thKAqemhQ4t9p/msvWicmKfI9ljXxB?= =?us-ascii?Q?XGEUaF2PLUVhWMZUsx9cR2Wz3iY4cdF8lFboozRSxfjsBBK4qHgsBh3SIdGe?= =?us-ascii?Q?YBoCEqbOLcgCsSJ0Igz7MzzUuPvOt5BfbFaMYz0ftuw1yK70NGfmEebvN+l7?= =?us-ascii?Q?9q5Bxufh/KmFDqZkVr9vGDJ8F8wf9uWAtoMQpokH+SuqvIuElVDYg6tmbWSb?= =?us-ascii?Q?R4IDf4wDFWUXWJ4oPt5cTJdhIAuT06rHauPtt6ikHBRsPAop2LkTxVZ66jKk?= =?us-ascii?Q?b8E2G/QhBizahjwoNzIPi7EZpHk2loTRcHv5QEnZQjV0olbrhmh972ZpP4IT?= =?us-ascii?Q?ECqBMN9+UyG7wMANZokkoYVk1XHkbh/5pBB1EePDC3zPIWkaPwTwx+VkRcA2?= =?us-ascii?Q?E8cgJJkxeWBqStlRT9KOmhw=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: fa2eeb43-ed28-4116-c777-08da9653c647 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2022 13:19:38.2672 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: QejewcWQvWqYa57gUk/QtX2rejJy6gSRNzSLWlFsrIh6jP+yhPuDS5GE3FZEKY+Q8GOeO0Cn6ho2cXLhXpkckWQVHa+Q+br/sFk+4Js5w6M= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4565 X-OriginatorOrg: intel.com 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 Wed, Sep 14, 2022 at 01:50:05PM +0200, Maxime Coquelin wrote: > > > 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 main > > > > > > > > > > > > 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 to > > > > > > > > > > > > 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, processes > > > > > > > > > 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, underlying > > > > > > > > > > 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 acc200 > > > > > > > > > > > > > > > > > > https://dickgrune.com/Programs/similarity_tester/ > > > > > > > > > > > > > > > > > > l=simum.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 the > > > > > > > > > devices' registers, they are the same. > > > > > > > > > > > > > > > > > > I raised this similarity issue with 100 vs 101. > > > > > > > > > > > > > > > > > > Having multiple copies is difficult to support and should > > > > > > > > > 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 accelerator > > > > > > > > we will use a > > > > > > > unified driver approach but for that very case that would be > > > > > > > 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 that > > > > > is proprietary information), and at bbdev level the interface is > > > > > 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 driver > > > > > > 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 unique > > > > > 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 the > > > > 2 pf_enum.h file, there are many registers that have same or > > > > similar names but have now different values being mapped hence you > > > > 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 using > > > > 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 merged > > > > these together. > > > > > > > > With regards to the pmd.h, some structure/defines are indeed common > > > > and could be moved to a common file (for instance turboencoder and > > > > LDPC encoder which are more vanilla and unlikely to change for > > > > future product unlike the decoders which have different feature set > > > > 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 which > > > > 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 for > > > > 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. > I think the expectation is that the two drivers will diverge in future, so having separate directories should be ok, even with common files placed in one directory are shared with another. With meson include paths its pretty 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. /Bruce