Hi Sahil,
Thank you for this patch.
I have some suggestions marked inline below, otherwise this patch looks good to me.
With that fixed,
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
From: sahil <sahil@arm.com> Add NOR flash library, this library provides APIs for getting the list of NOR flash devices on the platform.
[SAMI] I think the information in the commit message of patch 10/14 would be more useful here.
Not mandatory, but it may be useful to have an ASCII diagram to explain the flash partitioning.
[/SAMI]
Signed-off-by: sahil <sahil@arm.com> --- Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf | 34 ++++++++++ Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c | 65 ++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf new file mode 100644 index 000000000000..fad3bca79d3a --- /dev/null +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf @@ -0,0 +1,34 @@ +## @file +# NOR flash lib for ARM Neoverse N1 platform. +# +# Copyright (c) 2024, ARM Limited. All rights reserved.<BR> +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +## + +[Defines] + INF_VERSION = 0x0001001B + BASE_NAME = NorFlashNeoverseN1SocLib + FILE_GUID = 7006fcf1-a585-4272-92e3-b286b1dff5bb + MODULE_TYPE = DXE_DRIVER + VERSION_STRING = 1.0 + LIBRARY_CLASS = NorFlashPlatformLib + +[Sources.common] + NorFlashLib.c + +[Packages] + MdeModulePkg/MdeModulePkg.dec + MdePkg/MdePkg.dec + Platform/ARM/ARM.dec + Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec + +[LibraryClasses] + BaseLib + +[FixedPcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize diff --git a/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c new file mode 100644 index 000000000000..a48db9c74548 --- /dev/null +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c @@ -0,0 +1,65 @@ +/** @file +* NOR flash lib for ARM Neoverse N1 platform +* +* Copyright (c) 2024, ARM Limited. All rights reserved.<BR> +* +* SPDX-License-Identifier: BSD-2-Clause-Patent +* +**/ + +#include <Library/NorFlashPlatformLib.h> +#include <NeoverseN1Soc.h> +#include <PiDxe.h> + +#define FW_ENV_REGION_BASE FixedPcdGet32 (PcdFlashNvStorageVariableBase) +#define FW_ENV_REGION_SIZE (FixedPcdGet32 (PcdFlashNvStorageVariableSize) + \ + FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) + \ + FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize))
[SAMI] Would it be an issue if someone were to increase the storage variable sizes above?
How can you prevent someone overwriting the flash region used by the SCP?
Would it make sense to add a check in NorFlashPlatformInitialization() ?
[/SAMI]
+ +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = { + { + /// Environment variable region + NEOVERSEN1SOC_SCP_QSPI_AHB_BASE, ///< device base + FW_ENV_REGION_BASE, ///< region base + FW_ENV_REGION_SIZE, ///< region size + SIZE_4KB, ///< block size + }, +}; + +/** + Dummy implementation of NorFlashPlatformInitialization to + comply with NorFlashPlatformLib structure. + + @retval EFI_SUCCESS Success. +**/ +EFI_STATUS +NorFlashPlatformInitialization ( + VOID + ) +{ + return EFI_SUCCESS; +} + +/** + Get NOR flash region info + + @param[out] NorFlashDevices NOR flash regions info. + @param[out] Count number of flash instance. + + @retval EFI_SUCCESS Success. + @retval EFI_INVALID_PARAMETER The parameters specified are not valid. +**/ +EFI_STATUS +NorFlashPlatformGetDevices ( + OUT NOR_FLASH_DESCRIPTION **NorFlashDevices, + OUT UINT32 *Count + ) +{ + if ((NorFlashDevices == NULL) || (Count == NULL)) { + return EFI_INVALID_PARAMETER; + } + + *NorFlashDevices = mNorFlashDevices; + *Count = ARRAY_SIZE (mNorFlashDevices); + return EFI_SUCCESS; +}