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

Marcin Wojtas posted 25 patches 6 years, 11 months ago
[edk2] [platforms PATCH 17/25] Marvell/Library: ComPhyLib: Switch library to use MARVELL_BOARD_DESC
Posted by Marcin Wojtas 6 years, 11 months ago
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: Hua Jing <jinghua@marvell.com>
---
 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..8555c4c 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;
+  UINT8 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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms PATCH 17/25] Marvell/Library: ComPhyLib: Switch library to use MARVELL_BOARD_DESC
Posted by Leif Lindholm 6 years, 11 months ago
On Fri, Jun 08, 2018 at 05:34:15PM +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: Hua Jing <jinghua@marvell.com>
> ---
>  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..8555c4c 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;
> +  UINT8 Index;

UINTN.

/
    Leif

>  
>    /* 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
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel