Re: [edk2-platforms: PATCH 2/2] Marvell/Drivers: MvSpiFlashDxe: Implement progress bar

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

On Mon, Sep 09, 2019 at 05:52:12PM +0200, Patryk Duda wrote:
> This patch implements TFTP-like progress bar in
> MvSpiFlashUpdateWithProgress. This is necessary because
> CapsuleRuntimeDxe uses DxeRuntimeCapsuleLib which uses
> CapsuleProcessLibNull implementation of capsule process.

I would prefer not adding more progress output code into individual
drivers. Could this driver reuse
MdeModulePkg/Library/DisplayUpdateProgressLibText/ instead?

/
    Leif

> Signed-off-by: Patryk Duda <pdk@semihalf.com>
> ---
>  Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf |  1 +
>  Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.h   |  1 +
>  Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c   | 57 ++++++++++++++++----
>  3 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf
> index c6e93b82a1..088e9592e2 100644
> --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf
> +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf
> @@ -23,6 +23,7 @@
>    DebugLib
>    MemoryAllocationLib
>    NorFlashInfoLib
> +  PrintLib
>    TimerLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.h b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.h
> index 21877ba2a6..d51892c439 100755
> --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.h
> +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.h
> @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #include <Library/IoLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
> diff --git a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c
> index db12adb764..91e195b97c 100755
> --- a/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c
> +++ b/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c
> @@ -6,6 +6,25 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  *******************************************************************************/
>  #include "MvSpiFlashDxe.h"
>  
> +// Frame for the progression slider
> +STATIC CONST CHAR16 mProgressFrame[] = L"[                                        ]";
> +
> +// Number of steps in the progression slider
> +#define PROGRESS_SLIDER_STEPS  ((sizeof (mProgressFrame) / sizeof (CHAR16)) - 3)
> +
> +// Size in number of characters plus one (final zero) of the message to
> +// indicate the progress of update. The format is "[(progress slider:
> +// 40 characters)] (nb of KBytes written so far: 7 characters) Kb". There
> +// are thus the number of characters in mProgressFrame[] plus 11 characters
> +// (2 // spaces, "Kb" and seven characters for the number of KBytes).
> +#define PROGRESS_MESSAGE_SIZE  ((sizeof (mProgressFrame) / sizeof (CHAR16)) + 12)
> +
> +// String to delete progress message to be able to update it :
> +// (PROGRESS_MESSAGE_SIZE-1) '\b'
> +STATIC CONST CHAR16 mProgressDelete[] = L"\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b"
> +                                         "\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b"
> +                                         "\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b";
> +
>  STATIC EFI_EVENT            mMvSpiFlashVirtualAddrChangeEvent;
>  MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
>  SPI_FLASH_INSTANCE  *mSpiFlashInstance;
> @@ -380,12 +399,15 @@ MvSpiFlashUpdateWithProgress (
>    IN UINTN                                          EndPercentage
>    )
>  {
> +  CHAR16 ProgressBar[PROGRESS_MESSAGE_SIZE];
> +  UINTN ProgressIndex;
>    EFI_STATUS Status;
>    UINTN SectorSize;
>    UINTN SectorNum;
>    UINTN ToUpdate;
> -  UINTN Index;
>    UINT8 *TmpBuf;
> +  UINTN Index;
> +  UINTN Step;
>  
>    SectorSize = Slave->Info->SectorSize;
>    SectorNum = (ByteCount / SectorSize) + 1;
> @@ -397,12 +419,9 @@ MvSpiFlashUpdateWithProgress (
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  for (Index = 0; Index < SectorNum; Index++) {
> -    if (Progress != NULL) {
> -      Progress (StartPercentage +
> -                ((Index * (EndPercentage - StartPercentage)) / SectorNum));
> -    }
> +  Print(L"%s       0 Kb", mProgressFrame);
>  
> +  for (Index = 0; Index < SectorNum; Index++) {
>      // In the last chunk update only an actual number of remaining bytes.
>      if (Index + 1 == SectorNum) {
>        ToUpdate = ByteCount % SectorSize;
> @@ -418,12 +437,32 @@ MvSpiFlashUpdateWithProgress (
>        DEBUG ((DEBUG_ERROR, "%a: Error while updating\n", __FUNCTION__));
>        return Status;
>      }
> +
> +    ProgressBar[0] = L'\0';
> +    Step = ((Index * SectorSize + ToUpdate) * PROGRESS_SLIDER_STEPS) / ByteCount;
> +
> +    Print (L"%s", mProgressDelete);
> +
> +    Status = StrCpyS (ProgressBar, PROGRESS_MESSAGE_SIZE, mProgressFrame);
> +    if (EFI_ERROR(Status)) {
> +      return Status;
> +    }
> +    for (ProgressIndex = 1; ProgressIndex < Step; ProgressIndex++) {
> +      ProgressBar[ProgressIndex] = L'=';
> +    }
> +    ProgressBar[Step] = L'>';
> +    UnicodeSPrint (
> +      ProgressBar + (sizeof (mProgressFrame) / sizeof (CHAR16)) - 1,
> +      sizeof (ProgressBar) - sizeof (mProgressFrame),
> +      L" %7d Kb",
> +      (Index * SectorSize + ToUpdate) / 1024
> +      );
> +
> +    Print(L"%s", ProgressBar);
>    }
>    FreePool (TmpBuf);
>  
> -  if (Progress != NULL) {
> -    Progress (EndPercentage);
> -  }
> +  Print (L"\n");
>  
>    return EFI_SUCCESS;
>  }
> -- 
> 2.21.0
>