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 E039B45692; Tue, 23 Jul 2024 12:04:44 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8824940151; Tue, 23 Jul 2024 12:04:44 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) by mails.dpdk.org (Postfix) with ESMTP id DE76A40150 for ; Tue, 23 Jul 2024 12:03: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=1721729031; x=1753265031; h=message-id:date:subject:to:cc:references:from: in-reply-to:mime-version; bh=Sk9E/emShoN/8uZxLhLHy3cb63ptjl/oVES0GtX9omM=; b=arpAKAMEzRPnUk+EeW0+baDkyfcWoj8yMBPqQ2nwntOg2US+3L4BMmjk JYMco/z+b4Z8UP0qD7wRR+32JUN4h0ewMIt6SBE/kwSwWMB9n4l/xtqUR TIZR3us4cE7ZJoa/heZ2nczKqCQ5k5ZEnx332C33sya7mMXcBYEZzC3cq ByYTWDIlRxaUr+nvNlYrxzwUNLaNa3faNRFBZ/x9ci/FRhHpms+ur1KQU vpWNNQV6l5xY5S7YHPVEk4qvKTBhrNqpOf3gG4jzI8maJFcSEY6Os0JBS QPqRiDU5UukbxTecCzGTmXVcwfnONUSK/BG4VTtjnawQ5oPpp2YM1IoLH Q==; X-CSE-ConnectionGUID: XGsA34nHRAuYT8DIzUyHew== X-CSE-MsgGUID: DKkAU3/bQnqKKpRoLlBSRA== X-IronPort-AV: E=McAfee;i="6700,10204,11141"; a="12664509" X-IronPort-AV: E=Sophos;i="6.09,230,1716274800"; d="scan'208,217";a="12664509" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Jul 2024 03:03:50 -0700 X-CSE-ConnectionGUID: At++beR4TIORYuINUes/Rw== X-CSE-MsgGUID: XSQsW7nJTHuc6nBMyzzRww== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,230,1716274800"; d="scan'208,217";a="57307973" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by orviesa004.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 23 Jul 2024 03:03:50 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Tue, 23 Jul 2024 03:03:48 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Tue, 23 Jul 2024 03:03:48 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Tue, 23 Jul 2024 03:03:48 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.43) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 23 Jul 2024 03:03:48 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=yCHCQ1sPcVxH74hCtXSozzxYMg1jm4EUsQcj5F0VL23Hc43lTr9cgRXQi/Wibx+lniUPNtOmzWQ8umDZwsiEoLODs6THP0JkD6fgpWQe7P6yr7rD9SSu3GTa4SNRHZSiCLQ7/p2ov+ZDIdyP4E5zdKyHreM72BzWH1qtoLX0ydeF+GPZ1I18u7qPhqYvoMwBDkpoz2c0h6HfuhDtEzYZwLBb4NWdaAIRRet5Nvh7HvdGzXaIdK++JR90tjBXT9W30WO8/LWscsT5CmmWvAMwiu7PhpGwJDgeD+HTRDvexsjEF/nUCST8ZdJWs92PSb/Yo0hW4dUB0f1kNtky8SRi0Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=maahkljVVe2CqRj/uSMRmMxRRTSUt3Txcdz0elETO5A=; b=Lhjm2Eskf/KGqG9ch1EXQJT0VdzUVwuG3fVE3kJ3qlkKe+TRy6CaGHq0NgnFOFYGFnO1QBrxJN5FQlOY6DAvJRild4a+QYrEnfsUV4TgV68Cj5KGHo4THCTbj7URnt5q3vZZatg8XSoc0mFjKA1FPtVx9hRjxPy+w7bQ2mgadzmaxuOMTjs51vDp4srYqL6EpBkJqeMTT4K+vFKH2d4YWWrtsIatMml+K26V8iqhfqsXQvBD8BlFGlpk71uSewtzKxC89dJQ7u+xgRRFb8r5g0DOdWnkCvQXyqo/ND+2/Lub7pS13h5GoqDp4AZzZ1Xlq4OCh9y5zjlOf+JyMwMD0Q== 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 LV3PR11MB8579.namprd11.prod.outlook.com (2603:10b6:408:1b6::7) by SJ0PR11MB5008.namprd11.prod.outlook.com (2603:10b6:a03:2d5::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7784.18; Tue, 23 Jul 2024 10:03:40 +0000 Received: from LV3PR11MB8579.namprd11.prod.outlook.com ([fe80::ec3c:7b9a:2442:1ce4]) by LV3PR11MB8579.namprd11.prod.outlook.com ([fe80::ec3c:7b9a:2442:1ce4%4]) with mapi id 15.20.7784.016; Tue, 23 Jul 2024 10:03:40 +0000 Content-Type: multipart/alternative; boundary="------------YDlOEdJmZbd9XKl9PnxAPlSO" Message-ID: <794598fe-7e2b-421e-9cad-1ddeb44f6b3c@intel.com> Date: Tue, 23 Jul 2024 11:03:36 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 1/4] power: refactor core power management library To: Sivaprasad Tummala , , , , , , CC: References: <20240220153326.6236-1-sivaprasad.tummala@amd.com> <20240720165030.246294-1-sivaprasad.tummala@amd.com> <20240720165030.246294-2-sivaprasad.tummala@amd.com> Content-Language: en-US From: "Hunt, David" In-Reply-To: <20240720165030.246294-2-sivaprasad.tummala@amd.com> X-ClientProxiedBy: DUZPR01CA0336.eurprd01.prod.exchangelabs.com (2603:10a6:10:4b8::21) To LV3PR11MB8579.namprd11.prod.outlook.com (2603:10b6:408:1b6::7) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: LV3PR11MB8579:EE_|SJ0PR11MB5008:EE_ X-MS-Office365-Filtering-Correlation-Id: 7f1fc7f5-ef9c-4421-2679-08dcaafeba47 X-LD-Processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|366016|1800799024|376014; X-Microsoft-Antispam-Message-Info: =?utf-8?B?VGQvMElYaTMwZThlNHZhQlBvZmZ2MEczSnpvR3JzYTBLQ0tWRGlId015RmxG?= =?utf-8?B?ZXh5bGZ4Q0JFMDlhS0M1RlZsTXB2d3pIcEZYb01EK2s1MDN1aFpJeFhPMzFY?= =?utf-8?B?Ujd0cU8rSGFyamJaR3kyQ3hwanNLS2ZMblNXKzVBWHJ0MG44NXpyOUp4d3pj?= =?utf-8?B?ZU5iWWdWSFppOHgzTUs5Szl5SHEySTNRVlFJS0lDNklLak1kbThCQW5TQmNR?= =?utf-8?B?MXRKYmRMdmhjWTROUmQya1UvUUZTUnM1Tk5kcnEzVDBJNXdOUWlFSkhlUTBD?= =?utf-8?B?RFZqU3B4dnhnWjNiZTI1RDd0YmpTVXdkZGFLNGhXNW4vVHl0TDV6VDlQMlhy?= =?utf-8?B?VzNNb05NdGw3TzZqTDZMV2c0ajlrV2lQaEJyaG9RZ2R4K3BUYWZsR0F5aGZ5?= =?utf-8?B?TG1NK1l0VTNNVFNLSllIYWtYdXFOL1dLN0hjWUZmRVJBZk1yWjVTTnN5L1dW?= =?utf-8?B?YUloUExzOVREZWhWRWdyb1U2WWJSL25NSkw4bFIrRXRKTktwWnhDN0t3Wldz?= =?utf-8?B?SFRYTWZQcWkzZ0U3a1djc1ZMUVM0RHIrb2d5aGpVKzBVSk04UGpBWEZqc3N3?= =?utf-8?B?Y25VN2xqQ1F4Z0pBMGNpR09oL0lUd3pRRWhGSGJUbGFwT0FiL3p0ZGw5elcw?= =?utf-8?B?dWRpa0ZCaHFjSEJRSTY5djNoMkZCRzhkQTViR2tVVC9UZVpHSVlSWjE2WGlR?= =?utf-8?B?bHZ0QllVb1RkNTlsejBmcVpoaFpKQVNuZ1B3M1Nvc0IyWjAwSStYUllQWlB1?= =?utf-8?B?OVBQT08xSkhVVXJOa1dNTHZyM3JuSU45TXc2L0xtRDMrMHl4L2tYRzllR2tl?= =?utf-8?B?aTZKdTRBU0pYd3p2eWpnZVNhNTdrZmh4dkpWY0N5QlRpU3NKVWFGaTA0d1NR?= =?utf-8?B?OURQUi9QR2I1VkNxMEZHSnVLNU54dlFtTHhjZkRMVzROWTNlMy9nMFZBcm9R?= =?utf-8?B?a2VXMEpWMHE3YmsvUmlYQXdVQzR3Wld4TlRsUGhDR1c3MjVLMHFKTEd2NlFQ?= =?utf-8?B?cUNwZVhBeXJFbkpNZ2YzckVNWnpUcG8xTzdVUDhtTXpiSEM2S2hNUFZZbkdL?= =?utf-8?B?WDZuOGNaQVZ4Qys1SE5iL1VyeEFZRXVLa2JaQllzN2cwNWNWSjVOZ29Jb3ha?= =?utf-8?B?YStadFF2VUQ3VVkxYjA0NlRONHlFbXJLb21xY0FmTlVSL01sT0NRSXJnYzBw?= =?utf-8?B?ZXAzYlFCekNUOHpPcExyd1R5cE5VeDlTN2pXQTZzSzZXQWpGbVRzK2twYTl2?= =?utf-8?B?aTFCdERJMGp5YThPOC9pWitDQzNYNzdnaHZwV2VQNDVpRi9vL2k2TTkrcXpu?= =?utf-8?B?K0FZS2dKYmV2dWhHTUFGZE5TaW92NWNOYjBNVmFaaGhmWmJacmJFVHpOdEM3?= =?utf-8?B?Tmw1eEZUOFZOZmR0MXNMTmFtQThRU3FubEpxWXN3aFdyRzhzZkRTTU90L1ZU?= =?utf-8?B?ZXVnN3pNK2poRHJSaE9CY2crRUIyTE1XeXFvV2Z5bWg1NzBId3B2ZGdPeTFv?= =?utf-8?B?eFFxbmNaZGRydmhaeTlSaDluNTM2cWlrN1RhUmtGTVUrYlFwZUY4dEljSHN1?= =?utf-8?B?QWdmbzdMbGxONTlKRmd1dVBjc3JGc01SNCtpZmUzQUJ1WHhRMjJsNVk2WU1u?= =?utf-8?B?THVkaHZYUzhjV2VtdThFR2QvMkdYSkU2MVZJYmV6MUhkbVFBQ2I3NXkyd0hJ?= =?utf-8?B?OGI0TVI4cHRWN0Jrc2JORXBwM3VsS2dKMW1SZGd2S0l1RllaOHVVeUx5MHN1?= =?utf-8?B?SkpQQVloUWJiYmhicEVWMG1mTW15cGc5S2VveTJXWG5DNkZzRVM0Q3ZXSjhL?= =?utf-8?B?NUQrOWJuUXo5bmZhNGRuQT09?= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:LV3PR11MB8579.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230040)(366016)(1800799024)(376014); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UG9DMVpwUU14dVNxMi9IUmNlT1ROSkpJeFF3WjBwdmVTZVBEdWVTeGwzM3FU?= =?utf-8?B?NzI1cVZ6Q05DYzI0TFprK3Q3emp6clhsSWVrVUFkcWNwc21ockxaNVBOcGYw?= =?utf-8?B?aE56S2tCcTc4KzFEbFBSZHpqQ1ppMjEzQUIrc0txK1hvZjU3dlhiVGJXdWVP?= =?utf-8?B?eDhZS1UwRWxtS1BhSzNlMERJMXVpQnBYdEZzRjBjOWRkajRIVXB0NWp4dkND?= =?utf-8?B?RDdmT3BSVWJXdlZuMWVEOW9MZlRleWp6OFMwdmxDMTJuWDR4Ti9SR3hOeVJh?= =?utf-8?B?M0dWb1p2bDYvajl6TkNWYUZUQTVWcUpMekQ3K3VSSFNGYm82Rk0rQXFuSjVI?= =?utf-8?B?eEQ1SjdRcnpwSGVWTzJONlBXTTdLOXdTRTR3QjdyN1l2V3pYbHFkM1ZoK0hH?= =?utf-8?B?WUxIcnhqN3ByeGEyNTQ2OE9qYkxhNGVSdEZxUm5BRCtTdFM1YnU2emNxVndl?= =?utf-8?B?L3ZNKzR3cGVndW5tSFJsSVpSMC9DczY0RG9VdnVyVk8ybFVjUHo4cjhJSFdE?= =?utf-8?B?L0lpcVpQWjhDai8zQ0N0Qk9kVG4xYjkxN1RiRFliSnFjejQ1QTV0OFhuT2hG?= =?utf-8?B?b0oyVkJ3WjZZYkdFbUpqSE44VUpFdUJxRGhMdk1KVlpVYjUycFR0b2owbVNS?= =?utf-8?B?ak5pRThJY0U2ekR6cjVQcE8vdkcrSS9LaUhSYUlsV2lSZlRrVlQ0c3gyWEt5?= =?utf-8?B?T1hmYjZ0VThKb0s3Y05scUtlNXk1UTkxOTVrWkJLdHVjbFZJK2lad2pScDUz?= =?utf-8?B?TW5JYzI1bzg0NkRnOENFYnVaUW9MVVdKYVQ5WnlWY0FxaTlYUGdYa0RVa2gy?= =?utf-8?B?NTRHN2t0K3pmekRWYWxEM1VIcG1GUit0VGIvOEYxMWVTSEpTbVJFeEZobm5x?= =?utf-8?B?Ly96YlBMTzRmVmlMQUF6K29BU3dhU0tFdjNoTWJ3QlNpZkd2VTB1Y3NrclpM?= =?utf-8?B?SmljWnkzZ1pYYlRwRUpTTFNibllUMHNMd3llaFFQTHZYZUh1NjdxVDMyWXEz?= =?utf-8?B?ODJjeEZGNm53MWpDUTRWd1JyQldqRW13WTVjTEVpeWpuSEt4cjJhdmdMbWkz?= =?utf-8?B?eEhYdEFFZXozVlZqcU53bVk5bktyY2cxZzlIeHVqUjlMWVVyMEhzYnlyZnNp?= =?utf-8?B?bUZFOVJ2ZlZvZ1gxeUNQTWtYaEdqM0RPeTlkRjczdGxnU2pwWlg4cEhJc09U?= =?utf-8?B?T2RzTEs5Wm9XL21mUDlITE5tTUpiUUhHSzh4cTkySW8rOWtLd2Z2dzFibHRj?= =?utf-8?B?UHdzT1Qwd1BteUtoU1ZOVkF5OFNkTW1KOEU5K0taK01Bb1o4M0psMWg3d0Vy?= =?utf-8?B?LzdhQWVVSXZHcnVjQWNRdlVLSm9Tb1B6UlZPd0RjRkUxRmkwMWFWTXlJZ0VK?= =?utf-8?B?YnAyNG5LQjJsbVIvUjY4Vi9sTWl3RmZkYklWTnFhdlZMWHRyK0lOQWM0dnBW?= =?utf-8?B?YTZDaEtXYm9YRWxRcHBPdzRLQTRwV0wreG05OHE5OXRCc3Bma1RYbTdWNEJn?= =?utf-8?B?a0dpVmV1Z2l3bjdVUTViVzBVUVNCaFN4bUlWRUFKTVNGbHNtcHBpSnd0KzNJ?= =?utf-8?B?T0o3RlVIWndHTmpwejFic0RmK0J0c3ZMVjdrdHBDeVlSakJ6UzlwUFV5WlNJ?= =?utf-8?B?TmJVeUEyREgyUzljem1ZTEJsK25vUCtzS1RxOXFqNDFPVnZlbC92MExFMnBr?= =?utf-8?B?QnBoaGg3ZzJTenBlaVBRc2VEYmZ1Tk5kamZsWEJ1WFJhWVJacWRLbloyTy90?= =?utf-8?B?VlF2Y1oxb0loM2lGRkdMTjFoOW8zYnBZd3k3TzJIbmFuUDRwdzNwQWp6NldZ?= =?utf-8?B?VXcyK05YQXVuTjNtd0ZXYUpzWC8vOUdJVDJLVjlZUzZTZXpodDJya2pPb1lt?= =?utf-8?B?dUlvaysydGIvN3pHcWZ0eUVubDY1ejU0U0VVUytUS0hpVEcyZHJMUE9wdVpx?= =?utf-8?B?TFVDR243Zm1XSFVXQTZaOEswZlRGSlNGZUcvSmhjTE9EVW8wSmo4MGx2M3dv?= =?utf-8?B?bG82MmJZUHlxd1dXS2prNjFiUi81dGdFL3pZUmsvZUlqSldrMTFPMGR6MnJn?= =?utf-8?B?cDVRY0RuWTlLc1lVTEt1dnNIVE1oMDZhdEphbXN0a0RxWnpkSi9zaGxXbHpU?= =?utf-8?B?S3VrWFhDZDZNbWVRMGxzMzZvdllpUlpDbmVTMGtzVzB6d3o2VDQ1Q1l4KzBJ?= =?utf-8?B?TVE9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: 7f1fc7f5-ef9c-4421-2679-08dcaafeba47 X-MS-Exchange-CrossTenant-AuthSource: LV3PR11MB8579.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jul 2024 10:03:40.5213 (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: LwsLWCb7M6MQJaO5Fs04E/qbyEoeaUz3gvgrUZT2eJQrppTIlo8jnT9TRsL5XOmmkBVv5KYWxwerRiRkfOvstg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5008 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 --------------YDlOEdJmZbd9XKl9PnxAPlSO Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Hi Sivaprasad, A couple of comments below: On 20/07/2024 17:50, Sivaprasad Tummala wrote: > This patch introduces a comprehensive refactor to the core power > management library. The primary focus is on improving modularity > and organization by relocating specific driver implementations > from the 'lib/power' directory to dedicated directories within > 'drivers/power/core/*'. The adjustment of meson.build files > enables the selective activation of individual drivers. > > These changes contribute to a significant enhancement in code > organization, providing a clearer structure for driver implementations. > The refactor aims to improve overall code clarity and boost > maintainability. Additionally, it establishes a foundation for > future development, allowing for more focused work on individual > drivers and seamless integration of forthcoming enhancements. > > Signed-off-by: Sivaprasad Tummala > --- > drivers/meson.build | 1 + > .../power/acpi/acpi_cpufreq.c | 22 +- > .../power/acpi/acpi_cpufreq.h | 6 +- > drivers/power/acpi/meson.build | 10 + > .../power/amd_pstate/amd_pstate_cpufreq.c | 24 +- > .../power/amd_pstate/amd_pstate_cpufreq.h | 8 +- > drivers/power/amd_pstate/meson.build | 10 + > .../power/cppc/cppc_cpufreq.c | 22 +- > .../power/cppc/cppc_cpufreq.h | 8 +- > drivers/power/cppc/meson.build | 10 + > .../power/kvm_vm}/guest_channel.c | 0 > .../power/kvm_vm}/guest_channel.h | 0 > .../power/kvm_vm/kvm_vm.c | 22 +- > .../power/kvm_vm/kvm_vm.h | 6 +- > drivers/power/kvm_vm/meson.build | 16 + > drivers/power/meson.build | 12 + > drivers/power/pstate/meson.build | 10 + > .../power/pstate/pstate_cpufreq.c | 22 +- > .../power/pstate/pstate_cpufreq.h | 6 +- > lib/power/meson.build | 7 +- > lib/power/power_common.c | 2 +- > lib/power/power_common.h | 16 +- > lib/power/rte_power.c | 287 ++++++------------ > lib/power/rte_power.h | 139 ++++++--- > lib/power/rte_power_core_ops.h | 208 +++++++++++++ > lib/power/version.map | 14 + > 26 files changed, 618 insertions(+), 270 deletions(-) > rename lib/power/power_acpi_cpufreq.c => drivers/power/acpi/acpi_cpufreq.c (95%) > rename lib/power/power_acpi_cpufreq.h => drivers/power/acpi/acpi_cpufreq.h (98%) > create mode 100644 drivers/power/acpi/meson.build > rename lib/power/power_amd_pstate_cpufreq.c => drivers/power/amd_pstate/amd_pstate_cpufreq.c (95%) > rename lib/power/power_amd_pstate_cpufreq.h => drivers/power/amd_pstate/amd_pstate_cpufreq.h (97%) > create mode 100644 drivers/power/amd_pstate/meson.build > rename lib/power/power_cppc_cpufreq.c => drivers/power/cppc/cppc_cpufreq.c (95%) > rename lib/power/power_cppc_cpufreq.h => drivers/power/cppc/cppc_cpufreq.h (97%) > create mode 100644 drivers/power/cppc/meson.build > rename {lib/power => drivers/power/kvm_vm}/guest_channel.c (100%) > rename {lib/power => drivers/power/kvm_vm}/guest_channel.h (100%) > rename lib/power/power_kvm_vm.c => drivers/power/kvm_vm/kvm_vm.c (82%) > rename lib/power/power_kvm_vm.h => drivers/power/kvm_vm/kvm_vm.h (98%) > create mode 100644 drivers/power/kvm_vm/meson.build > create mode 100644 drivers/power/meson.build > create mode 100644 drivers/power/pstate/meson.build > rename lib/power/power_pstate_cpufreq.c => drivers/power/pstate/pstate_cpufreq.c (96%) > rename lib/power/power_pstate_cpufreq.h => drivers/power/pstate/pstate_cpufreq.h (98%) > create mode 100644 lib/power/rte_power_core_ops.h --snip-- > diff --git a/lib/power/rte_power.c b/lib/power/rte_power.c > index 36c3f3da98..8afb5949b9 100644 > --- a/lib/power/rte_power.c > +++ b/lib/power/rte_power.c > @@ -8,153 +8,86 @@ > #include > > #include "rte_power.h" > -#include "power_acpi_cpufreq.h" > -#include "power_cppc_cpufreq.h" > #include "power_common.h" > -#include "power_kvm_vm.h" > -#include "power_pstate_cpufreq.h" > -#include "power_amd_pstate_cpufreq.h" > > -enum power_management_env global_default_env = PM_ENV_NOT_SET; > +static enum power_management_env global_default_env = PM_ENV_NOT_SET; > +static struct rte_power_core_ops *global_power_core_ops; Suggest initialising this to NULL so we can check in rte_power_get_core_ops if it's null and throw an error. --snip-- > +struct rte_power_core_ops * > +rte_power_get_core_ops(void) > +{ Need a check here to see if rte_power_get_core_ops is NULL. If it is, then the developer has probably called a frequency change API before the relevant init function, so throw an error. Also, all the functions that call this need to check if it returns NULL so as to avoid a segfault when they attempts to call the op function. > + return global_power_core_ops; > +} > + --snip-- > diff --git a/lib/power/rte_power.h b/lib/power/rte_power.h > index 4fa4afe399..5e4aacf08b 100644 > --- a/lib/power/rte_power.h > +++ b/lib/power/rte_power.h > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: BSD-3-Clause > * Copyright(c) 2010-2014 Intel Corporation > + * Copyright(c) 2024 Advanced Micro Devices, Inc. > */ > > #ifndef _RTE_POWER_H > @@ -14,14 +15,21 @@ > #include > #include > > +#include "rte_power_core_ops.h" > + > #ifdef __cplusplus > extern "C" { > #endif > > /* Power Management Environment State */ > -enum power_management_env {PM_ENV_NOT_SET, PM_ENV_ACPI_CPUFREQ, PM_ENV_KVM_VM, > - PM_ENV_PSTATE_CPUFREQ, PM_ENV_CPPC_CPUFREQ, > - PM_ENV_AMD_PSTATE_CPUFREQ}; > +enum power_management_env { > + PM_ENV_NOT_SET = 0, > + PM_ENV_ACPI_CPUFREQ, > + PM_ENV_KVM_VM, > + PM_ENV_PSTATE_CPUFREQ, > + PM_ENV_CPPC_CPUFREQ, > + PM_ENV_AMD_PSTATE_CPUFREQ > +}; > > /** > * Check if a specific power management environment type is supported on a > @@ -66,6 +74,15 @@ void rte_power_unset_env(void); > */ > enum power_management_env rte_power_get_env(void); > > +/** > + * @internal Get the power ops struct from its index. > + * > + * @return > + * The pointer to the ops struct in the table if registered. > + */ > +struct rte_power_core_ops * > +rte_power_get_core_ops(void); > + > /** > * Initialize power management for a specific lcore. If rte_power_set_env() has > * not been called then an auto-detect of the environment will start and > @@ -108,10 +125,13 @@ int rte_power_exit(unsigned int lcore_id); > * @return > * The number of available frequencies. > */ > -typedef uint32_t (*rte_power_freqs_t)(unsigned int lcore_id, uint32_t *freqs, > - uint32_t num); > +static inline uint32_t > +rte_power_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t n) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > > -extern rte_power_freqs_t rte_power_freqs; > + return ops->get_avail_freqs(lcore_id, freqs, n); This function will segfault if is called before the appropriate init is performed. See comments above on global_power_core_ops. Same for all the functions below that call global_power_core_ops(). > +} > > /** > * Return the current index of available frequencies of a specific lcore. > @@ -124,9 +144,13 @@ extern rte_power_freqs_t rte_power_freqs; > * @return > * The current index of available frequencies. > */ > -typedef uint32_t (*rte_power_get_freq_t)(unsigned int lcore_id); > +static inline uint32_t > +rte_power_get_freq(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > > -extern rte_power_get_freq_t rte_power_get_freq; > + return ops->get_freq(lcore_id); > +} > > /** > * Set the new frequency for a specific lcore by indicating the index of > @@ -144,82 +168,101 @@ extern rte_power_get_freq_t rte_power_get_freq; > * - 0 on success without frequency changed. > * - Negative on error. > */ > -typedef int (*rte_power_set_freq_t)(unsigned int lcore_id, uint32_t index); > - > -extern rte_power_set_freq_t rte_power_set_freq; > +static inline uint32_t > +rte_power_set_freq(unsigned int lcore_id, uint32_t index) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > > -/** > - * Function pointer definition for generic frequency change functions. Review > - * each environments specific documentation for usage. > - * > - * @param lcore_id > - * lcore id. > - * > - * @return > - * - 1 on success with frequency changed. > - * - 0 on success without frequency changed. > - * - Negative on error. > - */ > -typedef int (*rte_power_freq_change_t)(unsigned int lcore_id); > + return ops->set_freq(lcore_id, index); > +} > > /** > * Scale up the frequency of a specific lcore according to the available > * frequencies. > * Review each environments specific documentation for usage. > */ > -extern rte_power_freq_change_t rte_power_freq_up; > +static inline int > +rte_power_freq_up(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->freq_up(lcore_id); > +} > > /** > * Scale down the frequency of a specific lcore according to the available > * frequencies. > * Review each environments specific documentation for usage. > */ > -extern rte_power_freq_change_t rte_power_freq_down; > +static inline int > +rte_power_freq_down(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->freq_down(lcore_id); > +} > > /** > * Scale up the frequency of a specific lcore to the highest according to the > * available frequencies. > * Review each environments specific documentation for usage. > */ > -extern rte_power_freq_change_t rte_power_freq_max; > +static inline int > +rte_power_freq_max(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->freq_max(lcore_id); > +} > > /** > * Scale down the frequency of a specific lcore to the lowest according to the > * available frequencies. > * Review each environments specific documentation for usage.. > */ > -extern rte_power_freq_change_t rte_power_freq_min; > +static inline int > +rte_power_freq_min(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->freq_min(lcore_id); > +} > > /** > * Query the Turbo Boost status of a specific lcore. > * Review each environments specific documentation for usage.. > */ > -extern rte_power_freq_change_t rte_power_turbo_status; > +static inline int > +rte_power_turbo_status(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->turbo_status(lcore_id); > +} > > /** > * Enable Turbo Boost for this lcore. > * Review each environments specific documentation for usage.. > */ > -extern rte_power_freq_change_t rte_power_freq_enable_turbo; > +static inline int > +rte_power_freq_enable_turbo(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > + > + return ops->enable_turbo(lcore_id); > +} > > /** > * Disable Turbo Boost for this lcore. > * Review each environments specific documentation for usage.. > */ > -extern rte_power_freq_change_t rte_power_freq_disable_turbo; > +static inline int > +rte_power_freq_disable_turbo(unsigned int lcore_id) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > > -/** > - * Power capabilities summary. > - */ > -struct rte_power_core_capabilities { > - union { > - uint64_t capabilities; > - struct { > - uint64_t turbo:1; /**< Turbo can be enabled. */ > - uint64_t priority:1; /**< SST-BF high freq core */ > - }; > - }; > -}; > + return ops->disable_turbo(lcore_id); > +} > > /** > * Returns power capabilities for a specific lcore. > @@ -235,10 +278,14 @@ struct rte_power_core_capabilities { > * - 0 on success. > * - Negative on error. > */ > -typedef int (*rte_power_get_capabilities_t)(unsigned int lcore_id, > - struct rte_power_core_capabilities *caps); > +static inline int > +rte_power_get_capabilities(unsigned int lcore_id, > + struct rte_power_core_capabilities *caps) > +{ > + struct rte_power_core_ops *ops = rte_power_get_core_ops(); > > -extern rte_power_get_capabilities_t rte_power_get_capabilities; > + return ops->get_caps(lcore_id, caps); > +} > > #ifdef __cplusplus > } --snip-- --------------YDlOEdJmZbd9XKl9PnxAPlSO Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 7bit

Hi Sivaprasad,

A couple of comments below:

On 20/07/2024 17:50, Sivaprasad Tummala wrote:
This patch introduces a comprehensive refactor to the core power
management library. The primary focus is on improving modularity
and organization by relocating specific driver implementations
from the 'lib/power' directory to dedicated directories within
'drivers/power/core/*'. The adjustment of meson.build files
enables the selective activation of individual drivers.

These changes contribute to a significant enhancement in code
organization, providing a clearer structure for driver implementations.
The refactor aims to improve overall code clarity and boost
maintainability. Additionally, it establishes a foundation for
future development, allowing for more focused work on individual
drivers and seamless integration of forthcoming enhancements.

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
 drivers/meson.build                           |   1 +
 .../power/acpi/acpi_cpufreq.c                 |  22 +-
 .../power/acpi/acpi_cpufreq.h                 |   6 +-
 drivers/power/acpi/meson.build                |  10 +
 .../power/amd_pstate/amd_pstate_cpufreq.c     |  24 +-
 .../power/amd_pstate/amd_pstate_cpufreq.h     |   8 +-
 drivers/power/amd_pstate/meson.build          |  10 +
 .../power/cppc/cppc_cpufreq.c                 |  22 +-
 .../power/cppc/cppc_cpufreq.h                 |   8 +-
 drivers/power/cppc/meson.build                |  10 +
 .../power/kvm_vm}/guest_channel.c             |   0
 .../power/kvm_vm}/guest_channel.h             |   0
 .../power/kvm_vm/kvm_vm.c                     |  22 +-
 .../power/kvm_vm/kvm_vm.h                     |   6 +-
 drivers/power/kvm_vm/meson.build              |  16 +
 drivers/power/meson.build                     |  12 +
 drivers/power/pstate/meson.build              |  10 +
 .../power/pstate/pstate_cpufreq.c             |  22 +-
 .../power/pstate/pstate_cpufreq.h             |   6 +-
 lib/power/meson.build                         |   7 +-
 lib/power/power_common.c                      |   2 +-
 lib/power/power_common.h                      |  16 +-
 lib/power/rte_power.c                         | 287 ++++++------------
 lib/power/rte_power.h                         | 139 ++++++---
 lib/power/rte_power_core_ops.h                | 208 +++++++++++++
 lib/power/version.map                         |  14 +
 26 files changed, 618 insertions(+), 270 deletions(-)
 rename lib/power/power_acpi_cpufreq.c => drivers/power/acpi/acpi_cpufreq.c (95%)
 rename lib/power/power_acpi_cpufreq.h => drivers/power/acpi/acpi_cpufreq.h (98%)
 create mode 100644 drivers/power/acpi/meson.build
 rename lib/power/power_amd_pstate_cpufreq.c => drivers/power/amd_pstate/amd_pstate_cpufreq.c (95%)
 rename lib/power/power_amd_pstate_cpufreq.h => drivers/power/amd_pstate/amd_pstate_cpufreq.h (97%)
 create mode 100644 drivers/power/amd_pstate/meson.build
 rename lib/power/power_cppc_cpufreq.c => drivers/power/cppc/cppc_cpufreq.c (95%)
 rename lib/power/power_cppc_cpufreq.h => drivers/power/cppc/cppc_cpufreq.h (97%)
 create mode 100644 drivers/power/cppc/meson.build
 rename {lib/power => drivers/power/kvm_vm}/guest_channel.c (100%)
 rename {lib/power => drivers/power/kvm_vm}/guest_channel.h (100%)
 rename lib/power/power_kvm_vm.c => drivers/power/kvm_vm/kvm_vm.c (82%)
 rename lib/power/power_kvm_vm.h => drivers/power/kvm_vm/kvm_vm.h (98%)
 create mode 100644 drivers/power/kvm_vm/meson.build
 create mode 100644 drivers/power/meson.build
 create mode 100644 drivers/power/pstate/meson.build
 rename lib/power/power_pstate_cpufreq.c => drivers/power/pstate/pstate_cpufreq.c (96%)
 rename lib/power/power_pstate_cpufreq.h => drivers/power/pstate/pstate_cpufreq.h (98%)
 create mode 100644 lib/power/rte_power_core_ops.h

--snip--

diff --git a/lib/power/rte_power.c b/lib/power/rte_power.c
index 36c3f3da98..8afb5949b9 100644
--- a/lib/power/rte_power.c
+++ b/lib/power/rte_power.c
@@ -8,153 +8,86 @@
 #include <rte_spinlock.h>
 
 #include "rte_power.h"
-#include "power_acpi_cpufreq.h"
-#include "power_cppc_cpufreq.h"
 #include "power_common.h"
-#include "power_kvm_vm.h"
-#include "power_pstate_cpufreq.h"
-#include "power_amd_pstate_cpufreq.h"
 
-enum power_management_env global_default_env = PM_ENV_NOT_SET;
+static enum power_management_env global_default_env = PM_ENV_NOT_SET;
+static struct rte_power_core_ops *global_power_core_ops;


Suggest initialising this to NULL so we can check in rte_power_get_core_ops if it's null and throw an error.


--snip--

+struct rte_power_core_ops *
+rte_power_get_core_ops(void)
+{


Need a check here to see if rte_power_get_core_ops is NULL. If it is, then the developer has probably called a frequency change API before the relevant init function, so throw an error.

Also, all the functions that call this need to check if it returns NULL so as to avoid a segfault when they attempts to call the op function.


+	return global_power_core_ops;
+}
+

--snip--


diff --git a/lib/power/rte_power.h b/lib/power/rte_power.h
index 4fa4afe399..5e4aacf08b 100644
--- a/lib/power/rte_power.h
+++ b/lib/power/rte_power.h
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2014 Intel Corporation
+ * Copyright(c) 2024 Advanced Micro Devices, Inc.
  */
 
 #ifndef _RTE_POWER_H
@@ -14,14 +15,21 @@
 #include <rte_log.h>
 #include <rte_power_guest_channel.h>
 
+#include "rte_power_core_ops.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /* Power Management Environment State */
-enum power_management_env {PM_ENV_NOT_SET, PM_ENV_ACPI_CPUFREQ, PM_ENV_KVM_VM,
-		PM_ENV_PSTATE_CPUFREQ, PM_ENV_CPPC_CPUFREQ,
-		PM_ENV_AMD_PSTATE_CPUFREQ};
+enum power_management_env {
+	PM_ENV_NOT_SET = 0,
+	PM_ENV_ACPI_CPUFREQ,
+	PM_ENV_KVM_VM,
+	PM_ENV_PSTATE_CPUFREQ,
+	PM_ENV_CPPC_CPUFREQ,
+	PM_ENV_AMD_PSTATE_CPUFREQ
+};
 
 /**
  * Check if a specific power management environment type is supported on a
@@ -66,6 +74,15 @@ void rte_power_unset_env(void);
  */
 enum power_management_env rte_power_get_env(void);
 
+/**
+ * @internal Get the power ops struct from its index.
+ *
+ * @return
+ *   The pointer to the ops struct in the table if registered.
+ */
+struct rte_power_core_ops *
+rte_power_get_core_ops(void);
+
 /**
  * Initialize power management for a specific lcore. If rte_power_set_env() has
  * not been called then an auto-detect of the environment will start and
@@ -108,10 +125,13 @@ int rte_power_exit(unsigned int lcore_id);
  * @return
  *  The number of available frequencies.
  */
-typedef uint32_t (*rte_power_freqs_t)(unsigned int lcore_id, uint32_t *freqs,
-		uint32_t num);
+static inline uint32_t
+rte_power_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t n)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
 
-extern rte_power_freqs_t rte_power_freqs;
+	return ops->get_avail_freqs(lcore_id, freqs, n);


This function will segfault if is called before the appropriate init is performed. See comments above on global_power_core_ops.

Same for all the functions below that call global_power_core_ops().


+}
 
 /**
  * Return the current index of available frequencies of a specific lcore.
@@ -124,9 +144,13 @@ extern rte_power_freqs_t rte_power_freqs;
  * @return
  *  The current index of available frequencies.
  */
-typedef uint32_t (*rte_power_get_freq_t)(unsigned int lcore_id);
+static inline uint32_t
+rte_power_get_freq(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
 
-extern rte_power_get_freq_t rte_power_get_freq;
+	return ops->get_freq(lcore_id);
+}
 
 /**
  * Set the new frequency for a specific lcore by indicating the index of
@@ -144,82 +168,101 @@ extern rte_power_get_freq_t rte_power_get_freq;
  *  - 0 on success without frequency changed.
  *  - Negative on error.
  */
-typedef int (*rte_power_set_freq_t)(unsigned int lcore_id, uint32_t index);
-
-extern rte_power_set_freq_t rte_power_set_freq;
+static inline uint32_t
+rte_power_set_freq(unsigned int lcore_id, uint32_t index)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
 
-/**
- * Function pointer definition for generic frequency change functions. Review
- * each environments specific documentation for usage.
- *
- * @param lcore_id
- *  lcore id.
- *
- * @return
- *  - 1 on success with frequency changed.
- *  - 0 on success without frequency changed.
- *  - Negative on error.
- */
-typedef int (*rte_power_freq_change_t)(unsigned int lcore_id);
+	return ops->set_freq(lcore_id, index);
+}
 
 /**
  * Scale up the frequency of a specific lcore according to the available
  * frequencies.
  * Review each environments specific documentation for usage.
  */
-extern rte_power_freq_change_t rte_power_freq_up;
+static inline int
+rte_power_freq_up(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->freq_up(lcore_id);
+}
 
 /**
  * Scale down the frequency of a specific lcore according to the available
  * frequencies.
  * Review each environments specific documentation for usage.
  */
-extern rte_power_freq_change_t rte_power_freq_down;
+static inline int
+rte_power_freq_down(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->freq_down(lcore_id);
+}
 
 /**
  * Scale up the frequency of a specific lcore to the highest according to the
  * available frequencies.
  * Review each environments specific documentation for usage.
  */
-extern rte_power_freq_change_t rte_power_freq_max;
+static inline int
+rte_power_freq_max(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->freq_max(lcore_id);
+}
 
 /**
  * Scale down the frequency of a specific lcore to the lowest according to the
  * available frequencies.
  * Review each environments specific documentation for usage..
  */
-extern rte_power_freq_change_t rte_power_freq_min;
+static inline int
+rte_power_freq_min(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->freq_min(lcore_id);
+}
 
 /**
  * Query the Turbo Boost status of a specific lcore.
  * Review each environments specific documentation for usage..
  */
-extern rte_power_freq_change_t rte_power_turbo_status;
+static inline int
+rte_power_turbo_status(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->turbo_status(lcore_id);
+}
 
 /**
  * Enable Turbo Boost for this lcore.
  * Review each environments specific documentation for usage..
  */
-extern rte_power_freq_change_t rte_power_freq_enable_turbo;
+static inline int
+rte_power_freq_enable_turbo(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
+
+	return ops->enable_turbo(lcore_id);
+}
 
 /**
  * Disable Turbo Boost for this lcore.
  * Review each environments specific documentation for usage..
  */
-extern rte_power_freq_change_t rte_power_freq_disable_turbo;
+static inline int
+rte_power_freq_disable_turbo(unsigned int lcore_id)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
 
-/**
- * Power capabilities summary.
- */
-struct rte_power_core_capabilities {
-	union {
-		uint64_t capabilities;
-		struct {
-			uint64_t turbo:1;	/**< Turbo can be enabled. */
-			uint64_t priority:1;	/**< SST-BF high freq core */
-		};
-	};
-};
+	return ops->disable_turbo(lcore_id);
+}
 
 /**
  * Returns power capabilities for a specific lcore.
@@ -235,10 +278,14 @@ struct rte_power_core_capabilities {
  *  - 0 on success.
  *  - Negative on error.
  */
-typedef int (*rte_power_get_capabilities_t)(unsigned int lcore_id,
-		struct rte_power_core_capabilities *caps);
+static inline int
+rte_power_get_capabilities(unsigned int lcore_id,
+		struct rte_power_core_capabilities *caps)
+{
+	struct rte_power_core_ops *ops = rte_power_get_core_ops();
 
-extern rte_power_get_capabilities_t rte_power_get_capabilities;
+	return ops->get_caps(lcore_id, caps);
+}
 
 #ifdef __cplusplus
 }


--snip--


--------------YDlOEdJmZbd9XKl9PnxAPlSO--