Re: [PATCH edk2-platforms 3/3] Silicon/SynQuacerPlatformFlashAccessLib: replace progress indication

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

On Thu, Jun 07, 2018 at 01:32:03PM +0200, Ard Biesheuvel wrote:
> Replace the home cooked progress indication with calls into the new
> (*Progress)() argument to PerformFlashWriteWithProgress(), which
> allows the flash access routine to report progress via a platform
> provided callback.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I think 1-2 depends on changes that aren't currently happening based
on discussions. If I'm wrong, set me straight.

However, does this one not make sense on its own?
If so, it has
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c                        |  1 +
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c   | 44 ++++++--------------
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf |  1 -
>  3 files changed, 14 insertions(+), 32 deletions(-)
> 
> diff --git a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
> index 38f1830b5c2e..c5e46dcd4ddf 100644
> --- a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
> +++ b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
> @@ -72,6 +72,7 @@ PerformFlashWriteWithProgress (
>  {
>    EFI_STATUS                      Status;
>    AMD_ISCP_DXE_PROTOCOL           *IscpDxeProtocol;
> +  UINTN                           Remaining;
>  
>    if (FlashAddressType != FlashAddressTypeRelativeAddress) {
>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeRelativeAddress supported\n",
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
> index 48d385993b38..ebb6ce189aa5 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
> @@ -17,7 +17,6 @@
>  #include <PiDxe.h>
>  
>  #include <Library/BaseMemoryLib.h>
> -#include <Library/BootLogoLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DxeServicesTableLib.h>
>  #include <Library/MemoryAllocationLib.h>
> @@ -195,17 +194,7 @@ PerformFlashWriteWithProgress (
>    EFI_LBA                             Lba;
>    EFI_PHYSICAL_ADDRESS                FvbBaseAddress;
>    UINTN                               NumBytes;
> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black;
> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;
> -  UINTN                               Resolution;
> -  UINTN                               CurrentProgress;
> -  BOOLEAN                             HaveBootGraphics;
> -
> -  Black.Raw = 0x00000000;
> -  White.Raw = 0x00FFFFFF;
> -
> -  Status = BootLogoEnableLogo ();
> -  HaveBootGraphics = !EFI_ERROR (Status);
> +  UINTN                               Remaining;
>  
>    if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {
>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeAbsoluteAddress supported\n",
> @@ -274,14 +263,8 @@ PerformFlashWriteWithProgress (
>      return Status;
>    }
>  
> -  if (HaveBootGraphics) {
> -    Resolution = (BlockSize * 100) / Length + 1;
> -    CurrentProgress = 0;
> -
> -    Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,
> -               L"Updating firmware - please wait", Black.Pixel, 100, 0);
> -  } else {
> -    Print (L"Updating firmware - please wait ");
> +  if (Progress == NULL) {
> +    Print (L"\n\nUpdating firmware - please wait ");
>    }
>  
>    //
> @@ -298,7 +281,8 @@ PerformFlashWriteWithProgress (
>      return Status;
>    }
>  
> -  while (Length > 0) {
> +  Remaining = Length;
> +  while (Remaining > 0) {
>      //
>      // Write the new data
>      //
> @@ -315,20 +299,18 @@ PerformFlashWriteWithProgress (
>        }
>      }
>  
> -    if (HaveBootGraphics) {
> -      Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,
> -                L"Updating firmware - please wait", White.Pixel,
> -                CurrentProgress + Resolution, CurrentProgress);
> -      CurrentProgress += Resolution;
> +    Buffer += BlockSize;
> +    Remaining -= BlockSize;
> +    Lba++;
> +
> +    if (Progress != NULL) {
> +      Progress (EndPercentage -
> +                (Remaining * (EndPercentage - StartPercentage)) / Length);
>      } else {
>        Print (L".");
>      }
> -
> -    Buffer += BlockSize;
> -    Length -= BlockSize;
> -    Lba++;
>    }
> -  if (!HaveBootGraphics) {
> +  if (Progress == NULL) {
>      Print (L"\n");
>    }
>  
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
> index 4dfa11372a38..6ca34ada1d03 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
> @@ -33,7 +33,6 @@ [Protocols]
>  
>  [LibraryClasses]
>    BaseMemoryLib
> -  BootLogoLib
>    DebugLib
>    DxeServicesTableLib
>    UefiBootServicesTableLib
> -- 
> 2.17.0
>