Re: [edk2-devel] [edk2-platforms][PATCH V1 2/5] Silicon/ARM/NeoverseN1Soc: NOR flash library for N1Sdp

["Sami Mujawar" <sami.mujawar@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 5104 bytes --]

Hi Sahil,

Please find my feedback marked inline as [SAMI].

With those fixed,

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

Regards,

Sami Mujawar

On 16/11/2023 11:45 am, sahil wrote:
> Add NOR flash library, this library provides APIs for getting the list
> of NOR flash devices on the platform.
>
> Signed-off-by: sahil<sahil@arm.com>
> ---
>   Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf | 36 ++++++++++++++
>   Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c   | 52 ++++++++++++++++++++
>   2 files changed, 88 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..14f81125c4e1
> --- /dev/null
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf
> @@ -0,0 +1,36 @@
> +## @file
>
> +#  NOR flash lib for ARM Neoverse N1 platform.
>
> +#
>
> +#  Copyright (c) 2023, 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

[SAMI] Since this library class is NorFlashPlatformLib, you need to 
implement NorFlashPlatformInitialization().

A platform may not need to do anything in 
NorFlashPlatformInitialization() and just return success.

[/SAMI]

>
> +
>
> +[Sources.common]
>
> +  NorFlashLib.c
>
> +
>
> +[Packages]
>
> +  MdeModulePkg/MdeModulePkg.dec
>
> +  MdePkg/MdePkg.dec
>
> +  Platform/ARM/ARM.dec
>
> +  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
>
> +
>
> +[LibraryClasses]
>
> +  BaseLib
>
> +  DebugLib
>
> +  IoLib
>
> +
>
> +[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..a508d7d77373
> --- /dev/null
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c
> @@ -0,0 +1,52 @@
> +/** @file
>
> +*  NOR flash lib for ARM Neoverse N1 platform
>
> +*
>
> +*  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
>
> +*
>
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +*
>
> +**/
>
> +
>
> +#include <Library/DebugLib.h>
>
> +#include <Library/IoLib.h>
>
> +#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] Please limit line length to 80 characters. Also run uncrustify.
>
> +
>
> +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
>
> +  },
>
> +};
>
> +
>
> +/**
>
> +  Get NOR flash region info
>
> +
>
> +  @param[out]    NorFlashDevices    NOR flash regions info.
>
> +  @param[out]    Count              number of flash instance.
>
> +
>
> +  @retval        EFI_SUCCESS        Success.
[SAMI] EFI_INVALID_PARAMETER ?
>
> +**/
>
> +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;
>
> +}
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112642): https://edk2.groups.io/g/devel/message/112642
Mute This Topic: https://groups.io/mt/102625034/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 6549 bytes --]