public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Marcin Wojtas <mw@semihalf.com>
Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org,
	nadavh@marvell.com, neta@marvell.com, kostap@marvell.com,
	jinghua@marvell.com, agraf@suse.de, jsd@semihalf.com
Subject: Re: [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection
Date: Fri, 1 Sep 2017 16:33:25 +0100	[thread overview]
Message-ID: <20170901153325.4lladsqfaomt5jcf@bivouac.eciton.net> (raw)
In-Reply-To: <1504271303-1782-11-git-send-email-mw@semihalf.com>

On Fri, Sep 01, 2017 at 03:08:22PM +0200, 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 reads the Id and goes through newly added table
> with JEDEC compliant devices and their description.
> 
> On the occasion fix the ReadId process by using master's
> ReadWrite routine instead of pure Transfer - no longer
> swapping and byte shifting is needed. Simplify code by
> using local array instead of dynamic allocation. Also
> reduced number of ReadId arguments allowed for cleaning
> Fupdate and Sf shell commands probe routines.
> 
> Additionally, use SpiFlashInfo fields instead of PCDs,
> and configure needed settings in MvSpiFlashInit.
> 
> Update PortingGuide documentation accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

So, this looks _really_ good, but I'm seeing three separate patches in
here really:
- Style fixups (yay). I love them, but squashing them in make the
  functional changes impossible to spot in the noise.
- SPI Flash Id table. Love it!
  But this is a completely generic feature. Medium-term, this should
  be living in EDK2. For now, can you break it out into a separate
  library that can be used by other platforms (and will be easier to
  move)?
- Functional improvements to existing driver - great! (but can't
  really review before the above two have been broken out).

/
    Leif

> ---
>  .../Marvell/Applications/FirmwareUpdate/FUpdate.c  |  23 +-
>  .../Applications/FirmwareUpdate/FUpdate.inf        |   3 -
>  .../Marvell/Applications/SpiTool/SpiFlashCmd.c     |  36 +--
>  .../Marvell/Applications/SpiTool/SpiFlashCmd.inf   |   1 -
>  Platform/Marvell/Armada/Armada70x0.dsc             |   5 -
>  Platform/Marvell/Documentation/PortingGuide.txt    |  18 --
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 267 +++++++++++++++++----
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |   2 +
>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |   7 -
>  Platform/Marvell/Include/Protocol/Spi.h            |  37 +++
>  Platform/Marvell/Include/Protocol/SpiFlash.h       |   4 +-
>  Platform/Marvell/Marvell.dec                       |   6 -
>  12 files changed, 271 insertions(+), 138 deletions(-)
> 
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> index 0951734..b0e94bc 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> @@ -94,32 +94,17 @@ SpiFlashProbe (
>    )
>  {
>    EFI_STATUS       Status;
> -  UINT32           IdBuffer, Id, RefId;
> -
> -  Id = PcdGet32 (PcdSpiFlashId);
> -
> -  IdBuffer = CMD_READ_ID & 0xff;
>  
>    // 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);
> +  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);
> -    return EFI_DEVICE_ERROR;
> +    return SHELL_ABORTED;
>    }
>  
>    return EFI_SUCCESS;
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> index 92c3333..43cac42 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
> @@ -64,9 +64,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 ee14270..b5db8b5 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
> @@ -166,37 +166,21 @@ FlashProbe (
>    )
>  {
>    EFI_STATUS Status;
> -  UINT8  IdBuffer[4];
> -  UINT32 Id, RefId;
>  
> -  Id = PcdGet32 (PcdSpiFlashId);
> -
> -  IdBuffer[0] = CMD_READ_ID;
> -
> -  SpiFlashProtocol->ReadId (
> -    Slave,
> -    4,
> -    IdBuffer
> -    );
> -
> -  RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];
> +  Status = SpiFlashProtocol->ReadId (Slave);
> +  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/Applications/SpiTool/SpiFlashCmd.inf b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> index c1ab770..343d5b5 100644
> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
> @@ -65,7 +65,6 @@
>   FileHandleLib
>  
>  [Pcd]
> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>   gMarvellTokenSpaceGuid.PcdSpiFlashCs
>   gMarvellTokenSpaceGuid.PcdSpiFlashMode
>  
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index df2ebdb..4633e32 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -93,11 +93,6 @@
>    gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>    gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>  
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
> -  gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
>  
> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> index f637fee..3ac35a0 100644
> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -288,24 +288,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 OpenPlatformPkg/Drivers/Spi/MvSpi.h))
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index f3fdba4..bfb8fa3 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -36,6 +36,136 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
>  SPI_FLASH_INSTANCE  *mSpiFlashInstance;
>  
> +#define INFO(JedecId, ExtId, SecSize, NSectors, FlashFlags)  \
> +  .Id = {                       \
> +    ((JedecId) >> 16) & 0xff,   \
> +    ((JedecId) >> 8) & 0xff,    \
> +    (JedecId) & 0xff,           \
> +    ((ExtId) >> 8) & 0xff,      \
> +    (ExtId) & 0xff,             \
> +    },                          \
> +  .IdLen = (!(JedecId) ? 0 : (3 + ((ExtId) ? 2 : 0))),  \
> +  .SectorSize = (SecSize),      \
> +  .SectorCount = (NSectors),    \
> +  .PageSize = 256,              \
> +  .Flags = (FlashFlags),
> +
> +static SPI_FLASH_INFO SpiFlashIds[] = {
> +  /* ATMEL */
> +  {L"at45db011d",        INFO(0x1f2200, 0x0, 64 * 1024,     4, SECT_4K) },
> +  {L"at45db021d",        INFO(0x1f2300, 0x0, 64 * 1024,     8, SECT_4K) },
> +  {L"at45db041d",        INFO(0x1f2400, 0x0, 64 * 1024,     8, SECT_4K) },
> +  {L"at45db081d",        INFO(0x1f2500, 0x0, 64 * 1024,    16, SECT_4K) },
> +  {L"at45db161d",        INFO(0x1f2600, 0x0, 64 * 1024,    32, SECT_4K) },
> +  {L"at45db321d",        INFO(0x1f2700, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at45db641d",        INFO(0x1f2800, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"at25df321a",        INFO(0x1f4701, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at25df321",         INFO(0x1f4700, 0x0, 64 * 1024,    64, SECT_4K) },
> +  {L"at26df081a",        INFO(0x1f4501, 0x0, 64 * 1024,    16, SECT_4K) },
> +  /* EON */
> +  {L"en25q32b",          INFO(0x1c3016, 0x0, 64 * 1024,    64, 0) },
> +  {L"en25q64",           INFO(0x1c3017, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"en25q128b",         INFO(0x1c3018, 0x0, 64 * 1024,   256, 0) },
> +  {L"en25s64",           INFO(0x1c3817, 0x0, 64 * 1024,   128, 0) },
> +  /* GIGADEVICE */
> +  {L"gd25q64b",          INFO(0xc84017, 0x0, 64 * 1024,   128, SECT_4K) },
> +  {L"gd25lq32",          INFO(0xc86016, 0x0, 64 * 1024,    64, SECT_4K) },
> +  /* ISSI */
> +  {L"is25lp032",         INFO(0x9d6016, 0x0, 64 * 1024,    64, 0) },
> +  {L"is25lp064",         INFO(0x9d6017, 0x0, 64 * 1024,   128, 0) },
> +  {L"is25lp128",         INFO(0x9d6018, 0x0, 64 * 1024,   256, 0) },
> +  /* MACRONIX */
> +  {L"mx25l2006e",        INFO(0xc22012, 0x0, 64 * 1024,     4, 0) },
> +  {L"mx25l4005",         INFO(0xc22013, 0x0, 64 * 1024,     8, 0) },
> +  {L"mx25l8005",         INFO(0xc22014, 0x0, 64 * 1024,    16, 0) },
> +  {L"mx25l1605d",        INFO(0xc22015, 0x0, 64 * 1024,    32, 0) },
> +  {L"mx25l3205d",        INFO(0xc22016, 0x0, 64 * 1024,    64, 0) },
> +  {L"mx25l6405d",        INFO(0xc22017, 0x0, 64 * 1024,   128, 0) },
> +  {L"mx25l12805",        INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"mx25l25635f",       INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP | ADDR_CYC_4) },
> +  {L"mx25l51235f",       INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"mx25l12855e",       INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"mx66u51235f",       INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"mx66l1g45g",        INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
> +  /* SPANSION */
> +  {L"s25fl008a",         INFO(0x010213, 0x0, 64 * 1024,    16, 0) },
> +  {L"s25fl016a",         INFO(0x010214, 0x0, 64 * 1024,    32, 0) },
> +  {L"s25fl032a",         INFO(0x010215, 0x0, 64 * 1024,    64, 0) },
> +  {L"s25fl064a",         INFO(0x010216, 0x0, 64 * 1024,   128, 0) },
> +  {L"s25fl116k",         INFO(0x014015, 0x0, 64 * 1024,   128, 0) },
> +  {L"s25fl164k",         INFO(0x014017, 0x0140,  64 * 1024,   128, 0) },
> +  {L"s25fl128p_256k",    INFO(0x012018, 0x0300, 256 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl128p_64k",     INFO(0x012018, 0x0301,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl032p",         INFO(0x010215, 0x4d00,  64 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl064p",         INFO(0x010216, 0x4d00,  64 * 1024,   128, RD_FULL | WR_QPP) },
> +  {L"s25fl128s_256k",    INFO(0x012018, 0x4d00, 256 * 1024,    64, RD_FULL | WR_QPP) },
> +  {L"s25fl128s_64k",     INFO(0x012018, 0x4d01,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl256s_256k",    INFO(0x010219, 0x4d00, 256 * 1024,   128, RD_FULL | WR_QPP) },
> +  {L"s25fl256s_64k",     INFO(0x010219, 0x4d01,  64 * 1024,   512, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_256k",    INFO(0x010220, 0x4d00, 256 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_64k",     INFO(0x010220, 0x4d01,  64 * 1024,  1024, RD_FULL | WR_QPP) },
> +  {L"s25fl512s_512k",    INFO(0x010220, 0x4f00, 256 * 1024,   256, RD_FULL | WR_QPP) },
> +  /* STMICRO */
> +  {L"m25p10",            INFO(0x202011, 0x0, 32 * 1024,     4, 0) },
> +  {L"m25p20",            INFO(0x202012, 0x0, 64 * 1024,     4, 0) },
> +  {L"m25p40",            INFO(0x202013, 0x0, 64 * 1024,     8, 0) },
> +  {L"m25p80",            INFO(0x202014, 0x0, 64 * 1024,    16, 0) },
> +  {L"m25p16",            INFO(0x202015, 0x0, 64 * 1024,    32, 0) },
> +  {L"m25pE16",           INFO(0x208015, 0x1000, 64 * 1024, 32, 0) },
> +  {L"m25pX16",           INFO(0x207115, 0x1000, 64 * 1024, 32, RD_QUAD | RD_DUAL) },
> +  {L"m25p32",            INFO(0x202016, 0x0,  64 * 1024,    64, 0) },
> +  {L"m25p64",            INFO(0x202017, 0x0,  64 * 1024,   128, 0) },
> +  {L"m25p128",           INFO(0x202018, 0x0, 256 * 1024,    64, 0) },
> +  {L"m25pX64",           INFO(0x207117, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"n25q016a",          INFO(0x20bb15, 0x0,  64 * 1024,    32, SECT_4K) },
> +  {L"n25q32",            INFO(0x20ba16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q32a",           INFO(0x20bb16, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q64",            INFO(0x20ba17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q64a",           INFO(0x20bb17, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q128",           INFO(0x20ba18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"n25q128a",          INFO(0x20bb18, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP) },
> +  {L"n25q256",           INFO(0x20ba19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q256a",          INFO(0x20bb19, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"n25q512",           INFO(0x20ba20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"n25q512a",          INFO(0x20bb20, 0x0,  64 * 1024,  1024, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"n25q1024",          INFO(0x20ba21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K | ADDR_CYC_4) },
> +  {L"n25q1024a",         INFO(0x20bb21, 0x0,  64 * 1024,  2048, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"mt25qu02g",         INFO(0x20bb22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  {L"mt25ql02g",         INFO(0x20ba22, 0x0,  64 * 1024,  4096, RD_FULL | WR_QPP | E_FSR | SECT_4K) },
> +  /* SST */
> +  {L"sst25vf040b",       INFO(0xbf258d, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
> +  {L"sst25vf080b",       INFO(0xbf258e, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
> +  {L"sst25vf016b",       INFO(0xbf2541, 0x0,  64 * 1024,    32, SECT_4K | SST_WR) },
> +  {L"sst25vf032b",       INFO(0xbf254a, 0x0,  64 * 1024,    64, SECT_4K | SST_WR) },
> +  {L"sst25vf064c",       INFO(0xbf254b, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"sst25wf512",        INFO(0xbf2501, 0x0,  64 * 1024,     1, SECT_4K | SST_WR) },
> +  {L"sst25wf010",        INFO(0xbf2502, 0x0,  64 * 1024,     2, SECT_4K | SST_WR) },
> +  {L"sst25wf020",        INFO(0xbf2503, 0x0,  64 * 1024,     4, SECT_4K | SST_WR) },
> +  {L"sst25wf040",        INFO(0xbf2504, 0x0,  64 * 1024,     8, SECT_4K | SST_WR) },
> +  {L"sst25wf040b",       INFO(0x621613, 0x0,  64 * 1024,     8, SECT_4K) },
> +  {L"sst25wf080",        INFO(0xbf2505, 0x0,  64 * 1024,    16, SECT_4K | SST_WR) },
> +  /* WINBOND */
> +  {L"w25p80",            INFO(0xef2014, 0x0,  64 * 1024,    16, 0) },
> +  {L"w25p16",            INFO(0xef2015, 0x0,  64 * 1024,    32, 0) },
> +  {L"w25p32",            INFO(0xef2016, 0x0,  64 * 1024,    64, 0) },
> +  {L"w25x40",            INFO(0xef3013, 0x0,  64 * 1024,     8, SECT_4K) },
> +  {L"w25x16",            INFO(0xef3015, 0x0,  64 * 1024,    32, SECT_4K) },
> +  {L"w25x32",            INFO(0xef3016, 0x0,  64 * 1024,    64, SECT_4K) },
> +  {L"w25x64",            INFO(0xef3017, 0x0,  64 * 1024,   128, SECT_4K) },
> +  {L"w25q80bl",          INFO(0xef4014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q16cl",          INFO(0xef4015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q32bv",          INFO(0xef4016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q64cv",          INFO(0xef4017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q128bv",         INFO(0xef4018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q256",           INFO(0xef4019, 0x0,  64 * 1024,   512, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q80bw",          INFO(0xef5014, 0x0,  64 * 1024,    16, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q16dw",          INFO(0xef6015, 0x0,  64 * 1024,    32, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q32dw",          INFO(0xef6016, 0x0,  64 * 1024,    64, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q64dw",          INFO(0xef6017, 0x0,  64 * 1024,   128, RD_FULL | WR_QPP | SECT_4K) },
> +  {L"w25q128fw",         INFO(0xef6018, 0x0,  64 * 1024,   256, RD_FULL | WR_QPP | SECT_4K) },
> +  {},  /* Empty entry to terminate the list */
> +};
> +
>  STATIC
>  VOID
>  SpiFlashFormatAddress (
> @@ -104,13 +234,13 @@ MvSpiFlashWriteCommon (
>    UINT8 CmdStatus = CMD_READ_STATUS;
>    UINT8 State;
>    UINT32 Counter = 0xFFFFF;
> -  UINT8 poll_bit = STATUS_REG_POLL_WIP;
> -  UINT8 check_status = 0x0;
> +  UINT8 PollBit = STATUS_REG_POLL_WIP;
> +  UINT8 CheckStatus = 0x0;
>  
> -  CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
> -  if (CmdStatus == CMD_FLAG_STATUS) {
> -    poll_bit = STATUS_REG_POLL_PEC;
> -    check_status = poll_bit;
> +  if (Slave->Info->Flags & E_FSR) {
> +    CmdStatus = CMD_FLAG_STATUS;
> +    PollBit = STATUS_REG_POLL_PEC;
> +    CheckStatus = STATUS_REG_POLL_PEC;
>    }
>  
>    // Send command
> @@ -127,7 +257,7 @@ MvSpiFlashWriteCommon (
>      SpiMasterProtocol->Transfer (SpiMasterProtocol, Slave, 1, NULL, &State,
>        0);
>      Counter--;
> -    if ((State & poll_bit) == check_status)
> +    if ((State & PollBit) == CheckStatus)
>        break;
>    } while (Counter > 0);
>    if (Counter == 0) {
> @@ -181,8 +311,19 @@ MvSpiFlashErase (
>    UINTN EraseSize;
>    UINT8 Cmd[5];
>  
> -  AddrSize  = PcdGet32 (PcdSpiFlashAddressCycles);
> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
> +
> +  if (Slave->Info->Flags & SECT_4K) {
> +    Cmd[0] = CMD_ERASE_4K;
> +    EraseSize = SPI_ERASE_SIZE_4K;
> +  } else {
> +    Cmd[0] = CMD_ERASE_64K;
> +    EraseSize = Slave->Info->SectorSize;
> +  }
>  
>    // Check input parameters
>    if (Offset % EraseSize || Length % EraseSize) {
> @@ -191,21 +332,6 @@ MvSpiFlashErase (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  switch (EraseSize) {
> -  case SPI_ERASE_SIZE_4K:
> -    Cmd[0] = CMD_ERASE_4K;
> -    break;
> -  case SPI_ERASE_SIZE_32K:
> -    Cmd[0] = CMD_ERASE_32K;
> -    break;
> -  case SPI_ERASE_SIZE_64K:
> -    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 +365,11 @@ MvSpiFlashRead (
>    UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>    UINTN BankSel = 0;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
>  
>    Cmd[0] = CMD_READ_ARRAY_FAST;
>  
> @@ -282,8 +412,13 @@ MvSpiFlashWrite (
>    UINT32 WriteAddr;
>    UINT8 Cmd[5], AddrSize;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> -  PageSize = PcdGet32 (PcdSpiFlashPageSize);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
> +
> +  PageSize = Slave->Info->PageSize;
>  
>    Cmd[0] = CMD_PAGE_PROGRAM;
>  
> @@ -370,7 +505,7 @@ MvSpiFlashUpdate (
>    UINT64 SectorSize, ToUpdate, Scale = 1;
>    UINT8 *TmpBuf, *End;
>  
> -  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
> +  SectorSize = Slave->Info->SectorSize;
>  
>    End = Buf + ByteCount;
>  
> @@ -400,38 +535,66 @@ MvSpiFlashUpdate (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC
> +VOID
> +MvPrintFlashInfo (
> +  IN     SPI_FLASH_INFO *Info
> +  )
> +{
> +  UINTN EraseSize;
> +
> +  if (Info->Flags & SECT_4K) {
> +    EraseSize = SPI_ERASE_SIZE_4K;
> +  } else {
> +    EraseSize = Info->SectorSize;
> +  }
> +
> +  DEBUG ((DEBUG_ERROR,
> +    "Detected %s SPI flash with page size %d B, erase size %d KB, total %d MB\n",
> +    Info->Name,
> +    Info->PageSize,
> +    EraseSize / 1024,
> +    (Info->SectorSize * Info->SectorCount) / 1024 / 1024));
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  MvSpiFlashReadId (
> -  IN     SPI_DEVICE *SpiDev,
> -  IN     UINT32     DataByteCount,
> -  IN OUT UINT8      *Buffer
> +  IN     SPI_DEVICE *SpiDev
>    )
>  {
> +  SPI_FLASH_INFO *Info;
>    EFI_STATUS Status;
> -  UINT8 *DataOut;
> -
> -  DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);
> -  if (DataOut == NULL) {
> -    DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -  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"));
> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
> +  UINT8 Cmd;
> +
> +  Cmd = CMD_READ_ID;
> +  Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,
> +                                SpiDev,
> +                                &Cmd,
> +                                SPI_CMD_LEN,
> +                                NULL,
> +                                Id,
> +                                SPI_FLASH_MAX_ID_LEN);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));
>      return Status;
>    }
>  
> -  // Bytes 1,2 and 3 contain SPI flash ID
> -  Buffer[0] = DataOut[1];
> -  Buffer[1] = DataOut[2];
> -  Buffer[2] = DataOut[3];
> +  Info = SpiFlashIds;
> +  for (; Info->Name != NULL; Info++) {
> +    if (Info->IdLen != 0) {
> +      if (CompareMem (Info->Id, Id, Info->IdLen) == 0) {
> +        SpiDev->Info = Info;
> +        MvPrintFlashInfo (Info);
> +        return EFI_SUCCESS;
> +      }
> +    }
> +  }
>  
> -  FreePool (DataOut);
> +  DEBUG ((DEBUG_ERROR, "ReadId: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n", Id[0], Id[1], Id[2]));
>  
> -  return EFI_SUCCESS;
> +  return EFI_NOT_FOUND;
>  }
>  
>  EFI_STATUS
> @@ -445,7 +608,11 @@ MvSpiFlashInit (
>    UINT8 Cmd, StatusRegister;
>    UINT32 AddrSize;
>  
> -  AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
> +  if (Slave->Info->Flags & ADDR_CYC_4) {
> +    AddrSize = 4;
> +  } else {
> +    AddrSize = 3;
> +  }
>  
>    if (AddrSize == 4) {
>      // Set 4 byte address mode
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> index 646598a..b876966 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/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> index 4519b02..f82c6e0 100644
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> @@ -53,13 +53,6 @@
>    DebugLib
>    MemoryAllocationLib
>  
> -[FixedPcd]
> -  gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles
> -  gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
> -  gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> -  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> -
>  [Protocols]
>    gMarvellSpiMasterProtocolGuid
>    gMarvellSpiFlashProtocolGuid
> diff --git a/Platform/Marvell/Include/Protocol/Spi.h b/Platform/Marvell/Include/Protocol/Spi.h
> index ae78a31..73f0d80 100644
> --- a/Platform/Marvell/Include/Protocol/Spi.h
> +++ b/Platform/Marvell/Include/Protocol/Spi.h
> @@ -38,6 +38,42 @@ extern EFI_GUID gMarvellSpiMasterProtocolGuid;
>  
>  typedef struct _MARVELL_SPI_MASTER_PROTOCOL MARVELL_SPI_MASTER_PROTOCOL;
>  
> +#define SPI_FLASH_MAX_ID_LEN    6
> +
> +typedef struct {
> +  /* Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) */
> +  UINT16 *Name;
> +
> +  /*
> +   * This array stores the ID bytes.
> +   * The first three bytes are the JEDIC ID.
> +   * JEDEC ID zero means "no ID" (mostly older chips).
> +   */
> +  UINT8 Id[SPI_FLASH_MAX_ID_LEN];
> +  UINT8 IdLen;
> +
> +  /*
> +   * The size listed here is what works with SPINOR_OP_SE, which isn't
> +   * necessarily called a "sector" by the vendor.
> +   */
> +  UINT32 SectorSize;
> +  UINT32 SectorCount;
> +
> +  UINT16 PageSize;
> +
> +  UINT16 Flags;
> +#define SECT_4K      1 << 0  /* CMD_ERASE_4K works uniformly */
> +#define E_FSR        1 << 1  /* use flag status register for */
> +#define SST_WR       1 << 2  /* use SST byte/word programming */
> +#define WR_QPP       1 << 3  /* use Quad Page Program */
> +#define RD_QUAD      1 << 4  /* use Quad Read */
> +#define RD_DUAL      1 << 5  /* use Dual Read */
> +#define RD_QUADIO    1 << 6  /* use Quad IO Read */
> +#define RD_DUALIO    1 << 7  /* use Dual IO Read */
> +#define RD_FULL      (RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
> +#define ADDR_CYC_4   1 << 8  /* use 4 byte addressing format */

Oh, and parentheses around all of these defines.

/
    Leif

> +} SPI_FLASH_INFO;
> +
>  typedef enum {
>    SPI_MODE0, // CPOL = 0 & CPHA = 0
>    SPI_MODE1, // CPOL = 0 & CPHA = 1
> @@ -49,6 +85,7 @@ typedef struct {
>    INTN Cs;
>    INTN MaxFreq;
>    SPI_MODE Mode;
> +  SPI_FLASH_INFO *Info;
>  } SPI_DEVICE;
>  
>  typedef
> diff --git a/Platform/Marvell/Include/Protocol/SpiFlash.h b/Platform/Marvell/Include/Protocol/SpiFlash.h
> index 743bb87..e12f55b 100644
> --- a/Platform/Marvell/Include/Protocol/SpiFlash.h
> +++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
> @@ -61,9 +61,7 @@ EFI_STATUS
>  typedef
>  EFI_STATUS
>  (EFIAPI *MV_SPI_FLASH_READ_ID) (
> -  IN     SPI_DEVICE *SpiDev,
> -  IN     UINT32     DataByteCount,
> -  IN OUT UINT8      *Buffer
> +  IN     SPI_DEVICE *SpiDev
>    );
>  
>  typedef
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index fc00f1a..418d960 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -123,12 +123,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|0|UINT32|0x3000056
>    gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
>    gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
>  
> -- 
> 1.8.3.1
> 


  reply	other threads:[~2017-09-01 15:30 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-01 13:08 [platforms: PATCH 00/11] Armada 70x0/80x0 SPI improvements Marcin Wojtas
2017-09-01 13:08 ` [platforms: PATCH 01/11] Platform/Marvell/Documentation: Refactor PortingGuide Marcin Wojtas
2017-09-01 14:36   ` Leif Lindholm
2017-09-01 15:08     ` Marcin Wojtas
2017-09-01 15:45       ` Leif Lindholm
2017-09-01 15:56         ` Marcin Wojtas
2017-09-01 13:08 ` [platforms: PATCH 02/11] Drivers/Spi/MvSpiDxe: Log and return correct error Marcin Wojtas
2017-09-01 14:05   ` Ard Biesheuvel
2017-09-01 13:08 ` [platforms: PATCH 03/11] Drivers/Spi/MvSpiDxe: Fix write bug Marcin Wojtas
2017-09-01 14:44   ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 04/11] Applications/SpiTool: Enable configurable CS and SCLK mode Marcin Wojtas
2017-09-01 14:47   ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 05/11] Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash Marcin Wojtas
2017-09-01 14:48   ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 06/11] Applications/SpiTool: Fix bug in error test Marcin Wojtas
2017-09-01 14:48   ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues Marcin Wojtas
2017-09-01 14:54   ` Leif Lindholm
2017-09-01 15:16     ` Ard Biesheuvel
2017-09-01 15:51       ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 08/11] Applications/SpiTool: " Marcin Wojtas
2017-09-01 14:56   ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter Marcin Wojtas
2017-09-01 15:21   ` Leif Lindholm
2017-09-01 15:25     ` Marcin Wojtas
2017-09-01 15:51       ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection Marcin Wojtas
2017-09-01 15:33   ` Leif Lindholm [this message]
2017-09-01 17:20     ` Marcin Wojtas
2017-09-01 22:03       ` Leif Lindholm
2017-09-01 13:08 ` [platforms: PATCH 11/11] Drivers/Spi/Devices/MvSpiFlash: Fix bank selection for Spansion Marcin Wojtas
2017-09-01 15:34   ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170901153325.4lladsqfaomt5jcf@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox