Re: [PATCH] Platform/Armada/Pp2Dxe: switch to ArmDmaLib

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

On Fri, Aug 25, 2017 at 01:25:54PM +0100, Ard Biesheuvel wrote:
> Instead of hardcoding the non-cache coherent nature of this device
> by invoking UncachedMemoryAllocationLib directly for allocating
> shared buffers, switch to DmaLib, which encapsulates this at a
> more abstract level. This allows the driver to be shared with
> platforms that are cache coherent (by simply switching to another
> DmaLib implementation), and removes the hardcoded dependency on
> UncachedMemoryAllocationLib, which will be removed from upstream
> EDK2.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Ah, and there is the non-bogus fix :)
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
> This depends on patches #1 and #2 of
>  https://lists.01.org/pipermail/edk2-devel/2017-August/013740.html
> 
>  Platform/Marvell/Armada/Armada.dsc.inc         |  1 -
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   | 35 ++++++++++++++------
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h   |  2 +-
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |  4 +--
>  4 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index 4e8f289fcbca..04b108619ac7 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -37,7 +37,6 @@
>    UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>  
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>  
>  # Basic utility libraries
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> index 8de2473d09f0..1e2ccd0dbe9d 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> @@ -175,9 +175,10 @@ Pp2DxeBmPoolInit (
>    VOID
>    )
>  {
> -  INTN Index;
> -  UINT8 *PoolAddr;
> -  UINT32 PoolSize = (sizeof(VOID *) * MVPP2_BM_SIZE) * 2 + MVPP2_BM_POOL_PTR_ALIGN;
> +  INTN          Index;
> +  UINT8         *PoolAddr;
> +  UINT32        PoolSize;
> +  EFI_STATUS    Status;
>  
>    ASSERT(MVPP2_BM_POOL_PTR_ALIGN >= sizeof(UINTN));
>  
> @@ -194,11 +195,16 @@ Pp2DxeBmPoolInit (
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  PoolAddr = UncachedAllocateAlignedZeroPool (PoolSize, MVPP2_BM_POOL_PTR_ALIGN);
> -  if (PoolAddr == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> +  Status = DmaAllocateAlignedBuffer (EfiBootServicesData,
> +                                     EFI_SIZE_TO_PAGES (PoolSize),
> +                                     MVPP2_BM_POOL_PTR_ALIGN,
> +                                     (VOID **)&PoolAddr);
> +  if (EFI_ERROR (Status)) {
> +    goto FreePools;
>    }
>  
> +  ZeroMem (PoolAddr, PoolSize);
> +
>    Mvpp2Shared->BmPools->Id = MVPP2_BM_POOL;
>    Mvpp2Shared->BmPools->VirtAddr = (UINT32 *)PoolAddr;
>    Mvpp2Shared->BmPools->PhysAddr = (UINTN)PoolAddr;
> @@ -206,6 +212,10 @@ Pp2DxeBmPoolInit (
>    Mvpp2BmPoolHwCreate(Mvpp2Shared, Mvpp2Shared->BmPools, MVPP2_BM_SIZE);
>  
>    return EFI_SUCCESS;
> +
> +FreePools:
> +  FreePool (Mvpp2Shared->BmPools);
> +  return Status;
>  }
>  
>  /* Enable and fill BM pool */
> @@ -1169,12 +1179,17 @@ Pp2DxeInitialise (
>    Mvpp2Shared->Tclk = PcdGet32 (PcdPp2ClockFrequency);
>  
>    /* Prepare buffers */
> -  BufferSpace = UncachedAllocateAlignedZeroPool (BD_SPACE, MVPP2_BUFFER_ALIGN_SIZE);
> -  if (BufferSpace == NULL) {
> -    DEBUG((DEBUG_ERROR, "Failed to allocate buffer space\n"));
> -    return EFI_OUT_OF_RESOURCES;
> +  Status = DmaAllocateAlignedBuffer (EfiBootServicesData,
> +                                     EFI_SIZE_TO_PAGES (BD_SPACE),
> +                                     MVPP2_BUFFER_ALIGN_SIZE,
> +                                     &BufferSpace);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to allocate buffer space\n"));
> +    return Status;
>    }
>  
> +  ZeroMem (BufferSpace, BD_SPACE);
> +
>    BufferLocation.TxDescs = BufferSpace;
>    BufferLocation.AggrTxDescs = (MVPP2_TX_DESC *)((UINTN)BufferSpace + MVPP2_MAX_TXD * sizeof(MVPP2_TX_DESC));
>    BufferLocation.RxDescs = (MVPP2_RX_DESC *)((UINTN)BufferSpace +
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> index 3bb0c4a65376..a179638fd609 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
> @@ -46,13 +46,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/BaseLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/DmaLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/NetLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiLib.h>
> -#include <Library/UncachedMemoryAllocationLib.h>
>  
>  #include "Mvpp2LibHw.h"
>  
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> index 87cc5e8ded74..9052fe27b7f3 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> @@ -43,13 +43,14 @@
>    Mvpp2Lib.c
>  
>  [Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> -  ArmPlatformPkg/ArmPlatformPkg.dec
>    ArmPkg/ArmPkg.dec
>    Platform/Marvell/Marvell.dec
>  
>  [LibraryClasses]
> +  DmaLib
>    IoLib
>    PcdLib
>    BaseLib
> @@ -60,7 +61,6 @@
>    UefiDriverEntryPoint
>    UefiBootServicesTableLib
>    MemoryAllocationLib
> -  UncachedMemoryAllocationLib
>    CacheMaintenanceLib
>  
>  [Protocols]
> -- 
> 2.11.0
>