[edk2] [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter

Marcin Wojtas posted 11 patches 7 years, 3 months ago
[edk2] [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter
Posted by Marcin Wojtas 7 years, 3 months ago
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/Documentation/PortingGuide.txt    |  3 +++
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
 .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
 Platform/Marvell/Marvell.dec                       |  1 +
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
index 3b79bd2..f637fee 100644
--- a/Platform/Marvell/Documentation/PortingGuide.txt
+++ b/Platform/Marvell/Documentation/PortingGuide.txt
@@ -297,6 +297,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)
 
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index 9a04493..f3fdba4 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 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;
 
@@ -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..646598a 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 SPI_ERASE_SIZE_4K               4096
+#define SPI_ERASE_SIZE_32K              32768
+#define SPI_ERASE_SIZE_64K              65536
+
 #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
-- 
1.8.3.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter
Posted by Leif Lindholm 7 years, 3 months ago
On Fri, Sep 01, 2017 at 03:08:21PM +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.

Another thing I immediately wish I had never learned.
Thanks :/

> 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/Documentation/PortingGuide.txt    |  3 +++
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
>  Platform/Marvell/Marvell.dec                       |  1 +
>  5 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> index 3b79bd2..f637fee 100644
> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> @@ -297,6 +297,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)
>  
> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> index 9a04493..f3fdba4 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 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;
>  
> @@ -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..646598a 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 SPI_ERASE_SIZE_4K               4096
> +#define SPI_ERASE_SIZE_32K              32768
> +#define SPI_ERASE_SIZE_64K              65536
> +

Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?

/
    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
> -- 
> 1.8.3.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter
Posted by Marcin Wojtas 7 years, 3 months ago
2017-09-01 17:21 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Sep 01, 2017 at 03:08:21PM +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.
>
> Another thing I immediately wish I had never learned.
> Thanks :/

Do you mean the code or flash quirks? :)

>
>> 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/Documentation/PortingGuide.txt    |  3 +++
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
>>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
>>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
>>  Platform/Marvell/Marvell.dec                       |  1 +
>>  5 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
>> index 3b79bd2..f637fee 100644
>> --- a/Platform/Marvell/Documentation/PortingGuide.txt
>> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
>> @@ -297,6 +297,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)
>>
>> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
>> index 9a04493..f3fdba4 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 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;
>>
>> @@ -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..646598a 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 SPI_ERASE_SIZE_4K               4096
>> +#define SPI_ERASE_SIZE_32K              32768
>> +#define SPI_ERASE_SIZE_64K              65536
>> +
>
> Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?
>

Sure, will do.

Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 09/11] Drivers/Spi/Devices/MvSpiFlash: Fix usage of erase size parameter
Posted by Leif Lindholm 7 years, 3 months ago
On Fri, Sep 01, 2017 at 05:25:00PM +0200, Marcin Wojtas wrote:
> 2017-09-01 17:21 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Fri, Sep 01, 2017 at 03:08:21PM +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.
> >
> > Another thing I immediately wish I had never learned.
> > Thanks :/
> 
> Do you mean the code or flash quirks? :)

The flash quirks...

/
    Leif

> >> 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/Documentation/PortingGuide.txt    |  3 +++
> >>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c  | 26 +++++++++++++++++-----
> >>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h  |  6 +++++
> >>  .../Marvell/Drivers/Spi/Devices/MvSpiFlash.inf     |  1 +
> >>  Platform/Marvell/Marvell.dec                       |  1 +
> >>  5 files changed, 31 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Platform/Marvell/Documentation/PortingGuide.txt b/Platform/Marvell/Documentation/PortingGuide.txt
> >> index 3b79bd2..f637fee 100644
> >> --- a/Platform/Marvell/Documentation/PortingGuide.txt
> >> +++ b/Platform/Marvell/Documentation/PortingGuide.txt
> >> @@ -297,6 +297,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)
> >>
> >> diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
> >> index 9a04493..f3fdba4 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 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;
> >>
> >> @@ -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..646598a 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 SPI_ERASE_SIZE_4K               4096
> >> +#define SPI_ERASE_SIZE_32K              32768
> >> +#define SPI_ERASE_SIZE_64K              65536
> >> +
> >
> > Maybe just replace these with SIZE_4KB, SIZE_32KB and SIZE_64KB?
> >
> 
> Sure, will do.
> 
> Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel