2017-10-31 10:07 GMT+01:00 Leif Lindholm <[email protected]>:
> On Tue, Oct 31, 2017 at 04:59:30AM +0100, Marcin Wojtas wrote:
>> Fix the ReadId routine by using master's ReadWrite callback
>> instead of the raw Transfer - no longer swapping and byte
>> shifting is needed. Simplify code by using local array
>> instead of dynamic allocation. Moreover store the FlashId
>> in an UINT8 array PCD instead of the concatenated UINT32
>> format - this way less overhead in the driver is needed
>> for comparing the buffers.
>>
>> The new handling allowed for cleaning Fupdate and Sf
>> shell commands FlashProbe routines.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <[email protected]>
>> ---
>> Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 22 +++--------
>> Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 37
>> ++++++------------
>> Platform/Marvell/Armada/Armada70x0.dsc | 2 +-
>> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 41
>> ++++++++++++--------
>> Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h | 2 +
>> Platform/Marvell/Include/Protocol/SpiFlash.h | 3 ++
>> Platform/Marvell/Marvell.dec | 2 +-
>> 7 files changed, 48 insertions(+), 61 deletions(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index 664411a..d70645d 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -94,28 +94,16 @@ SpiFlashProbe (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT32 IdBuffer, Id, RefId;
>> + UINT8 *FlashId;
>>
>> - Id = PcdGet32 (PcdSpiFlashId);
>> -
>> - IdBuffer = CMD_READ_ID & 0xff;
>> + FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>>
>> // 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, NOR_FLASH_ID_DEFAULT_LEN,
>> FlashId);
>
> Is the length not possible to calculate somehow?
> Having a MAX_LEN defined and then using a DEFAULT_LEN or explicitly
> extracting 3 bytes from somewhere feels suboptimal.
>
I know. It is however a change that was somewhat artificially
extracted, so that to make the next patch more readable
(NOR_FLASH_ID_DEFAULT_LEN is removed there). I will substitute it with
PcdGetSize (PcdSpiFlashId).
Marcin
> /
> Leif
>
>> + 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);
>> diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> index 9321f6b..a12f2ec 100644
>> --- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> +++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
>> @@ -166,37 +166,24 @@ FlashProbe (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT8 IdBuffer[4];
>> - UINT32 Id, RefId;
>> + UINT8 *FlashId;
>>
>> - Id = PcdGet32 (PcdSpiFlashId);
>> + FlashId = (UINT8 *)PcdGetPtr (PcdSpiFlashId);
>>
>> - IdBuffer[0] = CMD_READ_ID;
>> -
>> - SpiFlashProtocol->ReadId (
>> - Slave,
>> - 4,
>> - IdBuffer
>> - );
>> -
>> - RefId = (IdBuffer[0] << 16) + (IdBuffer[1] << 8) + IdBuffer[2];
>> + Status = SpiFlashProtocol->ReadId (Slave, NOR_FLASH_ID_DEFAULT_LEN,
>> FlashId);
>> + 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/Armada/Armada70x0.dsc
>> b/Platform/Marvell/Armada/Armada70x0.dsc
>> index 0396e8e..4d5f55f 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -94,7 +94,7 @@
>> gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|3
>> gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
>> gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
>> + 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 6f7d9c7..ab3ed6a 100755
>> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> @@ -409,28 +409,35 @@ MvSpiFlashReadId (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT8 *DataOut;
>> + UINT8 Id[NOR_FLASH_MAX_ID_LEN];
>> + UINT8 Cmd;
>> +
>> + Cmd = CMD_READ_ID;
>> + Status = SpiMasterProtocol->ReadWrite (SpiMasterProtocol,
>> + SpiDev,
>> + &Cmd,
>> + SPI_CMD_LEN,
>> + NULL,
>> + Id,
>> + NOR_FLASH_MAX_ID_LEN);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_ERROR, "ReadId: Spi transfer error\n"));
>> + return Status;
>> + }
>>
>> - DataOut = (UINT8 *) AllocateZeroPool (DataByteCount);
>> - if (DataOut == NULL) {
>> - DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
>> - return EFI_OUT_OF_RESOURCES;
>> + if (CompareMem (Id, Buffer, DataByteCount) != 0) {
>> + Status = EFI_NOT_FOUND;
>> }
>> - 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"));
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_ERROR,
>> + "%a: Unrecognized JEDEC Id bytes: 0x%02x%02x%02x\n",
>> + __FUNCTION__,
>> + Id[0],
>> + Id[1],
>> + Id[2]));
>> return Status;
>> }
>>
>> - // Bytes 1,2 and 3 contain SPI flash ID
>> - Buffer[0] = DataOut[1];
>> - Buffer[1] = DataOut[2];
>> - Buffer[2] = DataOut[3];
>> -
>> - FreePool (DataOut);
>> -
>> return EFI_SUCCESS;
>> }
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
>> index f90abb7..2583484 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/Include/Protocol/SpiFlash.h
>> b/Platform/Marvell/Include/Protocol/SpiFlash.h
>> index 743bb87..e0d62cc 100644
>> --- a/Platform/Marvell/Include/Protocol/SpiFlash.h
>> +++ b/Platform/Marvell/Include/Protocol/SpiFlash.h
>> @@ -47,6 +47,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> DAMAGE.
>> #define CMD_ERASE_64K 0xd8
>> #define CMD_4B_ADDR_ENABLE 0xb7
>>
>> +#define NOR_FLASH_MAX_ID_LEN 6
>> +#define NOR_FLASH_ID_DEFAULT_LEN 3
>> +
>> extern EFI_GUID gMarvellSpiFlashProtocolGuid;
>>
>> typedef struct _MARVELL_SPI_FLASH_PROTOCOL MARVELL_SPI_FLASH_PROTOCOL;
>> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
>> index cd800c8..679a9d0 100644
>> --- a/Platform/Marvell/Marvell.dec
>> +++ b/Platform/Marvell/Marvell.dec
>> @@ -133,7 +133,7 @@
>> gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x3000054
>> gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x3000055
>> gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x3000059
>> - gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x3000056
>> + gMarvellTokenSpaceGuid.PcdSpiFlashId|{ 0x0 }|VOID*|0x3000056
>> gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x3000057
>> gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x3000058
>>
>> --
>> 2.7.4
>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
