Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 04/14] Platform/ARM/NorFlashDxe: Move flash specific functions to NorFlash.c
Hi Sahil,
Thank you for this patch.
These changes look good to me.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 23/04/2024 06:56 am, Sahil Kaushal wrote:
From: sahil
Refactoring done in this patch has two major parts:
1. Moving out NorFlashUnlockAndEraseSingleBlock and
NorFlashWriteFullBlock functions from NorFlashDxe.c and
NorFlashStandaloneMm.c to NorFlash.c files.
2. At the same time, we are adding NorFlashLock and NorFlashUnlock
functions which will take care of TPL related operations needed by
functions mentioned in point 1. These functions are implemented
in NorFlashDxe.c but are just dummy placeholder functions in
NorFlashStandaloneMm.c file.
Signed-off-by: sahil
---
Platform/ARM/Drivers/NorFlashDxe/NorFlash.h | 26 +++
Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h | 14 --
Platform/ARM/Drivers/NorFlashDxe/NorFlash.c | 136 +-
Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c | 193
Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 151 +++
5 files changed, 225 insertions(+), 295 deletions(-)
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
index e0ebb1e2fd35..bd5c6a949cf0 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
@@ -220,4 +220,30 @@ NorFlashWriteSingleWord (
IN UINT32 WriteData
);
+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/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
index e329e0727617..c0a3b5861532 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
@@ -31,20 +31,6 @@
//
// NorFlashDxe.c
//
-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
NorFlashCreateInstance (
IN UINTNNorFlashDeviceBase,
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
index 4e5a97c83c7b..15000a692b02 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
@@ -10,7 +10,6 @@
#include
#include "NorFlash.h"
-#include "NorFlashCommon.h"
//
// Global variable declarations
@@ -817,3 +816,138 @@ NorFlashReset (
SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
return EFI_SUCCESS;
}
+
+/**
+ * This function unlock and erase an entire NOR Flash block.
+**/
+EFI_STATUS
+NorFlashUnlockAndEraseSingleBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN UINTN BlockAddress
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ EFI_TPL OriginalTPL;
+
+ NorFlashLock (&OriginalTPL);
+
+ Index = 0;
+ // The block erase might fail a first time (SW bug ?). Retry it ...
+ do {
+// Unlock the block if we have to
+Status = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = NorFlashEraseSingleBlock (Instance, BlockAddress);
+Index++;
+ } while ((Index < NOR_FLASH_ERASE_RETRY) && (Status == EFI_WRITE_PROTECTED));
+
+ if (Index == NOR_FLASH_ERASE_RETRY) {
+DEBUG ((DEBUG_ERROR, "EraseSingleBlock(BlockAddress=0x%08x: Block Locked Error
(try to erase %d times)\n", BlockAddress, Index));
+ }
+
+ NorFlashUnlock (OriginalTPL);
+
+ return Status;
+}
+
+EFI_STATUS
+NorFlashWriteFullBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN EFI_LBA Lba,
+ IN UINT32 *DataBuffer,
+ IN UINT32 BlockSizeInWords
+ )
+{
+ EFI_STATUS Status;
+ UINTN WordAddress;
+ UINT32 WordIndex;
+ UINTN BufferIndex;
+ UINTN BlockAddress;
+ UINTN BuffersInBlock;
+ UINTN RemainingWords;
+ EFI_TPL OriginalTPL;
+ UINTN Cnt;
+
+ Status = EFI_SUCCESS;
+
+ // Get the physical address of the block
+ BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
BlockSizeInWords * 4);
+
+ // Start writing from the first address at the start of the
Re: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 04/14] Platform/ARM/NorFlashDxe: Move flash specific functions to NorFlash.c
Reviewed-by: levi.yun
From: devel@edk2.groups.io on behalf of Sahil Kaushal
via groups.io
Sent: 23 April 2024 06:56
To: devel@edk2.groups.io
Cc: Ard Biesheuvel; Leif Lindholm ; Sami Mujawar; Sahil Kaushal
Subject: [edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 04/14]
Platform/ARM/NorFlashDxe: Move flash specific functions to NorFlash.c
From: sahil
Refactoring done in this patch has two major parts:
1. Moving out NorFlashUnlockAndEraseSingleBlock and
NorFlashWriteFullBlock functions from NorFlashDxe.c and
NorFlashStandaloneMm.c to NorFlash.c files.
2. At the same time, we are adding NorFlashLock and NorFlashUnlock
functions which will take care of TPL related operations needed by
functions mentioned in point 1. These functions are implemented
in NorFlashDxe.c but are just dummy placeholder functions in
NorFlashStandaloneMm.c file.
Signed-off-by: sahil
---
Platform/ARM/Drivers/NorFlashDxe/NorFlash.h | 26 +++
Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h | 14 --
Platform/ARM/Drivers/NorFlashDxe/NorFlash.c | 136 +-
Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c | 193
Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 151 +++
5 files changed, 225 insertions(+), 295 deletions(-)
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
index e0ebb1e2fd35..bd5c6a949cf0 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
@@ -220,4 +220,30 @@ NorFlashWriteSingleWord (
IN UINT32 WriteData
);
+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/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
index e329e0727617..c0a3b5861532 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
@@ -31,20 +31,6 @@
//
// NorFlashDxe.c
//
-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
NorFlashCreateInstance (
IN UINTNNorFlashDeviceBase,
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
index 4e5a97c83c7b..15000a692b02 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
@@ -10,7 +10,6 @@
#include
#include "NorFlash.h"
-#include "NorFlashCommon.h"
//
// Global variable declarations
@@ -817,3 +816,138 @@ NorFlashReset (
SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
return EFI_SUCCESS;
}
+
+/**
+ * This function unlock and erase an entire NOR Flash block.
+**/
+EFI_STATUS
+NorFlashUnlockAndEraseSingleBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN UINTN BlockAddress
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ EFI_TPL OriginalTPL;
+
+ NorFlashLock (&OriginalTPL);
+
+ Index = 0;
+ // The block erase might fail a first time (SW bug ?). Retry it ...
+ do {
+// Unlock the block if we have to
+Status = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = NorFlashEraseSingleBlock (Instance, BlockAddress);
+Index++;
+ } while ((Index < NOR_FLASH_ERASE_RETRY) && (Status == EFI_WRITE_PROTECTED));
+
+ if (Index == NOR_FLASH_ERASE_RETRY) {
+DEBUG ((DEBUG_ERROR, "EraseSingleBlock(BlockAddress=0x%08x: Block Locked
Error (try to erase %d times)\n", BlockAddress, Index));
+ }
+
+ NorFlashUnlock (OriginalTPL);
+
+ return Status;
+}
+
+EFI_STATUS
+NorFlashWriteFullBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN EFI_LBA Lba,
+ IN UINT32 *DataBuffer,
+ IN UINT32 BlockSizeInWords
+ )
+{
+ EFI_STATUS Status;
+ UINTN WordAddress;
+ UINT32 WordIndex;
+ UINTN BufferIndex;
+ UINTN BlockAddress;
+ UINTN BuffersInBlock;
+ UINTN RemainingWords;
+ EFI_TPL OriginalTPL;
+ UINTN Cnt
[edk2-devel] [PATCH RESEND edk2-platforms][PATCH V2 04/14] Platform/ARM/NorFlashDxe: Move flash specific functions to NorFlash.c
From: sahil
Refactoring done in this patch has two major parts:
1. Moving out NorFlashUnlockAndEraseSingleBlock and
NorFlashWriteFullBlock functions from NorFlashDxe.c and
NorFlashStandaloneMm.c to NorFlash.c files.
2. At the same time, we are adding NorFlashLock and NorFlashUnlock
functions which will take care of TPL related operations needed by
functions mentioned in point 1. These functions are implemented
in NorFlashDxe.c but are just dummy placeholder functions in
NorFlashStandaloneMm.c file.
Signed-off-by: sahil
---
Platform/ARM/Drivers/NorFlashDxe/NorFlash.h | 26 +++
Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h | 14 --
Platform/ARM/Drivers/NorFlashDxe/NorFlash.c | 136 +-
Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c | 193
Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 151 +++
5 files changed, 225 insertions(+), 295 deletions(-)
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
index e0ebb1e2fd35..bd5c6a949cf0 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.h
@@ -220,4 +220,30 @@ NorFlashWriteSingleWord (
IN UINT32 WriteData
);
+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/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
index e329e0727617..c0a3b5861532 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashCommon.h
@@ -31,20 +31,6 @@
//
// NorFlashDxe.c
//
-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
NorFlashCreateInstance (
IN UINTNNorFlashDeviceBase,
diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
index 4e5a97c83c7b..15000a692b02 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlash.c
@@ -10,7 +10,6 @@
#include
#include "NorFlash.h"
-#include "NorFlashCommon.h"
//
// Global variable declarations
@@ -817,3 +816,138 @@ NorFlashReset (
SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
return EFI_SUCCESS;
}
+
+/**
+ * This function unlock and erase an entire NOR Flash block.
+**/
+EFI_STATUS
+NorFlashUnlockAndEraseSingleBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN UINTN BlockAddress
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ EFI_TPL OriginalTPL;
+
+ NorFlashLock (&OriginalTPL);
+
+ Index = 0;
+ // The block erase might fail a first time (SW bug ?). Retry it ...
+ do {
+// Unlock the block if we have to
+Status = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress);
+if (EFI_ERROR (Status)) {
+ break;
+}
+
+Status = NorFlashEraseSingleBlock (Instance, BlockAddress);
+Index++;
+ } while ((Index < NOR_FLASH_ERASE_RETRY) && (Status == EFI_WRITE_PROTECTED));
+
+ if (Index == NOR_FLASH_ERASE_RETRY) {
+DEBUG ((DEBUG_ERROR, "EraseSingleBlock(BlockAddress=0x%08x: Block Locked
Error (try to erase %d times)\n", BlockAddress, Index));
+ }
+
+ NorFlashUnlock (OriginalTPL);
+
+ return Status;
+}
+
+EFI_STATUS
+NorFlashWriteFullBlock (
+ IN NOR_FLASH_INSTANCE *Instance,
+ IN EFI_LBA Lba,
+ IN UINT32 *DataBuffer,
+ IN UINT32 BlockSizeInWords
+ )
+{
+ EFI_STATUS Status;
+ UINTN WordAddress;
+ UINT32 WordIndex;
+ UINTN BufferIndex;
+ UINTN BlockAddress;
+ UINTN BuffersInBlock;
+ UINTN RemainingWords;
+ EFI_TPL OriginalTPL;
+ UINTN Cnt;
+
+ Status = EFI_SUCCESS;
+
+ // Get the physical address of the block
+ BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
BlockSizeInWords * 4);
+
+ // Start writing from the first address at the start of the block
+ WordAddress = BlockAddress;
+
+ NorFlashLock (&OriginalTPL);
+
+ Status = NorFlashUnlockAndEraseSingleBlock (Instance, BlockAddress);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "WriteSingleBlock: ERROR - Failed to Unlock and Erase
the single block at 0x%X\n", BlockAddress));
+goto EXIT;
+
