Re: [platforms: PATCH 09/10] Marvell/Drivers: MvSpiFlash: Fix usage of erase size parameter

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Sep 25, 2017 at 03:51:58AM +0200, Marcin Wojtas wrote:
> Although, hitherto support allowed for using configurable EraseSize,
> the erase command was fixed to CMD_ERASE_64K. Also it was
> assumed that EraseSize equals SectorSize, which is not true
> for some flash devices. Fix both issues by adding new PCD
> (gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
> this parameter properly in MvSpiFlashUpdate routine instead
> of the EraseSize. Also erase command is adjusted to the settings.
> Update PortingGuide accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c   | 26 +++++++++++++++-----
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h   |  6 +++++
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf |  1 +
>  Platform/Marvell/Marvell.dec                        |  1 +
>  Silicon/Marvell/Documentation/PortingGuide.txt      |  3 +++
>  5 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index 9a04493..c411296 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> @@ -191,7 +191,21 @@ MvSpiFlashErase (
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  Cmd[0] = CMD_ERASE_64K;
> +  switch (EraseSize) {
> +  case SIZE_4K:
> +    Cmd[0] = CMD_ERASE_4K;
> +    break;
> +  case SIZE_32K:
> +    Cmd[0] = CMD_ERASE_32K;
> +    break;
> +  case 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;
>  
> @@ -353,14 +367,14 @@ MvSpiFlashUpdate (
>    )
>  {
>    EFI_STATUS Status;
> -  UINT64 EraseSize, ToUpdate, Scale = 1;
> +  UINT64 SectorSize, ToUpdate, Scale = 1;
>    UINT8 *TmpBuf, *End;
>  
> -  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
> +  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
>  
>    End = Buf + ByteCount;
>  
> -  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
> +  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
>    if (TmpBuf == NULL) {
>      DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
>      return EFI_OUT_OF_RESOURCES;
> @@ -370,9 +384,9 @@ MvSpiFlashUpdate (
>      Scale = (End - Buf) / 100;
>  
>    for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
> -    ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
> +    ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
>      Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
> -    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, EraseSize);
> +    Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, SectorSize);
>  
>      if (EFI_ERROR (Status)) {
>        DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> index 3889643..49cce43 100755
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
> @@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define CMD_READ_ARRAY_FAST             0x0b
>  #define CMD_PAGE_PROGRAM                0x02
>  #define CMD_BANK_WRITE                  0xc5
> +#define CMD_ERASE_4K                    0x20
> +#define CMD_ERASE_32K                   0x52
>  #define CMD_ERASE_64K                   0xd8
>  #define CMD_4B_ADDR_ENABLE              0xb7
>  
> @@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define SPI_TRANSFER_BEGIN              0x01  // Assert CS before transfer
>  #define SPI_TRANSFER_END                0x02  // Deassert CS after transfers
>  
> +#define SIZE_4K                         4096
> +#define SIZE_32K                        32768
> +#define SIZE_64K                        65536
> +

Sorry, this was me being unclear: my meaning was "please just use the
existing defines from BaseLib".

If deleting this hunk is sufficient to resolve this, I can fold that
in when pushing.

/
    Leif

>  #define SPI_FLASH_16MB_BOUN             0x1000000
>  
>  typedef enum {
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> index d035d47..4519b02 100644
> --- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> +++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
> @@ -58,6 +58,7 @@
>    gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
>    gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>    gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
> +  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
>  
>  [Protocols]
>    gMarvellSpiMasterProtocolGuid
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index 869e376..fc00f1a 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -127,6 +127,7 @@
>    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
> diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
> index 2be658e..83ebe9d 100644
> --- a/Silicon/Marvell/Documentation/PortingGuide.txt
> +++ b/Silicon/Marvell/Documentation/PortingGuide.txt
> @@ -298,6 +298,9 @@ Folowing PCDs for spi flash driver configuration must be set properly:
>    - gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
>  	(Size of SPI flash page)
>  
> +  - gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
> +	(Size of SPI flash sector, 65536 bytes by default)
> +
>    - gMarvellTokenSpaceGuid.PcdSpiFlashId
>  	(Id of SPI flash)
>  
> -- 
> 1.8.3.1
>