From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 5D948AC0EE8 for ; Thu, 16 May 2024 15:18:36 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=i2qd9MSOhqlm5KFGSGhht4GAVS7CEefA6QBdi2neCIE=; c=relaxed/simple; d=groups.io; h=Received-SPF:Authentication-Results-Original:Message-ID:Date:User-Agent:Subject:To:Cc:References:From:In-Reply-To:MIME-Version:NoDisclaimer:Original-Authentication-Results:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Language; s=20240206; t=1715872714; v=1; b=W4cBLAyjENgKz8H2qjP/imyU2a+BCeTZWhEN9jzlSh3E7xCY+6Iwjcmt1/r2W+vJ7Q2YubJ3 8xsG7aHLcrKm9WAN40c9G2hIfYagYUz+nksjx0j8Ayyu4NXMN4TRqQRMCO9vp6u1ykAxAoNYh91 0T1HTuWVRlLv3EObaChrXwlKdsYJ1y+/B54EdD3qalbzlQ8AVyYIJhY5XRsSI5TTlXfQv/pRxOS nmcGmXVhC9XBWYcoaTVSPfNs1S/VsSSFwAqlXHAf2ueXI0MIIzFkacFNNpXJZtsJddIaHjfnOoJ LDh4zq8H8Q1ly/xoXbmJz+ag3bH+78HCwMuAY4rJb2rSg== X-Received: by 127.0.0.2 with SMTP id v8FuYY7687511xDcnVoPg7Xu; Thu, 16 May 2024 08:18:34 -0700 X-Received: from EUR01-HE1-obe.outbound.protection.outlook.com (EUR01-HE1-obe.outbound.protection.outlook.com [40.107.13.54]) by mx.groups.io with SMTP id smtpd.web10.16712.1715872713557726337 for ; Thu, 16 May 2024 08:18:34 -0700 X-Received: from DU6P191CA0016.EURP191.PROD.OUTLOOK.COM (2603:10a6:10:540::10) by PAWPR08MB8885.eurprd08.prod.outlook.com (2603:10a6:102:339::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7544.55; Thu, 16 May 2024 15:18:26 +0000 X-Received: from DB1PEPF00039233.eurprd03.prod.outlook.com (2603:10a6:10:540:cafe::91) by DU6P191CA0016.outlook.office365.com (2603:10a6:10:540::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7587.28 via Frontend Transport; Thu, 16 May 2024 15:18:26 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=arm.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C X-Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DB1PEPF00039233.mail.protection.outlook.com (10.167.8.106) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.20.7544.18 via Frontend Transport; Thu, 16 May 2024 15:18:25 +0000 X-Received: ("Tessian outbound daa456608199:v315"); Thu, 16 May 2024 15:18:25 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 5bb477bbe731a2e1 X-CR-MTA-TID: 64aa7808 X-Received: from 42611b6074e5.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 0521B368-A09C-4940-BC6F-8B855AD56F1E.1; Thu, 16 May 2024 15:18:19 +0000 X-Received: from EUR05-AM6-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 42611b6074e5.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 16 May 2024 15:18:19 +0000 Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by DB8PR08MB5339.eurprd08.prod.outlook.com (2603:10a6:10:114::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7587.28; Thu, 16 May 2024 15:18:17 +0000 X-Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::1e13:dc65:224e:219c]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::1e13:dc65:224e:219c%5]) with mapi id 15.20.7587.028; Thu, 16 May 2024 15:18:17 +0000 Message-ID: Date: Thu, 16 May 2024 16:18:16 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 05/14] Platform/ARM: Create NorFlashDeviceLib library interface for flash specific functions To: Sahil Kaushal , devel@edk2.groups.io Cc: Ard Biesheuvel , Leif Lindholm , sahil References: <20240423055638.1271531-1-Sahil.Kaushal@arm.com> <20240423055638.1271531-6-Sahil.Kaushal@arm.com> From: "Sami Mujawar" In-Reply-To: <20240423055638.1271531-6-Sahil.Kaushal@arm.com> X-ClientProxiedBy: LO4P123CA0086.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:190::19) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: AS8PR08MB6806:EE_|DB8PR08MB5339:EE_|DB1PEPF00039233:EE_|PAWPR08MB8885:EE_ X-MS-Office365-Filtering-Correlation-Id: 233f9561-06ba-448e-43a9-08dc75bb6eca x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0;ARA:13230031|1800799015|376005|366007; X-Microsoft-Antispam-Message-Info-Original: =?us-ascii?Q?RWzNaLG7H9sAzJU/iJnIDP0tl5DLy04GRLe3NRrH0JbEMr4n81M4LkJDI1Dy?= =?us-ascii?Q?DAc5oimW7QXv7V67crEjkUo2Hbyt4gzgdxnyFA4sldzVrml5g59nilWGKcz2?= =?us-ascii?Q?wyUOQmQQVMYmDv2wPMdGkdmu13TE/I3Aq+mceko5POHFV4jLGOa/cHJDcD8e?= =?us-ascii?Q?IKnCmRUbIlrZpjucXHgZjQWLW2xMviGrYoR9sPhDurO7lNPzqVIPV0jpGKTU?= =?us-ascii?Q?J0uvsD8nL12qeRZn9D92XAojKAo4qt5jgltcafk25T+9iMiLuH293gRIUHaY?= =?us-ascii?Q?/QYeS9clBWMqi54Os73ahZYLB9NxQIoQk8R2Uzgyn+uf4qDLF1taDQLxk2lN?= =?us-ascii?Q?671jDbLdcJxi9t5O+bbjSSgEv+LICSRduzJRUmst2EpdNemDCl8rOPb7KoYF?= =?us-ascii?Q?FZkH16nXoOKpGjS6Rxi5x0GK9H/3ihLuNDrg1MVs2gz5TSqH7ozx64kEhKWr?= =?us-ascii?Q?GTt/dqgeDSGT0EXZKwKbP4Tv57QCuPvrO57UDUuBNpILh1rZFkMDKs5Fkh04?= =?us-ascii?Q?8K+gE3aPLuTtbihaf8UoZBe506hVbMZLxALeROIwK5/hf5sN0kipl4U+nypO?= =?us-ascii?Q?Rj3cbUSNcTXJaz5mx6Rxr/z87++Ubq9QTJy6h5T7k55IeRUzI5HKopxWssKa?= =?us-ascii?Q?ct8i2BO7JeE7sRMO4rafpc9zDxpKn/yOWYEIlsPjGCzK7vNPCDNDiOcdc8po?= =?us-ascii?Q?oe0uBvB7zD4ra9Weio8MBpBWsEKvvacUN6OHMM1+TTF9K5EmVg9DnrqfmoWi?= =?us-ascii?Q?sM/cS+9FbLJVv91ZafZE7/b2t+bNUYCSn0Bedb5nODFwTMhzysMx3gkUHcDB?= =?us-ascii?Q?iByG+b1J0Zcrldi0/CWVdQ4TnK6yOx45QH06FpB8O/pHN9xJF2dMY34i1pIn?= =?us-ascii?Q?cBdVbYYUS/8nsHm6mz+Uz0V8okgQjrDIEu0VZkWthtr2VRsb5oLjRsZ9PGi8?= =?us-ascii?Q?pMuE3FqmElaGolS1hLpNyHqFaI5O+6n6MTlP+AALKY26E6k77a6fqwVdDepe?= =?us-ascii?Q?ioHwloM/h7SW7hKFkc9+ku9P5ONHEoW0wm+g4tcJMjA+k6UaF6qLly3jtnb0?= =?us-ascii?Q?V+Xv5DKsNj098iPVaW91ZUqyjDb00iINEeu04Z/3Q5+bksPVmYBdwSuzzLtf?= =?us-ascii?Q?j0W/GjNm8U6krVnjZLKVR5bERbL2SSJZKohB3IQOsSZnKDQUydLKGrdDIiaX?= =?us-ascii?Q?ZtxTFwgE1sLi25CULGC1eLWXB4P6Q2OveoZo3PcEhML9kfI9KFc4FHWDI7tq?= =?us-ascii?Q?h+vJuxAHy+U6G+wypzQfn5546Ysl+yHJJdlf1HzcuQ=3D=3D?= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(1800799015)(376005)(366007);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR08MB5339 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: DB1PEPF00039233.eurprd03.prod.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 7a573f1b-257a-431d-bacf-08dc75bb69af X-Microsoft-Antispam-Message-Info: =?utf-8?B?VVhaNitaZmMrYXNQSUptbkVGVW4zNkRpSGlheTZQU3ZYN0pYdDZaVVVhN3Zh?= =?utf-8?B?UDhDcVBmcEgrTHdsUnJQTHhnU3RrT0dDWFN6di9hTldFSFlDUTk2N0ozY3hL?= =?utf-8?B?K01mQ1Yvcy9PUGRMR3FKUlQrRnV6enF6NDRxb2ZicUhjTUxJcTVJaWQ5bXNW?= =?utf-8?B?TnFaK0NiVVNsb0FGWVhxZmtleGVXMUZaMmI3VkxYQUhOU1BKeW56TklVK3Z4?= =?utf-8?B?VGNtUERDTUVvZ3BYZkxaOVdFYS82T0EwZ0cxOVU5cnZhUnpaN2Y3eGRneDkz?= =?utf-8?B?Nk0zeXY1WE1GNHBjdXFROXlNQVJMNUQrVkJNM0ZVZ2Y5VEZjU2l4SGhLRWRn?= =?utf-8?B?VktWSXkzSXpNMytYUjA0UDRWSWMzWXJlZUVYdDdFWnR2a0dtOHZYS0xqZjdz?= =?utf-8?B?VGJOWEJaMEUwTmJFTHUzUG04RC8rM1ZRVlgxdER4K2RwUmo4ZFVyYkh0VnRO?= =?utf-8?B?Wk1aL0RuNWJmeGNkY1BHdk9KZ1lYREloNnRGYTI1NlMxVjZJMGtwT3RrSWJH?= =?utf-8?B?YW16RGlyc2VMN05pNnBleGJzY3N1WXdqZ3VGNlROdXcyWGdzekVzQ21mVE90?= =?utf-8?B?Z2dWdlU1NkR1VnAwdGU2cm9RVkZaZ3FGQjU5QVd6ZC82cHpadnltcWNPRU1F?= =?utf-8?B?Z2lyN24xTzZZUHg2YVJybFlYM3lTTlJkdElIL0QzTGl3dTE4WEVWZzRFa3Ar?= =?utf-8?B?VUw0UjBLaE50bXdzR0VYR0hvRU5pY2pRdzV1Z0IwOENLUVBOTmNZUXhoUU9m?= =?utf-8?B?d2hoKzhsUUdiQTVFUnU2RlBmUldieGptZTViVXFGZElod2FqaUYvZ3RVVEZh?= =?utf-8?B?M1IzcFZoOXRPV2twRURVdkxmSnh6alhXNHlib1laNmZzVzdibVM5WDMzTU8z?= =?utf-8?B?bGRxRzJSS3RKdURZbmNsTVM0RzhZTEt2R3dMUHFaS0VuZ08yREVqMjFXTVhX?= =?utf-8?B?TTVWMTR4NldxUGI2aXYraDZwN1hGWlJjVTZXSUhZdWVJNVhaeDgvcHZMa1Bs?= =?utf-8?B?eDlDNnBPVk5qK3hVVk5ZMVYrVTBnbmF2N09Cc2MvUjMzV3Q5YndDdXc4V1J2?= =?utf-8?B?OVNzcis1Z0RhdFZWQjdQMGZvVmhIQTY3aEpaSHYyeFZHMGh5MkJyNXJ4YjBV?= =?utf-8?B?dldhUm1hNmdoQXNId01XbVZPUFFQWXpObWRDai8rNUNHTVZPdE1jV3Ztd0FF?= =?utf-8?B?YVFaWkRQWmVDQU5VSFFIcVd5eDRpQjIzbEdZd21mOGtTWTJQTFFtS09Eb1RR?= =?utf-8?B?cUlQSkhnREljRHJ6UElmbVlkU1JXNkk1RFRDa3g4VUtxVVpnb3FPdnBxNVJL?= =?utf-8?B?bGpaeWpCL282VUx3U2k1ZWhaZzlxR1V5L3FMei9Za0JnN1daNFd5VU1ueHZw?= =?utf-8?B?NDNYbTVyRlZtSDVTYkhQWk5nN1hLNzdmSUlqeXQ2bDB6VCsxOE1pM29KaVZH?= =?utf-8?B?WklCTEFVWVB6ZjFMYTlHMGN1ZWtnOGFHQVVSNm1PYXZ5REZjd0V5bkJ5Q1Nr?= =?utf-8?B?THhqS1hoQzJNbU9OZWZ5Qyt4YUdCMkZEcGk3Z0x0U2dEMTdhaVg1TTE4WWxF?= =?utf-8?B?R3VVQjB5dGpveGxMdEVFNzB1SzhkSGwyOFlDRWxienRXYko1N0VWbVlQZm9H?= =?utf-8?B?M3Q0RnY2S0xKWUhpZ2Rwc0VFWDlqbmYxRzJkVzkrVEJ1WUtDOG41VWZURlY2?= =?utf-8?B?ZXJHbTEvdmxjVklOQjNLcXZpOFlzV1BmUVJQbWdLZEl5ZktMK1pXM3UrOVlJ?= =?utf-8?B?R3NmRlNFRFJHOFVIQnVDVit4dzRXcFdMVWRYaFc2dk9QdVZhREszcHdTaWg2?= =?utf-8?B?cHpza2toOVQ1M1NBcDdRcmU2ZWdxeFpwaEtrY2UyVzkzVlRqNEp3QUZ5cmtj?= =?utf-8?Q?+40S8r8Y6WWBD?= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2024 15:18:25.7851 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 233f9561-06ba-448e-43a9-08dc75bb6eca X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: DB1PEPF00039233.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB8885 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 16 May 2024 08:18:34 -0700 Resent-From: sami.mujawar@arm.com Reply-To: devel@edk2.groups.io,sami.mujawar@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: eAbHtCKWQlv5vpylsFdQ77mOx7686176AA= Content-Type: multipart/alternative; boundary="------------MRVhGOox7dBBKrsncZ5AuySM" Content-Language: en-GB X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=W4cBLAyj; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --------------MRVhGOox7dBBKrsncZ5AuySM Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: quoted-printable Hi Sahil, Thank you for this patch. I have some feedback marked inline as [SAMI]. Other than that, is is possible to add documentation header for the functio= ns and data streuctures in this file, please? With that fixed, Reviewed-by: Sami Mujawar Regards, Sami Mujawar On 23/04/2024 06:56 am, Sahil Kaushal wrote: From: sahil NorFlashDeviceLib can be used to provide implementations of different NOR Flash to NorFlashDxe, i.e. NorFlashDxe links with NorFlashDeviceLib and the platforms can specify their respective NorFlashDeviceLib instances. This patch splits NorFlash.h and moves out the function prototypes and macros that are expected by NorFlashDxe to be implemented by any Nor Flash implementation to NorFlashDeviceLib.h file. Signed-off-by: sahil --- Platform/ARM/ARM.dec | 1 + Platform/ARM/Drivers/NorFlashDxe/NorFlash.h | 143 +----------------- Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h | 1 + Platform/ARM/Include/Library/NorFlashDeviceLib.h | 156 ++++++++++++++++++= ++ 4 files changed, 159 insertions(+), 142 deletions(-) diff --git a/Platform/ARM/ARM.dec b/Platform/ARM/ARM.dec index be7e6dc83fde..86d1fcb4878e 100644 --- a/Platform/ARM/ARM.dec +++ b/Platform/ARM/ARM.dec @@ -17,6 +17,7 @@ [LibraryClasses] BdsLib|Include/Library/BdsLib.h + NorFlashDeviceLib|Include/Library/NorFlashDeviceLib.h NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h [Guids] diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h b/Platform/ARM/Dri= vers/NorFlashDxe/NorFlash.h index bd5c6a949cf0..6cb1f64b9875 100644 --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h @@ -20,6 +20,7 @@ #include #include +#include #define NOR_FLASH_ERASE_RETRY 10 @@ -40,7 +41,6 @@ #define CREATE_NOR_ADDRESS(BaseAddr, OffsetAddr) ((BaseAddr) + ((Off= setAddr) << 2)) #define CREATE_DUAL_CMD(Cmd) ( ( Cmd << 16) | ( = Cmd & LOW_16_BITS) ) #define SEND_NOR_COMMAND(BaseAddr, Offset, Cmd) MmioWrite32 (CREATE= _NOR_ADDRESS(BaseAddr,Offset), CREATE_DUAL_CMD(Cmd)) -#define GET_NOR_BLOCK_ADDRESS(BaseAddr, Lba, LbaSize) ( BaseAddr + (UINTN= )((Lba) * LbaSize) ) // Status Register Bits #define P30_SR_BIT_WRITE (BIT7 << 16 | BIT7) @@ -105,145 +105,4 @@ #define P30_CMD_READ_CONFIGURATION_REGISTER_SETUP 0x0060 #define P30_CMD_READ_CONFIGURATION_REGISTER 0x0003 -typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE; - -#pragma pack (1) -typedef struct { - VENDOR_DEVICE_PATH Vendor; - UINT8 Index; - EFI_DEVICE_PATH_PROTOCOL End; -} NOR_FLASH_DEVICE_PATH; -#pragma pack () - -struct _NOR_FLASH_INSTANCE { - UINT32 Signature; - EFI_HANDLE Handle; - - UINTN DeviceBaseAddress; - UINTN RegionBaseAddress; - UINTN Size; - EFI_LBA StartLba; - - EFI_BLOCK_IO_PROTOCOL BlockIoProtocol; - EFI_BLOCK_IO_MEDIA Media; - EFI_DISK_IO_PROTOCOL DiskIoProtocol; - - EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol; - VOID *ShadowBuffer; - - NOR_FLASH_DEVICE_PATH DevicePath; -}; - -EFI_STATUS -NorFlashReadCfiData ( - IN UINTN DeviceBaseAddress, - IN UINTN CFI_Offset, - IN UINT32 NumberOfBytes, - OUT UINT32 *Data - ); [SAMI] Where is this function implemented ? - -EFI_STATUS -NorFlashWriteBuffer ( - IN NOR_FLASH_INSTANCE *Instance, - IN UINTN TargetAddress, - IN UINTN BufferSizeInBytes, - IN UINT32 *Buffer - ); - -// -// NorFlash.c -// -EFI_STATUS -NorFlashWriteSingleBlock ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINTN Offset, - IN OUT UINTN *NumBytes, - IN UINT8 *Buffer - ); - -EFI_STATUS -NorFlashWriteBlocks ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINTN BufferSizeInBytes, - IN VOID *Buffer - ); - -EFI_STATUS -NorFlashReadBlocks ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINTN BufferSizeInBytes, - OUT VOID *Buffer - ); - -EFI_STATUS -NorFlashRead ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINTN Offset, - IN UINTN BufferSizeInBytes, - OUT VOID *Buffer - ); - -EFI_STATUS -NorFlashWrite ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINTN Offset, - IN OUT UINTN *NumBytes, - IN UINT8 *Buffer - ); - -EFI_STATUS -NorFlashReset ( - IN NOR_FLASH_INSTANCE *Instance - ); - -EFI_STATUS -NorFlashEraseSingleBlock ( - IN NOR_FLASH_INSTANCE *Instance, - IN UINTN BlockAddress - ); - -EFI_STATUS -NorFlashUnlockSingleBlockIfNecessary ( - IN NOR_FLASH_INSTANCE *Instance, - IN UINTN BlockAddress - ); [SAMI] Should this be a STATIC function in the Device Library implementatio= ns? Would it make more sense to have NorFlashBlockIsLocked() exposed via th= is library interface instead. Also, would it make sense to integrate the check to see if the block is loc= ked in the NorFlashUnlockSingleBlock() instead and expose it via the librar= y interface? [/SAMI] - -EFI_STATUS -NorFlashWriteSingleWord ( - IN NOR_FLASH_INSTANCE *Instance, - IN UINTN WordAddress, - IN UINT32 WriteData - ); [SAMI] Should this function be made STATIC in the Device Library implementa= tions? - -EFI_STATUS -NorFlashWriteFullBlock ( - IN NOR_FLASH_INSTANCE *Instance, - IN EFI_LBA Lba, - IN UINT32 *DataBuffer, - IN UINT32 BlockSizeInWords - ); - -EFI_STATUS -NorFlashUnlockAndEraseSingleBlock ( - IN NOR_FLASH_INSTANCE *Instance, - IN UINTN BlockAddress - ); - -VOID -EFIAPI -NorFlashLock ( - IN EFI_TPL *OriginalTPL - ); - -VOID -EFIAPI -NorFlashUnlock ( - IN EFI_TPL OriginalTPL - ); - #endif /* __NOR_FLASH_H__ */ diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h b/Platform/A= RM/Drivers/NorFlashDxe/NorFlashCommon.h index c0a3b5861532..7fcb949843e8 100644 --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include diff --git a/Platform/ARM/Include/Library/NorFlashDeviceLib.h b/Platform/AR= M/Include/Library/NorFlashDeviceLib.h new file mode 100644 index 000000000000..e5017130a091 --- /dev/null +++ b/Platform/ARM/Include/Library/NorFlashDeviceLib.h @@ -0,0 +1,156 @@ +/** @file NorFlashDeviceLib.h + + Copyright (c) 2011 - 2024, Arm Limited. All rights reserved.
+ + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#ifndef NOR_FLASH_DEVICE_LIB_H_ +#define NOR_FLASH_DEVICE_LIB_H_ + +#include +#include +#include + +typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE; + +#define GET_NOR_BLOCK_ADDRESS(BaseAddr, Lba, LbaSize) ( BaseAddr + (UINTN= )((Lba) * LbaSize) ) + +#pragma pack (1) +typedef struct { + VENDOR_DEVICE_PATH Vendor; + UINT8 Index; + EFI_DEVICE_PATH_PROTOCOL End; +} NOR_FLASH_DEVICE_PATH; +#pragma pack () + +struct _NOR_FLASH_INSTANCE { + UINT32 Signature; + EFI_HANDLE Handle; + + UINTN DeviceBaseAddress; + UINTN RegionBaseAddress; + UINTN Size; + EFI_LBA StartLba; + + EFI_BLOCK_IO_PROTOCOL BlockIoProtocol; + EFI_BLOCK_IO_MEDIA Media; + EFI_DISK_IO_PROTOCOL DiskIoProtocol; + + EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol; + VOID *ShadowBuffer; + + NOR_FLASH_DEVICE_PATH DevicePath; +}; + +EFI_STATUS +NorFlashReadCfiData ( + IN UINTN DeviceBaseAddress, + IN UINTN CFI_Offset, + IN UINT32 NumberOfBytes, + OUT UINT32 *Data + ); + +EFI_STATUS +NorFlashWriteBuffer ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN TargetAddress, + IN UINTN BufferSizeInBytes, + IN UINT32 *Buffer + ); + +EFI_STATUS +NorFlashWriteFullBlock ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINT32 *DataBuffer, + IN UINT32 BlockSizeInWords + ); + +EFI_STATUS +NorFlashUnlockAndEraseSingleBlock ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN BlockAddress + ); + +EFI_STATUS +NorFlashWriteSingleBlock ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN Offset, + IN OUT UINTN *NumBytes, + IN UINT8 *Buffer + ); + +EFI_STATUS +NorFlashWriteBlocks ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN BufferSizeInBytes, + IN VOID *Buffer + ); + +EFI_STATUS +NorFlashReadBlocks ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN BufferSizeInBytes, + OUT VOID *Buffer + ); + +EFI_STATUS +NorFlashRead ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN Offset, + IN UINTN BufferSizeInBytes, + OUT VOID *Buffer + ); + +EFI_STATUS +NorFlashWrite ( + IN NOR_FLASH_INSTANCE *Instance, + IN EFI_LBA Lba, + IN UINTN Offset, + IN OUT UINTN *NumBytes, + IN UINT8 *Buffer + ); + +EFI_STATUS +NorFlashReset ( + IN NOR_FLASH_INSTANCE *Instance + ); + +EFI_STATUS +NorFlashEraseSingleBlock ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN BlockAddress + ); + +EFI_STATUS +NorFlashUnlockSingleBlockIfNecessary ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN BlockAddress + ); + +EFI_STATUS +NorFlashWriteSingleWord ( + IN NOR_FLASH_INSTANCE *Instance, + IN UINTN WordAddress, + IN UINT32 WriteData + ); + +VOID +EFIAPI +NorFlashLock ( + IN EFI_TPL *OriginalTPL + ); + +VOID +EFIAPI +NorFlashUnlock ( + IN EFI_TPL OriginalTPL + ); + +#endif /* NOR_FLASH_DEVICE_LIB_H_ */ IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you. -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118961): https://edk2.groups.io/g/devel/message/118961 Mute This Topic: https://groups.io/mt/105690940/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --------------MRVhGOox7dBBKrsncZ5AuySM Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hi Sahil,

Thank you for this patch.

I have some feedback marked inline as [SAMI].

Other than that, is is possible to add documentation header for the func= tions and data streuctures in this file, please?

With that fixed,

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/04/2024 06:56 am, Sahil Kaushal wrote:= 
From: sahil <sahil@arm.com>

NorFlashDeviceLib can be used to provide implementations of different
NOR Flash to NorFlashDxe, i.e. NorFlashDxe links with NorFlashDeviceLib
and the platforms can specify their respective NorFlashDeviceLib
instances.

This patch splits NorFlash.h and moves out the function prototypes and
macros that are expected by NorFlashDxe to be implemented by any
Nor Flash implementation to NorFlashDeviceLib.h file.

Signed-off-by: sahil <sahil@arm.com>
---
 Platform/ARM/ARM.dec                              |   1 +
 Platform/ARM/Drivers/NorFlashDxe/NorFlash.h       | 143 +-----------------
 Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h |   1 +
 Platform/ARM/Include/Library/NorFlashDeviceLib.h  | 156 ++++++++++++++++++=
++
 4 files changed, 159 insertions(+), 142 deletions(-)

diff --git a/Platform/ARM/ARM.dec b/Platform/ARM/ARM.dec
index be7e6dc83fde..86d1fcb4878e 100644
--- a/Platform/ARM/ARM.dec
+++ b/Platform/ARM/ARM.dec
@@ -17,6 +17,7 @@
=20

 [LibraryClasses]

   BdsLib|Include/Library/BdsLib.h

+  NorFlashDeviceLib|Include/Library/NorFlashDeviceLib.h

   NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h

=20

 [Guids]

diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h b/Platform/ARM/Dri=
vers/NorFlashDxe/NorFlash.h
index bd5c6a949cf0..6cb1f64b9875 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
@@ -20,6 +20,7 @@
=20

 #include <Library/DebugLib.h>

 #include <Library/IoLib.h>

+#include <Library/NorFlashDeviceLib.h>

=20

 #define NOR_FLASH_ERASE_RETRY  10

=20

@@ -40,7 +41,6 @@
 #define CREATE_NOR_ADDRESS(BaseAddr, OffsetAddr)       ((BaseAddr) + ((Off=
setAddr) << 2))

 #define CREATE_DUAL_CMD(Cmd)                           ( ( Cmd << 16=
) | ( Cmd & LOW_16_BITS) )

 #define SEND_NOR_COMMAND(BaseAddr, Offset, Cmd)        MmioWrite32 (CREATE=
_NOR_ADDRESS(BaseAddr,Offset), CREATE_DUAL_CMD(Cmd))

-#define GET_NOR_BLOCK_ADDRESS(BaseAddr, Lba, LbaSize)  ( BaseAddr + (UINTN=
)((Lba) * LbaSize) )

=20

 // Status Register Bits

 #define P30_SR_BIT_WRITE            (BIT7 << 16 | BIT7)

@@ -105,145 +105,4 @@
 #define P30_CMD_READ_CONFIGURATION_REGISTER_SETUP  0x0060

 #define P30_CMD_READ_CONFIGURATION_REGISTER        0x0003

=20

-typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE;

-

-#pragma pack (1)

-typedef struct {

-  VENDOR_DEVICE_PATH          Vendor;

-  UINT8                       Index;

-  EFI_DEVICE_PATH_PROTOCOL    End;

-} NOR_FLASH_DEVICE_PATH;

-#pragma pack ()

-

-struct _NOR_FLASH_INSTANCE {

-  UINT32                                 Signature;

-  EFI_HANDLE                             Handle;

-

-  UINTN                                  DeviceBaseAddress;

-  UINTN                                  RegionBaseAddress;

-  UINTN                                  Size;

-  EFI_LBA                                StartLba;

-

-  EFI_BLOCK_IO_PROTOCOL                  BlockIoProtocol;

-  EFI_BLOCK_IO_MEDIA                     Media;

-  EFI_DISK_IO_PROTOCOL                   DiskIoProtocol;

-

-  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL    FvbProtocol;

-  VOID                                   *ShadowBuffer;

-

-  NOR_FLASH_DEVICE_PATH                  DevicePath;

-};

-

-EFI_STATUS

-NorFlashReadCfiData (

-  IN  UINTN   DeviceBaseAddress,

-  IN  UINTN   CFI_Offset,

-  IN  UINT32  NumberOfBytes,

-  OUT UINT32  *Data

-  );
[SAMI] Where is this function implemented ?

-

-EFI_STATUS

-NorFlashWriteBuffer (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN UINTN               TargetAddress,

-  IN UINTN               BufferSizeInBytes,

-  IN UINT32              *Buffer

-  );

-

-//

-// NorFlash.c

-//

-EFI_STATUS

-NorFlashWriteSingleBlock (

-  IN        NOR_FLASH_INSTANCE  *Instance,

-  IN        EFI_LBA             Lba,

-  IN        UINTN               Offset,

-  IN OUT    UINTN               *NumBytes,

-  IN        UINT8               *Buffer

-  );

-

-EFI_STATUS

-NorFlashWriteBlocks (

-  IN  NOR_FLASH_INSTANCE  *Instance,

-  IN  EFI_LBA             Lba,

-  IN  UINTN               BufferSizeInBytes,

-  IN  VOID                *Buffer

-  );

-

-EFI_STATUS

-NorFlashReadBlocks (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN EFI_LBA             Lba,

-  IN UINTN               BufferSizeInBytes,

-  OUT VOID               *Buffer

-  );

-

-EFI_STATUS

-NorFlashRead (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN EFI_LBA             Lba,

-  IN UINTN               Offset,

-  IN UINTN               BufferSizeInBytes,

-  OUT VOID               *Buffer

-  );

-

-EFI_STATUS

-NorFlashWrite (

-  IN        NOR_FLASH_INSTANCE  *Instance,

-  IN        EFI_LBA             Lba,

-  IN        UINTN               Offset,

-  IN OUT    UINTN               *NumBytes,

-  IN        UINT8               *Buffer

-  );

-

-EFI_STATUS

-NorFlashReset (

-  IN  NOR_FLASH_INSTANCE  *Instance

-  );

-

-EFI_STATUS

-NorFlashEraseSingleBlock (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN UINTN               BlockAddress

-  );

-

-EFI_STATUS

-NorFlashUnlockSingleBlockIfNecessary (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN UINTN               BlockAddress

-  );

[SAMI] Should this be a STATIC function in the Device Library implementa= tions? Would it make more sense to have NorFlashBlockIsLocke= d() exposed via this library interface instead.

Also, would it ma= ke sense to integrate the check to see if the block is locked in the NorFlashUnloc= kSingleBlock() instead and expose it via the library interface?

[/SAMI] 

-

-EFI_STATUS

-NorFlashWriteSingleWord (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN UINTN               WordAddress,

-  IN UINT32              WriteData

-  );
[SAMI] Should this function be made STATIC in the Device Library implementa= tions? 

-

-EFI_STATUS

-NorFlashWriteFullBlock (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN EFI_LBA             Lba,

-  IN UINT32              *DataBuffer,

-  IN UINT32              BlockSizeInWords

-  );

-

-EFI_STATUS

-NorFlashUnlockAndEraseSingleBlock (

-  IN NOR_FLASH_INSTANCE  *Instance,

-  IN UINTN               BlockAddress

-  );

-

-VOID

-EFIAPI

-NorFlashLock (

-  IN EFI_TPL  *OriginalTPL

-  );

-

-VOID

-EFIAPI

-NorFlashUnlock (

-  IN EFI_TPL OriginalTPL

-  );

-

 #endif /* __NOR_FLASH_H__ */

diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h b/Platform/A=
RM/Drivers/NorFlashDxe/NorFlashCommon.h
index c0a3b5861532..7fcb949843e8 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
@@ -19,6 +19,7 @@
 #include <Protocol/FirmwareVolumeBlock.h>

=20

 #include <Library/DebugLib.h>

+#include <Library/NorFlashDeviceLib.h>

 #include <Library/NorFlashPlatformLib.h>

 #include <Library/UefiLib.h>

 #include <Library/UefiRuntimeLib.h>

diff --git a/Platform/ARM/Include/Library/NorFlashDeviceLib.h b/Platform/AR=
M/Include/Library/NorFlashDeviceLib.h
new file mode 100644
index 000000000000..e5017130a091
--- /dev/null
+++ b/Platform/ARM/Include/Library/NorFlashDeviceLib.h
@@ -0,0 +1,156 @@
+/** @file  NorFlashDeviceLib.h

+

+  Copyright (c) 2011 - 2024, Arm Limited. All rights reserved.<BR>

+

+  SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#ifndef NOR_FLASH_DEVICE_LIB_H_

+#define NOR_FLASH_DEVICE_LIB_H_

+

+#include <Protocol/BlockIo.h>

+#include <Protocol/DiskIo.h>

+#include <Protocol/FirmwareVolumeBlock.h>

+

+typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE;

+

+#define GET_NOR_BLOCK_ADDRESS(BaseAddr, Lba, LbaSize)  ( BaseAddr + (UINTN=
)((Lba) * LbaSize) )

+

+#pragma pack (1)

+typedef struct {

+  VENDOR_DEVICE_PATH          Vendor;

+  UINT8                       Index;

+  EFI_DEVICE_PATH_PROTOCOL    End;

+} NOR_FLASH_DEVICE_PATH;

+#pragma pack ()

+

+struct _NOR_FLASH_INSTANCE {

+  UINT32                                 Signature;

+  EFI_HANDLE                             Handle;

+

+  UINTN                                  DeviceBaseAddress;

+  UINTN                                  RegionBaseAddress;

+  UINTN                                  Size;

+  EFI_LBA                                StartLba;

+

+  EFI_BLOCK_IO_PROTOCOL                  BlockIoProtocol;

+  EFI_BLOCK_IO_MEDIA                     Media;

+  EFI_DISK_IO_PROTOCOL                   DiskIoProtocol;

+

+  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL    FvbProtocol;

+  VOID                                   *ShadowBuffer;

+

+  NOR_FLASH_DEVICE_PATH                  DevicePath;

+};

+

+EFI_STATUS

+NorFlashReadCfiData (

+  IN  UINTN   DeviceBaseAddress,

+  IN  UINTN   CFI_Offset,

+  IN  UINT32  NumberOfBytes,

+  OUT UINT32  *Data

+  );

+

+EFI_STATUS

+NorFlashWriteBuffer (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN UINTN               TargetAddress,

+  IN UINTN               BufferSizeInBytes,

+  IN UINT32              *Buffer

+  );

+

+EFI_STATUS

+NorFlashWriteFullBlock (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN EFI_LBA             Lba,

+  IN UINT32              *DataBuffer,

+  IN UINT32              BlockSizeInWords

+  );

+

+EFI_STATUS

+NorFlashUnlockAndEraseSingleBlock (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN UINTN               BlockAddress

+  );

+

+EFI_STATUS

+NorFlashWriteSingleBlock (

+  IN        NOR_FLASH_INSTANCE  *Instance,

+  IN        EFI_LBA             Lba,

+  IN        UINTN               Offset,

+  IN OUT    UINTN               *NumBytes,

+  IN        UINT8               *Buffer

+  );

+

+EFI_STATUS

+NorFlashWriteBlocks (

+  IN  NOR_FLASH_INSTANCE  *Instance,

+  IN  EFI_LBA             Lba,

+  IN  UINTN               BufferSizeInBytes,

+  IN  VOID                *Buffer

+  );

+

+EFI_STATUS

+NorFlashReadBlocks (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN EFI_LBA             Lba,

+  IN UINTN               BufferSizeInBytes,

+  OUT VOID               *Buffer

+  );

+

+EFI_STATUS

+NorFlashRead (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN EFI_LBA             Lba,

+  IN UINTN               Offset,

+  IN UINTN               BufferSizeInBytes,

+  OUT VOID               *Buffer

+  );

+

+EFI_STATUS

+NorFlashWrite (

+  IN        NOR_FLASH_INSTANCE  *Instance,

+  IN        EFI_LBA             Lba,

+  IN        UINTN               Offset,

+  IN OUT    UINTN               *NumBytes,

+  IN        UINT8               *Buffer

+  );

+

+EFI_STATUS

+NorFlashReset (

+  IN  NOR_FLASH_INSTANCE  *Instance

+  );

+

+EFI_STATUS

+NorFlashEraseSingleBlock (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN UINTN               BlockAddress

+  );

+

+EFI_STATUS

+NorFlashUnlockSingleBlockIfNecessary (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN UINTN               BlockAddress

+  );

+

+EFI_STATUS

+NorFlashWriteSingleWord (

+  IN NOR_FLASH_INSTANCE  *Instance,

+  IN UINTN               WordAddress,

+  IN UINT32              WriteData

+  );

+

+VOID

+EFIAPI

+NorFlashLock (

+  IN EFI_TPL  *OriginalTPL

+  );

+

+VOID

+EFIAPI

+NorFlashUnlock (

+  IN EFI_TPL  OriginalTPL

+  );

+

+#endif /* NOR_FLASH_DEVICE_LIB_H_ */
IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#118961) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------MRVhGOox7dBBKrsncZ5AuySM--