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
>>
next prev parent 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