Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 00/14] Split NorFlashDxe driver and add CadenceQspiNorFlashDeviceLib library

["PierreGondois" <pierre.gondois@arm.com>]

Hello Sahil,

I had some comments for:
- [PATCH V2 12/14] Platform/ARM: Add CadenceQspiNorFlashDeviceLib for NorFlashDxe
for all the other patches:
   Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Also, unless I missed something, shouldn't your mail address/signed-off tag be:
   'Sahil <sahil@arm.com>' -> 'Sahil Kaushal <sahil.kaushal@arm.com>'
?

Regards,
Pierre

On 4/23/24 07:56, Sahil Kaushal via groups.io wrote:
> From: sahil <sahil@arm.com>
> 
> This patch series adds the following changes:
> 
> 1. Splits the NorFlashDxe driver to introduce a NorFlashDeviceLib that
> implements the specifics for the respective flash. This will allow us
> to plug different libraries implementing functionality of various NOR
> Flash. The flash specific code in NorFlashDxe has been moved to
> P30NorFlashDeviceLib library.
> 
> 2. Adds support for CadenceQspiNorFlashDeviceLib which is used by N1Sdp
> platform along with NorFlashDxe driver. N1Sdp uses an emulated variable
> storage on DDR memory for the variable storage. But this emulated
> variable storage is a volatile memory and so the values of variables
> can't persist on next reboot or in power cycle. In N1Sdp platform, the
> SoC is connected to IOFPGA which has a Cadence Quad SPI (QSPI)
> controller. This QSPI controller manages the flash chip device via QSPI
> bus. With these changes we use this NOR flash device for persistent
> variable storage.
> 
> v2:
>    - Fixed code review comments
>    - Split the NorFlashDxe driver and moved flash specific code to
>      P30NorFlashDeviceLib
>    - Added NOR flash Dxe Driver for N1Sdp as a library instead of a
>      driver
> 
> Links to v1:
> https://edk2.groups.io/g/devel/topic/102625035
> https://edk2.groups.io/g/devel/topic/102625033
> https://edk2.groups.io/g/devel/topic/102625034
> https://edk2.groups.io/g/devel/topic/102625036
> https://edk2.groups.io/g/devel/topic/102625037
> https://edk2.groups.io/g/devel/topic/102625038
> 
> Link to branch with the patches in this series -
> https://github.com/sah01Kaushal/edk2-platforms/tree/n1sdp_persistent_storage_v2
> 
> sahil (14):
>    Platform/ARM/NorFlashDxe: Move DiskIo related functions out of
>      NorFlash.c
>    Platform/ARM/NorFlashDxe: Move NorFlashVirtualNotifyEvent
>    Platform/ARM/NorFlashDxe: Add NorFlashCommon.h header file
>    Platform/ARM/NorFlashDxe: Move flash specific functions to NorFlash.c
>    Platform/ARM: Create NorFlashDeviceLib library interface for flash
>      specific functions
>    Platform/ARM: Add P30NorFlashDeviceLib Library
>    Platform/ARM/NorFlashDxe: Switch from NorFlash.c to NorFlashDeviceLib
>    Platform/ARM: Add HostRegisterBaseAddress variable
>    Platform/ARM: Add optional provision to fetch and print NOR Flash info
>    Silicon/ARM/NeoverseN1Soc: Enable SCP QSPI flash region
>    Silicon/ARM/NeoverseN1Soc: NOR flash library for N1Sdp
>    Platform/ARM: Add CadenceQspiNorFlashDeviceLib for NorFlashDxe
>    Platform/ARM/N1Sdp: Persistent storage for N1Sdp
>    Platform/ARM/N1Sdp: Enable FaultTolerantWrite Dxe driver for N1Sdp
> 
>   Platform/ARM/ARM.dec                                                                                 |    4 +
>   Platform/ARM/SgiPkg/SgiPlatform.dsc.inc                                                              |    3 +
>   Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc                                                            |    3 +
>   Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc                                                         |    3 +
>   Platform/ARM/JunoPkg/ArmJuno.dsc                                                                     |    3 +
>   Platform/ARM/N1Sdp/N1SdpPlatform.dsc                                                                 |   24 +-
>   Platform/ARM/VExpressPkg/PlatformStandaloneMm.dsc                                                    |    3 +
>   Platform/ARM/N1Sdp/N1SdpPlatform.fdf                                                                 |    3 +
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.inf                                                     |    8 +-
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf                                            |    8 +-
>   Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.inf                   |   32 +
>   Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.inf                                   |   35 +
>   Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf                                        |   34 +
>   Platform/ARM/Drivers/NorFlashDxe/NorFlash.h                                                          |  422 --------
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h                                                    |  209 ++++
>   Platform/ARM/Include/Library/NorFlashDeviceLib.h                                                     |  163 ++++
>   Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.h                     |   44 +
>   Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.h                                     |   98 ++
>   Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h                                                    |    7 +
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c                                                |  131 ++-
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c                                                       |  292 +++---
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashFvb.c                                                       |    2 +-
>   Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c                                              |  184 ++--
>   Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.c                     | 1011 ++++++++++++++++++++
>   Platform/ARM/{Drivers/NorFlashDxe/NorFlash.c => Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.c} |  330 +++----
>   Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c                                          |   65 ++
>   Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c                                       |    8 +-
>   27 files changed, 2225 insertions(+), 904 deletions(-)
>   create mode 100644 Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.inf
>   create mode 100644 Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.inf
>   create mode 100644 Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.inf
>   delete mode 100644 Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
>   create mode 100644 Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
>   create mode 100644 Platform/ARM/Include/Library/NorFlashDeviceLib.h
>   create mode 100644 Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.h
>   create mode 100644 Platform/ARM/Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.h
>   create mode 100644 Platform/ARM/Library/CadenceQspiNorFlashDeviceLib/CadenceQspiNorFlashDeviceLib.c
>   rename Platform/ARM/{Drivers/NorFlashDxe/NorFlash.c => Library/P30NorFlashDeviceLib/P30NorFlashDeviceLib.c} (77%)
>   create mode 100644 Silicon/ARM/NeoverseN1Soc/Library/NorFlashLib/NorFlashLib.c
> 


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