Marvell/Drivers: MvSpiFlash: Improve ReadId

Fix the ReadId routine by using master's ReadWrite callback
instead of the raw Transfer - no longer swapping and byte
shifting is needed. Simplify code by using local array
instead of dynamic allocation. Moreover store the FlashId
in an UINT8 array PCD instead of the concatenated UINT32
format - this way less overhead in the driver is needed
for comparing the buffers.

The new handling allowed for cleaning Fupdate and Sf
shell commands FlashProbe routines.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

7 files changed, 51 insertions, 61 deletions

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index 664411a97b..8a2ad3fb54 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -94,28 +94,18 @@ SpiFlashProbe (
   )

 {

   EFI_STATUS       Status;

-  UINT32           IdBuffer, Id, RefId;

+  UINT32           FlashIdLen;

+  UINT8           *FlashId;

 

-  Id = PcdGet32 (PcdSpiFlashId);

-

-  IdBuffer = CMD_READ_ID & 0xff;

+  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);

+  FlashIdLen = PcdGetSize (PcdSpiFlashId);

 

   // Read SPI flash ID

-  SpiFlashProtocol->ReadId (Slave, sizeof (UINT32), (UINT8 *)&IdBuffer);

-

-  // Swap and extract 3 bytes of the ID

-  RefId = SwapBytes32 (IdBuffer) >> 8;

-

-  if (RefId == 0) {

-    Print (L"%s: No SPI flash detected");

-    return EFI_DEVICE_ERROR;

-  } else if (RefId != Id) {

-    Print (L"%s: Unsupported SPI flash detected with ID=%2x\n", CMD_NAME_STRING, RefId);

-    return EFI_DEVICE_ERROR;

+  Status = SpiFlashProtocol->ReadId (Slave, FlashIdLen, FlashId);

+  if (EFI_ERROR (Status)) {

+    return SHELL_ABORTED;

   }

 

-  Print (L"%s: Detected supported SPI flash with ID=%3x\n", CMD_NAME_STRING, RefId);

-

   Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);

   if (EFI_ERROR(Status)) {

     Print (L"%s: Cannot initialize flash device\n", CMD_NAME_STRING);

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index 9321f6b47f..7c81bfc0d0 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -166,37 +166,26 @@ FlashProbe (
   )

 {

   EFI_STATUS Status;

-  UINT8  IdBuffer[4];

-  UINT32 Id, RefId;

+  UINT32     FlashIdLen;

+  UINT8      *FlashId;

 

-  Id = PcdGet32 (PcdSpiFlashId);

+  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);

+  FlashIdLen = PcdGetSize (PcdSpiFlashId);

 

-  IdBuffer[0] = CMD_READ_ID;

-

-  SpiFlashProtocol->ReadId (

-    Slave,

-    4,

-    IdBuffer

-    );

-

-  RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];

+  Status = SpiFlashProtocol->ReadId (Slave, FlashIdLen, FlashId);

+  if (EFI_ERROR (Status)) {

+    return SHELL_ABORTED;

+  }

 

-  if (RefId == Id) {

-    Print (L"sf: Detected supported SPI flash with ID=%3x\n", RefId);

-    Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);

-    if (EFI_ERROR(Status)) {

-      Print (L"sf: Cannot initialize flash device\n");

-      return SHELL_ABORTED;

-    }

-    InitFlag = 0;

-    return EFI_SUCCESS;

-  } else if (RefId != 0) {

-    Print (L"sf: Unsupported SPI flash detected with ID=%2x\n", RefId);

+  Status = SpiFlashProtocol->Init (SpiFlashProtocol, Slave);

+  if (EFI_ERROR (Status)) {

+    Print (L"sf: Cannot initialize flash device\n");

     return SHELL_ABORTED;

   }

 

-  Print (L"sf: No SPI flash detected");

-  return SHELL_ABORTED;

+  InitFlag = 0;

+

+  return SHELL_SUCCESS;

 }

 

 SHELL_STATUS

diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index 0396e8e124..4d5f55f076 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -94,7 +94,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3

   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536

   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256

-  gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18

+  gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x20, 0xBA, 0x18 }

   gMarvellTokenSpaceGuid.PcdSpiFlashMode|3

   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0

 

diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index 6f7d9c7f07..ab3ed6a55a 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -409,28 +409,35 @@ MvSpiFlashReadId (
   )

 {

   EFI_STATUS Status;

-  UINT8 *DataOut;

+  UINT8 Id[NOR_FLASH_MAX_ID_LEN];

+  UINT8 Cmd;

+

+  Cmd = CMD_READ_ID;

+  Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,

+                                SpiDev,

+                                &Cmd,

+                                SPI_CMD_LEN,

+                                NULL,

+                                Id,

+                                NOR_FLASH_MAX_ID_LEN);

+  if (EFI_ERROR (Status)) {

+    DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));

+    return Status;

+  }

 

-  DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);

-  if (DataOut == NULL) {

-    DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));

-    return EFI_OUT_OF_RESOURCES;

+  if (CompareMem (Id, Buffer, DataByteCount) != 0) {

+    Status = EFI_NOT_FOUND;

   }

-  Status = SpiMasterProtocol->Transfer (SpiMasterProtocol, SpiDev,

-    DataByteCount, Buffer, DataOut, SPI_TRANSFER_BEGIN | SPI_TRANSFER_END);

-  if (EFI_ERROR(Status)) {

-    FreePool (DataOut);

-    DEBUG((DEBUG_ERROR, "SpiFlash: Spi transfer error\n"));

+  if (EFI_ERROR (Status)) {

+    DEBUG ((DEBUG_ERROR,

+      "%a: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n",

+      __FUNCTION__,

+      Id[0],

+      Id[1],

+      Id[2]));

     return Status;

   }

 

-  // Bytes 1,2 and 3 contain SPI flash ID

-  Buffer[0] = DataOut[1];

-  Buffer[1] = DataOut[2];

-  Buffer[2] = DataOut[3];

-

-  FreePool (DataOut);

-

   return EFI_SUCCESS;

 }

 

diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
index f90abb771e..2583484afe 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
@@ -62,6 +62,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_ERASE_64K                   0xd8

 #define CMD_4B_ADDR_ENABLE              0xb7

 

+#define SPI_CMD_LEN                     1

+

 #define STATUS_REG_POLL_WIP             (1 << 0)

 #define STATUS_REG_POLL_PEC             (1 << 7)

 

diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
index 743bb87553..f65a12d188 100644
--- a/Platform/Marvell/Include/Protocol/SpiFlash.h
+++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
@@ -47,6 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_ERASE_64K                   0xd8

 #define CMD_4B_ADDR_ENABLE              0xb7

 

+#define NOR_FLASH_MAX_ID_LEN            6

+

 extern EFI_GUID gMarvellSpiFlashProtocolGuid;

 

 typedef struct _MARVELL_SPI_FLASH_PROTOCOL MARVELL_SPI_FLASH_PROTOCOL;

diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index cd800c8106..679a9d0c80 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -133,7 +133,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054

   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055

   gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059

-  gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056

+  gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x0 }|VOID*|0x3000056

   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057

   gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058