Leif,
2017-11-01 4:25 GMT+01:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Tue, Oct 31, 2017 at 04:59:31AM +0100, Marcin Wojtas wrote:
>> Hitherto mechanism of fixing SPI flash model in the PCDs,
>> occured to be very inefficient and problematic. Enable
>> dynamic detection by reworking MvSpiFlashReadId() command,
>> which now uses newly added NorFlashInfoLib, that helps to
>> obtain description of the JEDEC compliant devices.
>>
>> This patch updates the MvSpiFlashProtocol ReadId() protocol
>> callback on both producer's (MvFlashDxe) and consumers' sides
>> (FirmwareUpdate and SpiTool applications). Because all
>> information about detected SPI NOR flash is now stored in
>> the obtained NorFlashInfo structure fields, use them instead
>> of the PCDs.
>>
>> Enable compilation of the NorFlashInfoLib and update
>> PortingGuide documentation accordingly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <m...@semihalf.com>
>> ---
>> Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 +-
>> Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf | 4 +-
>> Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 5 +-
>> Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf | 2 +-
>> Platform/Marvell/Armada/Armada.dsc.inc | 1 +
>> Platform/Marvell/Armada/Armada70x0.dsc | 5 --
>> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 68
>> +++++++++++---------
>> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf | 9 +--
>> Platform/Marvell/Drivers/Spi/MvSpiDxe.inf | 2 +
>> Platform/Marvell/Include/Protocol/Spi.h | 3 +
>> Platform/Marvell/Include/Protocol/SpiFlash.h | 6 +-
>> Platform/Marvell/Marvell.dec | 6 --
>> Silicon/Marvell/Documentation/PortingGuide.txt | 18 ------
>> 13 files changed, 51 insertions(+), 83 deletions(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index d70645d..750e52a 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -94,12 +94,9 @@ SpiFlashProbe (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT8 *FlashId;
>> -
>> - FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>>
>> // Read SPI flash ID
>> - Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN,
>> FlashId);
>> + Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>> if (EFI_ERROR (Status)) {
>> return SHELL_ABORTED;
>> }
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> index 92c3333..53ea491 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf
>> @@ -44,6 +44,7 @@
>> FUpdate.uni
>>
>> [Packages]
>> + EmbeddedPkg/EmbeddedPkg.dec
>> MdeModulePkg/MdeModulePkg.dec
>> MdePkg/MdePkg.dec
>> Platform/Marvell/Marvell.dec
>> @@ -64,9 +65,6 @@
>> UefiLib
>> UefiRuntimeServicesTableLib
>>
>> -[Pcd]
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>> -
>> [Protocols]
>> gMarvellSpiFlashProtocolGuid
>> gMarvellSpiMasterProtocolGuid
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> index a12f2ec..68a6cf7 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> @@ -166,11 +166,8 @@ FlashProbe (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT8 *FlashId;
>>
>> - FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>> -
>> - Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN,
>> FlashId);
>> + Status = SpiFlashProtocol->ReadId (Slave, FALSE);
>> if (EFI_ERROR (Status)) {
>> return SHELL_ABORTED;
>> }
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> index 887b9a5..a52906b 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
>> @@ -44,6 +44,7 @@
>> SpiFlashCmd.uni
>>
>> [Packages]
>> + EmbeddedPkg/EmbeddedPkg.dec
>> MdePkg/MdePkg.dec
>> ShellPkg/ShellPkg.dec
>> MdeModulePkg/MdeModulePkg.dec
>> @@ -66,7 +67,6 @@
>>
>> [Pcd]
>> gMarvellTokenSpaceGuid.PcdSpiFlashCs
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId
>> gMarvellTokenSpaceGuid.PcdSpiFlashMode
>>
>> [Protocols]
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc
>> b/Platform/Marvell/Armada/Armada.dsc.inc
>> index b9fc384..2cd96e6 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -33,6 +33,7 @@
>>
>> ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
>> ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
>> MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
>> + NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
>> UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>>
>> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc
>> b/Platform/Marvell/Armada/Armada70x0.dsc
>> index 4d5f55f..8e4cdb2 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -90,11 +90,6 @@
>> gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|10000000
>> gMarvellTokenSpaceGuid.PcdSpiClockFrequency|200000000
>>
>> - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd|0x70
>> - gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
>> - gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
>> - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x20, 0xBA, 0x18 }
>> gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
>> gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> index ab3ed6a..703994c 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> @@ -107,10 +107,10 @@ MvSpiFlashWriteCommon (
>> UINT8 PollBit = STATUS_REG_POLL_WIP;
>> UINT8 CheckStatus = 0x0;
>>
>> - CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
>> - if (CmdStatus == CMD_FLAG_STATUS) {
>> + if (Slave->Info->Flags & NF_WRITE_FSR) {
>> + CmdStatus = CMD_FLAG_STATUS;
>> PollBit = STATUS_REG_POLL_PEC;
>> - CheckStatus = PollBit;
>> + CheckStatus = STATUS_REG_POLL_PEC;
>> }
>>
>> // Send command
>> @@ -181,8 +181,19 @@ MvSpiFlashErase (
>> UINTN EraseSize;
>> UINT8 Cmd[5];
>>
>> - AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> - EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
>> + if (Slave->Info->Flags & NF_4B_ADDR) {
>> + AddrSize = 4;
>> + } else {
>> + AddrSize = 3;
>> + }
>> +
>> + if (Slave->Info->Flags & NF_ERASE_4K) {
>> + Cmd[0] = CMD_ERASE_4K;
>> + EraseSize = SIZE_4KB;
>> + } else {
>
> Are 4 and 64K the only legal sizes?
> This patch deletes a detection of 32K size and the error message (and
> return) if an unknown size is encountered.
I didn't find 32K flash in Linux and U-Boot, maybe there some exotic
models, which use it. Anyway, there is no size check, as there are two
valid options now - 64KB and 4KB size, which is determined by
Info->Flag. Previously it was explicit EraseSize value, so the check
for wrong value was needed. Although unused, I can add NF_ERASE_32K
flag to NorFlashInfoLib and extend a check here.
>
>> + Cmd[0] = CMD_ERASE_64K;
>> + EraseSize = Slave->Info->SectorSize;
>> + }
>>
>> // Check input parameters
>> if (Offset % EraseSize || Length % EraseSize) {
>> @@ -191,21 +202,6 @@ MvSpiFlashErase (
>> return EFI_DEVICE_ERROR;
>> }
>>
>> - switch (EraseSize) {
>> - case SIZE_4KB:
>> - Cmd[0] = CMD_ERASE_4K;
>> - break;
>> - case SIZE_32KB:
>> - Cmd[0] = CMD_ERASE_32K;
>> - break;
>> - case SIZE_64KB:
>> - Cmd[0] = CMD_ERASE_64K;
>> - break;
>> - default:
>> - DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
>> - return EFI_INVALID_PARAMETER;
>> - }
>> -
>> while (Length) {
>> EraseAddr = Offset;
>>
>> @@ -239,7 +235,11 @@ MvSpiFlashRead (
>> UINT32 AddrSize, ReadAddr, ReadLength, RemainLength;
>> UINTN BankSel = 0;
>>
>> - AddrSize = PcdGet32 (PcdSpiFlashAddressCycles);
>> + if (Slave->Info->Flags & NF_4B_ADDR) {
>> + AddrSize = 4;
>> + } else {
>> + AddrSize = 3;
>> + }
>
> I see this sequence repeated many times.
> Could it be replaced by a macro or static helper function? Or cached
> in a device structure?
>
Ok.
Thanks,
Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
