Re: [platforms: PATCH v2 17/25] Marvell/Library: ComPhyLib: Switch library to use MARVELL_BOARD_DESC

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

On Sun, Jun 17, 2018 at 10:11:57PM +0200, Marcin Wojtas wrote:
> MvComPhyLib library used to get Armada7k8k SerDes multiplexing
> 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 library.
> 
> This patch updates the library, so that it can obtain the
> 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: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Silicon/Marvell/Include/Library/MvHwDescLib.h | 39 -----------
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c | 74 ++++++++++++--------
>  2 files changed, 45 insertions(+), 68 deletions(-)
> 
> diff --git a/Silicon/Marvell/Include/Library/MvHwDescLib.h b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> index 9f383f4..423ca17 100644
> --- a/Silicon/Marvell/Include/Library/MvHwDescLib.h
> +++ b/Silicon/Marvell/Include/Library/MvHwDescLib.h
> @@ -35,8 +35,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #ifndef __MVHWDESCLIB_H__
>  #define __MVHWDESCLIB_H__
>  
> -#include <Library/MvComPhyLib.h>
> -
>  //
>  // Helper macros
>  //
> @@ -45,20 +43,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #define MVHW_DEV_ENABLED(type, index) (type ## DeviceTable[index])
>  
>  //
> -// CommonPhy devices description template definition
> -//
> -#define MVHW_MAX_COMPHY_DEVS       4
> -
> -typedef struct {
> -  UINT8 ComPhyDevCount;
> -  UINTN ComPhyBaseAddresses[MVHW_MAX_COMPHY_DEVS];
> -  UINTN ComPhyHpipe3BaseAddresses[MVHW_MAX_COMPHY_DEVS];
> -  UINTN ComPhyLaneCount[MVHW_MAX_COMPHY_DEVS];
> -  UINTN ComPhyMuxBitCount[MVHW_MAX_COMPHY_DEVS];
> -  MV_COMPHY_CHIP_TYPE ComPhyChipType[MVHW_MAX_COMPHY_DEVS];
> -} MVHW_COMPHY_DESC;
> -
> -//
>  // I2C devices description template definition
>  //
>  #define MVHW_MAX_I2C_DEVS         4
> @@ -79,29 +63,6 @@ typedef struct {
>  } MVHW_MDIO_DESC;
>  
>  //
> -// Platform description of CommonPhy devices
> -//
> -#define MVHW_CP0_COMPHY_BASE       0xF2441000
> -#define MVHW_CP0_HPIPE3_BASE       0xF2120000
> -#define MVHW_CP0_COMPHY_LANES      6
> -#define MVHW_CP0_COMPHY_MUX_BITS   4
> -#define MVHW_CP1_COMPHY_BASE       0xF4441000
> -#define MVHW_CP1_HPIPE3_BASE       0xF4120000
> -#define MVHW_CP1_COMPHY_LANES      6
> -#define MVHW_CP1_COMPHY_MUX_BITS   4
> -
> -#define DECLARE_A7K8K_COMPHY_TEMPLATE \
> -STATIC \
> -MVHW_COMPHY_DESC mA7k8kComPhyDescTemplate = {\
> -  2,\
> -  { MVHW_CP0_COMPHY_BASE, MVHW_CP1_COMPHY_BASE },\
> -  { MVHW_CP0_HPIPE3_BASE, MVHW_CP1_HPIPE3_BASE },\
> -  { MVHW_CP0_COMPHY_LANES, MVHW_CP1_COMPHY_LANES },\
> -  { MVHW_CP0_COMPHY_MUX_BITS, MVHW_CP1_COMPHY_MUX_BITS },\
> -  { MvComPhyTypeCp110, MvComPhyTypeCp110 }\
> -}
> -
> -//
>  // Platform description of I2C devices
>  //
>  #define MVHW_CP0_I2C0_BASE       0xF2701000
> diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c
> index b03bc35..2ef9af4 100644
> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c
> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c
> @@ -34,9 +34,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  #include "ComPhyLib.h"
>  #include <Library/MvComPhyLib.h>
> -#include <Library/MvHwDescLib.h>
> -
> -DECLARE_A7K8K_COMPHY_TEMPLATE;
>  
>  CHAR16 * TypeStringTable [] = {L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2",
>                             L"PCIE3", L"SATA0", L"SATA1", L"SATA2", L"SATA3",
> @@ -182,22 +179,20 @@ VOID
>  InitComPhyConfig (
>    IN  OUT  CHIP_COMPHY_CONFIG *ChipConfig,
>    IN  OUT  PCD_LANE_MAP       *LaneData,
> -  IN       UINT8               Id
> +  IN       MV_BOARD_COMPHY_DESC *Desc
>    )
>  {
> -  MVHW_COMPHY_DESC *Desc = &mA7k8kComPhyDescTemplate;
> -
> -  ChipConfig->ChipType = Desc->ComPhyChipType[Id];
> -  ChipConfig->ComPhyBaseAddr = Desc->ComPhyBaseAddresses[Id];
> -  ChipConfig->Hpipe3BaseAddr = Desc->ComPhyHpipe3BaseAddresses[Id];
> -  ChipConfig->LanesCount = Desc->ComPhyLaneCount[Id];
> -  ChipConfig->MuxBitCount = Desc->ComPhyMuxBitCount[Id];
> -  ChipConfig->ChipId = Id;
> +  ChipConfig->ChipType = Desc->SoC->ComPhyChipType;
> +  ChipConfig->ComPhyBaseAddr = Desc->SoC->ComPhyBaseAddress;
> +  ChipConfig->Hpipe3BaseAddr = Desc->SoC->ComPhyHpipe3BaseAddress;
> +  ChipConfig->LanesCount = Desc->SoC->ComPhyLaneCount;
> +  ChipConfig->MuxBitCount = Desc->SoC->ComPhyMuxBitCount;
> +  ChipConfig->ChipId = Desc->SoC->ComPhyId;
>  
>    /*
>     * Below macro contains variable name concatenation (used to form PCD's name).
>     */
> -  switch (Id) {
> +  switch (ChipConfig->ChipId) {
>    case 0:
>      GetComPhyPcd (LaneData, 0);
>      break;
> @@ -219,32 +214,49 @@ MvComPhyInit (
>    )
>  {
>    EFI_STATUS Status;
> -  CHIP_COMPHY_CONFIG ChipConfig[MVHW_MAX_COMPHY_DEVS], *PtrChipCfg;
> -  PCD_LANE_MAP LaneData[MVHW_MAX_COMPHY_DEVS];
> +  CHIP_COMPHY_CONFIG *ChipConfig, *PtrChipCfg;
> +  MARVELL_BOARD_DESC_PROTOCOL *BoardDescProtocol;
> +  MV_BOARD_COMPHY_DESC *ComPhyBoardDesc;
> +  PCD_LANE_MAP *LaneData;
>    UINT32 Lane, MaxComphyCount;
> -  UINT8 *ComPhyDeviceTable, Index;
> +  UINTN Index;
>  
>    /* Obtain table with enabled ComPhy devices */
> -  ComPhyDeviceTable = (UINT8 *)PcdGetPtr (PcdComPhyDevices);
> -  if (ComPhyDeviceTable == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Missing PcdComPhyDevices\n"));
> -    return EFI_INVALID_PARAMETER;
> +  Status = gBS->LocateProtocol (&gMarvellBoardDescProtocolGuid,
> +                  NULL,
> +                  (VOID **)&BoardDescProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = BoardDescProtocol->BoardDescComPhyGet (BoardDescProtocol,
> +                                &ComPhyBoardDesc);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
> -  if (PcdGetSize (PcdComPhyDevices) > MVHW_MAX_COMPHY_DEVS) {
> -    DEBUG ((DEBUG_ERROR, "Wrong PcdComPhyDevices format\n"));
> -    return EFI_INVALID_PARAMETER;
> +  ChipConfig = AllocateZeroPool (ComPhyBoardDesc->ComPhyDevCount *
> +                                 sizeof (CHIP_COMPHY_CONFIG));
> +  if (ChipConfig == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> +    BoardDescProtocol->BoardDescFree (ComPhyBoardDesc);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  LaneData = AllocateZeroPool (ComPhyBoardDesc->ComPhyDevCount *
> +                               sizeof (PCD_LANE_MAP));
> +  if (ChipConfig == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> +    BoardDescProtocol->BoardDescFree (ComPhyBoardDesc);
> +    FreePool (ChipConfig);
> +    return EFI_OUT_OF_RESOURCES;
>    }
>  
>    /* Initialize enabled chips */
> -  for (Index = 0; Index < PcdGetSize (PcdComPhyDevices); Index++) {
> -    if (!MVHW_DEV_ENABLED (ComPhy, Index)) {
> -      DEBUG ((DEBUG_ERROR, "Skip ComPhy chip %d\n", Index));
> -      continue;
> -    }
> +  for (Index = 0; Index < ComPhyBoardDesc->ComPhyDevCount; Index++) {
>  
>      PtrChipCfg = &ChipConfig[Index];
> -    InitComPhyConfig(PtrChipCfg, LaneData, Index);
> +    InitComPhyConfig (PtrChipCfg, LaneData, &ComPhyBoardDesc[Index]);
>  
>      /* Get the count of the SerDes of the specific chip */
>      MaxComphyCount = PtrChipCfg->LanesCount;
> @@ -275,5 +287,9 @@ MvComPhyInit (
>      PtrChipCfg->Init (PtrChipCfg);
>    }
>  
> +  BoardDescProtocol->BoardDescFree (ComPhyBoardDesc);
> +  FreePool (ChipConfig);
> +  FreePool (LaneData);
> +
>    return EFI_SUCCESS;
>  }
> -- 
> 2.7.4
>