Re: [platforms PATCH 10/25] Marvell/Drivers: Pp2Dxe: Switch to use MARVELL_BOARD_DESC protocol

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

On Fri, Jun 08, 2018 at 05:34:08PM +0200, Marcin Wojtas wrote:
> Pp2Dxe driver used to get Armada7k8k PP2 controller description from
> hardcoded values stored in the header file MvHwDescLib.h.
> As a result it is very hard to support other Armada SoC families
> with this driver.
> 
> This patch updates the driver to get PP2 controller description from
> newly introduced MARVELL_BOARD_DESC protocol, and removes the dependency
> on the hardcoded structures.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Reviewed-by: Hua Jing <jinghua@marvell.com>
> ---
>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   | 43 ++++++++------------
>  Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |  2 +-
>  Silicon/Marvell/Include/Library/MvHwDescLib.h | 26 ------------
>  3 files changed, 19 insertions(+), 52 deletions(-)
> 
> diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> index 3ed10f6..4ddce22 100644
> --- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> +++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> @@ -32,6 +32,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  *******************************************************************************/
>  
> +#include <Protocol/BoardDesc.h>
>  #include <Protocol/DevicePath.h>
>  #include <Protocol/DriverBinding.h>
>  #include <Protocol/SimpleNetwork.h>
> @@ -42,7 +43,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/DebugLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/MemoryAllocationLib.h>
> -#include <Library/MvHwDescLib.h>
>  #include <Library/NetLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> @@ -54,8 +54,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  #define ReturnUnlock(tpl, status) do { gBS->RestoreTPL (tpl); return (status); } while(0)
>  
> -DECLARE_A7K8K_PP2_TEMPLATE;
> -
>  STATIC PP2_DEVICE_PATH Pp2DevicePathTemplate = {
>    {
>      {
> @@ -1343,35 +1341,28 @@ Pp2DxeInitialise (
>    IN EFI_SYSTEM_TABLE *SystemTable
>    )
>  {
> -  MVHW_PP2_DESC *Desc = &mA7k8kPp2DescTemplate;
> -  UINT8 *Pp2DeviceTable, Index;
> +  MARVELL_BOARD_DESC_PROTOCOL *BoardDescProtocol;
> +  MV_BOARD_PP2_DESC *Pp2BoardDesc;
>    MVPP2_SHARED *Mvpp2Shared;
>    EFI_STATUS Status;
> +  UINT8 Index;

UINTN please (if we're already changing things).

/
    Leif

>  
>    /* Obtain table with enabled Pp2 devices */
> -  Pp2DeviceTable = (UINT8 *)PcdGetPtr (PcdPp2Controllers);
> -  if (Pp2DeviceTable == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Missing PcdPp2Controllers\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  if (PcdGetSize (PcdPp2Controllers) > MVHW_MAX_PP2_DEVS) {
> -    DEBUG ((DEBUG_ERROR, "Wrong PcdPp2Controllers format\n"));
> -    return EFI_INVALID_PARAMETER;
> +  Status = gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid,
> +                  NULL,
> +                  (VOID **)&BoardDescProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
> -  /* Check amount of declared ports */
> -  if (PcdGetSize (PcdPp2Port2Controller) > Desc->Pp2DevCount * MVPP2_MAX_PORT) {
> -    DEBUG ((DEBUG_ERROR, "Pp2Dxe: Wrong too many ports declared\n"));
> -    return EFI_INVALID_PARAMETER;
> +  Status = BoardDescProtocol->BoardDescPp2Get (BoardDescProtocol,
> +                                &Pp2BoardDesc);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
>    /* Initialize enabled chips */
> -  for (Index = 0; Index < PcdGetSize (PcdPp2Controllers); Index++) {
> -    if (!MVHW_DEV_ENABLED (Pp2, Index)) {
> -      DEBUG ((DEBUG_ERROR, "Skip Pp2 controller %d\n", Index));
> -      continue;
> -    }
> +  for (Index = 0; Index < Pp2BoardDesc->Pp2DevCount; Index++) {
>  
>      /* Initialize private data */
>      Mvpp2Shared = AllocateZeroPool (sizeof (MVPP2_SHARED));
> @@ -1383,8 +1374,8 @@ Pp2DxeInitialise (
>      Status = Pp2DxeInitialiseController (
>                      Index,
>                      Mvpp2Shared,
> -                    Desc->Pp2BaseAddresses[Index],
> -                    Desc->Pp2ClockFrequency[Index]
> +                    Pp2BoardDesc[Index].SoC->Pp2BaseAddress,
> +                    Pp2BoardDesc[Index].SoC->Pp2ClockFrequency
>                      );
>      if (EFI_ERROR(Status)) {
>        FreePool (Mvpp2Shared);
> @@ -1393,5 +1384,7 @@ Pp2DxeInitialise (
>      }
>    }
>  
> +  BoardDescProtocol->BoardDescFree (Pp2BoardDesc);
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> index fcd0611..be536ab 100644
> --- a/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> +++ b/Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
> @@ -67,11 +67,11 @@
>    gEfiSimpleNetworkProtocolGuid
>    gEfiDevicePathProtocolGuid
>    gEfiCpuArchProtocolGuid
> +  gMarvellBoardDescProtocolGuid
>    gMarvellMdioProtocolGuid
>    gMarvellPhyProtocolGuid
>  
>  [Pcd]
> -  gMarvellTokenSpaceGuid.PcdPp2Controllers
>    gMarvellTokenSpaceGuid.PcdPp2GopIndexes
>    gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
>    gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
> diff --git a/Silicon/Marvell/Include/Library/MvHwDescLib.h b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> index 34d03d4..5fd514c 100644
> --- a/Silicon/Marvell/Include/Library/MvHwDescLib.h
> +++ b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> @@ -105,17 +105,6 @@ typedef struct {
>  } MVHW_NONDISCOVERABLE_DESC;
>  
>  //
> -// PP2 NIC devices description template definition
> -//
> -#define MVHW_MAX_PP2_DEVS         4
> -
> -typedef struct {
> -  UINT8 Pp2DevCount;
> -  UINTN Pp2BaseAddresses[MVHW_MAX_PP2_DEVS];
> -  UINTN Pp2ClockFrequency[MVHW_MAX_PP2_DEVS];
> -} MVHW_PP2_DESC;
> -
> -//
>  // Platform description of CommonPhy devices
>  //
>  #define MVHW_CP0_COMPHY_BASE       0xF2441000
> @@ -200,19 +189,4 @@ MVHW_NONDISCOVERABLE_DESC mA7k8kNonDiscoverableDescTemplate = {\
>    { NonDiscoverableDeviceDmaTypeCoherent, NonDiscoverableDeviceDmaTypeCoherent }\
>  }
>  
> -//
> -// Platform description of Pp2 NIC devices
> -//
> -#define MVHW_CP0_PP2_BASE       0xF2000000
> -#define MVHW_CP1_PP2_BASE       0xF4000000
> -#define MVHW_PP2_CLK_FREQ       333333333
> -
> -#define DECLARE_A7K8K_PP2_TEMPLATE \
> -STATIC \
> -MVHW_PP2_DESC mA7k8kPp2DescTemplate = {\
> -  2,\
> -  { MVHW_CP0_PP2_BASE, MVHW_CP1_PP2_BASE },\
> -  { MVHW_PP2_CLK_FREQ, MVHW_PP2_CLK_FREQ } \
> -}
> -
>  #endif /* __MVHWDESCLIB_H__ */
> -- 
> 2.7.4
>