1. Patchew
  2. EDK2
  3. [edk2] [platforms: PATCH 0/6] Armada 7k/8k SPI improvements pt 2.
  4. View patch

[edk2] [platforms: PATCH 2/6] Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection

Marcin Wojtas posted 6 patches 7 years, 1 month ago
There is a newer version of this series
[edk2] [platforms: PATCH 2/6] Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection
Posted by Marcin Wojtas 7 years, 1 month ago 
Hitherto mechanism of fixing SPI flash model in the PCDs,
occured to be very inefficient and problematic. Enable
dynamic detection by reworking MvSpiFlashReadId() command,
which now uses newly added NorFlashInfoLib, that helps to
obtain description of the JEDEC compliant devices.

This patch updates the MvSpiFlashProtocol ReadId() protocol
callback on both producer's (MvFlashDxe) and consumers' sides
(FirmwareUpdate and SpiTool applications). Because all
information about detected SPI NOR flash is now stored in
the obtained NorFlashInfo structure fields, use them instead
of the PCDs.

Enable compilation of the NorFlashInfoLib and update
PortingGuide documentation accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c   |  5 +-
 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf |  4 +-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c      |  5 +-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf    |  2 +-
 Platform/Marvell/Armada/Armada.dsc.inc                   |  1 +
 Platform/Marvell/Armada/Armada70x0.dsc                   |  5 --
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c        | 68 +++++++++++---------
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf      |  9 +--
 Platform/Marvell/Drivers/Spi/MvSpiDxe.inf                |  2 +
 Platform/Marvell/Include/Protocol/Spi.h                  |  3 +
 Platform/Marvell/Include/Protocol/SpiFlash.h             |  6 +-
 Platform/Marvell/Marvell.dec                             |  6 --
 Silicon/Marvell/Documentation/PortingGuide.txt           | 18 ------
 13 files changed, 51 insertions(+), 83 deletions(-)

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index d70645d..750e52a 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -94,12 +94,9 @@ SpiFlashProbe (
   )
 {
   EFI_STATUS       Status;
-  UINT8           *FlashId;
-
-  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
 
   // Read SPI flash ID
-  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
+  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
   if (EFI_ERROR (Status)) {
     return SHELL_ABORTED;
   }
diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
index 92c3333..53ea491 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
@@ -44,6 +44,7 @@
   FUpdate.uni
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   Platform/Marvell/Marvell.dec
@@ -64,9 +65,6 @@
   UefiLib
   UefiRuntimeServicesTableLib
 
-[Pcd]
- gMarvellTokenSpaceGuid.PcdSpiFlashId
-
 [Protocols]
  gMarvellSpiFlashProtocolGuid
  gMarvellSpiMasterProtocolGuid
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index a12f2ec..68a6cf7 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -166,11 +166,8 @@ FlashProbe (
   )
 {
   EFI_STATUS Status;
-  UINT8      *FlashId;
 
-  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
-
-  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
+  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
   if (EFI_ERROR (Status)) {
     return SHELL_ABORTED;
   }
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
index 887b9a5..a52906b 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
@@ -44,6 +44,7 @@
  SpiFlashCmd.uni
 
 [Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
  MdePkg/MdePkg.dec
  ShellPkg/ShellPkg.dec
  MdeModulePkg/MdeModulePkg.dec
@@ -66,7 +67,6 @@
 
 [Pcd]
  gMarvellTokenSpaceGuid.PcdSpiFlashCs
- gMarvellTokenSpaceGuid.PcdSpiFlashId
  gMarvellTokenSpaceGuid.PcdSpiFlashMode
 
 [Protocols]
diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index b9fc384..2cd96e6 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -33,6 +33,7 @@
   ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
   ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
   MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
+  NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
   UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
 
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index 4d5f55f..8e4cdb2 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -90,11 +90,6 @@
   gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
   gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
 
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
-  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 ab3ed6a..703994c 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -107,10 +107,10 @@ MvSpiFlashWriteCommon (
   UINT8 PollBit = STATUS_REG_POLL_WIP;
   UINT8 CheckStatus = 0x0;
 
-  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
-  if (CmdStatus == CMD_FLAG_STATUS) {
+  if (Slave->Info->Flags & NF_WRITE_FSR) {
+    CmdStatus = CMD_FLAG_STATUS;
     PollBit = STATUS_REG_POLL_PEC;
-    CheckStatus = PollBit;
+    CheckStatus = STATUS_REG_POLL_PEC;
   }
 
   // Send command
@@ -181,8 +181,19 @@ MvSpiFlashErase (
   UINTN EraseSize;
   UINT8 Cmd[5];
 
-  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
-  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
+  if (Slave->Info->Flags & NF_4B_ADDR) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
+
+  if (Slave->Info->Flags & NF_ERASE_4K) {
+    Cmd[0] = CMD_ERASE_4K;
+    EraseSize = SIZE_4KB;
+  } else {
+    Cmd[0] = CMD_ERASE_64K;
+    EraseSize = Slave->Info->SectorSize;
+  }
 
   // Check input parameters
   if (Offset % EraseSize || Length % EraseSize) {
@@ -191,21 +202,6 @@ MvSpiFlashErase (
     return EFI_DEVICE_ERROR;
   }
 
-  switch (EraseSize) {
-  case SIZE_4KB:
-    Cmd[0] = CMD_ERASE_4K;
-    break;
-  case SIZE_32KB:
-    Cmd[0] = CMD_ERASE_32K;
-    break;
-  case SIZE_64KB:
-    Cmd[0] = CMD_ERASE_64K;
-    break;
-  default:
-    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
-    return EFI_INVALID_PARAMETER;
-  }
-
   while (Length) {
     EraseAddr = Offset;
 
@@ -239,7 +235,11 @@ MvSpiFlashRead (
   UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
   UINTN BankSel = 0;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
+  if (Slave->Info->Flags & NF_4B_ADDR) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
 
   Cmd[0] = CMD_READ_ARRAY_FAST;
 
@@ -282,8 +282,13 @@ MvSpiFlashWrite (
   UINT32 WriteAddr;
   UINT8 Cmd[5], AddrSize;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
-  PageSize = PcdGet32 (PcdSpiFlashPageSize);
+  if (Slave->Info->Flags & NF_4B_ADDR) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
+
+  PageSize = Slave->Info->PageSize;
 
   Cmd[0] = CMD_PAGE_PROGRAM;
 
@@ -370,7 +375,7 @@ MvSpiFlashUpdate (
   UINT64 SectorSize, ToUpdate, Scale = 1;
   UINT8 *TmpBuf, *End;
 
-  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
+  SectorSize = Slave->Info->SectorSize;
 
   End = Buf + ByteCount;
 
@@ -404,8 +409,7 @@ EFI_STATUS
 EFIAPI
 MvSpiFlashReadId (
   IN     SPI_DEVICE *SpiDev,
-  IN     UINT32     DataByteCount,
-  IN OUT UINT8      *Buffer
+  IN     BOOLEAN     UseInRuntime
   )
 {
   EFI_STATUS Status;
@@ -425,9 +429,7 @@ MvSpiFlashReadId (
     return Status;
   }
 
-  if (CompareMem (Id, Buffer, DataByteCount) != 0) {
-    Status = EFI_NOT_FOUND;
-  }
+  Status = GetNorFlashInfo (Id, &SpiDev->Info, UseInRuntime);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR,
       "%a: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n",
@@ -438,6 +440,8 @@ MvSpiFlashReadId (
     return Status;
   }
 
+  PrintNorFlashInfo (SpiDev->Info);
+
   return EFI_SUCCESS;
 }
 
@@ -452,7 +456,11 @@ MvSpiFlashInit (
   UINT8 Cmd, StatusRegister;
   UINT32 AddrSize;
 
-  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
+  if (Slave->Info->Flags & NF_4B_ADDR) {
+    AddrSize = 4;
+  } else {
+    AddrSize = 3;
+  }
 
   if (AddrSize == 4) {
     // Set 4 byte address mode
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
index 4519b02..6587f69 100644
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
@@ -42,10 +42,12 @@
   MvSpiFlash.h
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   Platform/Marvell/Marvell.dec
 
 [LibraryClasses]
+  NorFlashInfoLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   TimerLib
@@ -53,13 +55,6 @@
   DebugLib
   MemoryAllocationLib
 
-[FixedPcd]
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
-  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
-
 [Protocols]
   gMarvellSpiMasterProtocolGuid
   gMarvellSpiFlashProtocolGuid
diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.inf b/Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
index d38d331..08c6c04 100644
--- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
+++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.inf
@@ -42,10 +42,12 @@
   MvSpiDxe.h
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   Platform/Marvell/Marvell.dec
 
 [LibraryClasses]
+  NorFlashInfoLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   TimerLib
diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
index ae78a31..93a8ec0 100644
--- a/Platform/Marvell/Include/Protocol/Spi.h
+++ b/Platform/Marvell/Include/Protocol/Spi.h
@@ -34,6 +34,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #ifndef __MARVELL_SPI_MASTER_PROTOCOL_H__
 #define __MARVELL_SPI_MASTER_PROTOCOL_H__
 
+#include <Library/NorFlashInfoLib.h>
+
 extern EFI_GUID gMarvellSpiMasterProtocolGuid;
 
 typedef struct _MARVELL_SPI_MASTER_PROTOCOL MARVELL_SPI_MASTER_PROTOCOL;
@@ -49,6 +51,7 @@ typedef struct {
   INTN Cs;
   INTN MaxFreq;
   SPI_MODE Mode;
+  NOR_FLASH_INFO *Info;
 } SPI_DEVICE;
 
 typedef
diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
index e0d62cc..4a3053e 100644
--- a/Platform/Marvell/Include/Protocol/SpiFlash.h
+++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
@@ -47,9 +47,6 @@ 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
-#define NOR_FLASH_ID_DEFAULT_LEN        3
-
 extern EFI_GUID gMarvellSpiFlashProtocolGuid;
 
 typedef struct _MARVELL_SPI_FLASH_PROTOCOL MARVELL_SPI_FLASH_PROTOCOL;
@@ -65,8 +62,7 @@ typedef
 EFI_STATUS
 (EFIAPI *MV_SPI_FLASH_READ_ID) (
   IN     SPI_DEVICE *SpiDev,
-  IN     UINT32     DataByteCount,
-  IN OUT UINT8      *Buffer
+  IN     BOOLEAN     UseInRuntime
   );
 
 typedef
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 679a9d0..8255895 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -128,12 +128,6 @@
   gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052
   gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053
 
-  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0|UINT32|0x3000052
-  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x3000053
-  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
-  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
-  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
-  gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x0 }|VOID*|0x3000056
   gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
   gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
 
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
index cbe3bed..d5deed5 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -312,24 +312,6 @@ SpiFlash configuration
 ======================
 Folowing PCDs for spi flash driver configuration must be set properly:
 
-  - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
-        (Size of SPI flash address in bytes (3 or 4) )
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
-        (Size of minimal erase block in bytes)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
-        (Size of SPI flash page)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
-        (Size of SPI flash sector, 65536 bytes by default)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashId
-        (Id of SPI flash)
-
-  - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
-        (Spi flash polling flag)
-
   - gMarvellTokenSpaceGuid.PcdSpiFlashMode
         (Default SCLK mode (see SPI_MODE enum in file
          edk2-platforms/Platform/Marvell/Drivers/Spi/MvSpi.h))
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 2/6] Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection
Posted by Leif Lindholm 7 years, 1 month ago 
On Tue, Oct 31, 2017 at 04:59:31AM +0100, Marcin Wojtas wrote:
> Hitherto mechanism of fixing SPI flash model in the PCDs,
> occured to be very inefficient and problematic. Enable
> dynamic detection by reworking MvSpiFlashReadId() command,
> which now uses newly added NorFlashInfoLib, that helps to
> obtain description of the JEDEC compliant devices.
> 
> This patch updates the MvSpiFlashProtocol ReadId() protocol
> callback on both producer's (MvFlashDxe) and consumers' sides
> (FirmwareUpdate and SpiTool applications). Because all
> information about detected SPI NOR flash is now stored in
> the obtained NorFlashInfo structure fields, use them instead
> of the PCDs.
> 
> Enable compilation of the NorFlashInfoLib and update
> PortingGuide documentation accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c   |  5 +-
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf |  4 +-
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c      |  5 +-
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf    |  2 +-
>  Platform/Marvell/Armada/Armada.dsc.inc                   |  1 +
>  Platform/Marvell/Armada/Armada70x0.dsc                   |  5 --
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c        | 68 +++++++++++---------
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf      |  9 +--
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.inf                |  2 +
>  Platform/Marvell/Include/Protocol/Spi.h                  |  3 +
>  Platform/Marvell/Include/Protocol/SpiFlash.h             |  6 +-
>  Platform/Marvell/Marvell.dec                             |  6 --
>  Silicon/Marvell/Documentation/PortingGuide.txt           | 18 ------
>  13 files changed, 51 insertions(+), 83 deletions(-)
> 
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> index d70645d..750e52a 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> @@ -94,12 +94,9 @@ SpiFlashProbe (
>    )
>  {
>    EFI_STATUS       Status;
> -  UINT8           *FlashId;
> -
> -  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>  
>    // Read SPI flash ID
> -  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
> +  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>    if (EFI_ERROR (Status)) {
>      return SHELL_ABORTED;
>    }
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> index 92c3333..53ea491 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> @@ -44,6 +44,7 @@
>    FUpdate.uni
>  
>  [Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
>    Platform/Marvell/Marvell.dec
> @@ -64,9 +65,6 @@
>    UefiLib
>    UefiRuntimeServicesTableLib
>  
> -[Pcd]
> - gMarvellTokenSpaceGuid.PcdSpiFlashId
> -
>  [Protocols]
>   gMarvellSpiFlashProtocolGuid
>   gMarvellSpiMasterProtocolGuid
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> index a12f2ec..68a6cf7 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -166,11 +166,8 @@ FlashProbe (
>    )
>  {
>    EFI_STATUS Status;
> -  UINT8      *FlashId;
>  
> -  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
> -
> -  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
> +  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>    if (EFI_ERROR (Status)) {
>      return SHELL_ABORTED;
>    }
> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> index 887b9a5..a52906b 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> @@ -44,6 +44,7 @@
>   SpiFlashCmd.uni
>  
>  [Packages]
> + EmbeddedPkg/EmbeddedPkg.dec
>   MdePkg/MdePkg.dec
>   ShellPkg/ShellPkg.dec
>   MdeModulePkg/MdeModulePkg.dec
> @@ -66,7 +67,6 @@
>  
>  [Pcd]
>   gMarvellTokenSpaceGuid.PcdSpiFlashCs
> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>   gMarvellTokenSpaceGuid.PcdSpiFlashMode
>  
>  [Protocols]
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index b9fc384..2cd96e6 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -33,6 +33,7 @@
>    ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>    ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
>    MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
> +  NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
>    UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>  
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index 4d5f55f..8e4cdb2 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -90,11 +90,6 @@
>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>  
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
> -  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 ab3ed6a..703994c 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -107,10 +107,10 @@ MvSpiFlashWriteCommon (
>    UINT8 PollBit = STATUS_REG_POLL_WIP;
>    UINT8 CheckStatus = 0x0;
>  
> -  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
> -  if (CmdStatus == CMD_FLAG_STATUS) {
> +  if (Slave->Info->Flags & NF_WRITE_FSR) {
> +    CmdStatus = CMD_FLAG_STATUS;
>      PollBit = STATUS_REG_POLL_PEC;
> -    CheckStatus = PollBit;
> +    CheckStatus = STATUS_REG_POLL_PEC;
>    }
>  
>    // Send command
> @@ -181,8 +181,19 @@ MvSpiFlashErase (
>    UINTN EraseSize;
>    UINT8 Cmd[5];
>  
> -  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> +  if (Slave->Info->Flags & NF_4B_ADDR) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
> +
> +  if (Slave->Info->Flags & NF_ERASE_4K) {
> +    Cmd[0] = CMD_ERASE_4K;
> +    EraseSize = SIZE_4KB;
> +  } else {

Are 4 and 64K the only legal sizes?
This patch deletes a detection of 32K size and the error message (and
return) if an unknown size is encountered.

> +    Cmd[0] = CMD_ERASE_64K;
> +    EraseSize = Slave->Info->SectorSize;
> +  }
>  
>    // Check input parameters
>    if (Offset % EraseSize || Length % EraseSize) {
> @@ -191,21 +202,6 @@ MvSpiFlashErase (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  switch (EraseSize) {
> -  case SIZE_4KB:
> -    Cmd[0] = CMD_ERASE_4K;
> -    break;
> -  case SIZE_32KB:
> -    Cmd[0] = CMD_ERASE_32K;
> -    break;
> -  case SIZE_64KB:
> -    Cmd[0] = CMD_ERASE_64K;
> -    break;
> -  default:
> -    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
>    while (Length) {
>      EraseAddr = Offset;
>  
> @@ -239,7 +235,11 @@ MvSpiFlashRead (
>    UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>    UINTN BankSel = 0;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> +  if (Slave->Info->Flags & NF_4B_ADDR) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }

I see this sequence repeated many times.
Could it be replaced by a macro or static helper function? Or cached
in a device structure?

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 2/6] Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection
Posted by Marcin Wojtas 7 years, 1 month ago 
Leif,

2017-11-01 4:25 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Tue, Oct 31, 2017 at 04:59:31AM +0100, Marcin Wojtas wrote:
>> Hitherto mechanism of fixing SPI flash model in the PCDs,
>> occured to be very inefficient and problematic. Enable
>> dynamic detection by reworking MvSpiFlashReadId() command,
>> which now uses newly added NorFlashInfoLib, that helps to
>> obtain description of the JEDEC compliant devices.
>>
>> This patch updates the MvSpiFlashProtocol ReadId() protocol
>> callback on both producer's (MvFlashDxe) and consumers' sides
>> (FirmwareUpdate and SpiTool applications). Because all
>> information about detected SPI NOR flash is now stored in
>> the obtained NorFlashInfo structure fields, use them instead
>> of the PCDs.
>>
>> Enable compilation of the NorFlashInfoLib and update
>> PortingGuide documentation accordingly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c   |  5 +-
>>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf |  4 +-
>>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c      |  5 +-
>>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf    |  2 +-
>>  Platform/Marvell/Armada/Armada.dsc.inc                   |  1 +
>>  Platform/Marvell/Armada/Armada70x0.dsc                   |  5 --
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c        | 68 +++++++++++---------
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf      |  9 +--
>>  Platform/Marvell/Drivers/Spi/MvSpiDxe.inf                |  2 +
>>  Platform/Marvell/Include/Protocol/Spi.h                  |  3 +
>>  Platform/Marvell/Include/Protocol/SpiFlash.h             |  6 +-
>>  Platform/Marvell/Marvell.dec                             |  6 --
>>  Silicon/Marvell/Documentation/PortingGuide.txt           | 18 ------
>>  13 files changed, 51 insertions(+), 83 deletions(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index d70645d..750e52a 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -94,12 +94,9 @@ SpiFlashProbe (
>>    )
>>  {
>>    EFI_STATUS       Status;
>> -  UINT8           *FlashId;
>> -
>> -  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>>
>>    // Read SPI flash ID
>> -  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
>> +  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>>    if (EFI_ERROR (Status)) {
>>      return SHELL_ABORTED;
>>    }
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> index 92c3333..53ea491 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> @@ -44,6 +44,7 @@
>>    FUpdate.uni
>>
>>  [Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>>    MdeModulePkg/MdeModulePkg.dec
>>    MdePkg/MdePkg.dec
>>    Platform/Marvell/Marvell.dec
>> @@ -64,9 +65,6 @@
>>    UefiLib
>>    UefiRuntimeServicesTableLib
>>
>> -[Pcd]
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>> -
>>  [Protocols]
>>   gMarvellSpiFlashProtocolGuid
>>   gMarvellSpiMasterProtocolGuid
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> index a12f2ec..68a6cf7 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> @@ -166,11 +166,8 @@ FlashProbe (
>>    )
>>  {
>>    EFI_STATUS Status;
>> -  UINT8      *FlashId;
>>
>> -  FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>> -
>> -  Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN, FlashId);
>> +  Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>>    if (EFI_ERROR (Status)) {
>>      return SHELL_ABORTED;
>>    }
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> index 887b9a5..a52906b 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> @@ -44,6 +44,7 @@
>>   SpiFlashCmd.uni
>>
>>  [Packages]
>> + EmbeddedPkg/EmbeddedPkg.dec
>>   MdePkg/MdePkg.dec
>>   ShellPkg/ShellPkg.dec
>>   MdeModulePkg/MdeModulePkg.dec
>> @@ -66,7 +67,6 @@
>>
>>  [Pcd]
>>   gMarvellTokenSpaceGuid.PcdSpiFlashCs
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>>   gMarvellTokenSpaceGuid.PcdSpiFlashMode
>>
>>  [Protocols]
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
>> index b9fc384..2cd96e6 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -33,6 +33,7 @@
>>    ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>>    ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
>>    MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
>> +  NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
>>    UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>>
>>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
>> index 4d5f55f..8e4cdb2 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -90,11 +90,6 @@
>>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>>
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
>> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
>> -  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 ab3ed6a..703994c 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> @@ -107,10 +107,10 @@ MvSpiFlashWriteCommon (
>>    UINT8 PollBit = STATUS_REG_POLL_WIP;
>>    UINT8 CheckStatus = 0x0;
>>
>> -  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
>> -  if (CmdStatus == CMD_FLAG_STATUS) {
>> +  if (Slave->Info->Flags & NF_WRITE_FSR) {
>> +    CmdStatus = CMD_FLAG_STATUS;
>>      PollBit = STATUS_REG_POLL_PEC;
>> -    CheckStatus = PollBit;
>> +    CheckStatus = STATUS_REG_POLL_PEC;
>>    }
>>
>>    // Send command
>> @@ -181,8 +181,19 @@ MvSpiFlashErase (
>>    UINTN EraseSize;
>>    UINT8 Cmd[5];
>>
>> -  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
>> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
>> +  if (Slave->Info->Flags & NF_4B_ADDR) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>> +
>> +  if (Slave->Info->Flags & NF_ERASE_4K) {
>> +    Cmd[0] = CMD_ERASE_4K;
>> +    EraseSize = SIZE_4KB;
>> +  } else {
>
> Are 4 and 64K the only legal sizes?
> This patch deletes a detection of 32K size and the error message (and
> return) if an unknown size is encountered.

I didn't find 32K flash in Linux and U-Boot, maybe there some exotic
models, which use it. Anyway, there is no size check, as there are two
valid options now - 64KB and 4KB size, which is determined by
Info->Flag. Previously it was explicit EraseSize value, so the check
for wrong value was needed. Although unused, I can add NF_ERASE_32K
flag to NorFlashInfoLib and extend a check here.

>
>> +    Cmd[0] = CMD_ERASE_64K;
>> +    EraseSize = Slave->Info->SectorSize;
>> +  }
>>
>>    // Check input parameters
>>    if (Offset % EraseSize || Length % EraseSize) {
>> @@ -191,21 +202,6 @@ MvSpiFlashErase (
>>      return EFI_DEVICE_ERROR;
>>    }
>>
>> -  switch (EraseSize) {
>> -  case SIZE_4KB:
>> -    Cmd[0] = CMD_ERASE_4K;
>> -    break;
>> -  case SIZE_32KB:
>> -    Cmd[0] = CMD_ERASE_32K;
>> -    break;
>> -  case SIZE_64KB:
>> -    Cmd[0] = CMD_ERASE_64K;
>> -    break;
>> -  default:
>> -    DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
>> -    return EFI_INVALID_PARAMETER;
>> -  }
>> -
>>    while (Length) {
>>      EraseAddr = Offset;
>>
>> @@ -239,7 +235,11 @@ MvSpiFlashRead (
>>    UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>>    UINTN BankSel = 0;
>>
>> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> +  if (Slave->Info->Flags & NF_4B_ADDR) {
>> +    AddrSize = 4;
>> +  } else {
>> +    AddrSize = 3;
>> +  }
>
> I see this sequence repeated many times.
> Could it be replaced by a macro or static helper function? Or cached
> in a device structure?
>

Ok.

Thanks,
Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.