[edk2] Platform/Armada/Pp2Dxe: switch to ArmDmaLib

Message ID 20170825122554.17819-1-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • [edk2] Platform/Armada/Pp2Dxe: switch to ArmDmaLib
Related show

Commit Message

Ard Biesheuvel Aug. 25, 2017, 12:25 p.m.
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>

---
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(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Aug. 29, 2017, 3:46 p.m. | #1
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

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Marcin Wojtas Aug. 29, 2017, 3:47 p.m. | #2
2017-08-29 17:46 GMT+02:00 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>


Please let me just run this on the HW before merge - I'll do it tomorrow.

Best regards,
Marcin

>

>> ---

>> 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

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 29, 2017, 3:48 p.m. | #3
On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:
> 2017-08-29 17:46 GMT+02:00 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>

>

> Please let me just run this on the HW before merge - I'll do it tomorrow.

>


Sure. Note that you will need to sync EDK2 after I merged the DmaLib
changes (which I will do today)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Marcin Wojtas Aug. 29, 2017, 3:53 p.m. | #4
2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>> 2017-08-29 17:46 GMT+02:00 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>

>>

>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>

>

> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

> changes (which I will do today)


Ok, please let know once it's available and also it would be great if
this patch could be shared via github repo.

Thanks,
Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 29, 2017, 5:01 p.m. | #5
On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:
> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>> 2017-08-29 17:46 GMT+02:00 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>

>>>

>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>

>>

>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>> changes (which I will do today)

>

> Ok, please let know once it's available and also it would be great if

> this patch could be shared via github repo.

>


The prerequisite patches for DmaLib are in EDK2 now.

https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 30, 2017, 1:26 p.m. | #6
On 29 August 2017 at 18:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:

>> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>>> 2017-08-29 17:46 GMT+02:00 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>

>>>>

>>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>>

>>>

>>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>>> changes (which I will do today)

>>

>> Ok, please let know once it's available and also it would be great if

>> this patch could be shared via github repo.

>>

>

> The prerequisite patches for DmaLib are in EDK2 now.

>

> https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib


Note that I just updated this branch to use NonCoherentDmaLib rather
than ArmDmaLib
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Marcin Wojtas Aug. 31, 2017, 1:35 p.m. | #7
Hi Ard,

2017-08-30 15:26 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 29 August 2017 at 18:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:

>>> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>>>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>>>> 2017-08-29 17:46 GMT+02:00 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>

>>>>>

>>>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>>>

>>>>

>>>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>>>> changes (which I will do today)

>>>

>>> Ok, please let know once it's available and also it would be great if

>>> this patch could be shared via github repo.

>>>

>>

>> The prerequisite patches for DmaLib are in EDK2 now.

>>

>> https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib

>

> Note that I just updated this branch to use NonCoherentDmaLib rather

> than ArmDmaLib


Network continues to work fine on Armada 7040 DB, so

Tested-by: Marcin Wojtas <mw@semihalf.com>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 31, 2017, 1:40 p.m. | #8
On 31 August 2017 at 14:35, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi Ard,

>

> 2017-08-30 15:26 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>> On 29 August 2017 at 18:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>>> On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:

>>>> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>>>>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>>>>> 2017-08-29 17:46 GMT+02:00 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>

>>>>>>

>>>>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>>>>

>>>>>

>>>>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>>>>> changes (which I will do today)

>>>>

>>>> Ok, please let know once it's available and also it would be great if

>>>> this patch could be shared via github repo.

>>>>

>>>

>>> The prerequisite patches for DmaLib are in EDK2 now.

>>>

>>> https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib

>>

>> Note that I just updated this branch to use NonCoherentDmaLib rather

>> than ArmDmaLib

>

> Network continues to work fine on Armada 7040 DB, so

>

> Tested-by: Marcin Wojtas <mw@semihalf.com>


Thank you!
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Aug. 31, 2017, 1:51 p.m. | #9
On 30 August 2017 at 14:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 29 August 2017 at 18:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:

>>> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>>>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>>>> 2017-08-29 17:46 GMT+02:00 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>

>>>>>

>>>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>>>

>>>>

>>>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>>>> changes (which I will do today)

>>>

>>> Ok, please let know once it's available and also it would be great if

>>> this patch could be shared via github repo.

>>>

>>

>> The prerequisite patches for DmaLib are in EDK2 now.

>>

>> https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib

>

> Note that I just updated this branch to use NonCoherentDmaLib rather

> than ArmDmaLib


For clarity - my Reviewed-by: still applies.

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 31, 2017, 1:58 p.m. | #10
On 31 August 2017 at 14:51, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On 30 August 2017 at 14:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> On 29 August 2017 at 18:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>>> On 29 August 2017 at 16:53, Marcin Wojtas <mw@semihalf.com> wrote:

>>>> 2017-08-29 17:48 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:

>>>>> On 29 August 2017 at 16:47, Marcin Wojtas <mw@semihalf.com> wrote:

>>>>>> 2017-08-29 17:46 GMT+02:00 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>

>>>>>>

>>>>>> Please let me just run this on the HW before merge - I'll do it tomorrow.

>>>>>>

>>>>>

>>>>> Sure. Note that you will need to sync EDK2 after I merged the DmaLib

>>>>> changes (which I will do today)

>>>>

>>>> Ok, please let know once it's available and also it would be great if

>>>> this patch could be shared via github repo.

>>>>

>>>

>>> The prerequisite patches for DmaLib are in EDK2 now.

>>>

>>> https://git.linaro.org/people/ard.biesheuvel/edk2-platforms.git/log/?h=mvpp2-dmalib

>>

>> Note that I just updated this branch to use NonCoherentDmaLib rather

>> than ArmDmaLib

>

> For clarity - my Reviewed-by: still applies.

>


Thank. Pushed as bff9700855e43e4948fb4f7249b9a4fb5451072e
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

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]