public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Marcin Wojtas <mw@semihalf.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	 nadavh@marvell.com, Neta Zur Hershkovits <neta@marvell.com>,
	 Kostya Porotchkin <kostap@marvell.com>,
	Hua Jing <jinghua@marvell.com>, Alexander Graf <agraf@suse.de>,
	semihalf-dabros-jan <jsd@semihalf.com>
Subject: Re: [platforms: PATCH 10/11] Drivers/Spi/Devices/MvSpiFlash: Enable dynamic SPI Flash detection
Date: Fri, 1 Sep 2017 19:20:06 +0200	[thread overview]
Message-ID: <CAPv3WKckc+aBKoeft1S3tzX5mwJJActBy0F1pMJMMn=G=5Uh9A@mail.gmail.com> (raw)
In-Reply-To: <20170901153325.4lladsqfaomt5jcf@bivouac.eciton.net>

2017-09-01 17:33 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> 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).
>

I'll check and do the style fixup/functional improvement split.

About flash ids in a separate library - we already have
SpiFlashProtocol->ReadId callback. In fact despite Mv prefix the
MvSpiFlash.c driver and its associated SPI_FLASH_PROTOCOL are
completely generic for now and SoC controller agnostic. It works in
very similar way as U-Boot driver (drivers/mtd/spi) and Linux one
(drivers/mtd/spi-nor/spi-nor.c). Due to lack of similar solution, how
about moving it to EDK2? Do you think there's chance to promote such
solution?

Marcin

> /
>     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 17:17 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
2017-09-01 17:20     ` Marcin Wojtas [this message]
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='CAPv3WKckc+aBKoeft1S3tzX5mwJJActBy0F1pMJMMn=G=5Uh9A@mail.gmail.com' \
    --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