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 090DD45564; Wed, 3 Jul 2024 20:31:31 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C9B47427C1; Wed, 3 Jul 2024 20:31:30 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by mails.dpdk.org (Postfix) with ESMTP id 945664278B for ; Wed, 3 Jul 2024 20:31:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720031489; x=1751567489; h=message-id:date:subject:to:cc:references:from: in-reply-to:mime-version; bh=LCWokreJ45ZmIjUHXn/qiMjw+o7HIdVNKASxOQdqbGU=; b=cfWLHKecZPDQ6gHGiMFe/cPvlUtP5h4O9E5tw3/BThKYyKrrqRFdTGkd sla6Jdtah3jQqH2EmcAaAN93zTAFI2fMXC99IVvjteIJQy9R1ZT3Eeh4j pYiFrAzm7ZvGihWkOA5TnJ3AXneoESBZzhZt6sEa3nLATzlmTctaSdMUy FaPJ1PanrBec90FtTPE96C0WJZSb/WALgh/DHmu1aTjN5sF7Ck/gGbKBM lINjJlb2VQuH+kwErTs99GmWuOMP/PVx/WmIHSHBbAQyJCEbenEsit+d6 oTqXQ3Dy+G0pd8nwxutsOsKvyDsj6o+z42HsaM/4N6+ZK/1YB3BFWOb7y A==; X-CSE-ConnectionGUID: cxbVWlALS/u3ql6NORU6xg== X-CSE-MsgGUID: iGUvgOlaRsm/QJofJUNE0g== X-IronPort-AV: E=McAfee;i="6700,10204,11122"; a="17005860" X-IronPort-AV: E=Sophos;i="6.09,182,1716274800"; d="scan'208,217";a="17005860" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jul 2024 11:31:26 -0700 X-CSE-ConnectionGUID: iIkefbKpSQW7+6gGOqYd6g== X-CSE-MsgGUID: AGSHJvWrRfWkY7lQXqwe5w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,182,1716274800"; d="scan'208,217";a="47004721" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by orviesa007.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 03 Jul 2024 11:31:26 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Wed, 3 Jul 2024 11:31:26 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Wed, 3 Jul 2024 11:31:25 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Wed, 3 Jul 2024 11:31:25 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.168) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 3 Jul 2024 11:31:25 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=juc91ecyL9VQmL2LL/LV33dv+eEDGq7VwZG1vUV8XjTof3j/RTPFCc2UJoNV0PkhMe0xpGL9jqzqc86ENOESGtzpd4tOXRAzRDzPEND8k3Aw3UUV5bTwU7UeOjySvfV9pxHKTkSuRSthyjcnMAF3IFSnCBhWk6o2Dt3CbsZcSQRzMhzIwW+h8wqR//MM+mKeJ0Wd29myPD/BtLcNlU7SQtRTZeZg86Mt9tvN/+RqnB31N8bj8r4Uyu4am6NL8azlQyUJI8Ves2GGtf8TT50XfdTRAkRWDXzaTUeF4wvMX4kjJxgSK9s5LP6sn1kuYTVxlOQoKr+QB4sdEOrr/HmJeA== 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=w6CCzACUW/XQmtywKAv0+mjNnq4We5AfSQAER8wsM38=; b=nCRU4k4AGfLSIzE4V9k6LxTDtO/BR5Q5n6tshIdvj7n159sH7wFAR1NhluZi62j5knxEVhVU0q+wJHMNgaS6ILa5cyOqqUEmb1fbOR7lmcYgiIRkoHV5YjBqYS/gJqE3p2qlxW+dUTCQ6OQL0NP48c4WT63lPpFg70gNEhtP9GpXXeKZ/HzFlYT45ubI1sjVgM54/ARUxoyPlbCHcLRX1+5jQdT9FvHy3BmjzNqBQ1cufdF/Vdy7Bf8eMrU8FhuX1tKGo30atFH8BbHxf4+cmNsZ5SKmlIe5EPUXzenmusemXPNC7COJX/6bg8jJJi8FbdubLBP1vETlRnvvilTO1Q== 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 SJ0PR11MB5772.namprd11.prod.outlook.com (2603:10b6:a03:422::8) by DS0PR11MB7335.namprd11.prod.outlook.com (2603:10b6:8:11e::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7741.26; Wed, 3 Jul 2024 18:31:22 +0000 Received: from SJ0PR11MB5772.namprd11.prod.outlook.com ([fe80::5851:319:3da6:850b]) by SJ0PR11MB5772.namprd11.prod.outlook.com ([fe80::5851:319:3da6:850b%4]) with mapi id 15.20.7719.029; Wed, 3 Jul 2024 18:31:22 +0000 Content-Type: multipart/alternative; boundary="------------7JBesJHrh9oWZf5vKKtLaqtb" Message-ID: <6d9ad0c4-e5bf-47a0-b14d-512689262922@intel.com> Date: Wed, 3 Jul 2024 19:31:17 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] net/ice: support FEC feature To: Mingjin Ye , CC: Qiming Yang References: <20240411094548.1622662-1-mingjinx.ye@intel.com> <20240702080244.1190884-1-mingjinx.ye@intel.com> Content-Language: en-US From: "Medvedkin, Vladimir" In-Reply-To: <20240702080244.1190884-1-mingjinx.ye@intel.com> X-ClientProxiedBy: DUZPR01CA0144.eurprd01.prod.exchangelabs.com (2603:10a6:10:4bd::26) To SJ0PR11MB5772.namprd11.prod.outlook.com (2603:10b6:a03:422::8) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SJ0PR11MB5772:EE_|DS0PR11MB7335:EE_ X-MS-Office365-Filtering-Correlation-Id: a9b036b9-8271-4f0f-bc55-08dc9b8e566b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|366016|1800799024; X-Microsoft-Antispam-Message-Info: =?utf-8?B?NzFCNXVYQ3VFUGFDb092cXAzb2Ixak1IOUhMRVpZWi83djF5NDI3YThUYllw?= =?utf-8?B?WDg5eldRRFhHZFQwMkkzZXJiSkNDMm9HNjY2NzdtWGJBdjk3TUo3bTdVN2FN?= =?utf-8?B?UXI3UEE5ZWFQNU5adllKSFJUbnI5MllPZ3VOb2lxTXdmbVJHUWxvaC92Wmxx?= =?utf-8?B?cW9udVRESzkrVUZGcEpxcDAwbm1GQnpFVmlqUHdMZGF6MDlXSlpZYXJob3Ir?= =?utf-8?B?cm9iazQ2QkRmNTJMZlJySkkwd0RxdmwvLzFLekkzeVFUQVNHRXFYTGN1Qmhw?= =?utf-8?B?R0VKVWRSRkRTSnZ4RDJydzN4dGN4Q2VucHFzbG5DYjgxZDl0Q1lwVlhmc2wr?= =?utf-8?B?Q05OU0hpblNZS3JZZHJtU2NFQktIYzArZFJEWExaWWVzN1Z0YlNrQjM5dlp6?= =?utf-8?B?QTVHZUhSQmc5N0cwU05GSnhUcmMvcC9mZmp4Tk5FMVNXK2sxb3QxM3kvTUN4?= =?utf-8?B?UUxNM0RqY0Qrb250TStBOUZZbG9wSFVTb3AxVE5jWjJGZW9OZ1NWMitYL3dD?= =?utf-8?B?SW5JWXplbUhQS0xUTi9OK3pLVEhabTdXQ2gwa1IwZ1hmUk93cXVKcXdYd1hu?= =?utf-8?B?aHVja1FFOERwZmM1K1VoZGNSTzV4Rmc2cXlBR0pWeXBtNXI1eUNEV2FLMkR5?= =?utf-8?B?N2RUYmNKOWJ0dVY1QWR4Y1J4dkFncDB6Tmg0T2F3VkwvQ3JGazU0VmVQczhX?= =?utf-8?B?dnU0QXp2ODlDOVUrdXgyNkhReThFNk5sY29oMFdzeC9jYzFCY240NkpkVENa?= =?utf-8?B?ckQxTzB3Q3pxL3IwZU0vWWFVcS9jcDZLbnRrdUxhaEtWWG9qVTR6VnFuenFO?= =?utf-8?B?Rk5UN0hDaHdlWitLSnFwcjRYQ3ArZDZaQ0ppNEIzTGhvVmhmYXgybUlXYlJ1?= =?utf-8?B?YjlXVnN2eUpPODlpdWlibmsyM3liMHUwWkpodU02aTY1aUdrcnhUaXhkL2ZN?= =?utf-8?B?bTJpQ0NLRE1FSzYxSjNaVlRnUDN4MVZMRXMvcmppYlMzQnZOd05uMzlDNk9J?= =?utf-8?B?OE1GS0NYdlIyYlZtdy9XOHFielZvVmgwa2dWMGhOZ2dqd0x4aHQ3ZHhST05t?= =?utf-8?B?cld5VHJWZEdvS3dUZGlPVEVkRTY0OEZjZS83QXZIdDNnenFJbHE3aXJhU0VH?= =?utf-8?B?UnUyTkZFVXUraUFSR1hQaG1PcGE3d2hvZzFuQVk0VkZMaUl2QXlFSFVwREwr?= =?utf-8?B?MG81eGhPSGlxZ2d2Y0I0elFoZzFpYXI0cGxTOG41RDd1QmM1dzBnaGh1ZnRh?= =?utf-8?B?VjlCR1l1TzZkUFpFSkY5K0poRyt3Y25XZ2QwTjBLYitEeTgvbVdHa05Gek9y?= =?utf-8?B?cHhlc0IzNTNEdHBheHpRTDM3QXRJZUFac1dBSFNWMFlYVUpESzVUZEJ6Yzho?= =?utf-8?B?V3AvZTBFSTlOcFpUaS9tU2pNRkE0NC82Z0hUWllvOGU4VEJLSmJsMDNDSHB1?= =?utf-8?B?WDB1LzNXQTVPU2Q3RHVQN1lORWVjcDVYQTRvTWpKV3UveFpuVUJ5WWN5U0Ex?= =?utf-8?B?L2tkcGZ5Vkg5N01JRTh6VW5SQmc2WnJoUDI4MGduTDZBc1BQRmpvQy94cURq?= =?utf-8?B?VkZJQjVDY1liR043WUdBbzVObzIwcXZoeUQvU0NuWks3TFAwcUQ0bDZGSTQy?= =?utf-8?B?aU15MVZMNlBLaGhyRkhrVjk2TlhpY1BYNHIzSjZ6N2dkb0p6djFmSEk4c1JC?= =?utf-8?B?YTQvVHZvcDZQZnF4NU1kMWtkRTlEeGJTU2dxNWdhMEpJZDNza0JhSjN2VE5S?= =?utf-8?B?QkN3OEFzekVmZXpMcVIyUzlSdWVGWXgyMlIybzhPTDhXazRNWDFlUE5kdjFU?= =?utf-8?B?a0xKUmczUFpCUGZ0RlNMZz09?= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SJ0PR11MB5772.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230040)(376014)(366016)(1800799024); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Q0VadjZEbTJHVS8zQzJTcTQwTTBXRDFyeU90SFJQSThsVm96YnByWlFqTnZY?= =?utf-8?B?MUN1V1hiZjlKS0UxUTI0MXlqU1B5NFU0eDlkY2svYjQwdUhjMmF1a3lwc0J4?= =?utf-8?B?cVdOQ0hPblpMQ3ErZGxxdHl1OWMwRGRWVXFkaVJFWUtuOFVaNXd1cWl5Lzlv?= =?utf-8?B?TlZCZURRNXJFOGFjRVVSRHN0bTJHbHQ2dVlSbk1NUmRHSUExMHE3RmtqbzRH?= =?utf-8?B?aUl0WXZCMG92Vi9HRWg5eVpVNmZPZWNPNzNJWkhLWWJzZExPSWFHa0U2S0VM?= =?utf-8?B?NGFEL2NTMlRkb25Ha2Frc0dSMU9rcktXekZyTXJvUTROU3RHbVJWa1NqNFZh?= =?utf-8?B?dTc2a0pvR0F0T2c3SjNtbWNTV1BGdEVBdUlxdFNPOVptdTdHWHFVc01NcUx0?= =?utf-8?B?emxtc0FVUzQ0aE1VTHFnRVNpb2tZMy9ES1dad213bVMyeW5RbkxjaXhBdzZQ?= =?utf-8?B?N214bmtZTVE5bVFReWVsMUFkM0hjSzJBcjNxNm51MWVNQnpzUzdaVWt5b0Fs?= =?utf-8?B?ejhONERkM1pQZmJyU3A1dnZ1N2grSTh1cEtuSVVKRzRtb2hTTWZCQm44Mlo5?= =?utf-8?B?aXdaaitUalM0MjhlU29OVU5ONVdZU0dEcUNvUksvM05zaDJZcjJGeGxXSzho?= =?utf-8?B?VUJaZXYrVG41MXM0MVZlRUdCTmp2alpqanYzSkRjUzk5Qmx6ME0rWjQ1QTdB?= =?utf-8?B?Yk5nVlUraDF0TG1FQ1NuOXZZU3h6cWdGeURPeDBYK29hNUNIV3VZUXI3eTRR?= =?utf-8?B?eFRPMkZpeHhuNVhycUZDbExHVmpHQ2xBTk16OEtYL1JRTEZxcE1YVTNua2s2?= =?utf-8?B?MUFEWTNTSzNCUUk0NkVSWmVIaGxVempqTFNreTY3SEt4alhvQno3WUtYM3Aw?= =?utf-8?B?d2xxR0NCT0N6cmpORnViSFJ1RkpaMDFiSmlaWnhIQUpodDBMNUZzN3NPVVlu?= =?utf-8?B?TFRTMVJVT2NvZDBlWG9UU3FuZk9vNDFsa29SRWg3a3lSeHUwYzIzbWJ2aGlu?= =?utf-8?B?dWtMVEJHMW11N2VCYWVrYThlM1B5SVRoaFA5dE5WcmpDVFYrWFVJa0M4QUda?= =?utf-8?B?eEtrZ1NHeldKd1lKRUwxZ2lCMTIrcFNSc3pBVVMzb25xbW1tUHJ6UWNjcGpG?= =?utf-8?B?OHZoNnI4U0dnNWVnd0dIRTc0K3hOTXU4aVI3L2FhbjVCa3VFSWc0OVphMjhv?= =?utf-8?B?cWxRWiszZW9wQS9jcldWRnlFeXhpTnhnN1pSUzNHNzFKQ1J4Uzk0N3F6OU54?= =?utf-8?B?cEVPcEtSbldLdkpiYlYzMmNuUDBJUFpGbjBVSW9zTVZhdUNVS0R1eUJXSWR4?= =?utf-8?B?eURHSTJDaTBibWlpUG1TeFJoSEtGVGVGcUEyVGtaWUtmdjlOa1BmU3lyb1U2?= =?utf-8?B?QUFFRlhFQ0Z2QjFJdjJRRWdqditPaWpmaVY0RWRMNGVOY3ozenpCczh6NHFz?= =?utf-8?B?RmtSV2c3YTJwaTFyd2hSU0t6VVVjZUF3S1RiNWpsK3NEci8wMDdERi94UGk2?= =?utf-8?B?a0RrdGRQZDJBK2hmVzRFYklPTklIczVIN2oyQ3JlcmlpZG5DcWtnOTc3NGxo?= =?utf-8?B?RUpZL01BVE5ONGhZbUVjbEx4YnIyREFNbXAvcUdGWkU3K2dsUG9VVm5GQ3pv?= =?utf-8?B?b29xblpScXdLWDNqSlYxMDJBbW52SXk3WS9HNkhyMG45b3ZYYkJkK0pjMUx1?= =?utf-8?B?RFk4VkNLSkpvU0t6b25ZZzBCR3dDcWRwYmMwTy9PQWx6NEFMSlo5SjFLSlIv?= =?utf-8?B?K2tTM3JTSm9SYWRtdU0xNGplRnhjaUpwSHpOc3dremU5U0lOMEdJMGhPNzBG?= =?utf-8?B?a1hlZkl1MS9JdVByTjV0TjBZTzh6MDlzaHZmV0R2NHlZU3VtNVJCQi9qYWxL?= =?utf-8?B?YWFNVWlBZkRQVGdNWXk1SXo0REt1YkQxcnZSaDNwWFB6RXJLenhicEFHeWRE?= =?utf-8?B?NGJaT1d1SHY5aVdmNU9QNkdKdUpOMjROYk11NUQ2bmkxZVBjeUUzdEE2M1pn?= =?utf-8?B?R0V0elJlNStyMURxY1pXQnJGbkJEbTVkMXREb1dyUlRSc3czS2VZbXFqVmJl?= =?utf-8?B?c2oraTdDeWhldnlDSkpLTmJoT2hyN3pJVHIyc1hyK0xOdXZ3R21TSGVlUi9x?= =?utf-8?B?SVZLV2YvbXdQT2s1SlZ2cUFYWExWN0twcXhtZ0YwYlF6MUFER01sL2c5OWVq?= =?utf-8?B?QXc9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: a9b036b9-8271-4f0f-bc55-08dc9b8e566b X-MS-Exchange-CrossTenant-AuthSource: SJ0PR11MB5772.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jul 2024 18:31:21.9562 (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: gc2FLZZSUS7dp6iIvIZ+iYsUWN8bjtdbivENShecpO6SDTdXzqjdeM68/eNakSfGRB+Dn+O91sb/S1Iz6PC7wM0WHKSs7M2ls0S3zpkEp/o= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7335 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 --------------7JBesJHrh9oWZf5vKKtLaqtb Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit Hi Minjin, - please update release notes - see comments inline On 02/07/2024 09:02, Mingjin Ye wrote: > This patch enable three Forward Error Correction(FEC) related ops > in ice driver. As no speed information can get from HW, this patch > only show FEC capability. > > Signed-off-by: Qiming Yang > Signed-off-by: Mingjin Ye > --- > v2: fix some logic > --- > doc/guides/nics/features/ice.ini | 1 + > doc/guides/nics/ice.rst | 5 + > drivers/net/ice/ice_ethdev.c | 292 +++++++++++++++++++++++++++++++ > 3 files changed, 298 insertions(+) > > +static int > +ice_fec_get_capability(struct rte_eth_dev *dev, struct rte_eth_fec_capa *speed_fec_capa, > + unsigned int num) > +{ > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct ice_aqc_get_phy_caps_data pcaps = {0}; > + unsigned int capa_num; > + int ret; > + > + ret = ice_aq_get_phy_caps(hw->port_info, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA, > + &pcaps, NULL); > + if (ret != ICE_SUCCESS) > + goto done; This breaks API. Please redefine with some supported error status And please get rid of goto here, it is not necessary anymore > + > + /* first time to get capa_num */ > + capa_num = ice_fec_get_capa_num(&pcaps, NULL); > + if (!speed_fec_capa || num < capa_num) { > + ret = capa_num; > + goto done; > + } > + > + ret = ice_fec_get_capa_num(&pcaps, speed_fec_capa); > + > +done: > + return ret; > +} > + > +static int > +ice_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa) > +{ > +#define FEC_CAPA_NUM 10 I believe this macro is not needed > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); > + bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false; > + bool link_up; > + u32 temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC); > + struct ice_link_status link_status = {0}; > + struct ice_aqc_get_phy_caps_data pcaps = {0}; > + struct ice_port_info *pi = hw->port_info; > + u8 fec_config; > + int ret; > + > + if (!pi) > + return -ENOTSUP; > + > + ret = ice_get_link_info_safe(pf, enable_lse, &link_status); > + if (ret != ICE_SUCCESS) { > + PMD_DRV_LOG(ERR, "Failed to get link information: %d\n", > + ret); > + goto done; Same problem here and below with API and returning error statuses. And I think it is worth to get rid of goto here in this function as well. > + } > + > + link_up = link_status.link_info & ICE_AQ_LINK_UP; > + > + ret = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA, > + &pcaps, NULL); > + if (ret != ICE_SUCCESS) > + goto done; > + > + /* Get current FEC mode from port info */ > + if (link_up) { > + switch (link_status.fec_info) { > + case ICE_AQ_LINK_25G_KR_FEC_EN: > + temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(BASER); > + break; > + case ICE_AQ_LINK_25G_RS_528_FEC_EN: > + /* fall-through */ > + case ICE_AQ_LINK_25G_RS_544_FEC_EN: > + temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(RS); > + break; > + default: > + temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC); > + break; > + } > + goto done; > + } > + > + if (pcaps.caps & ICE_AQC_PHY_EN_AUTO_FEC) { > + temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(AUTO); > + return 0; > + } > + > + fec_config = pcaps.link_fec_options & ICE_AQC_PHY_FEC_MASK; > + > + if (fec_config & (ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN | > + ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN | > + ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ | > + ICE_AQC_PHY_FEC_25G_KR_REQ)) > + temp_fec_capa |= RTE_ETH_FEC_MODE_CAPA_MASK(BASER); > + > + if (fec_config & (ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN | > + ICE_AQC_PHY_FEC_25G_RS_528_REQ | > + ICE_AQC_PHY_FEC_25G_RS_544_REQ)) > + temp_fec_capa |= RTE_ETH_FEC_MODE_CAPA_MASK(RS); > + > +done: > + *fec_capa = temp_fec_capa; > + return ret; > +} > + > +static int > +ice_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa) > +{ > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct ice_port_info *pi = hw->port_info; > + struct ice_aqc_set_phy_cfg_data cfg = { 0 }; > + bool fec_auto = false, fec_kr = false, fec_rs = false; > + > + if (!pi) > + return -ENOTSUP; > + > + if (fec_capa & ~(RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) | > + RTE_ETH_FEC_MODE_CAPA_MASK(BASER) | > + RTE_ETH_FEC_MODE_CAPA_MASK(RS))) please use additional tab to indent properly according to dpdk coding style > + return -EINVAL; > + /* Copy the current user PHY configuration. The current user PHY > + * configuration is initialized during probe from PHY capabilities > + * software mode, and updated on set PHY configuration. > + */ > + memcpy(&cfg, &pi->phy.curr_user_phy_cfg, sizeof(cfg)); > + > + if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(AUTO)) > + fec_auto = true; > + > + if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(BASER)) > + fec_kr = true; > + > + if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(RS)) > + fec_rs = true; > + > + if (fec_auto) { > + if (fec_kr || fec_rs) { > + if (fec_rs) { > + cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN | > + ICE_AQC_PHY_FEC_25G_RS_528_REQ | > + ICE_AQC_PHY_FEC_25G_RS_544_REQ; > + } > + if (fec_kr) { > + cfg.link_fec_opt |= ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN | > + ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN | > + ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ | > + ICE_AQC_PHY_FEC_25G_KR_REQ; > + } > + } else { > + cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN | > + ICE_AQC_PHY_FEC_25G_RS_528_REQ | > + ICE_AQC_PHY_FEC_25G_RS_544_REQ | > + ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN | > + ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN | > + ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ | > + ICE_AQC_PHY_FEC_25G_KR_REQ; > + } > + } else { > + if (fec_kr ^ fec_rs) { > + if (fec_rs) { > + cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN | > + ICE_AQC_PHY_FEC_25G_RS_528_REQ | > + ICE_AQC_PHY_FEC_25G_RS_544_REQ; > + } else { > + cfg.link_fec_opt = ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN | > + ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN | > + ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ | > + ICE_AQC_PHY_FEC_25G_KR_REQ; > + } > + } else { > + return -EINVAL; > + } > + } > + > + /* Recovery of accidentally rewritten bit */ > + if (pi->phy.curr_user_phy_cfg.link_fec_opt & > + ~ICE_AQC_PHY_FEC_MASK) same indentation issue here, please add an extra tab also it would be a bit more readable if you test link_fec_opt against ICE_AQC_PHY_FEC_DIS instead of ~ICE_AQC_PHY_FEC_MASK. Additionally no need to clean up this flag in cfg if it wasn't there in the first place. > + cfg.link_fec_opt |= ICE_AQC_PHY_FEC_DIS; > + else > + cfg.link_fec_opt &= ICE_AQC_PHY_FEC_MASK; > + > + /* Proceed only if requesting different FEC mode */ > + if (pi->phy.curr_user_phy_cfg.link_fec_opt == cfg.link_fec_opt) > + return 0; > + > + cfg.caps |= ICE_AQ_PHY_ENA_AUTO_LINK_UPDT; > + > + if (ice_aq_set_phy_cfg(pi->hw, pi, &cfg, NULL)) > + return -EAGAIN; > + > + return 0; > +} > + > static int > ice_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > struct rte_pci_device *pci_dev) -- Regards, Vladimir --------------7JBesJHrh9oWZf5vKKtLaqtb Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit

Hi Minjin,

- please update release notes

- see comments inline

On 02/07/2024 09:02, Mingjin Ye wrote:
This patch enable three Forward Error Correction(FEC) related ops
in ice driver. As no speed information can get from HW, this patch
only show FEC capability.

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v2: fix some logic
---
 doc/guides/nics/features/ice.ini |   1 +
 doc/guides/nics/ice.rst          |   5 +
 drivers/net/ice/ice_ethdev.c     | 292 +++++++++++++++++++++++++++++++
 3 files changed, 298 insertions(+)
<snip> 
+static int
+ice_fec_get_capability(struct rte_eth_dev *dev, struct rte_eth_fec_capa *speed_fec_capa,
+			   unsigned int num)
+{
+	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ice_aqc_get_phy_caps_data pcaps = {0};
+	unsigned int capa_num;
+	int ret;
+
+	ret = ice_aq_get_phy_caps(hw->port_info, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA,
+				  &pcaps, NULL);
+	if (ret != ICE_SUCCESS)
+		goto done;

This breaks API. Please redefine with some supported error status

And please get rid of goto here, it is not necessary anymore

+
+	/* first time to get capa_num */
+	capa_num = ice_fec_get_capa_num(&pcaps, NULL);
+	if (!speed_fec_capa || num < capa_num) {
+		ret = capa_num;
+		goto done;
+	}
+
+	ret = ice_fec_get_capa_num(&pcaps, speed_fec_capa);
+
+done:
+	return ret;
+}
+
+static int
+ice_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+#define FEC_CAPA_NUM 10
I believe this macro is not needed
+	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	bool link_up;
+	u32 temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC);
+	struct ice_link_status link_status = {0};
+	struct ice_aqc_get_phy_caps_data pcaps = {0};
+	struct ice_port_info *pi = hw->port_info;
+	u8 fec_config;
+	int ret;
+
+	if (!pi)
+		return -ENOTSUP;
+
+	ret = ice_get_link_info_safe(pf, enable_lse, &link_status);
+	if (ret != ICE_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to get link information: %d\n",
+			ret);
+		goto done;
Same problem here and below with API and returning error statuses. And I think it is worth to get rid of goto here in this function as well.
+	}
+
+	link_up = link_status.link_info & ICE_AQ_LINK_UP;
+
+	ret = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA,
+				  &pcaps, NULL);
+	if (ret != ICE_SUCCESS)
+		goto done;
+
+	/* Get current FEC mode from port info */
+	if (link_up) {
+		switch (link_status.fec_info) {
+		case ICE_AQ_LINK_25G_KR_FEC_EN:
+			temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+			break;
+		case ICE_AQ_LINK_25G_RS_528_FEC_EN:
+			/* fall-through */
+		case ICE_AQ_LINK_25G_RS_544_FEC_EN:
+			temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+			break;
+		default:
+			temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(NOFEC);
+			break;
+		}
+		goto done;
+	}
+
+	if (pcaps.caps & ICE_AQC_PHY_EN_AUTO_FEC) {
+		temp_fec_capa = RTE_ETH_FEC_MODE_CAPA_MASK(AUTO);
+		return 0;
+	}
+
+	fec_config = pcaps.link_fec_options & ICE_AQC_PHY_FEC_MASK;
+
+	if (fec_config & (ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
+				ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN |
+				ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
+				ICE_AQC_PHY_FEC_25G_KR_REQ))
+		temp_fec_capa |= RTE_ETH_FEC_MODE_CAPA_MASK(BASER);
+
+	if (fec_config & (ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN |
+				ICE_AQC_PHY_FEC_25G_RS_528_REQ |
+				ICE_AQC_PHY_FEC_25G_RS_544_REQ))
+		temp_fec_capa |= RTE_ETH_FEC_MODE_CAPA_MASK(RS);
+
+done:
+	*fec_capa = temp_fec_capa;
+	return ret;
+}
+
+static int
+ice_fec_set(struct rte_eth_dev *dev, uint32_t fec_capa)
+{
+	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ice_port_info *pi = hw->port_info;
+	struct ice_aqc_set_phy_cfg_data cfg = { 0 };
+	bool fec_auto = false, fec_kr = false, fec_rs = false;
+
+	if (!pi)
+		return -ENOTSUP;
+
+	if (fec_capa & ~(RTE_ETH_FEC_MODE_CAPA_MASK(AUTO) |
+		RTE_ETH_FEC_MODE_CAPA_MASK(BASER) |
+		RTE_ETH_FEC_MODE_CAPA_MASK(RS)))
please use additional tab to indent properly according to dpdk coding style
+		return -EINVAL;
+	/* Copy the current user PHY configuration. The current user PHY
+	 * configuration is initialized during probe from PHY capabilities
+	 * software mode, and updated on set PHY configuration.
+	 */
+	memcpy(&cfg, &pi->phy.curr_user_phy_cfg, sizeof(cfg));
+
+	if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(AUTO))
+		fec_auto = true;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(BASER))
+		fec_kr = true;
+
+	if (fec_capa & RTE_ETH_FEC_MODE_CAPA_MASK(RS))
+		fec_rs = true;
+
+	if (fec_auto) {
+		if (fec_kr || fec_rs) {
+			if (fec_rs) {
+				cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN |
+					ICE_AQC_PHY_FEC_25G_RS_528_REQ |
+					ICE_AQC_PHY_FEC_25G_RS_544_REQ;
+			}
+			if (fec_kr) {
+				cfg.link_fec_opt |= ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
+					ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN |
+					ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
+					ICE_AQC_PHY_FEC_25G_KR_REQ;
+			}
+		} else {
+			cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN |
+				ICE_AQC_PHY_FEC_25G_RS_528_REQ |
+				ICE_AQC_PHY_FEC_25G_RS_544_REQ |
+				ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
+				ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN |
+				ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
+				ICE_AQC_PHY_FEC_25G_KR_REQ;
+		}
+	} else {
+		if (fec_kr ^ fec_rs) {
+			if (fec_rs) {
+				cfg.link_fec_opt = ICE_AQC_PHY_FEC_25G_RS_CLAUSE91_EN |
+					ICE_AQC_PHY_FEC_25G_RS_528_REQ |
+					ICE_AQC_PHY_FEC_25G_RS_544_REQ;
+			} else {
+				cfg.link_fec_opt = ICE_AQC_PHY_FEC_10G_KR_40G_KR4_EN |
+					ICE_AQC_PHY_FEC_25G_KR_CLAUSE74_EN |
+					ICE_AQC_PHY_FEC_10G_KR_40G_KR4_REQ |
+					ICE_AQC_PHY_FEC_25G_KR_REQ;
+			}
+		} else {
+			return -EINVAL;
+		}
+	}
+
+	/* Recovery of accidentally rewritten bit */
+	if (pi->phy.curr_user_phy_cfg.link_fec_opt &
+		~ICE_AQC_PHY_FEC_MASK)

same indentation issue here, please add an extra tab

also it would be a bit more readable if you test link_fec_opt against ICE_AQC_PHY_FEC_DIS instead of ~ICE_AQC_PHY_FEC_MASK. Additionally no need to clean up this flag in cfg if it wasn't there in the first place.

+		cfg.link_fec_opt |= ICE_AQC_PHY_FEC_DIS;
+	else
+		cfg.link_fec_opt &= ICE_AQC_PHY_FEC_MASK;
+
+	/* Proceed only if requesting different FEC mode */
+	if (pi->phy.curr_user_phy_cfg.link_fec_opt == cfg.link_fec_opt)
+		return 0;
+
+	cfg.caps |= ICE_AQ_PHY_ENA_AUTO_LINK_UPDT;
+
+	if (ice_aq_set_phy_cfg(pi->hw, pi, &cfg, NULL))
+		return -EAGAIN;
+
+	return 0;
+}
+
 static int
 ice_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	      struct rte_pci_device *pci_dev)

-- 
Regards,
Vladimir
--------------7JBesJHrh9oWZf5vKKtLaqtb--