public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal
@ 2017-10-08 10:56 Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing Marcin Wojtas
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Hi,

This is a second version of the patchset with the complete
removal of ParsePcdLib and cleanup of boards' PCD representation.
According to v1 remarks, I modified PCD names and introduced macros
in Armada.dsc.inc, so that the readability of boards .dsc could
increse.
More details can befound in the changelog and commits.

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/parsepcd-upstream-r20171008

I'm looking forward to your comments or remarks.

Best regards,
Marcin

Changelog:
v1  -> v2
1/5
  * Add new [Defines] section was added to Armada.dsc.inc
    file in order to increase readability.

2/5
  * s/PcdI2cControllers/PcdI2cControllersEnabled/

3/5
  * s/PcdUtmiControllers/PcdUtmiControllersEnabled/
  * Add defines for PcdUtmiPortType

4/5:
  * s/PcdMdioControllers/PcdMdioControllersEnabled/
  * Add macros for network PHY type and speed in Armada.dsc.inc

5/5
  * Add RBs

Marcin Wojtas (5):
  Marvell/Library: ComPhyLib: Remove PCD string parsing
  Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib
  Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  Marvell/Drivers: Pp2Dxe: Rework PHY handling
  Platform/Marvell/Armada: Remove ParsePcdLib

 Platform/Marvell/Armada/Armada.dsc.inc                 |  68 +++++-
 Platform/Marvell/Armada/Armada70x0.dsc                 |  30 +--
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c       |  42 ++--
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf     |   3 +-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c     |  35 ++-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf   |   3 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c   | 122 ++++++-----
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h   |   2 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf |   4 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c           |  16 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h           |   2 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf         |   4 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h         |  95 ++++++++
 Platform/Marvell/Include/Library/ParsePcdLib.h         |  46 ----
 Platform/Marvell/Include/Protocol/Mdio.h               |   6 +
 Platform/Marvell/Include/Protocol/MvPhy.h              |   1 +
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.c         |  65 +-----
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.h         |  25 +--
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf       |   1 -
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c     | 228 --------------------
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf   |  50 -----
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c       | 150 ++++++-------
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h       |   1 -
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf     |  11 +-
 Platform/Marvell/Marvell.dec                           |  17 +-
 Silicon/Marvell/Documentation/PortingGuide.txt         | 165 ++++++++------
 26 files changed, 528 insertions(+), 664 deletions(-)
 delete mode 100644 Platform/Marvell/Include/Library/ParsePcdLib.h
 delete mode 100644 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
 delete mode 100644 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing
  2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
@ 2017-10-08 10:56 ` Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib Marcin Wojtas
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Simplify obtaining lane data, using arrays with direct enum values,
rather than strings. This is another step to completely remove
ParsePcdLib.

This patch replaces string-based description of ComPhy lanes
on Armada 70x0 DB with the enum values of type and speed -
for that purpose new [Defines] section was added to Armada.dsc.inc
file in order to increase readability.
PortingGuide is updated accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada.dsc.inc           | 44 +++++++++++++
 Platform/Marvell/Armada/Armada70x0.dsc           | 11 +++-
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.c   | 65 ++++---------------
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.h   | 25 +++-----
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf |  1 -
 Silicon/Marvell/Documentation/PortingGuide.txt   | 67 +++++++++++++++-----
 6 files changed, 124 insertions(+), 89 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 9549091..cd26506 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -479,3 +479,47 @@
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
       gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
+
+################################################################################
+#
+# Defines - platform description macros
+#
+################################################################################
+[Defines]
+  # ComPhy speed
+  DEFINE CP_1_25G           = 0x1
+  DEFINE CP_1_5G            = 0x2
+  DEFINE CP_2_5G            = 0x3
+  DEFINE CP_3G              = 0x4
+  DEFINE CP_3_125G          = 0x5
+  DEFINE CP_5G              = 0x6
+  DEFINE CP_5_15625G        = 0x7
+  DEFINE CP_6G              = 0x8
+  DEFINE CP_6_25G           = 0x9
+  DEFINE CP_10_3125G        = 0xA
+
+  # ComPhy type
+  DEFINE CP_UNCONNECTED     = 0x0
+  DEFINE CP_PCIE0           = 0x1
+  DEFINE CP_PCIE1           = 0x2
+  DEFINE CP_PCIE2           = 0x3
+  DEFINE CP_PCIE3           = 0x4
+  DEFINE CP_SATA0           = 0x5
+  DEFINE CP_SATA1           = 0x6
+  DEFINE CP_SATA2           = 0x7
+  DEFINE CP_SATA3           = 0x8
+  DEFINE CP_SGMII0          = 0x9
+  DEFINE CP_SGMII1          = 0xA
+  DEFINE CP_SGMII2          = 0xB
+  DEFINE CP_SGMII3          = 0xC
+  DEFINE CP_QSGMII          = 0xD
+  DEFINE CP_USB3_HOST0      = 0xE
+  DEFINE CP_USB3_HOST1      = 0xF
+  DEFINE CP_USB3_DEVICE     = 0x10
+  DEFINE CP_XAUI0           = 0x11
+  DEFINE CP_XAUI1           = 0x12
+  DEFINE CP_XAUI2           = 0x13
+  DEFINE CP_XAUI3           = 0x14
+  DEFINE CP_RXAUI0          = 0x15
+  DEFINE CP_RXAUI1          = 0x16
+  DEFINE CP_SFI             = 0x17
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index 467dfa3..dae9715 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -100,8 +100,15 @@
 
   #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
-  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
+  # ComPhy0
+  # 0: SGMII1        1.25 Gbps
+  # 1: USB3_HOST0    5 Gbps
+  # 2: SFI           10.31 Gbps
+  # 3: SATA1         5 Gbps
+  # 4: USB3_HOST1    5 Gbps
+  # 5: PCIE2         5 Gbps
+  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{ $(CP_SGMII1), $(CP_USB3_HOST0), $(CP_SFI), $(CP_SATA1), $(CP_USB3_HOST1), $(CP_PCIE2) }
+  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
 
   #UtmiPhy
   gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
diff --git a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
index 3eb5d9f..bf21dca 100644
--- a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
+++ b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
@@ -113,47 +113,6 @@ RegSetSilent16(
   MmioWrite16 (Addr, RegData);
 }
 
-/* This function returns enum with SerDesType */
-UINT32
-ParseSerdesTypeString (
-  CHAR16* String
-  )
-{
-  UINT32 i;
-
-  if (String == NULL)
-    return COMPHY_TYPE_INVALID;
-
-  for (i = 0; i < COMPHY_TYPE_MAX; i++) {
-    if (StrCmp (String, TypeStringTable[i]) == 0) {
-      return i;
-    }
-  }
-
-  /* PCD string doesn't match any supported SerDes Type */
-  return COMPHY_TYPE_INVALID;
-}
-
-/* This function converts SerDes speed in MHz to enum with SerDesSpeed */
-UINT32
-ParseSerdesSpeed (
-  UINT32 Value
-  )
-{
-  UINT32 i;
-  UINT32 ValueTable [] = {0, 1250, 1500, 2500, 3000, 3125,
-                          5000, 5156, 6000, 6250, 10310};
-
-  for (i = 0; i < COMPHY_SPEED_MAX; i++) {
-    if (Value == ValueTable[i]) {
-      return i;
-    }
-  }
-
-  /* PCD SerDes speed value doesn't match any supported SerDes speed */
-  return COMPHY_SPEED_INVALID;
-}
-
 CHAR16 *
 GetTypeString (
   UINT32 Type
@@ -182,7 +141,8 @@ GetSpeedString (
 
 VOID
 ComPhyPrint (
-  IN CHIP_COMPHY_CONFIG *PtrChipCfg
+  IN CHIP_COMPHY_CONFIG *PtrChipCfg,
+  IN UINT8 Index
   )
 {
   UINT32 Lane;
@@ -191,7 +151,7 @@ ComPhyPrint (
   for (Lane = 0; Lane < PtrChipCfg->LanesCount; Lane++) {
     SpeedStr = GetSpeedString(PtrChipCfg->MapData[Lane].Speed);
     TypeStr = GetTypeString(PtrChipCfg->MapData[Lane].Type);
-    DEBUG((DEBUG_ERROR, "Comphy-%d: %-13s %-10s\n", Lane, TypeStr, SpeedStr));
+    DEBUG ((DEBUG_ERROR, "Comphy%d-%d: %-13s %-10s\n", Index, Lane, TypeStr, SpeedStr));
   }
 
   DEBUG((DEBUG_ERROR, "\n"));
@@ -238,16 +198,16 @@ InitComPhyConfig (
    */
   switch (Id) {
   case 0:
-    GetComPhyPcd (ChipConfig, LaneData, 0);
+    GetComPhyPcd (LaneData, 0);
     break;
   case 1:
-    GetComPhyPcd (ChipConfig, LaneData, 1);
+    GetComPhyPcd (LaneData, 1);
     break;
   case 2:
-    GetComPhyPcd (ChipConfig, LaneData, 2);
+    GetComPhyPcd (LaneData, 2);
     break;
   case 3:
-    GetComPhyPcd (ChipConfig, LaneData, 3);
+    GetComPhyPcd (LaneData, 3);
     break;
   }
 }
@@ -288,12 +248,9 @@ MvComPhyInit (
     /* Get the count of the SerDes of the specific chip */
     MaxComphyCount = PtrChipCfg->LanesCount;
     for (Lane = 0; Lane < MaxComphyCount; Lane++) {
-      /* Parse PCD with string indicating SerDes Type */
-      PtrChipCfg->MapData[Lane].Type =
-        ParseSerdesTypeString (LaneData[Index].TypeStr[Lane]);
-      PtrChipCfg->MapData[Lane].Speed =
-        ParseSerdesSpeed (LaneData[Index].SpeedValue[Lane]);
-      PtrChipCfg->MapData[Lane].Invert = (UINT32)LaneData[Index].InvFlag[Lane];
+      PtrChipCfg->MapData[Lane].Type = LaneData[Index].Type[Lane];
+      PtrChipCfg->MapData[Lane].Speed = LaneData[Index].SpeedValue[Lane];
+      PtrChipCfg->MapData[Lane].Invert = LaneData[Index].InvFlag[Lane];
 
       if ((PtrChipCfg->MapData[Lane].Speed == COMPHY_SPEED_INVALID) ||
           (PtrChipCfg->MapData[Lane].Speed == COMPHY_SPEED_ERROR) ||
@@ -311,7 +268,7 @@ MvComPhyInit (
      return Status;
     }
 
-    ComPhyPrint (PtrChipCfg);
+    ComPhyPrint (PtrChipCfg, Index);
 
     /* PHY power UP sequence */
     PtrChipCfg->Init (PtrChipCfg);
diff --git a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.h b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.h
index 3898978..5899a4a 100644
--- a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.h
+++ b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.h
@@ -43,7 +43,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <Library/MvComPhyLib.h>
 #include <Library/IoLib.h>
 #include <Library/TimerLib.h>
-#include <Library/ParsePcdLib.h>
 
 #define MAX_LANE_OPTIONS          10
 
@@ -52,14 +51,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define GET_LANE_SPEED(id)        PcdGetPtr(PcdChip##id##ComPhySpeeds)
 #define GET_LANE_INV(id)          PcdGetPtr(PcdChip##id##ComPhyInvFlags)
 
-#define FillLaneMap(chip_struct, lane_struct, id) { \
-  ParsePcdString((CHAR16 *) GET_LANE_TYPE(id), chip_struct[id].LanesCount, NULL, lane_struct[id].TypeStr);     \
-  ParsePcdString((CHAR16 *) GET_LANE_SPEED(id), chip_struct[id].LanesCount, lane_struct[id].SpeedValue, NULL); \
-  ParsePcdString((CHAR16 *) GET_LANE_INV(id), chip_struct[id].LanesCount, lane_struct[id].InvFlag, NULL);      \
-}
-
-#define GetComPhyPcd(chip_struct, lane_struct, id) {               \
-  FillLaneMap(chip_struct, lane_struct, id);                       \
+#define GetComPhyPcd(lane_struct, id) {                      \
+  lane_struct[id].Type = (UINT8 *)GET_LANE_TYPE(id);         \
+  lane_struct[id].SpeedValue = (UINT8 *)GET_LANE_SPEED(id);  \
+  lane_struct[id].InvFlag = (UINT8 *)GET_LANE_SPEED(id);     \
 }
 
 /***** ComPhy *****/
@@ -573,15 +568,15 @@ typedef struct {
 } COMPHY_MUX_DATA;
 
 typedef struct {
-  UINT32 Type;
-  UINT32 Speed;
-  UINT32 Invert;
+  UINT8 Type;
+  UINT8 Speed;
+  UINT8 Invert;
 } COMPHY_MAP;
 
 typedef struct {
-  CHAR16 *TypeStr[MAX_LANE_OPTIONS];
-  UINTN SpeedValue[MAX_LANE_OPTIONS];
-  UINTN InvFlag[MAX_LANE_OPTIONS];
+  UINT8 *Type;
+  UINT8 *SpeedValue;
+  UINT8 *InvFlag;
 } PCD_LANE_MAP;
 
 typedef
diff --git a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
index e0f4634..c223fe5 100644
--- a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
+++ b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
@@ -51,7 +51,6 @@
   MemoryAllocationLib
   PcdLib
   IoLib
-  ParsePcdLib
 
 [Sources.common]
   ComPhyLib.c
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
index 83ebe9d..25cb66b 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -57,35 +57,68 @@ Every ComPhy PCD has <Num> part where <Num> stands for chip ID (order is not
 important, but configuration will be set for first PcdComPhyChipCount chips).
 
 Every chip has 3 ComPhy PCDs and three of them comprise per-board lanes
-settings for this chip. Their format is unicode string, containing settings
-for up to 10 lanes. Setting for each one is separated with semicolon.
-These PCDs together describe outputs of PHY integrated in simple cihp.
-Below is example for the first chip (Chip0).
+settings for this chip. Their format is array of up to 10 values reflecting
+defined numbers for SPEED/TYPE/INVERT, whose description can be found in:
 
-  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
-	(Unicode string indicating PHY types. Currently supported are:
+  OpenPlatformPkg/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
 
-		{ L"unconnected", L"PCIE0", L"PCIE1", L"PCIE2", L"PCIE3",
-		  L"SATA0", L"SATA1", L"SATA2", L"SATA3", L"SGMII0",
-		  L"SGMII1", L"SGMII2", L"SGMII3", L"QSGMII",
-		  L"USB3_HOST0", L"USB3_HOST1", L"USB3_DEVICE",
-		  L"XAUI0", L"XAUI1", L"XAUI2", L"XAUI3", L"RXAUI0",
-		  L"RXAUI1", L"KR" } )
+  - gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes
+	(Array of types - currently supported are:
+
+	CP_UNCONNECTED                      0x0
+	CP_PCIE0                            0x1
+	CP_PCIE1                            0x2
+	CP_PCIE2                            0x3
+	CP_PCIE3                            0x4
+	CP_SATA0                            0x5
+	CP_SATA1                            0x6
+	CP_SATA2                            0x7
+	CP_SATA3                            0x8
+	CP_SGMII0                           0x9
+	CP_SGMII1                           0xA
+	CP_SGMII2                           0xB
+	CP_SGMII3                           0xC
+	CP_QSGMII                           0xD
+	CP_USB3_HOST0                       0xE
+	CP_USB3_HOST1                       0xF
+	CP_USB3_DEVICE                      0x10
+	CP_XAUI0                            0x11
+	CP_XAUI1                            0x12
+	CP_XAUI2                            0x13
+	CP_XAUI3                            0x14
+	CP_RXAUI0                           0x15
+	CP_RXAUI1                           0x16
+	CP_SFI                              0x17 )
 
   - gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds
-	(Indicates PHY speeds in MHz. Currently supported are:
-		{ 1250, 1500, 2500, 3000, 3125, 5000, 6000, 6250, 1031 }  )
+	(Array of speeds - currently supported are:
+
+	CP_1_25G                            0x1
+	CP_1_5G                             0x2
+	CP_2_5G                             0x3
+	CP_3G                               0x4
+	CP_3_125G                           0x5
+	CP_5G                               0x6
+	CP_5_15625G                         0x7
+	CP_6G                               0x8
+	CP_6_25G                            0x9
+	CP_10_3125G                         0xA )
 
   - gMarvellTokenSpaceGuid.PcdChip0ComPhyInvFlags
-	(Indicates lane polarity invert)
+	(Array of lane inversion types - currently supported are:
+
+	CP_NO_INVERT                        0x0
+	CP_TXD_INVERT                       0x1
+	CP_RXD_INVERT                       0x2
+	CP_ALL_INVERT                       0x3 )
 
 Example
 -------
 
 		  #ComPhy
 		  gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-		  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
-		  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
+		  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{ $(CP_SGMII1), $(CP_USB3_HOST0), $(CP_SFI), $(CP_SATA1), $(CP_USB3_HOST1), $(CP_PCIE2) }
+		  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
 
 
 PHY Driver configuration
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [platforms: PATCH v2 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib
  2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing Marcin Wojtas
@ 2017-10-08 10:56 ` Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: " Marcin Wojtas
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

This patch introduces I2c description, using the new structures
and template in MvHwDescLib. This change enables more flexible
addition of multiple I2c controllers and also allows for
removal of string PCD parsing. Update Armada 70x0 DB description
and PortingGuide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada70x0.dsc             |  2 +-
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   | 42 +++++++++++---------
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |  3 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h     | 25 ++++++++++++
 Platform/Marvell/Marvell.dec                       |  2 +-
 Silicon/Marvell/Documentation/PortingGuide.txt     |  4 +-
 6 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index dae9715..c11a973 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -78,7 +78,7 @@
   # I2C
   gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60 }
   gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0 }
-  gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
+  gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x1 }
   gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57 }
   gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdI2cClockFrequency|250000000
diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
index fa79ebc..d85ee0b 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
@@ -42,12 +42,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiLib.h>
-#include <Library/ParsePcdLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/MvHwDescLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
 #include "MvI2cDxe.h"
 
+DECLARE_A7K8K_I2C_TEMPLATE;
+
 STATIC MV_I2C_BAUD_RATE baud_rate;
 
 STATIC MV_I2C_DEVICE_PATH MvI2cDevicePathProtocol = {
@@ -172,35 +174,39 @@ MvI2cInitialise (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  MVHW_I2C_DESC *Desc = &mA7k8kI2cDescTemplate;
+  UINT8 *I2cDeviceTable, Index;
   EFI_STATUS Status;
-  UINT32 BusCount;
-  EFI_PHYSICAL_ADDRESS I2cBaseAddresses[PcdGet32 (PcdI2cBusCount)];
-  INTN i;
 
-  BusCount = PcdGet32 (PcdI2cBusCount);
-  if (BusCount == 0)
-    return EFI_SUCCESS;
+  /* Obtain table with enabled I2c devices */
+  I2cDeviceTable = (UINT8 *)PcdGetPtr (PcdI2cControllersEnabled);
+  if (I2cDeviceTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "Missing PcdI2cControllersEnabled\n"));
+    return EFI_INVALID_PARAMETER;
+  }
 
-  Status = ParsePcdString (
-      (CHAR16 *) PcdGetPtr (PcdI2cBaseAddresses),
-      BusCount,
-      I2cBaseAddresses,
-      NULL
-      );
-  if (EFI_ERROR(Status))
-    return Status;
+  if (PcdGetSize (PcdI2cControllersEnabled) > MVHW_MAX_I2C_DEVS) {
+    DEBUG ((DEBUG_ERROR, "Wrong PcdI2cControllersEnabled format\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  /* Initialize enabled chips */
+  for (Index = 0; Index < PcdGetSize (PcdI2cControllersEnabled); Index++) {
+    if (!MVHW_DEV_ENABLED (I2c, Index)) {
+      DEBUG ((DEBUG_ERROR, "Skip I2c chip %d\n", Index));
+      continue;
+    }
 
-  for (i = 0; i < BusCount; i++) {
     Status = MvI2cInitialiseController(
         ImageHandle,
         SystemTable,
-        I2cBaseAddresses[i]
+        Desc->I2cBaseAddresses[Index]
         );
     if (EFI_ERROR(Status))
       return Status;
   }
 
-  return Status;
+  return EFI_SUCCESS;
 }
 
 STATIC
diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
index 16374ef..80655f1 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
@@ -55,7 +55,6 @@
   UefiLib
   UefiDriverEntryPoint
   UefiBootServicesTableLib
-  ParsePcdLib
 
 [Protocols]
   gEfiI2cMasterProtocolGuid
@@ -66,7 +65,7 @@
 [Pcd]
   gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses
   gMarvellTokenSpaceGuid.PcdI2cSlaveBuses
-  gMarvellTokenSpaceGuid.PcdI2cBaseAddresses
+  gMarvellTokenSpaceGuid.PcdI2cControllersEnabled
   gMarvellTokenSpaceGuid.PcdI2cClockFrequency
   gMarvellTokenSpaceGuid.PcdI2cBaudRate
   gMarvellTokenSpaceGuid.PcdI2cBusCount
diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h
index 6a86865..e029b50 100644
--- a/Platform/Marvell/Include/Library/MvHwDescLib.h
+++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
@@ -60,6 +60,16 @@ typedef struct {
 } MVHW_COMPHY_DESC;
 
 //
+// I2C devices description template definition
+//
+#define MVHW_MAX_I2C_DEVS         4
+
+typedef struct {
+  UINT8 I2cDevCount;
+  UINTN I2cBaseAddresses[MVHW_MAX_I2C_DEVS];
+} MVHW_I2C_DESC;
+
+//
 // NonDiscoverable devices description template definition
 //
 #define MVHW_MAX_XHCI_DEVS         4
@@ -130,6 +140,21 @@ MVHW_COMPHY_DESC mA7k8kComPhyDescTemplate = {\
 }
 
 //
+// Platform description of I2C devices
+//
+#define MVHW_CP0_I2C0_BASE       0xF2701000
+#define MVHW_CP0_I2C1_BASE       0xF2701100
+#define MVHW_CP1_I2C0_BASE       0xF4701000
+#define MVHW_CP1_I2C1_BASE       0xF4701100
+
+#define DECLARE_A7K8K_I2C_TEMPLATE \
+STATIC \
+MVHW_I2C_DESC mA7k8kI2cDescTemplate = {\
+  4,\
+  { MVHW_CP0_I2C0_BASE, MVHW_CP0_I2C1_BASE, MVHW_CP1_I2C0_BASE, MVHW_CP1_I2C1_BASE }\
+}
+
+//
 // Platform description of NonDiscoverable devices
 //
 #define MVHW_CP0_XHCI0_BASE        0xF2500000
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index fc00f1a..d2ab0a9 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -113,7 +113,7 @@
   gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0 }|VOID*|0x3000184
   gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x0 }|VOID*|0x3000050
   gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0 }|VOID*|0x3000185
-  gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|{ 0x0 }|VOID*|0x3000047
+  gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x0 }|VOID*|0x3000047
   gMarvellTokenSpaceGuid.PcdI2cClockFrequency|0|UINT32|0x3000048
   gMarvellTokenSpaceGuid.PcdI2cBaudRate|0|UINT32|0x3000049
   gMarvellTokenSpaceGuid.PcdI2cBusCount|0|UINT32|0x3000183
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
index 25cb66b..b2bb595 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -189,8 +189,8 @@ In order to enable driver on a new platform, following steps need to be taken:
 		(buses to which accoring slaves are attached)
 	- gMarvellTokenSpaceGuid.PcdI2cBusCount|2
 		(number of SoC's I2C buses)
-	- gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
-		(base addresses of I2C controller buses)
+	- gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x1 }
+		(array with used controllers)
 	- gMarvellTokenSpaceGuid.PcdI2cClockFrequency|200000000
 		(I2C host controller clock frequency)
 	- gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib Marcin Wojtas
@ 2017-10-08 10:56 ` Marcin Wojtas
  2017-10-09 16:00   ` Leif Lindholm
  2017-10-08 10:56 ` [platforms: PATCH v2 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 5/5] Platform/Marvell/Armada: Remove ParsePcdLib Marcin Wojtas
  4 siblings, 1 reply; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

This patch introduces UTMI description, using the new structures
and template in MvHwDescLib. This change enables more flexible
addition of multiple CP with UTMI PHY's and also significantly
reduces amount of used PCD's for that purpose. Update PortingGuide
documentation accordingly.

This patch replaces string-based description of Utmi on
Armada 70x0 DB with new, reduced format, which uses macros
in Armada.dsc.inc file for better readability.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
 Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
 Platform/Marvell/Marvell.dec                       |   7 +-
 Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
 8 files changed, 148 insertions(+), 110 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index cd26506..7d0dc39 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -523,3 +523,8 @@
   DEFINE CP_RXAUI0          = 0x15
   DEFINE CP_RXAUI1          = 0x16
   DEFINE CP_SFI             = 0x17
+
+  #UTMI PHY connection type
+  DEFINE UTMI_USB_HOST0     = 0x0
+  DEFINE UTMI_USB_HOST1     = 0x1
+  DEFINE UTMI_USB_DEVICE0   = 0x2
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index c11a973..b40766b 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -111,11 +111,8 @@
   gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
 
   #UtmiPhy
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
+  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
+  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
 
   #MDIO
   gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h
index e029b50..6ad1bc2 100644
--- a/Platform/Marvell/Include/Library/MvHwDescLib.h
+++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
@@ -117,6 +117,19 @@ typedef struct {
 } MVHW_RTC_DESC;
 
 //
+// UTMI PHY's description template definition
+//
+
+typedef struct {
+  UINT8 UtmiDevCount;
+  UINT32 UtmiPhyId[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiBaseAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiConfigAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiUsbConfigAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiMuxBitCount[MVHW_MAX_XHCI_DEVS];
+} MVHW_UTMI_DESC;
+
+//
 // Platform description of CommonPhy devices
 //
 #define MVHW_CP0_COMPHY_BASE       0xF2441000
@@ -217,4 +230,38 @@ MVHW_RTC_DESC mA7k8kRtcDescTemplate = {\
   { SIZE_4KB, SIZE_4KB }\
 }
 
+//
+// Platform description of UTMI PHY's
+//
+#define MVHW_CP0_UTMI0_BASE            0xF2580000
+#define MVHW_CP0_UTMI0_CFG_BASE        0xF2440440
+#define MVHW_CP0_UTMI0_USB_CFG_BASE    0xF2440420
+#define MVHW_CP0_UTMI0_ID              0x0
+#define MVHW_CP0_UTMI1_BASE            0xF2581000
+#define MVHW_CP0_UTMI1_CFG_BASE        0xF2440444
+#define MVHW_CP0_UTMI1_USB_CFG_BASE    0xF2440420
+#define MVHW_CP0_UTMI1_ID              0x1
+#define MVHW_CP1_UTMI0_BASE            0xF4580000
+#define MVHW_CP1_UTMI0_CFG_BASE        0xF4440440
+#define MVHW_CP1_UTMI0_USB_CFG_BASE    0xF4440420
+#define MVHW_CP1_UTMI0_ID              0x0
+#define MVHW_CP1_UTMI1_BASE            0xF4581000
+#define MVHW_CP1_UTMI1_CFG_BASE        0xF4440444
+#define MVHW_CP1_UTMI1_USB_CFG_BASE    0xF4440420
+#define MVHW_CP1_UTMI1_ID              0x1
+
+#define DECLARE_A7K8K_UTMI_TEMPLATE \
+STATIC \
+MVHW_UTMI_DESC mA7k8kUtmiDescTemplate = {\
+  4,\
+  { MVHW_CP0_UTMI0_ID, MVHW_CP0_UTMI1_ID,\
+    MVHW_CP1_UTMI0_ID, MVHW_CP1_UTMI1_ID },\
+  { MVHW_CP0_UTMI0_BASE, MVHW_CP0_UTMI1_BASE,\
+    MVHW_CP1_UTMI0_BASE, MVHW_CP1_UTMI1_BASE },\
+  { MVHW_CP0_UTMI0_CFG_BASE, MVHW_CP0_UTMI1_CFG_BASE,\
+    MVHW_CP1_UTMI0_CFG_BASE, MVHW_CP1_UTMI1_CFG_BASE },\
+  { MVHW_CP0_UTMI0_USB_CFG_BASE, MVHW_CP0_UTMI1_USB_CFG_BASE,\
+    MVHW_CP1_UTMI0_USB_CFG_BASE, MVHW_CP1_UTMI1_USB_CFG_BASE }\
+}
+
 #endif /* __MVHWDESCLIB_H__ */
diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
index 95b5698..f1819c4 100644
--- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
+++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
@@ -33,12 +33,16 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *******************************************************************************/
 
 #include "UtmiPhyLib.h"
+#include <Library/MvHwDescLib.h>
+
+DECLARE_A7K8K_UTMI_TEMPLATE;
 
 typedef struct {
   EFI_PHYSICAL_ADDRESS UtmiBaseAddr;
   EFI_PHYSICAL_ADDRESS UsbCfgAddr;
   EFI_PHYSICAL_ADDRESS UtmiCfgAddr;
   UINT32 UtmiPhyPort;
+  UINT32 PhyId;
 } UTMI_PHY_DATA;
 
 STATIC
@@ -236,48 +240,52 @@ UtmiPhyPowerUp (
 STATIC
 VOID
 Cp110UtmiPhyInit (
-  IN UINT32 UtmiPhyCount,
   IN UTMI_PHY_DATA *UtmiData
   )
 {
-  UINT32 i;
+  EFI_STATUS Status;
 
-  for (i = 0; i < UtmiPhyCount; i++) {
-    UtmiPhyPowerDown(i, UtmiData[i].UtmiBaseAddr,
-      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
-      UtmiData[i].UtmiPhyPort);
-  }
+  UtmiPhyPowerDown (
+          UtmiData->PhyId,
+          UtmiData->UtmiBaseAddr,
+          UtmiData->UsbCfgAddr,
+          UtmiData->UtmiCfgAddr,
+          UtmiData->UtmiPhyPort
+          );
 
   /* Power down PLL */
   DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power down PLL\n"));
-  RegSet (UtmiData[0].UsbCfgAddr, 0x0 << UTMI_USB_CFG_PLL_OFFSET,
-    UTMI_USB_CFG_PLL_MASK);
-
-  for (i = 0; i < UtmiPhyCount; i++) {
-    UtmiPhyConfig(i, UtmiData[i].UtmiBaseAddr,
-      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
-      UtmiData[i].UtmiPhyPort);
+  MmioAnd32 (UtmiData->UsbCfgAddr, ~UTMI_USB_CFG_PLL_MASK);
+
+  UtmiPhyConfig (
+          UtmiData->PhyId,
+          UtmiData->UtmiBaseAddr,
+          UtmiData->UsbCfgAddr,
+          UtmiData->UtmiCfgAddr,
+          UtmiData->UtmiPhyPort
+          );
+
+  Status = UtmiPhyPowerUp (
+          UtmiData->PhyId,
+          UtmiData->UtmiBaseAddr,
+          UtmiData->UsbCfgAddr,
+          UtmiData->UtmiCfgAddr,
+          UtmiData->UtmiPhyPort
+          );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", UtmiData->PhyId));
+    return;
   }
 
-  for (i = 0; i < UtmiPhyCount; i++) {
-    if (EFI_ERROR(UtmiPhyPowerUp(i, UtmiData[i].UtmiBaseAddr,
-        UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
-        UtmiData[i].UtmiPhyPort))) {
-      DEBUG((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", i));
-      continue;
-    }
-    DEBUG((DEBUG_ERROR, "UTMI PHY %d initialized to ", i));
-
-    if (UtmiData[i].UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
-      DEBUG((DEBUG_ERROR, "USB Device\n"));
-    else
-      DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData[i].UtmiPhyPort));
-  }
+  DEBUG ((DEBUG_ERROR, "UTMI PHY %d initialized to ", UtmiData->PhyId));
+  if (UtmiData->UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
+    DEBUG((DEBUG_ERROR, "USB Device\n"));
+  else
+    DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData->UtmiPhyPort));
 
   /* Power up PLL */
   DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power up PLL\n"));
-  RegSet (UtmiData[0].UsbCfgAddr, 0x1 << UTMI_USB_CFG_PLL_OFFSET,
-    UTMI_USB_CFG_PLL_MASK);
+  MmioOr32 (UtmiData->UsbCfgAddr, UTMI_USB_CFG_PLL_MASK);
 }
 
 EFI_STATUS
@@ -285,69 +293,67 @@ UtmiPhyInit (
   VOID
   )
 {
-  EFI_STATUS Status;
-  UTMI_PHY_DATA UtmiData[PcdGet32 (PcdUtmiPhyCount)];
-  EFI_PHYSICAL_ADDRESS RegUtmiUnit[PcdGet32 (PcdUtmiPhyCount)];
-  EFI_PHYSICAL_ADDRESS RegUsbCfg[PcdGet32 (PcdUtmiPhyCount)];
-  EFI_PHYSICAL_ADDRESS RegUtmiCfg[PcdGet32 (PcdUtmiPhyCount)];
-  UINTN UtmiPort[PcdGet32 (PcdUtmiPhyCount)];
-  UINTN i, Count;
-
-  Count = PcdGet32 (PcdUtmiPhyCount);
-  if (Count == 0) {
+  UTMI_PHY_DATA UtmiData;
+  UINT8 *UtmiDeviceTable, *XhciDeviceTable, *UtmiPortType, Index;
+  MVHW_UTMI_DESC *Desc = &mA7k8kUtmiDescTemplate;
+
+  /* Obtain table with enabled Utmi PHY's*/
+  UtmiDeviceTable = (UINT8 *)PcdGetPtr (PcdUtmiControllersEnabled);
+  if (UtmiDeviceTable == NULL) {
     /* No UTMI PHY on platform */
     return EFI_SUCCESS;
   }
 
-  DEBUG((DEBUG_INFO, "UtmiPhy: Initialize USB UTMI PHYs\n"));
-  /* Parse UtmiPhy PCDs */
-  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiUnit),
-    Count, RegUtmiUnit, NULL);
-  if (EFI_ERROR(Status)) {
-    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiUnit format\n"));
+  if (PcdGetSize (PcdUtmiControllersEnabled) > MVHW_MAX_XHCI_DEVS) {
+    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiControllersEnabled format\n"));
     return EFI_INVALID_PARAMETER;
   }
 
-  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUsbCfg),
-    Count, RegUsbCfg, NULL);
-  if (EFI_ERROR(Status)) {
-    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUsbCfg format\n"));
+  /* Make sure XHCI controllers table is present */
+  XhciDeviceTable = (UINT8 *)PcdGetPtr (PcdPciEXhci);
+  if (XhciDeviceTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "UTMI: Missing PcdPciEXhci\n"));
     return EFI_INVALID_PARAMETER;
   }
 
-  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiCfg),
-    Count, RegUtmiCfg, NULL);
-  if (EFI_ERROR(Status)) {
-    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiCfg format\n"));
+  /* Obtain port type table */
+  UtmiPortType = (UINT8 *)PcdGetPtr (PcdUtmiPortType);
+  if (UtmiPortType == NULL ||
+      PcdGetSize (PcdUtmiPortType) != PcdGetSize (PcdUtmiControllersEnabled)) {
+    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiPortType format\n"));
     return EFI_INVALID_PARAMETER;
   }
 
-  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyUtmiPort),
-    Count, UtmiPort, NULL);
-  if (EFI_ERROR(Status)) {
-    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyUtmiPort format\n"));
-    return EFI_INVALID_PARAMETER;
-  }
+  /* Initialize enabled chips */
+  for (Index = 0; Index < PcdGetSize (PcdUtmiControllersEnabled); Index++) {
+    if (!MVHW_DEV_ENABLED (Utmi, Index)) {
+      continue;
+    }
+
+    /* UTMI PHY without enabled XHCI controller is useless */
+    if (!MVHW_DEV_ENABLED (Xhci, Index)) {
+      DEBUG ((DEBUG_ERROR, "UTMI: Disabled Xhci controller %d\n", Index));
+      return EFI_INVALID_PARAMETER;
+    }
 
-  for (i = 0 ; i < Count ; i++) {
     /* Get base address of UTMI phy */
-    UtmiData[i].UtmiBaseAddr = RegUtmiUnit[i];
+    UtmiData.UtmiBaseAddr = Desc->UtmiBaseAddresses[Index];
 
     /* Get usb config address */
-    UtmiData[i].UsbCfgAddr = RegUsbCfg[i];
+    UtmiData.UsbCfgAddr = Desc->UtmiUsbConfigAddresses[Index];
 
     /* Get UTMI config address */
-    UtmiData[i].UtmiCfgAddr = RegUtmiCfg[i];
+    UtmiData.UtmiCfgAddr = Desc->UtmiConfigAddresses[Index];
 
-    /*
-     * Get the usb port number, which will be used to check if
-     * the utmi connected to host or device
-     */
-    UtmiData[i].UtmiPhyPort = UtmiPort[i];
-  }
+    /* Get UTMI PHY ID */
+    UtmiData.PhyId = Desc->UtmiPhyId[Index];
 
-  /* Currently only Cp110 is supported */
-  Cp110UtmiPhyInit (Count, UtmiData);
+    /* Get the usb port type */
+    UtmiData.UtmiPhyPort = UtmiPortType[Index];
+
+    /* Currently only Cp110 is supported */
+    Cp110UtmiPhyInit (&UtmiData);
+  }
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
index f9b4933..0d7d72e 100644
--- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
+++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
@@ -42,7 +42,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <Library/MemoryAllocationLib.h>
 #include <Library/IoLib.h>
 #include <Library/TimerLib.h>
-#include <Library/ParsePcdLib.h>
 
 #define UTMI_USB_CFG_DEVICE_EN_OFFSET             0
 #define UTMI_USB_CFG_DEVICE_EN_MASK               (0x1 << UTMI_USB_CFG_DEVICE_EN_OFFSET)
diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
index f1e57f4..b56c43b 100644
--- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
+++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
@@ -50,15 +50,12 @@
   DebugLib
   IoLib
   MemoryAllocationLib
-  ParsePcdLib
   PcdLib
 
 [Sources.common]
   UtmiPhyLib.c
 
-[FixedPcd]
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
+[Pcd]
+  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
+  gMarvellTokenSpaceGuid.PcdUtmiPortType
+  gMarvellTokenSpaceGuid.PcdPciEXhci
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index d2ab0a9..e23607f 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -156,11 +156,8 @@
   gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags|{ 0x0 }|VOID*|0x30000177
 
 #UtmiPhy
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|0|UINT32|0x30000205
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|{ 0x0 }|VOID*|0x30000206
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|{ 0x0 }|VOID*|0x30000207
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|{ 0x0 }|VOID*|0x30000208
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|{ 0x0 }|VOID*|0x30000209
+  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x0 }|VOID*|0x30000206
+  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ 0x0 }|VOID*|0x30000207
 
 #MDIO
   gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0|UINT64|0x3000043
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
index b2bb595..fa429d1 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -279,33 +279,23 @@ UTMI PHY configuration
 ======================
 In order to configure UTMI, following PCDs are available:
 
-  - gMarvellTokenSpaceGuid.PcdUtmiPhyCount
-	(Indicates how many UTMI PHYs are available on platform)
-
-Next four PCDs are in unicode string format containing settings for all devices
-separated with semicolon.
-
-  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
-	(Indicates base address of the UTMI unit)
-
-  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
-	(Indicates address of USB Configuration register)
+  - gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
+	(Array with used controllers
+	 Set to 0x1 for enabled, 0x0 for disabled)
 
-  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
-	(Indicates address of external UTMI configuration)
+  - gMarvellTokenSpaceGuid.PcdUtmiPortType
+	(Indicates type of the connected USB port:
 
-  - gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
-	(Indicates type of the connected USB port)
+	UTMI_USB_HOST0                     0x0
+	UTMI_USB_HOST1                     0x1
+	UTMI_USB_DEVICE0                   0x2 )
 
 Example
 -------
 
 		# UtmiPhy
-		  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
-		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
-		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
-		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
-		  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
+		  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
+		  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
 
 
 SPI driver configuration
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [platforms: PATCH v2 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling
  2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
                   ` (2 preceding siblings ...)
  2017-10-08 10:56 ` [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: " Marcin Wojtas
@ 2017-10-08 10:56 ` Marcin Wojtas
  2017-10-08 10:56 ` [platforms: PATCH v2 5/5] Platform/Marvell/Armada: Remove ParsePcdLib Marcin Wojtas
  4 siblings, 0 replies; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Hitherto PHY handling in Pp2Dxe was not flexible. It allowed for using
only single MDIO controller, which may not be true on Armada 80x0 SoCs.
For this purpose introduce the MDIO description, using the new structures
and template in MvHwDescLib. This change enables addition of multiple
CP110 hardware blocks with MDIO controllers.

This change required different PHY handling and obtaining data over
desired MDIO bus. Now given Pp2 port is matched with the PHY via
its index in gMarvellTokenSpaceGuid.PcdPhyDeviceIds. The PHY itself
is mapped to the MDIO controller, using
gMarvellTokenSpaceGuid.PcdPhy2MdioController. Also obtaining
SMI addresses was moved to the PHY initialization routine.
All above allow for much cleaner and logical PHY description
in the .dsc file, which now uses macros for connection type
and speed.

Update PortingGuide documentation accordingly and Armada 70x0 DB
NIC/PHY description.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada.dsc.inc                 |  18 +++
 Platform/Marvell/Armada/Armada70x0.dsc                 |  10 +-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c     |  35 ++++--
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf   |   3 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c   | 122 ++++++++++++--------
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h   |   2 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf |   4 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c           |  16 +--
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h           |   2 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf         |   4 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h         |  23 ++++
 Platform/Marvell/Include/Protocol/Mdio.h               |   6 +
 Platform/Marvell/Include/Protocol/MvPhy.h              |   1 +
 Platform/Marvell/Marvell.dec                           |   8 +-
 Silicon/Marvell/Documentation/PortingGuide.txt         |  66 +++++------
 15 files changed, 203 insertions(+), 117 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 7d0dc39..7258017 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -524,6 +524,24 @@
   DEFINE CP_RXAUI1          = 0x16
   DEFINE CP_SFI             = 0x17
 
+  #Network interface speed
+  DEFINE PHY_SPEED_10       = 0x1
+  DEFINE PHY_SPEED_100      = 0x2
+  DEFINE PHY_SPEED_1000     = 0x3
+  DEFINE PHY_SPEED_2500     = 0x4
+  DEFINE PHY_SPEED_10000    = 0x5
+
+  #Network PHY type
+  DEFINE PHY_RGMII          = 0x0
+  DEFINE PHY_RGMII_ID       = 0x1
+  DEFINE PHY_RGMII_TXID     = 0x2
+  DEFINE PHY_RGMII_RXID     = 0x3
+  DEFINE PHY_SGMII          = 0x4
+  DEFINE PHY_RTBI           = 0x5
+  DEFINE PHY_XAUI           = 0x6
+  DEFINE PHY_RXAUI          = 0x7
+  DEFINE PHY_SFI            = 0x8
+
   #UTMI PHY connection type
   DEFINE UTMI_USB_HOST0     = 0x0
   DEFINE UTMI_USB_HOST1     = 0x1
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
index b40766b..430803c 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -115,18 +115,20 @@
   gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
 
   #MDIO
-  gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
+  gMarvellTokenSpaceGuid.PcdMdioControllersEnabled|{ 0x1, 0x0 }
 
   #PHY
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x8, 0x4, 0x0 }
+  gMarvellTokenSpaceGuid.PcdPhy2MdioController|{ 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
+  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
 
   #NET
-  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0xff, 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPp2GopIndexes|{ 0x0, 0x2, 0x3 }
   gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp|{ 0x0, 0x0, 0x0 }
-  gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ 0x5, 0x3, 0x3 }
+  gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ $(PHY_SPEED_10000), $(PHY_SPEED_1000), $(PHY_SPEED_1000) }
+  gMarvellTokenSpaceGuid.PcdPp2PhyConnectionTypes|{ $(PHY_SFI), $(PHY_SGMII), $(PHY_RGMII) }
+  gMarvellTokenSpaceGuid.PcdPp2PhyIndexes|{ 0xFF, 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPp2Port2Controller|{ 0x0, 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdPp2PortIds|{ 0x0, 0x1, 0x2 }
   gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x1 }
diff --git a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
index ae466d7..12aabad 100644
--- a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
+++ b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
@@ -46,7 +46,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "MvMdioDxe.h"
 
-UINT64 MdioBase = 0;
+DECLARE_A7K8K_MDIO_TEMPLATE;
 
 STATIC
 EFI_STATUS
@@ -70,7 +70,7 @@ MdioCheckParam (
 STATIC
 EFI_STATUS
 MdioWaitReady (
-  VOID
+  UINT32 MdioBase
   )
 {
   UINT32 Timeout = MVEBU_SMI_TIMEOUT;
@@ -92,7 +92,7 @@ MdioWaitReady (
 STATIC
 EFI_STATUS
 MdioWaitValid (
-  VOID
+  UINT32 MdioBase
   )
 {
   UINT32 Timeout = MVEBU_SMI_TIMEOUT;
@@ -116,11 +116,13 @@ EFI_STATUS
 MdioOperation (
   IN CONST MARVELL_MDIO_PROTOCOL *This,
   IN UINT32 PhyAddr,
+  IN UINT32 MdioIndex,
   IN UINT32 RegOff,
   IN BOOLEAN Write,
   IN OUT UINT32 *Data
   )
 {
+  UINT32 MdioBase = This->BaseAddresses[MdioIndex];
   UINT32 MdioReg;
   EFI_STATUS Status;
 
@@ -131,7 +133,7 @@ MdioOperation (
   }
 
   /* wait till the SMI is not busy */
-  Status = MdioWaitReady ();
+  Status = MdioWaitReady (MdioBase);
   if (EFI_ERROR(Status)) {
     DEBUG((DEBUG_ERROR, "MdioDxe: MdioWaitReady error\n"));
     return Status;
@@ -151,7 +153,7 @@ MdioOperation (
   MdioRegWrite32 (MdioReg, MdioBase);
 
   /* make sure that the write transaction  is over */
-  Status = Write ? MdioWaitReady () : MdioWaitValid ();
+  Status = Write ? MdioWaitReady (MdioBase) : MdioWaitValid (MdioBase);
   if (EFI_ERROR(Status)) {
     DEBUG((DEBUG_ERROR, "MdioDxe: MdioWaitReady error\n"));
     return Status;
@@ -169,6 +171,7 @@ EFI_STATUS
 MvMdioRead (
   IN CONST MARVELL_MDIO_PROTOCOL *This,
   IN UINT32 PhyAddr,
+  IN UINT32 MdioIndex,
   IN UINT32 RegOff,
   IN UINT32 *Data
   )
@@ -178,6 +181,7 @@ MvMdioRead (
   Status = MdioOperation (
             This,
             PhyAddr,
+            MdioIndex,
             RegOff,
             FALSE,
             Data
@@ -190,6 +194,7 @@ EFI_STATUS
 MvMdioWrite (
   IN CONST MARVELL_MDIO_PROTOCOL *This,
   IN UINT32 PhyAddr,
+  IN UINT32 MdioIndex,
   IN UINT32 RegOff,
   IN UINT32 Data
   )
@@ -197,6 +202,7 @@ MvMdioWrite (
   return MdioOperation (
             This,
             PhyAddr,
+            MdioIndex,
             RegOff,
             TRUE,
             &Data
@@ -210,18 +216,27 @@ MvMdioDxeInitialise (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  MVHW_MDIO_DESC *Desc = &mA7k8kMdioDescTemplate;
+  UINT8 Index;
   MARVELL_MDIO_PROTOCOL *Mdio;
   EFI_STATUS Status;
   EFI_HANDLE Handle = NULL;
 
   Mdio = AllocateZeroPool (sizeof (MARVELL_MDIO_PROTOCOL));
+  if (Mdio == NULL) {
+    DEBUG ((DEBUG_ERROR, "MdioDxe: Protocol allocation failed\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  /* Obtain base addresses of all possible controllers */
+  for (Index = 0; Index < Desc->MdioDevCount; Index++) {
+    Mdio->BaseAddresses[Index] = Desc->MdioBaseAddresses[Index];
+  }
+
+  Mdio->ControllerCount = Desc->MdioDevCount;
   Mdio->Read = MvMdioRead;
   Mdio->Write = MvMdioWrite;
-  MdioBase = PcdGet64 (PcdMdioBaseAddress);
-  if (MdioBase == 0) {
-    DEBUG((DEBUG_ERROR, "MdioDxe: PcdMdioBaseAddress not set\n"));
-    return EFI_INVALID_PARAMETER;
-  }
+
   Status = gBS->InstallMultipleProtocolInterfaces (
                   &Handle,
                   &gMarvellMdioProtocolGuid, Mdio,
diff --git a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf
index faab1f7..d9878eb 100644
--- a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf
+++ b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf
@@ -62,8 +62,5 @@
 [Protocols]
   gMarvellMdioProtocolGuid
 
-[Pcd]
-  gMarvellTokenSpaceGuid.PcdMdioBaseAddress
-
 [Depex]
   TRUE
diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
index aeb6f7a..e776a91 100644
--- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
+++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c
@@ -41,6 +41,7 @@ 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/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
@@ -51,6 +52,19 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 STATIC MARVELL_MDIO_PROTOCOL *Mdio;
 
+//
+// Table with available Mdio controllers
+//
+STATIC UINT8 * CONST MdioDeviceTable = PcdGetPtr (PcdMdioControllersEnabled);
+//
+// Table with PHY to Mdio controller mappings
+//
+STATIC UINT8 * CONST Phy2MdioController = PcdGetPtr (PcdPhy2MdioController);
+//
+// Table with PHYs' SMI addresses
+//
+STATIC UINT8 * CONST PhySmiAddresses = PcdGetPtr (PcdPhySmiAddresses);
+
 STATIC MV_PHY_DEVICE MvPhyDevices[] = {
   { MV_PHY_DEVICE_1512, MvPhyInit1512 },
   { 0, NULL }
@@ -64,18 +78,18 @@ MvPhyStatus (
 
 EFI_STATUS
 MvPhyReset (
-  IN UINT32 PhyAddr
+  IN PHY_DEVICE *PhyDev
   )
 {
   UINT32 Reg = 0;
   INTN timeout = TIMEOUT;
 
-  Mdio->Read(Mdio, PhyAddr, MII_BMCR, &Reg);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMCR, &Reg);
   Reg |= BMCR_RESET;
-  Mdio->Write(Mdio, PhyAddr, MII_BMCR, Reg);
+  Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMCR, Reg);
 
   while ((Reg & BMCR_RESET) && timeout--) {
-    Mdio->Read(Mdio, PhyAddr, MII_BMCR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMCR, &Reg);
     gBS->Stall(1000);
   }
 
@@ -99,7 +113,7 @@ MvPhyM88e1111sConfig (
       (PhyDev->Connection == PHY_CONNECTION_RGMII_ID) ||
       (PhyDev->Connection == PHY_CONNECTION_RGMII_RXID) ||
       (PhyDev->Connection == PHY_CONNECTION_RGMII_TXID)) {
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_CR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_CR, &Reg);
 
     if ((PhyDev->Connection == PHY_CONNECTION_RGMII) ||
       (PhyDev->Connection == PHY_CONNECTION_RGMII_ID)) {
@@ -112,9 +126,9 @@ MvPhyM88e1111sConfig (
       Reg |= MIIM_88E1111_TX_DELAY;
     }
 
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_CR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_CR, Reg);
 
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, &Reg);
 
     Reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK);
 
@@ -123,50 +137,50 @@ MvPhyM88e1111sConfig (
     else
       Reg |= MIIM_88E1111_HWCFG_MODE_COPPER_RGMII;
 
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, Reg);
   }
 
   if (PhyDev->Connection == PHY_CONNECTION_SGMII) {
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, &Reg);
 
     Reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK);
     Reg |= MIIM_88E1111_HWCFG_MODE_SGMII_NO_CLK;
     Reg |= MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
 
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, Reg);
   }
 
   if (PhyDev->Connection == PHY_CONNECTION_RTBI) {
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_CR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_CR, &Reg);
     Reg |= (MIIM_88E1111_RX_DELAY | MIIM_88E1111_TX_DELAY);
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_CR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_CR, Reg);
 
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, &Reg);
     Reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK |
       MIIM_88E1111_HWCFG_FIBER_COPPER_RES);
     Reg |= 0x7 | MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, Reg);
 
     /* Soft reset */
-    MvPhyReset(PhyDev->Addr);
+    MvPhyReset (PhyDev);
 
-    Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, &Reg);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, &Reg);
     Reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK |
       MIIM_88E1111_HWCFG_FIBER_COPPER_RES);
     Reg |= MIIM_88E1111_HWCFG_MODE_COPPER_RTBI |
       MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
-    Mdio->Write(Mdio, PhyDev->Addr, MIIM_88E1111_PHY_EXT_SR, Reg);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1111_PHY_EXT_SR, Reg);
   }
 
-  Mdio->Read(Mdio, PhyDev->Addr, MII_BMCR, &Reg);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMCR, &Reg);
   Reg |= (BMCR_ANENABLE | BMCR_ANRESTART);
   Reg &= ~BMCR_ISOLATE;
-  Mdio->Write(Mdio, PhyDev->Addr, MII_BMCR, Reg);
+  Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMCR, Reg);
 
   /* Soft reset */
-  MvPhyReset(PhyDev->Addr);
+  MvPhyReset (PhyDev);
 
-  MvPhyReset(PhyDev->Addr);
+  MvPhyReset (PhyDev);
 
   return EFI_SUCCESS;
 }
@@ -179,7 +193,7 @@ MvPhyParseStatus (
   UINT32 Data;
   UINT32 Speed;
 
-  Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1xxx_PHY_STATUS, &Data);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1xxx_PHY_STATUS, &Data);
 
   if ((Data & MIIM_88E1xxx_PHYSTAT_LINK) &&
     !(Data & MIIM_88E1xxx_PHYSTAT_SPDDONE)) {
@@ -196,7 +210,7 @@ MvPhyParseStatus (
       if ((i++ % 1000) == 0)
         DEBUG((DEBUG_ERROR, "."));
       gBS->Stall(1000);
-      Mdio->Read(Mdio, PhyDev->Addr, MIIM_88E1xxx_PHY_STATUS, &Data);
+      Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MIIM_88E1xxx_PHY_STATUS, &Data);
     }
     DEBUG((DEBUG_ERROR," done\n"));
     gBS->Stall(500000);
@@ -241,7 +255,7 @@ MvPhyParseStatus (
 STATIC
 VOID
 MvPhy1512WriteBits (
-  IN UINT32 PhyAddr,
+  IN PHY_DEVICE *PhyDev,
   IN UINT8 RegNum,
   IN UINT16 Offset,
   IN UINT16 Len,
@@ -254,19 +268,18 @@ MvPhy1512WriteBits (
   else
     Mask = (1 << (Len + Offset)) - (1 << Offset);
 
-  Mdio->Read(Mdio, PhyAddr, RegNum, &Reg);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, RegNum, &Reg);
 
   Reg &= ~Mask;
   Reg |= Data << Offset;
 
-  Mdio->Write(Mdio, PhyAddr, RegNum, Reg);
+  Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, RegNum, Reg);
 }
 
 STATIC
 EFI_STATUS
 MvPhyInit1512 (
     IN CONST MARVELL_PHY_PROTOCOL *Snp,
-    IN UINT32 PhyAddr,
     IN OUT PHY_DEVICE *PhyDev
     )
 {
@@ -278,28 +291,28 @@ MvPhyInit1512 (
      * Marvell Release Notes - Alaska 88E1510/88E1518/88E1512 Rev A0,
      * Errata Section 3.1 - needed in SGMII mode.
      */
-    Mdio->Write(Mdio, PhyAddr, 22, 0x00ff);
-    Mdio->Write(Mdio, PhyAddr, 17, 0x214B);
-    Mdio->Write(Mdio, PhyAddr, 16, 0x2144);
-    Mdio->Write(Mdio, PhyAddr, 17, 0x0C28);
-    Mdio->Write(Mdio, PhyAddr, 16, 0x2146);
-    Mdio->Write(Mdio, PhyAddr, 17, 0xB233);
-    Mdio->Write(Mdio, PhyAddr, 16, 0x214D);
-    Mdio->Write(Mdio, PhyAddr, 17, 0xCC0C);
-    Mdio->Write(Mdio, PhyAddr, 16, 0x2159);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 22, 0x00ff);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 17, 0x214B);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 16, 0x2144);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 17, 0x0C28);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 16, 0x2146);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 17, 0xB233);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 16, 0x214D);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 17, 0xCC0C);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 16, 0x2159);
 
     /* Reset page selection and select page 0x12 */
-    Mdio->Write(Mdio, PhyAddr, 22, 0x0000);
-    Mdio->Write(Mdio, PhyAddr, 22, 0x0012);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 22, 0x0000);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 22, 0x0012);
 
     /* Write HWCFG_MODE = SGMII to Copper */
-    MvPhy1512WriteBits(PhyAddr, 20, 0, 3, 1);
+    MvPhy1512WriteBits(PhyDev, 20, 0, 3, 1);
 
     /* Phy reset - necessary after changing mode */
-    MvPhy1512WriteBits(PhyAddr, 20, 15, 1, 1);
+    MvPhy1512WriteBits(PhyDev, 20, 15, 1, 1);
 
     /* Reset page selection */
-    Mdio->Write(Mdio, PhyAddr, 22, 0x0000);
+    Mdio->Write (Mdio, PhyDev->Addr, PhyDev->MdioIndex, 22, 0x0000);
     gBS->Stall(100);
   }
 
@@ -309,7 +322,7 @@ MvPhyInit1512 (
   if (!PcdGetBool (PcdPhyStartupAutoneg))
     return EFI_SUCCESS;
 
-  Mdio->Read(Mdio, PhyAddr, MII_BMSR, &Data);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMSR, &Data);
 
   if ((Data & BMSR_ANEGCAPABLE) && !(Data & BMSR_ANEGCOMPLETE)) {
 
@@ -322,12 +335,12 @@ MvPhyInit1512 (
       }
 
       gBS->Stall(1000);  /* 1 ms */
-      Mdio->Read(Mdio, PhyAddr, MII_BMSR, &Data);
+      Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMSR, &Data);
     }
     PhyDev->LinkUp = TRUE;
     DEBUG((DEBUG_INFO, "MvPhyDxe: link up\n"));
   } else {
-    Mdio->Read(Mdio, PhyAddr, MII_BMSR, &Data);
+    Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMSR, &Data);
 
     if (Data & BMSR_LSTATUS) {
       PhyDev->LinkUp = TRUE;
@@ -345,7 +358,7 @@ MvPhyInit1512 (
 EFI_STATUS
 MvPhyInit (
   IN CONST MARVELL_PHY_PROTOCOL *Snp,
-  IN UINT32 PhyAddr,
+  IN UINT32 PhyIndex,
   IN PHY_CONNECTION PhyConnection,
   IN OUT PHY_DEVICE **OutPhyDev
   )
@@ -353,6 +366,7 @@ MvPhyInit (
   EFI_STATUS Status;
   PHY_DEVICE *PhyDev;
   UINT8 *DeviceIds;
+  UINT8 MdioIndex;
   INTN i;
 
   Status = gBS->LocateProtocol (
@@ -363,12 +377,20 @@ MvPhyInit (
   if (EFI_ERROR(Status))
     return Status;
 
+  MdioIndex = Phy2MdioController[PhyIndex];
+
+  /* Verify correctness of PHY <-> MDIO assignment */
+  if (!MVHW_DEV_ENABLED (Mdio, MdioIndex) || MdioIndex >= Mdio->ControllerCount) {
+    DEBUG ((DEBUG_ERROR, "MvPhyDxe: Incorrect Mdio controller assignment for PHY#%d", PhyIndex));
+    return EFI_INVALID_PARAMETER;
+  }
+
   /* perform setup common for all PHYs */
   PhyDev = AllocateZeroPool (sizeof (PHY_DEVICE));
-  PhyDev->Addr = PhyAddr;
+  PhyDev->Addr = PhySmiAddresses[PhyIndex];
   PhyDev->Connection = PhyConnection;
   DEBUG((DEBUG_INFO, "MvPhyDxe: PhyAddr is %d, connection %d\n",
-        PhyAddr, PhyConnection));
+        PhyDev->Addr, PhyConnection));
   *OutPhyDev = PhyDev;
 
   DeviceIds = PcdGetPtr (PcdPhyDeviceIds);
@@ -377,7 +399,7 @@ MvPhyInit (
     if (MvPhyDevices[i].DevId == DeviceIds[i]) {
       ASSERT (MvPhyDevices[i].DevInit != NULL);
       /* proceed with PHY-specific initialization */
-      return MvPhyDevices[i].DevInit(Snp, PhyAddr, PhyDev);
+      return MvPhyDevices[i].DevInit (Snp, PhyDev);
     }
   }
 
@@ -395,8 +417,8 @@ MvPhyStatus (
 {
   UINT32 Data;
 
-  Mdio->Read(Mdio, PhyDev->Addr, MII_BMSR, &Data);
-  Mdio->Read(Mdio, PhyDev->Addr, MII_BMSR, &Data);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMSR, &Data);
+  Mdio->Read (Mdio, PhyDev->Addr, PhyDev->MdioIndex, MII_BMSR, &Data);
 
   if ((Data & BMSR_LSTATUS) == 0) {
     PhyDev->LinkUp = FALSE;
diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
index 6bd06c5..0c3d935 100644
--- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
+++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h
@@ -174,7 +174,6 @@ typedef
 EFI_STATUS
 (*MV_PHY_DEVICE_INIT) (
     IN CONST MARVELL_PHY_PROTOCOL *Snp,
-    IN UINT32 PhyAddr,
     IN OUT PHY_DEVICE *PhyDev
     );
 
@@ -187,7 +186,6 @@ STATIC
 EFI_STATUS
 MvPhyInit1512 (
     IN CONST MARVELL_PHY_PROTOCOL *Snp,
-    IN UINT32 PhyAddr,
     IN OUT PHY_DEVICE *PhyDev
     );
 
diff --git a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf
index c262ce4..2abd673 100644
--- a/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf
+++ b/Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf
@@ -63,8 +63,10 @@
   gMarvellPhyProtocolGuid
 
 [Pcd]
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
+  gMarvellTokenSpaceGuid.PcdMdioControllersEnabled
+  gMarvellTokenSpaceGuid.PcdPhy2MdioController
   gMarvellTokenSpaceGuid.PcdPhyDeviceIds
+  gMarvellTokenSpaceGuid.PcdPhySmiAddresses
   gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
 
 [Depex]
diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 620bd5c..2827976 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -519,14 +519,14 @@ Pp2DxePhyInitialize (
     return Status;
   }
 
-  if (Pp2Context->Port.PhyAddr == 0xff) {
+  if (Pp2Context->Port.PhyIndex == 0xff) {
     /* PHY iniitalization not required */
     return EFI_SUCCESS;
   }
 
   Status = Pp2Context->Phy->Init(
                Pp2Context->Phy,
-               Pp2Context->Port.PhyAddr,
+               Pp2Context->Port.PhyIndex,
                Pp2Context->Port.PhyInterface,
                &Pp2Context->PhyDev
              );
@@ -1147,25 +1147,25 @@ Pp2DxeParsePortPcd (
   IN INTN Index
   )
 {
-  UINT8 *PortIds, *GopIndexes, *PhyConnectionTypes, *AlwaysUp, *Speed, *PhyAddresses;
+  UINT8 *PortIds, *GopIndexes, *PhyConnectionTypes, *AlwaysUp, *Speed, *PhyIndexes;
 
   PortIds = PcdGetPtr (PcdPp2PortIds);
   GopIndexes = PcdGetPtr (PcdPp2GopIndexes);
-  PhyConnectionTypes = PcdGetPtr (PcdPhyConnectionTypes);
-  PhyAddresses = PcdGetPtr (PcdPhySmiAddresses);
+  PhyConnectionTypes = PcdGetPtr (PcdPp2PhyConnectionTypes);
+  PhyIndexes = PcdGetPtr (PcdPp2PhyIndexes);
   AlwaysUp = PcdGetPtr (PcdPp2InterfaceAlwaysUp);
   Speed = PcdGetPtr (PcdPp2InterfaceSpeed);
 
   ASSERT (PcdGetSize (PcdPp2GopIndexes) == PcdGetSize (PcdPp2PortIds));
-  ASSERT (PcdGetSize (PcdPhyConnectionTypes) == PcdGetSize (PcdPp2PortIds));
+  ASSERT (PcdGetSize (PcdPp2PhyConnectionTypes) == PcdGetSize (PcdPp2PortIds));
   ASSERT (PcdGetSize (PcdPp2InterfaceAlwaysUp) == PcdGetSize (PcdPp2PortIds));
   ASSERT (PcdGetSize (PcdPp2InterfaceSpeed) == PcdGetSize (PcdPp2PortIds));
-  ASSERT (PcdGetSize (PcdPhySmiAddresses) == PcdGetSize (PcdPp2PortIds));
+  ASSERT (PcdGetSize (PcdPp2PhyIndexes) == PcdGetSize (PcdPp2PortIds));
 
   Pp2Context->Port.Id = PortIds[Index];
   Pp2Context->Port.GopIndex = GopIndexes[Index];
   Pp2Context->Port.PhyInterface = PhyConnectionTypes[Index];
-  Pp2Context->Port.PhyAddr = PhyAddresses[Index];
+  Pp2Context->Port.PhyIndex = PhyIndexes[Index];
   Pp2Context->Port.AlwaysUp = AlwaysUp[Index];
   Pp2Context->Port.Speed = Speed[Index];
 }
diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
index cde2995..60f40be 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
@@ -327,7 +327,7 @@ struct Pp2DxePort {
   UINT16 RxRingSize;
 
   INT32 PhyInterface;
-  UINTN PhyAddr;
+  UINT32 PhyIndex;
   BOOLEAN Link;
   BOOLEAN Duplex;
   BOOLEAN AlwaysUp;
diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
index 752fcc0..b4568d8 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
@@ -71,12 +71,12 @@
   gMarvellPhyProtocolGuid
 
 [Pcd]
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
-  gMarvellTokenSpaceGuid.PcdPhySmiAddresses
   gMarvellTokenSpaceGuid.PcdPp2Controllers
   gMarvellTokenSpaceGuid.PcdPp2GopIndexes
   gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp
   gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
+  gMarvellTokenSpaceGuid.PcdPp2PhyConnectionTypes
+  gMarvellTokenSpaceGuid.PcdPp2PhyIndexes
   gMarvellTokenSpaceGuid.PcdPp2Port2Controller
   gMarvellTokenSpaceGuid.PcdPp2PortIds
 
diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h
index 6ad1bc2..9ae03d0 100644
--- a/Platform/Marvell/Include/Library/MvHwDescLib.h
+++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
@@ -70,6 +70,16 @@ typedef struct {
 } MVHW_I2C_DESC;
 
 //
+// MDIO devices description template definition
+//
+#define MVHW_MAX_MDIO_DEVS         2
+
+typedef struct {
+  UINT8 MdioDevCount;
+  UINTN MdioBaseAddresses[MVHW_MAX_MDIO_DEVS];
+} MVHW_MDIO_DESC;
+
+//
 // NonDiscoverable devices description template definition
 //
 #define MVHW_MAX_XHCI_DEVS         4
@@ -168,6 +178,19 @@ MVHW_I2C_DESC mA7k8kI2cDescTemplate = {\
 }
 
 //
+// Platform description of MDIO devices
+//
+#define MVHW_CP0_MDIO_BASE       0xF212A200
+#define MVHW_CP1_MDIO_BASE       0xF412A200
+
+#define DECLARE_A7K8K_MDIO_TEMPLATE \
+STATIC \
+MVHW_MDIO_DESC mA7k8kMdioDescTemplate = {\
+  2,\
+  { MVHW_CP0_MDIO_BASE, MVHW_CP1_MDIO_BASE }\
+}
+
+//
 // Platform description of NonDiscoverable devices
 //
 #define MVHW_CP0_XHCI0_BASE        0xF2500000
diff --git a/Platform/Marvell/Include/Protocol/Mdio.h b/Platform/Marvell/Include/Protocol/Mdio.h
index 10acad4..d077a8f 100644
--- a/Platform/Marvell/Include/Protocol/Mdio.h
+++ b/Platform/Marvell/Include/Protocol/Mdio.h
@@ -35,6 +35,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #ifndef __MDIO_H__
 #define __MDIO_H__
 
+#include <Library/MvHwDescLib.h>
+
 #define MARVELL_MDIO_PROTOCOL_GUID { 0x40010b03, 0x5f08, 0x496a, { 0xa2, 0x64, 0x10, 0x5e, 0x72, 0xd3, 0x71, 0xaa }}
 
 typedef struct _MARVELL_MDIO_PROTOCOL MARVELL_MDIO_PROTOCOL;
@@ -44,6 +46,7 @@ EFI_STATUS
 (EFIAPI *MARVELL_MDIO_READ) (
   IN CONST MARVELL_MDIO_PROTOCOL *This,
   IN UINT32 PhyAddr,
+  IN UINT32 MdioIndex,
   IN UINT32 RegOff,
   IN UINT32 *Data
   );
@@ -53,6 +56,7 @@ EFI_STATUS
 (EFIAPI *MARVELL_MDIO_WRITE) (
   IN CONST MARVELL_MDIO_PROTOCOL *This,
   IN UINT32 PhyAddr,
+  IN UINT32 MdioIndex,
   IN UINT32 RegOff,
   IN UINT32 Data
   );
@@ -60,6 +64,8 @@ EFI_STATUS
 struct _MARVELL_MDIO_PROTOCOL {
   MARVELL_MDIO_READ Read;
   MARVELL_MDIO_WRITE Write;
+  UINTN BaseAddresses[MVHW_MAX_MDIO_DEVS];
+  UINTN ControllerCount;
 };
 
 extern EFI_GUID gMarvellMdioProtocolGuid;
diff --git a/Platform/Marvell/Include/Protocol/MvPhy.h b/Platform/Marvell/Include/Protocol/MvPhy.h
index a91759a..99c75b3 100644
--- a/Platform/Marvell/Include/Protocol/MvPhy.h
+++ b/Platform/Marvell/Include/Protocol/MvPhy.h
@@ -62,6 +62,7 @@ typedef enum {
 
 typedef struct {
   UINT32          Addr;
+  UINT8           MdioIndex;
   BOOLEAN         LinkUp;
   BOOLEAN         FullDuplex;
   BOOLEAN         AutoNegotiation;
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index e23607f..0902086 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -160,19 +160,21 @@
   gMarvellTokenSpaceGuid.PcdUtmiPortType|{ 0x0 }|VOID*|0x30000207
 
 #MDIO
-  gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0|UINT64|0x3000043
+  gMarvellTokenSpaceGuid.PcdMdioControllersEnabled|{ 0x0 }|VOID*|0x3000043
 
 #PHY
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0 }|VOID*|0x3000044
+  gMarvellTokenSpaceGuid.PcdPhy2MdioController|{ 0x0 }|VOID*|0x3000027
   gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0 }|VOID*|0x3000095
+  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0x0 }|VOID*|0x3000024
   gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE|BOOLEAN|0x3000070
 
 #NET
-  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0x0 }|VOID*|0x3000024
   gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x0 }|VOID*|0x3000028
   gMarvellTokenSpaceGuid.PcdPp2GopIndexes|{ 0x0 }|VOID*|0x3000029
   gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp|{ 0x0 }|VOID*|0x300002A
   gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ 0x0 }|VOID*|0x300002B
+  gMarvellTokenSpaceGuid.PcdPp2PhyConnectionTypes|{ 0x0 }|VOID*|0x3000044
+  gMarvellTokenSpaceGuid.PcdPp2PhyIndexes|{ 0x0 }|VOID*|0x3000045
   gMarvellTokenSpaceGuid.PcdPp2Port2Controller|{ 0x0 }|VOID*|0x300002D
   gMarvellTokenSpaceGuid.PcdPp2PortIds|{ 0x0 }|VOID*|0x300002C
 
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
index fa429d1..f0da515 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -126,25 +126,15 @@ PHY Driver configuration
 MvPhyDxe provides basic initialization and status routines for Marvell PHYs.
 Currently only 1518 series PHYs are supported. Following PCDs are required:
 
-  - gMarvellTokenSpaceGuid.PcdPhyConnectionTypes
-	(list of values corresponding to PHY_CONNECTION enum)
   - gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg
 	(boolean - if true, driver waits for autonegotiation on startup)
   - gMarvellTokenSpaceGuid.PcdPhyDeviceIds
 	(list of values corresponding to MV_PHY_DEVICE_ID enum)
+  - gMarvellTokenSpaceGuid.PcdPhySmiAddresses
+	(addresses of PHY devices)
+  - gMarvellTokenSpaceGuid.PcdPhy2MdioController
+	(Array specifying, which Mdio controller the PHY is attached to)
 
-PHY_CONNECTION enum type is defined as follows:
-
-		typedef enum {
-			0    PHY_CONNECTION_RGMII,
-			1    PHY_CONNECTION_RGMII_ID,
-			2    PHY_CONNECTION_RGMII_TXID,
-			3    PHY_CONNECTION_RGMII_RXID,
-			4    PHY_CONNECTION_SGMII,
-			5    PHY_CONNECTION_RTBI,
-			6    PHY_CONNECTION_XAUI,
-			7    PHY_CONNECTION_RXAUI
-		} PHY_CONNECTION;
 
 MV_PHY_DEVICE_ID:
 
@@ -153,11 +143,8 @@ MV_PHY_DEVICE_ID:
 		} MV_PHY_DEVICE_ID;
 
 It should be extended when adding support for other PHY models.
-Thus in order to set RGMII for 1st PHY and SGMII for 2nd, PCD should be:
-
- gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x0, 0x4 }
 
-with disabled autonegotiation:
+Disable autonegotiation:
 
  gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
 
@@ -171,8 +158,9 @@ MDIO configuration
 MDIO driver provides access to network PHYs' registers via EFI_MDIO_READ and
 EFI_MDIO_WRITE functions (EFI_MDIO_PROTOCOL). Following PCD is required:
 
-  - gMarvellTokenSpaceGuid.PcdMdioBaseAddress
-	(base address of SMI management register)
+  - gMarvellTokenSpaceGuid.PcdMdioControllers
+	(Array with used controllers
+	 Set to 0x1 for enabled, 0x0 for disabled)
 
 
 I2C configuration
@@ -249,8 +237,24 @@ are required to operate:
   - gMarvellTokenSpaceGuid.PcdPp2Port2Controller
 	(Array specifying, to which controller the port belongs to)
 
-  - gMarvellTokenSpaceGuid.PcdPhySmiAddresses
-	(Addresses of PHY devices)
+  - gMarvellTokenSpaceGuid.PcdPp2PhyConnectionTypes
+	(Indicates speed of the network interface:
+
+	PHY_RGMII                        0x0
+	PHY_RGMII_ID                     0x1
+	PHY_RGMII_TXID                   0x2
+	PHY_RGMII_RXID                   0x3
+	PHY_SGMII                        0x4
+	PHY_RTBI                         0x5
+	PHY_XAUI                         0x6
+	PHY_RXAUI                        0x7
+	PHY_SFI                          0x8 )
+
+  - gMarvellTokenSpaceGuid.PcdPp2PhyIndexes
+	(Array specifying, to which PHY from
+	 gMarvellTokenSpaceGuid.PcdPhyDeviceIds is used. If none,
+	 e.g. in 10G SFI in-band link detection, 0xFF value must
+	 be specified)
 
   - gMarvellTokenSpaceGuid.PcdPp2PortIds
 	(Identificators of PP2 ports)
@@ -262,17 +266,13 @@ are required to operate:
 	(Set to 0x1 for always-up interface, 0x0 otherwise)
 
   - gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed
-	(Values corresponding to PHY_SPEED enum.
-	 PHY_SPEED is defined as follows:
-
-		  typedef enum {
-			  0  NO_SPEED,
-			  1  SPEED_10,
-			  2  SPEED_100,
-			  3  SPEED_1000,
-			  4  SPEED_2500,
-			  5  SPEED_10000
-		  } PHY_SPEED;
+	(Indicates speed of the network interface:
+
+	PHY_SPEED_10                     0x1
+	PHY_SPEED_100                    0x2
+	PHY_SPEED_1000                   0x3
+	PHY_SPEED_2500                   0x4
+	PHY_SPEED_10000                  0x5 )
 
 
 UTMI PHY configuration
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [platforms: PATCH v2 5/5] Platform/Marvell/Armada: Remove ParsePcdLib
  2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
                   ` (3 preceding siblings ...)
  2017-10-08 10:56 ` [platforms: PATCH v2 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling Marcin Wojtas
@ 2017-10-08 10:56 ` Marcin Wojtas
  4 siblings, 0 replies; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-08 10:56 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Current PCD handling in libraries and drivers allow to get
rid of this code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/Marvell/Armada/Armada.dsc.inc               |   1 -
 Platform/Marvell/Include/Library/ParsePcdLib.h       |  46 ----
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c   | 228 --------------------
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf |  50 -----
 4 files changed, 325 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 7258017..89fb7e7 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -33,7 +33,6 @@
   ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
   ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
   MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
-  ParsePcdLib|Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf
   UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
 
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/Platform/Marvell/Include/Library/ParsePcdLib.h b/Platform/Marvell/Include/Library/ParsePcdLib.h
deleted file mode 100644
index a255685..0000000
--- a/Platform/Marvell/Include/Library/ParsePcdLib.h
+++ /dev/null
@@ -1,46 +0,0 @@
-/********************************************************************************
-Copyright (C) 2016 Marvell International Ltd.
-
-Marvell BSD License Option
-
-If you received this File from Marvell, you may opt to use, redistribute and/or
-modify this File under the following licensing terms.
-Redistribution and use in source and binary forms, with or without modification,
-are permitted provided that the following conditions are met:
-
-* Redistributions of source code must retain the above copyright notice,
-  this list of conditions and the following disclaimer.
-
-* Redistributions in binary form must reproduce the above copyright
-  notice, this list of conditions and the following disclaimer in the
-  documentation and/or other materials provided with the distribution.
-
-* Neither the name of Marvell nor the names of its contributors may be
-  used to endorse or promote products derived from this software without
-  specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-*******************************************************************************/
-
-#ifndef __PARSEPCDLIB_H__
-#define __PARSEPCDLIB_H__
-
-EFI_STATUS
-ParsePcdString (
-  IN  CHAR16 *PcdString,
-  IN  UINT8  Count,
-  OUT UINTN  *ValueTable,
-  OUT CHAR16 **StrTable
-  );
-
-#endif
diff --git a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c b/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
deleted file mode 100644
index 9a4be8e..0000000
--- a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
+++ /dev/null
@@ -1,228 +0,0 @@
-/********************************************************************************
-Copyright (C) 2016 Marvell International Ltd.
-
-Marvell BSD License Option
-
-If you received this File from Marvell, you may opt to use, redistribute and/or
-modify this File under the following licensing terms.
-Redistribution and use in source and binary forms, with or without modification,
-are permitted provided that the following conditions are met:
-
-* Redistributions of source code must Retain the above copyright notice,
-  this list of conditions and the following disclaimer.
-
-* Redistributions in binary form must reproduce the above copyright
-  notice, this list of conditions and the following disclaimer in the
-  documentation and/or other materials provided with the distribution.
-
-* Neither the name of Marvell nor the names of its contributors may be
-  used to endorse or promote products derived from this software without
-  specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-*******************************************************************************/
-#define CHAR_NULL 0x0000
-
-#include <Library/BaseLib.h>
-#include <Uefi.h>
-#include <Library/UefiLib.h>
-#include <Library/DebugLib.h>
-
-STATIC
-CHAR16
-CharToUpper (
-  IN CHAR16 Char
-  )
-{
-
-  if (Char >= L'a' && Char <= L'z') {
-    return (CHAR16) (Char - (L'a' - L'A'));
-  }
-
-  return Char;
-}
-
-STATIC
-BOOLEAN
-IsDecimalDigitChar (
-  IN CHAR16 Char
-  )
-{
-
-  return (BOOLEAN) (Char >= L'0' && Char <= L'9');
-}
-
-
-STATIC
-UINTN
-HexCharToUintn (
-  IN CHAR16 Char
-  )
-{
-  if (IsDecimalDigitChar (Char)) {
-    return Char - L'0';
-  }
-
-  return (UINTN) (10 + CharToUpper (Char) - L'A');
-}
-
-STATIC
-BOOLEAN
-IsHexDigitCharacter (
-  CHAR16 Char
-  )
-{
-
-  return (BOOLEAN) ((Char >= L'0' && Char <= L'9') || (Char >= L'A' &&
-    Char <= L'F') || (Char >= L'a' && Char <= L'f'));
-}
-
-STATIC
-UINTN
-HexStrToUintn (
-  CHAR16 *String
-  )
-{
-  UINTN Result = 0;
-
-  if (String == NULL || StrSize(String) == 0) {
-    return (UINTN)(-1);
-  }
-
-  // Ignore spaces and tabs
-  while ((*String == L' ') || (*String == L'\t')) {
-    String++;
-  }
-
-  // Ignore leading zeros after spaces
-  while (*String == L'0') {
-    String++;
-  }
-
-  if (CharToUpper (*String) != L'X') {
-    return (UINTN)(-1);
-  }
-
-  // Skip 'x'
-  String++;
-
-  while (IsHexDigitCharacter (*String)) {
-    Result <<= 4;
-    Result += HexCharToUintn (*String);
-    String++;
-  }
-
-  return (UINTN) Result;
-}
-
-STATIC
-UINTN
-DecimalStrToUintn (
-  CHAR16 *String
-  )
-{
-  UINTN Result = 0;
-
-  while (IsDecimalDigitChar (*String)) {
-    Result = 10 * Result + (*String - L'0');
-    String++;
-  }
-
-  return Result;
-}
-
-STATIC
-UINTN
-StrToUintn (
-  CHAR16 *String
-  )
-{
-  CHAR16 *Walker;
-
-  // Chop off leading spaces
-  for (Walker = String; Walker != NULL && *Walker != CHAR_NULL && *Walker == L' '; Walker++);
-
-  if (StrnCmp(Walker, L"0x", 2) == 0 || StrnCmp(Walker, L"0X", 2) == 0) {
-    return HexStrToUintn (Walker);
-  } else {
-    return DecimalStrToUintn (Walker);
-  }
-}
-
-EFI_STATUS
-ParsePcdString (
-  IN  CHAR16 *PcdString,
-  IN  UINT8  Count,
-  OUT UINTN  *ValueTable,
-  OUT CHAR16 **StrTable
-  )
-{
-  BOOLEAN ValueFlag = FALSE;
-  CHAR16 *Walker;
-  UINTN i, Tmp = 0;
-
-  if (ValueTable != NULL) {
-    ValueFlag = TRUE;
-  }
-
-  // Set pointer at the end of PCD string
-  Walker = PcdString + StrLen (PcdString);
-  for (i = 0; i < Count; i++) {
-    while ((--Walker) >= PcdString) {
-      if (*Walker == L';') {
-        // Cut off parsed chunk from PCD string by replacing ';' with
-        // null-terminator
-        *Walker = '\0';
-        if (ValueFlag) {
-          Tmp = StrToUintn ((Walker + 1));
-          if ((UINTN)(-1) == Tmp) {
-            return EFI_INVALID_PARAMETER;
-          }
-          // Entry is parsed from the end to the beginning
-          // so fill table in the same manner
-          ValueTable[Count - (i + 1)] = Tmp;
-        } else {
-          StrTable[Count - (i + 1)] = Walker + 1;
-        }
-        Walker--;
-        break;
-      }
-      if (Walker == PcdString) {
-        if (ValueFlag) {
-          Tmp = StrToUintn ((Walker));
-          if (Tmp == (UINTN)(-1)) {
-            return EFI_INVALID_PARAMETER;
-          }
-        }
-        // Last device's entry should be added to the table here.
-        // If not, return error
-        if (i != (Count - 1)) {
-          DEBUG((DEBUG_ERROR, "ParsePcdLib: Please set PCD value for every "
-            "device\n"));
-          return EFI_INVALID_PARAMETER;
-        }
-        // We parse from the end to the beginning
-        // so fill table in the same manner
-        if (ValueFlag) {
-          ValueTable[Count - (i + 1)] = Tmp;
-        } else {
-          StrTable[Count - (i + 1)] = Walker;
-        }
-        // End both loops
-        return EFI_SUCCESS;
-      }
-    }
-  }
-
-  return EFI_SUCCESS;
-}
diff --git a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf b/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf
deleted file mode 100644
index b4db621..0000000
--- a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf
+++ /dev/null
@@ -1,50 +0,0 @@
-# Copyright (C) 2016 Marvell International Ltd.
-#
-# Marvell BSD License Option
-#
-# If you received this File from Marvell, you may opt to use, redistribute and/or
-# modify this File under the following licensing terms.
-# Redistribution and use in source and binary forms, with or without modification,
-# are permitted provided that the following conditions are met:
-#
-# * Redistributions of source code must retain the above copyright notice,
-#   this list of conditions and the following disclaimer.
-#
-# * Redistributions in binary form must reproduce the above copyright
-#   notice, this list of conditions and the following disclaimer in the
-#   documentation and/or other materials provided with the distribution.
-#
-# * Neither the name of Marvell nor the names of its contributors may be
-#   used to endorse or promote products derived from this software without
-#   specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-#
-
-[Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = ParsePcdLib
-  FILE_GUID                      = 698d85a0-a952-453e-b8a4-1d6ea338a38e
-  MODULE_TYPE                    = BASE
-  VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = ParsePcdLib
-
-[Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
-
-[LibraryClasses]
-  ArmLib
-  DebugLib
-
-[Sources.common]
-  ParsePcdLib.c
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-08 10:56 ` [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: " Marcin Wojtas
@ 2017-10-09 16:00   ` Leif Lindholm
  2017-10-09 16:06     ` Marcin Wojtas
  0 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2017-10-09 16:00 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel, ard.biesheuvel, nadavh, neta, kostap, jinghua, jsd

On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
> This patch introduces UTMI description, using the new structures
> and template in MvHwDescLib. This change enables more flexible
> addition of multiple CP with UTMI PHY's and also significantly
> reduces amount of used PCD's for that purpose. Update PortingGuide
> documentation accordingly.
> 
> This patch replaces string-based description of Utmi on
> Armada 70x0 DB with new, reduced format, which uses macros
> in Armada.dsc.inc file for better readability.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
>  Platform/Marvell/Marvell.dec                       |   7 +-
>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
>  8 files changed, 148 insertions(+), 110 deletions(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index cd26506..7d0dc39 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -523,3 +523,8 @@
>    DEFINE CP_RXAUI0          = 0x15
>    DEFINE CP_RXAUI1          = 0x16
>    DEFINE CP_SFI             = 0x17
> +
> +  #UTMI PHY connection type
> +  DEFINE UTMI_USB_HOST0     = 0x0
> +  DEFINE UTMI_USB_HOST1     = 0x1
> +  DEFINE UTMI_USB_DEVICE0   = 0x2
> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
> index c11a973..b40766b 100644
> --- a/Platform/Marvell/Armada/Armada70x0.dsc
> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
> @@ -111,11 +111,8 @@
>    gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
>  
>    #UtmiPhy
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
> +  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
>  
>    #MDIO
>    gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
> diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h
> index e029b50..6ad1bc2 100644
> --- a/Platform/Marvell/Include/Library/MvHwDescLib.h
> +++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
> @@ -117,6 +117,19 @@ typedef struct {
>  } MVHW_RTC_DESC;
>  
>  //
> +// UTMI PHY's description template definition
> +//
> +
> +typedef struct {
> +  UINT8 UtmiDevCount;
> +  UINT32 UtmiPhyId[MVHW_MAX_XHCI_DEVS];
> +  UINTN UtmiBaseAddresses[MVHW_MAX_XHCI_DEVS];
> +  UINTN UtmiConfigAddresses[MVHW_MAX_XHCI_DEVS];
> +  UINTN UtmiUsbConfigAddresses[MVHW_MAX_XHCI_DEVS];
> +  UINTN UtmiMuxBitCount[MVHW_MAX_XHCI_DEVS];
> +} MVHW_UTMI_DESC;
> +
> +//
>  // Platform description of CommonPhy devices
>  //
>  #define MVHW_CP0_COMPHY_BASE       0xF2441000
> @@ -217,4 +230,38 @@ MVHW_RTC_DESC mA7k8kRtcDescTemplate = {\
>    { SIZE_4KB, SIZE_4KB }\
>  }
>  
> +//
> +// Platform description of UTMI PHY's
> +//
> +#define MVHW_CP0_UTMI0_BASE            0xF2580000
> +#define MVHW_CP0_UTMI0_CFG_BASE        0xF2440440
> +#define MVHW_CP0_UTMI0_USB_CFG_BASE    0xF2440420
> +#define MVHW_CP0_UTMI0_ID              0x0
> +#define MVHW_CP0_UTMI1_BASE            0xF2581000
> +#define MVHW_CP0_UTMI1_CFG_BASE        0xF2440444
> +#define MVHW_CP0_UTMI1_USB_CFG_BASE    0xF2440420
> +#define MVHW_CP0_UTMI1_ID              0x1
> +#define MVHW_CP1_UTMI0_BASE            0xF4580000
> +#define MVHW_CP1_UTMI0_CFG_BASE        0xF4440440
> +#define MVHW_CP1_UTMI0_USB_CFG_BASE    0xF4440420
> +#define MVHW_CP1_UTMI0_ID              0x0
> +#define MVHW_CP1_UTMI1_BASE            0xF4581000
> +#define MVHW_CP1_UTMI1_CFG_BASE        0xF4440444
> +#define MVHW_CP1_UTMI1_USB_CFG_BASE    0xF4440420
> +#define MVHW_CP1_UTMI1_ID              0x1
> +
> +#define DECLARE_A7K8K_UTMI_TEMPLATE \
> +STATIC \
> +MVHW_UTMI_DESC mA7k8kUtmiDescTemplate = {\
> +  4,\
> +  { MVHW_CP0_UTMI0_ID, MVHW_CP0_UTMI1_ID,\
> +    MVHW_CP1_UTMI0_ID, MVHW_CP1_UTMI1_ID },\
> +  { MVHW_CP0_UTMI0_BASE, MVHW_CP0_UTMI1_BASE,\
> +    MVHW_CP1_UTMI0_BASE, MVHW_CP1_UTMI1_BASE },\
> +  { MVHW_CP0_UTMI0_CFG_BASE, MVHW_CP0_UTMI1_CFG_BASE,\
> +    MVHW_CP1_UTMI0_CFG_BASE, MVHW_CP1_UTMI1_CFG_BASE },\
> +  { MVHW_CP0_UTMI0_USB_CFG_BASE, MVHW_CP0_UTMI1_USB_CFG_BASE,\
> +    MVHW_CP1_UTMI0_USB_CFG_BASE, MVHW_CP1_UTMI1_USB_CFG_BASE }\
> +}
> +
>  #endif /* __MVHWDESCLIB_H__ */
> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> index 95b5698..f1819c4 100644
> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
> @@ -33,12 +33,16 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  *******************************************************************************/
>  
>  #include "UtmiPhyLib.h"
> +#include <Library/MvHwDescLib.h>
> +
> +DECLARE_A7K8K_UTMI_TEMPLATE;
>  
>  typedef struct {
>    EFI_PHYSICAL_ADDRESS UtmiBaseAddr;
>    EFI_PHYSICAL_ADDRESS UsbCfgAddr;
>    EFI_PHYSICAL_ADDRESS UtmiCfgAddr;
>    UINT32 UtmiPhyPort;
> +  UINT32 PhyId;
>  } UTMI_PHY_DATA;
>  
>  STATIC
> @@ -236,48 +240,52 @@ UtmiPhyPowerUp (
>  STATIC
>  VOID
>  Cp110UtmiPhyInit (
> -  IN UINT32 UtmiPhyCount,
>    IN UTMI_PHY_DATA *UtmiData
>    )
>  {
> -  UINT32 i;
> +  EFI_STATUS Status;
>  
> -  for (i = 0; i < UtmiPhyCount; i++) {
> -    UtmiPhyPowerDown(i, UtmiData[i].UtmiBaseAddr,
> -      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
> -      UtmiData[i].UtmiPhyPort);
> -  }
> +  UtmiPhyPowerDown (
> +          UtmiData->PhyId,

This indentation does not appear to follow any of the patterns
permitted by the coding style. Please address here and in the two
instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).

No need to resubmit the whole series - just the single patch.

> +          UtmiData->UtmiBaseAddr,
> +          UtmiData->UsbCfgAddr,
> +          UtmiData->UtmiCfgAddr,
> +          UtmiData->UtmiPhyPort
> +          );
>  
>    /* Power down PLL */
>    DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power down PLL\n"));
> -  RegSet (UtmiData[0].UsbCfgAddr, 0x0 << UTMI_USB_CFG_PLL_OFFSET,
> -    UTMI_USB_CFG_PLL_MASK);
> -
> -  for (i = 0; i < UtmiPhyCount; i++) {
> -    UtmiPhyConfig(i, UtmiData[i].UtmiBaseAddr,
> -      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
> -      UtmiData[i].UtmiPhyPort);
> +  MmioAnd32 (UtmiData->UsbCfgAddr, ~UTMI_USB_CFG_PLL_MASK);
> +
> +  UtmiPhyConfig (
> +          UtmiData->PhyId,
> +          UtmiData->UtmiBaseAddr,
> +          UtmiData->UsbCfgAddr,
> +          UtmiData->UtmiCfgAddr,
> +          UtmiData->UtmiPhyPort
> +          );
> +
> +  Status = UtmiPhyPowerUp (
> +          UtmiData->PhyId,
> +          UtmiData->UtmiBaseAddr,
> +          UtmiData->UsbCfgAddr,
> +          UtmiData->UtmiCfgAddr,
> +          UtmiData->UtmiPhyPort
> +          );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", UtmiData->PhyId));
> +    return;
>    }
>  
> -  for (i = 0; i < UtmiPhyCount; i++) {
> -    if (EFI_ERROR(UtmiPhyPowerUp(i, UtmiData[i].UtmiBaseAddr,
> -        UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
> -        UtmiData[i].UtmiPhyPort))) {
> -      DEBUG((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", i));
> -      continue;
> -    }
> -    DEBUG((DEBUG_ERROR, "UTMI PHY %d initialized to ", i));
> -
> -    if (UtmiData[i].UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
> -      DEBUG((DEBUG_ERROR, "USB Device\n"));
> -    else
> -      DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData[i].UtmiPhyPort));
> -  }
> +  DEBUG ((DEBUG_ERROR, "UTMI PHY %d initialized to ", UtmiData->PhyId));
> +  if (UtmiData->UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
> +    DEBUG((DEBUG_ERROR, "USB Device\n"));
> +  else
> +    DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData->UtmiPhyPort));
>  
>    /* Power up PLL */
>    DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power up PLL\n"));
> -  RegSet (UtmiData[0].UsbCfgAddr, 0x1 << UTMI_USB_CFG_PLL_OFFSET,
> -    UTMI_USB_CFG_PLL_MASK);
> +  MmioOr32 (UtmiData->UsbCfgAddr, UTMI_USB_CFG_PLL_MASK);
>  }
>  
>  EFI_STATUS
> @@ -285,69 +293,67 @@ UtmiPhyInit (
>    VOID
>    )
>  {
> -  EFI_STATUS Status;
> -  UTMI_PHY_DATA UtmiData[PcdGet32 (PcdUtmiPhyCount)];
> -  EFI_PHYSICAL_ADDRESS RegUtmiUnit[PcdGet32 (PcdUtmiPhyCount)];
> -  EFI_PHYSICAL_ADDRESS RegUsbCfg[PcdGet32 (PcdUtmiPhyCount)];
> -  EFI_PHYSICAL_ADDRESS RegUtmiCfg[PcdGet32 (PcdUtmiPhyCount)];
> -  UINTN UtmiPort[PcdGet32 (PcdUtmiPhyCount)];
> -  UINTN i, Count;
> -
> -  Count = PcdGet32 (PcdUtmiPhyCount);
> -  if (Count == 0) {
> +  UTMI_PHY_DATA UtmiData;
> +  UINT8 *UtmiDeviceTable, *XhciDeviceTable, *UtmiPortType, Index;
> +  MVHW_UTMI_DESC *Desc = &mA7k8kUtmiDescTemplate;
> +
> +  /* Obtain table with enabled Utmi PHY's*/
> +  UtmiDeviceTable = (UINT8 *)PcdGetPtr (PcdUtmiControllersEnabled);
> +  if (UtmiDeviceTable == NULL) {
>      /* No UTMI PHY on platform */
>      return EFI_SUCCESS;
>    }
>  
> -  DEBUG((DEBUG_INFO, "UtmiPhy: Initialize USB UTMI PHYs\n"));
> -  /* Parse UtmiPhy PCDs */
> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiUnit),
> -    Count, RegUtmiUnit, NULL);
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiUnit format\n"));
> +  if (PcdGetSize (PcdUtmiControllersEnabled) > MVHW_MAX_XHCI_DEVS) {
> +    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiControllersEnabled format\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUsbCfg),
> -    Count, RegUsbCfg, NULL);
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUsbCfg format\n"));
> +  /* Make sure XHCI controllers table is present */
> +  XhciDeviceTable = (UINT8 *)PcdGetPtr (PcdPciEXhci);
> +  if (XhciDeviceTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "UTMI: Missing PcdPciEXhci\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiCfg),
> -    Count, RegUtmiCfg, NULL);
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiCfg format\n"));
> +  /* Obtain port type table */
> +  UtmiPortType = (UINT8 *)PcdGetPtr (PcdUtmiPortType);
> +  if (UtmiPortType == NULL ||
> +      PcdGetSize (PcdUtmiPortType) != PcdGetSize (PcdUtmiControllersEnabled)) {
> +    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiPortType format\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyUtmiPort),
> -    Count, UtmiPort, NULL);
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyUtmiPort format\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> +  /* Initialize enabled chips */
> +  for (Index = 0; Index < PcdGetSize (PcdUtmiControllersEnabled); Index++) {
> +    if (!MVHW_DEV_ENABLED (Utmi, Index)) {
> +      continue;
> +    }
> +
> +    /* UTMI PHY without enabled XHCI controller is useless */
> +    if (!MVHW_DEV_ENABLED (Xhci, Index)) {
> +      DEBUG ((DEBUG_ERROR, "UTMI: Disabled Xhci controller %d\n", Index));
> +      return EFI_INVALID_PARAMETER;
> +    }
>  
> -  for (i = 0 ; i < Count ; i++) {
>      /* Get base address of UTMI phy */
> -    UtmiData[i].UtmiBaseAddr = RegUtmiUnit[i];
> +    UtmiData.UtmiBaseAddr = Desc->UtmiBaseAddresses[Index];
>  
>      /* Get usb config address */
> -    UtmiData[i].UsbCfgAddr = RegUsbCfg[i];
> +    UtmiData.UsbCfgAddr = Desc->UtmiUsbConfigAddresses[Index];
>  
>      /* Get UTMI config address */
> -    UtmiData[i].UtmiCfgAddr = RegUtmiCfg[i];
> +    UtmiData.UtmiCfgAddr = Desc->UtmiConfigAddresses[Index];
>  
> -    /*
> -     * Get the usb port number, which will be used to check if
> -     * the utmi connected to host or device
> -     */
> -    UtmiData[i].UtmiPhyPort = UtmiPort[i];
> -  }
> +    /* Get UTMI PHY ID */
> +    UtmiData.PhyId = Desc->UtmiPhyId[Index];
>  
> -  /* Currently only Cp110 is supported */
> -  Cp110UtmiPhyInit (Count, UtmiData);
> +    /* Get the usb port type */
> +    UtmiData.UtmiPhyPort = UtmiPortType[Index];
> +
> +    /* Currently only Cp110 is supported */
> +    Cp110UtmiPhyInit (&UtmiData);
> +  }
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> index f9b4933..0d7d72e 100644
> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
> @@ -42,7 +42,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/TimerLib.h>
> -#include <Library/ParsePcdLib.h>
>  
>  #define UTMI_USB_CFG_DEVICE_EN_OFFSET             0
>  #define UTMI_USB_CFG_DEVICE_EN_MASK               (0x1 << UTMI_USB_CFG_DEVICE_EN_OFFSET)
> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
> index f1e57f4..b56c43b 100644
> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
> @@ -50,15 +50,12 @@
>    DebugLib
>    IoLib
>    MemoryAllocationLib
> -  ParsePcdLib
>    PcdLib
>  
>  [Sources.common]
>    UtmiPhyLib.c
>  
> -[FixedPcd]
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
> +[Pcd]
> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
> +  gMarvellTokenSpaceGuid.PcdUtmiPortType
> +  gMarvellTokenSpaceGuid.PcdPciEXhci
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index d2ab0a9..e23607f 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -156,11 +156,8 @@
>    gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags|{ 0x0 }|VOID*|0x30000177
>  
>  #UtmiPhy
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|0|UINT32|0x30000205
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|{ 0x0 }|VOID*|0x30000206
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|{ 0x0 }|VOID*|0x30000207
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|{ 0x0 }|VOID*|0x30000208
> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|{ 0x0 }|VOID*|0x30000209
> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x0 }|VOID*|0x30000206
> +  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ 0x0 }|VOID*|0x30000207
>  
>  #MDIO
>    gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0|UINT64|0x3000043
> diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
> index b2bb595..fa429d1 100644
> --- a/Silicon/Marvell/Documentation/PortingGuide.txt
> +++ b/Silicon/Marvell/Documentation/PortingGuide.txt
> @@ -279,33 +279,23 @@ UTMI PHY configuration
>  ======================
>  In order to configure UTMI, following PCDs are available:
>  
> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyCount
> -	(Indicates how many UTMI PHYs are available on platform)
> -
> -Next four PCDs are in unicode string format containing settings for all devices
> -separated with semicolon.
> -
> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
> -	(Indicates base address of the UTMI unit)
> -
> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
> -	(Indicates address of USB Configuration register)
> +  - gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
> +	(Array with used controllers
> +	 Set to 0x1 for enabled, 0x0 for disabled)
>  
> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
> -	(Indicates address of external UTMI configuration)
> +  - gMarvellTokenSpaceGuid.PcdUtmiPortType
> +	(Indicates type of the connected USB port:
>  
> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
> -	(Indicates type of the connected USB port)
> +	UTMI_USB_HOST0                     0x0
> +	UTMI_USB_HOST1                     0x1
> +	UTMI_USB_DEVICE0                   0x2 )
>  
>  Example
>  -------
>  
>  		# UtmiPhy
> -		  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
> -		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
> -		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
> -		  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
> -		  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
> +		  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
> +		  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }

Actually, looking at this bit made me realise the PortingGuide.txt
uses tab characters and uses \n line endings.

This is not caused by this set, so does not need to be addressed as
part of this series, but if you could follow up with a patch adjusting
the formating of documentation, I would be grateful.

This is also made painfully clear when running CheckPatch.py.

Regards,

Leif

>  
>  
>  SPI driver configuration
> -- 
> 1.8.3.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-09 16:00   ` Leif Lindholm
@ 2017-10-09 16:06     ` Marcin Wojtas
  2017-10-09 16:23       ` Leif Lindholm
  0 siblings, 1 reply; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-09 16:06 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
	Kostya Porotchkin, Hua Jing, semihalf-dabros-jan

2017-10-09 18:00 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
>> This patch introduces UTMI description, using the new structures
>> and template in MvHwDescLib. This change enables more flexible
>> addition of multiple CP with UTMI PHY's and also significantly
>> reduces amount of used PCD's for that purpose. Update PortingGuide
>> documentation accordingly.
>>
>> This patch replaces string-based description of Utmi on
>> Armada 70x0 DB with new, reduced format, which uses macros
>> in Armada.dsc.inc file for better readability.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
>>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
>>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
>>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
>>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
>>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
>>  Platform/Marvell/Marvell.dec                       |   7 +-
>>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
>>  8 files changed, 148 insertions(+), 110 deletions(-)
>>
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
>> index cd26506..7d0dc39 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -523,3 +523,8 @@
>>    DEFINE CP_RXAUI0          = 0x15
>>    DEFINE CP_RXAUI1          = 0x16
>>    DEFINE CP_SFI             = 0x17
>> +
>> +  #UTMI PHY connection type
>> +  DEFINE UTMI_USB_HOST0     = 0x0
>> +  DEFINE UTMI_USB_HOST1     = 0x1
>> +  DEFINE UTMI_USB_DEVICE0   = 0x2
>> diff --git a/Platform/Marvell/Armada/Armada70x0.dsc b/Platform/Marvell/Armada/Armada70x0.dsc
>> index c11a973..b40766b 100644
>> --- a/Platform/Marvell/Armada/Armada70x0.dsc
>> +++ b/Platform/Marvell/Armada/Armada70x0.dsc
>> @@ -111,11 +111,8 @@
>>    gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
>>
>>    #UtmiPhy
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
>> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
>> +  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
>>
>>    #MDIO
>>    gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
>> diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h b/Platform/Marvell/Include/Library/MvHwDescLib.h
>> index e029b50..6ad1bc2 100644
>> --- a/Platform/Marvell/Include/Library/MvHwDescLib.h
>> +++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
>> @@ -117,6 +117,19 @@ typedef struct {
>>  } MVHW_RTC_DESC;
>>
>>  //
>> +// UTMI PHY's description template definition
>> +//
>> +
>> +typedef struct {
>> +  UINT8 UtmiDevCount;
>> +  UINT32 UtmiPhyId[MVHW_MAX_XHCI_DEVS];
>> +  UINTN UtmiBaseAddresses[MVHW_MAX_XHCI_DEVS];
>> +  UINTN UtmiConfigAddresses[MVHW_MAX_XHCI_DEVS];
>> +  UINTN UtmiUsbConfigAddresses[MVHW_MAX_XHCI_DEVS];
>> +  UINTN UtmiMuxBitCount[MVHW_MAX_XHCI_DEVS];
>> +} MVHW_UTMI_DESC;
>> +
>> +//
>>  // Platform description of CommonPhy devices
>>  //
>>  #define MVHW_CP0_COMPHY_BASE       0xF2441000
>> @@ -217,4 +230,38 @@ MVHW_RTC_DESC mA7k8kRtcDescTemplate = {\
>>    { SIZE_4KB, SIZE_4KB }\
>>  }
>>
>> +//
>> +// Platform description of UTMI PHY's
>> +//
>> +#define MVHW_CP0_UTMI0_BASE            0xF2580000
>> +#define MVHW_CP0_UTMI0_CFG_BASE        0xF2440440
>> +#define MVHW_CP0_UTMI0_USB_CFG_BASE    0xF2440420
>> +#define MVHW_CP0_UTMI0_ID              0x0
>> +#define MVHW_CP0_UTMI1_BASE            0xF2581000
>> +#define MVHW_CP0_UTMI1_CFG_BASE        0xF2440444
>> +#define MVHW_CP0_UTMI1_USB_CFG_BASE    0xF2440420
>> +#define MVHW_CP0_UTMI1_ID              0x1
>> +#define MVHW_CP1_UTMI0_BASE            0xF4580000
>> +#define MVHW_CP1_UTMI0_CFG_BASE        0xF4440440
>> +#define MVHW_CP1_UTMI0_USB_CFG_BASE    0xF4440420
>> +#define MVHW_CP1_UTMI0_ID              0x0
>> +#define MVHW_CP1_UTMI1_BASE            0xF4581000
>> +#define MVHW_CP1_UTMI1_CFG_BASE        0xF4440444
>> +#define MVHW_CP1_UTMI1_USB_CFG_BASE    0xF4440420
>> +#define MVHW_CP1_UTMI1_ID              0x1
>> +
>> +#define DECLARE_A7K8K_UTMI_TEMPLATE \
>> +STATIC \
>> +MVHW_UTMI_DESC mA7k8kUtmiDescTemplate = {\
>> +  4,\
>> +  { MVHW_CP0_UTMI0_ID, MVHW_CP0_UTMI1_ID,\
>> +    MVHW_CP1_UTMI0_ID, MVHW_CP1_UTMI1_ID },\
>> +  { MVHW_CP0_UTMI0_BASE, MVHW_CP0_UTMI1_BASE,\
>> +    MVHW_CP1_UTMI0_BASE, MVHW_CP1_UTMI1_BASE },\
>> +  { MVHW_CP0_UTMI0_CFG_BASE, MVHW_CP0_UTMI1_CFG_BASE,\
>> +    MVHW_CP1_UTMI0_CFG_BASE, MVHW_CP1_UTMI1_CFG_BASE },\
>> +  { MVHW_CP0_UTMI0_USB_CFG_BASE, MVHW_CP0_UTMI1_USB_CFG_BASE,\
>> +    MVHW_CP1_UTMI0_USB_CFG_BASE, MVHW_CP1_UTMI1_USB_CFG_BASE }\
>> +}
>> +
>>  #endif /* __MVHWDESCLIB_H__ */
>> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
>> index 95b5698..f1819c4 100644
>> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
>> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
>> @@ -33,12 +33,16 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  *******************************************************************************/
>>
>>  #include "UtmiPhyLib.h"
>> +#include <Library/MvHwDescLib.h>
>> +
>> +DECLARE_A7K8K_UTMI_TEMPLATE;
>>
>>  typedef struct {
>>    EFI_PHYSICAL_ADDRESS UtmiBaseAddr;
>>    EFI_PHYSICAL_ADDRESS UsbCfgAddr;
>>    EFI_PHYSICAL_ADDRESS UtmiCfgAddr;
>>    UINT32 UtmiPhyPort;
>> +  UINT32 PhyId;
>>  } UTMI_PHY_DATA;
>>
>>  STATIC
>> @@ -236,48 +240,52 @@ UtmiPhyPowerUp (
>>  STATIC
>>  VOID
>>  Cp110UtmiPhyInit (
>> -  IN UINT32 UtmiPhyCount,
>>    IN UTMI_PHY_DATA *UtmiData
>>    )
>>  {
>> -  UINT32 i;
>> +  EFI_STATUS Status;
>>
>> -  for (i = 0; i < UtmiPhyCount; i++) {
>> -    UtmiPhyPowerDown(i, UtmiData[i].UtmiBaseAddr,
>> -      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
>> -      UtmiData[i].UtmiPhyPort);
>> -  }
>> +  UtmiPhyPowerDown (
>> +          UtmiData->PhyId,
>
> This indentation does not appear to follow any of the patterns
> permitted by the coding style. Please address here and in the two
> instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).
>
> No need to resubmit the whole series - just the single patch.

Sure, will send right away.

>
>> +          UtmiData->UtmiBaseAddr,
>> +          UtmiData->UsbCfgAddr,
>> +          UtmiData->UtmiCfgAddr,
>> +          UtmiData->UtmiPhyPort
>> +          );
>>
>>    /* Power down PLL */
>>    DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power down PLL\n"));
>> -  RegSet (UtmiData[0].UsbCfgAddr, 0x0 << UTMI_USB_CFG_PLL_OFFSET,
>> -    UTMI_USB_CFG_PLL_MASK);
>> -
>> -  for (i = 0; i < UtmiPhyCount; i++) {
>> -    UtmiPhyConfig(i, UtmiData[i].UtmiBaseAddr,
>> -      UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
>> -      UtmiData[i].UtmiPhyPort);
>> +  MmioAnd32 (UtmiData->UsbCfgAddr, ~UTMI_USB_CFG_PLL_MASK);
>> +
>> +  UtmiPhyConfig (
>> +          UtmiData->PhyId,
>> +          UtmiData->UtmiBaseAddr,
>> +          UtmiData->UsbCfgAddr,
>> +          UtmiData->UtmiCfgAddr,
>> +          UtmiData->UtmiPhyPort
>> +          );
>> +
>> +  Status = UtmiPhyPowerUp (
>> +          UtmiData->PhyId,
>> +          UtmiData->UtmiBaseAddr,
>> +          UtmiData->UsbCfgAddr,
>> +          UtmiData->UtmiCfgAddr,
>> +          UtmiData->UtmiPhyPort
>> +          );
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", UtmiData->PhyId));
>> +    return;
>>    }
>>
>> -  for (i = 0; i < UtmiPhyCount; i++) {
>> -    if (EFI_ERROR(UtmiPhyPowerUp(i, UtmiData[i].UtmiBaseAddr,
>> -        UtmiData[i].UsbCfgAddr, UtmiData[i].UtmiCfgAddr,
>> -        UtmiData[i].UtmiPhyPort))) {
>> -      DEBUG((DEBUG_ERROR, "UtmiPhy: Failed to initialize UTMI PHY %d\n", i));
>> -      continue;
>> -    }
>> -    DEBUG((DEBUG_ERROR, "UTMI PHY %d initialized to ", i));
>> -
>> -    if (UtmiData[i].UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
>> -      DEBUG((DEBUG_ERROR, "USB Device\n"));
>> -    else
>> -      DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData[i].UtmiPhyPort));
>> -  }
>> +  DEBUG ((DEBUG_ERROR, "UTMI PHY %d initialized to ", UtmiData->PhyId));
>> +  if (UtmiData->UtmiPhyPort == UTMI_PHY_TO_USB_DEVICE0)
>> +    DEBUG((DEBUG_ERROR, "USB Device\n"));
>> +  else
>> +    DEBUG((DEBUG_ERROR, "USB Host%d\n", UtmiData->UtmiPhyPort));
>>
>>    /* Power up PLL */
>>    DEBUG((DEBUG_INFO, "UtmiPhy: stage: PHY power up PLL\n"));
>> -  RegSet (UtmiData[0].UsbCfgAddr, 0x1 << UTMI_USB_CFG_PLL_OFFSET,
>> -    UTMI_USB_CFG_PLL_MASK);
>> +  MmioOr32 (UtmiData->UsbCfgAddr, UTMI_USB_CFG_PLL_MASK);
>>  }
>>
>>  EFI_STATUS
>> @@ -285,69 +293,67 @@ UtmiPhyInit (
>>    VOID
>>    )
>>  {
>> -  EFI_STATUS Status;
>> -  UTMI_PHY_DATA UtmiData[PcdGet32 (PcdUtmiPhyCount)];
>> -  EFI_PHYSICAL_ADDRESS RegUtmiUnit[PcdGet32 (PcdUtmiPhyCount)];
>> -  EFI_PHYSICAL_ADDRESS RegUsbCfg[PcdGet32 (PcdUtmiPhyCount)];
>> -  EFI_PHYSICAL_ADDRESS RegUtmiCfg[PcdGet32 (PcdUtmiPhyCount)];
>> -  UINTN UtmiPort[PcdGet32 (PcdUtmiPhyCount)];
>> -  UINTN i, Count;
>> -
>> -  Count = PcdGet32 (PcdUtmiPhyCount);
>> -  if (Count == 0) {
>> +  UTMI_PHY_DATA UtmiData;
>> +  UINT8 *UtmiDeviceTable, *XhciDeviceTable, *UtmiPortType, Index;
>> +  MVHW_UTMI_DESC *Desc = &mA7k8kUtmiDescTemplate;
>> +
>> +  /* Obtain table with enabled Utmi PHY's*/
>> +  UtmiDeviceTable = (UINT8 *)PcdGetPtr (PcdUtmiControllersEnabled);
>> +  if (UtmiDeviceTable == NULL) {
>>      /* No UTMI PHY on platform */
>>      return EFI_SUCCESS;
>>    }
>>
>> -  DEBUG((DEBUG_INFO, "UtmiPhy: Initialize USB UTMI PHYs\n"));
>> -  /* Parse UtmiPhy PCDs */
>> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiUnit),
>> -    Count, RegUtmiUnit, NULL);
>> -  if (EFI_ERROR(Status)) {
>> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiUnit format\n"));
>> +  if (PcdGetSize (PcdUtmiControllersEnabled) > MVHW_MAX_XHCI_DEVS) {
>> +    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiControllersEnabled format\n"));
>>      return EFI_INVALID_PARAMETER;
>>    }
>>
>> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUsbCfg),
>> -    Count, RegUsbCfg, NULL);
>> -  if (EFI_ERROR(Status)) {
>> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUsbCfg format\n"));
>> +  /* Make sure XHCI controllers table is present */
>> +  XhciDeviceTable = (UINT8 *)PcdGetPtr (PcdPciEXhci);
>> +  if (XhciDeviceTable == NULL) {
>> +    DEBUG ((DEBUG_ERROR, "UTMI: Missing PcdPciEXhci\n"));
>>      return EFI_INVALID_PARAMETER;
>>    }
>>
>> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyRegUtmiCfg),
>> -    Count, RegUtmiCfg, NULL);
>> -  if (EFI_ERROR(Status)) {
>> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyRegUtmiCfg format\n"));
>> +  /* Obtain port type table */
>> +  UtmiPortType = (UINT8 *)PcdGetPtr (PcdUtmiPortType);
>> +  if (UtmiPortType == NULL ||
>> +      PcdGetSize (PcdUtmiPortType) != PcdGetSize (PcdUtmiControllersEnabled)) {
>> +    DEBUG ((DEBUG_ERROR, "UTMI: Wrong PcdUtmiPortType format\n"));
>>      return EFI_INVALID_PARAMETER;
>>    }
>>
>> -  Status = ParsePcdString ((CHAR16 *) PcdGetPtr (PcdUtmiPhyUtmiPort),
>> -    Count, UtmiPort, NULL);
>> -  if (EFI_ERROR(Status)) {
>> -    DEBUG((DEBUG_ERROR, "UtmiPhy: Wrong PcdUtmiPhyUtmiPort format\n"));
>> -    return EFI_INVALID_PARAMETER;
>> -  }
>> +  /* Initialize enabled chips */
>> +  for (Index = 0; Index < PcdGetSize (PcdUtmiControllersEnabled); Index++) {
>> +    if (!MVHW_DEV_ENABLED (Utmi, Index)) {
>> +      continue;
>> +    }
>> +
>> +    /* UTMI PHY without enabled XHCI controller is useless */
>> +    if (!MVHW_DEV_ENABLED (Xhci, Index)) {
>> +      DEBUG ((DEBUG_ERROR, "UTMI: Disabled Xhci controller %d\n", Index));
>> +      return EFI_INVALID_PARAMETER;
>> +    }
>>
>> -  for (i = 0 ; i < Count ; i++) {
>>      /* Get base address of UTMI phy */
>> -    UtmiData[i].UtmiBaseAddr = RegUtmiUnit[i];
>> +    UtmiData.UtmiBaseAddr = Desc->UtmiBaseAddresses[Index];
>>
>>      /* Get usb config address */
>> -    UtmiData[i].UsbCfgAddr = RegUsbCfg[i];
>> +    UtmiData.UsbCfgAddr = Desc->UtmiUsbConfigAddresses[Index];
>>
>>      /* Get UTMI config address */
>> -    UtmiData[i].UtmiCfgAddr = RegUtmiCfg[i];
>> +    UtmiData.UtmiCfgAddr = Desc->UtmiConfigAddresses[Index];
>>
>> -    /*
>> -     * Get the usb port number, which will be used to check if
>> -     * the utmi connected to host or device
>> -     */
>> -    UtmiData[i].UtmiPhyPort = UtmiPort[i];
>> -  }
>> +    /* Get UTMI PHY ID */
>> +    UtmiData.PhyId = Desc->UtmiPhyId[Index];
>>
>> -  /* Currently only Cp110 is supported */
>> -  Cp110UtmiPhyInit (Count, UtmiData);
>> +    /* Get the usb port type */
>> +    UtmiData.UtmiPhyPort = UtmiPortType[Index];
>> +
>> +    /* Currently only Cp110 is supported */
>> +    Cp110UtmiPhyInit (&UtmiData);
>> +  }
>>
>>    return EFI_SUCCESS;
>>  }
>> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
>> index f9b4933..0d7d72e 100644
>> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
>> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h
>> @@ -42,7 +42,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/IoLib.h>
>>  #include <Library/TimerLib.h>
>> -#include <Library/ParsePcdLib.h>
>>
>>  #define UTMI_USB_CFG_DEVICE_EN_OFFSET             0
>>  #define UTMI_USB_CFG_DEVICE_EN_MASK               (0x1 << UTMI_USB_CFG_DEVICE_EN_OFFSET)
>> diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>> index f1e57f4..b56c43b 100644
>> --- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>> +++ b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>> @@ -50,15 +50,12 @@
>>    DebugLib
>>    IoLib
>>    MemoryAllocationLib
>> -  ParsePcdLib
>>    PcdLib
>>
>>  [Sources.common]
>>    UtmiPhyLib.c
>>
>> -[FixedPcd]
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
>> +[Pcd]
>> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
>> +  gMarvellTokenSpaceGuid.PcdUtmiPortType
>> +  gMarvellTokenSpaceGuid.PcdPciEXhci
>> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
>> index d2ab0a9..e23607f 100644
>> --- a/Platform/Marvell/Marvell.dec
>> +++ b/Platform/Marvell/Marvell.dec
>> @@ -156,11 +156,8 @@
>>    gMarvellTokenSpaceGuid.PcdChip3ComPhyInvFlags|{ 0x0 }|VOID*|0x30000177
>>
>>  #UtmiPhy
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|0|UINT32|0x30000205
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|{ 0x0 }|VOID*|0x30000206
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|{ 0x0 }|VOID*|0x30000207
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|{ 0x0 }|VOID*|0x30000208
>> -  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|{ 0x0 }|VOID*|0x30000209
>> +  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x0 }|VOID*|0x30000206
>> +  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ 0x0 }|VOID*|0x30000207
>>
>>  #MDIO
>>    gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0|UINT64|0x3000043
>> diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt b/Silicon/Marvell/Documentation/PortingGuide.txt
>> index b2bb595..fa429d1 100644
>> --- a/Silicon/Marvell/Documentation/PortingGuide.txt
>> +++ b/Silicon/Marvell/Documentation/PortingGuide.txt
>> @@ -279,33 +279,23 @@ UTMI PHY configuration
>>  ======================
>>  In order to configure UTMI, following PCDs are available:
>>
>> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyCount
>> -     (Indicates how many UTMI PHYs are available on platform)
>> -
>> -Next four PCDs are in unicode string format containing settings for all devices
>> -separated with semicolon.
>> -
>> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit
>> -     (Indicates base address of the UTMI unit)
>> -
>> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg
>> -     (Indicates address of USB Configuration register)
>> +  - gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled
>> +     (Array with used controllers
>> +      Set to 0x1 for enabled, 0x0 for disabled)
>>
>> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg
>> -     (Indicates address of external UTMI configuration)
>> +  - gMarvellTokenSpaceGuid.PcdUtmiPortType
>> +     (Indicates type of the connected USB port:
>>
>> -  - gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort
>> -     (Indicates type of the connected USB port)
>> +     UTMI_USB_HOST0                     0x0
>> +     UTMI_USB_HOST1                     0x1
>> +     UTMI_USB_DEVICE0                   0x2 )
>>
>>  Example
>>  -------
>>
>>               # UtmiPhy
>> -               gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
>> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
>> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
>> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
>> -               gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
>> +               gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
>> +               gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
>
> Actually, looking at this bit made me realise the PortingGuide.txt
> uses tab characters and uses \n line endings.
>
> This is not caused by this set, so does not need to be addressed as
> part of this series, but if you could follow up with a patch adjusting
> the formating of documentation, I would be grateful.
>
> This is also made painfully clear when running CheckPatch.py.
>

Well, yes. It's in Sphinx acceptable format and it is generated into
Marvell documentation along with all other projects. If I align it to
edk2 coding style, I'd have to maintain 2 copies of this file... If
you insist, I'll change it, but I would be very grateful for accepting
this one exception, if possible:) Please let know your decision
(looking at my branch, there won't be much updates of this file).

Best regards,
Marcin


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-09 16:06     ` Marcin Wojtas
@ 2017-10-09 16:23       ` Leif Lindholm
  2017-10-09 16:25         ` Marcin Wojtas
  0 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2017-10-09 16:23 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
	Kostya Porotchkin, Hua Jing, semihalf-dabros-jan

On Mon, Oct 09, 2017 at 06:06:53PM +0200, Marcin Wojtas wrote:
> 2017-10-09 18:00 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
> >> This patch introduces UTMI description, using the new structures
> >> and template in MvHwDescLib. This change enables more flexible
> >> addition of multiple CP with UTMI PHY's and also significantly
> >> reduces amount of used PCD's for that purpose. Update PortingGuide
> >> documentation accordingly.
> >>
> >> This patch replaces string-based description of Utmi on
> >> Armada 70x0 DB with new, reduced format, which uses macros
> >> in Armada.dsc.inc file for better readability.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
> >>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
> >>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
> >>  Platform/Marvell/Marvell.dec                       |   7 +-
> >>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
> >>  8 files changed, 148 insertions(+), 110 deletions(-)
> >>

> > This indentation does not appear to follow any of the patterns
> > permitted by the coding style. Please address here and in the two
> > instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).
> >
> > No need to resubmit the whole series - just the single patch.
> 
> Sure, will send right away.

Thx.

> >>  Example
> >>  -------
> >>
> >>               # UtmiPhy
> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
> >> +               gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
> >> +               gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
> >
> > Actually, looking at this bit made me realise the PortingGuide.txt
> > uses tab characters and uses \n line endings.
> >
> > This is not caused by this set, so does not need to be addressed as
> > part of this series, but if you could follow up with a patch adjusting
> > the formating of documentation, I would be grateful.
> >
> > This is also made painfully clear when running CheckPatch.py.
> >
> 
> Well, yes. It's in Sphinx acceptable format

Apart from a massive structure in Egypt, what is a Sphinx?

> and it is generated into
> Marvell documentation along with all other projects. If I align it to
> edk2 coding style, I'd have to maintain 2 copies of this file... If
> you insist, I'll change it, but I would be very grateful for accepting
> this one exception, if possible:) Please let know your decision
> (looking at my branch, there won't be much updates of this file).

Well, I don't see how we can keep a file that makes PatchCheck.py
barf whenever we touch it. If you want to keep this format, please
send a proposal to exclude files of this type from said checks by
PatchCheck.py to edk2-devel.

Is there a default "Sphinx source" file extension that can be used to
describe this format, rather than changing the rule for anything
called ".txt"? (If not, should we make one?)

/
    Leif


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-09 16:23       ` Leif Lindholm
@ 2017-10-09 16:25         ` Marcin Wojtas
  2017-10-09 16:43           ` Leif Lindholm
  0 siblings, 1 reply; 11+ messages in thread
From: Marcin Wojtas @ 2017-10-09 16:25 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
	Kostya Porotchkin, Hua Jing, semihalf-dabros-jan

2017-10-09 18:23 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Mon, Oct 09, 2017 at 06:06:53PM +0200, Marcin Wojtas wrote:
>> 2017-10-09 18:00 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> > On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
>> >> This patch introduces UTMI description, using the new structures
>> >> and template in MvHwDescLib. This change enables more flexible
>> >> addition of multiple CP with UTMI PHY's and also significantly
>> >> reduces amount of used PCD's for that purpose. Update PortingGuide
>> >> documentation accordingly.
>> >>
>> >> This patch replaces string-based description of Utmi on
>> >> Armada 70x0 DB with new, reduced format, which uses macros
>> >> in Armada.dsc.inc file for better readability.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> >> ---
>> >>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
>> >>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
>> >>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
>> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
>> >>  Platform/Marvell/Marvell.dec                       |   7 +-
>> >>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
>> >>  8 files changed, 148 insertions(+), 110 deletions(-)
>> >>
>
>> > This indentation does not appear to follow any of the patterns
>> > permitted by the coding style. Please address here and in the two
>> > instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).
>> >
>> > No need to resubmit the whole series - just the single patch.
>>
>> Sure, will send right away.
>
> Thx.

Rebased patches are available here:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/parsepcd-upstream-r20171009

>
>> >>  Example
>> >>  -------
>> >>
>> >>               # UtmiPhy
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF2580000;0xF2581000"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
>> >> -               gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
>> >> +               gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
>> >> +               gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), $(UTMI_USB_HOST1) }
>> >
>> > Actually, looking at this bit made me realise the PortingGuide.txt
>> > uses tab characters and uses \n line endings.
>> >
>> > This is not caused by this set, so does not need to be addressed as
>> > part of this series, but if you could follow up with a patch adjusting
>> > the formating of documentation, I would be grateful.
>> >
>> > This is also made painfully clear when running CheckPatch.py.
>> >
>>
>> Well, yes. It's in Sphinx acceptable format
>
> Apart from a massive structure in Egypt, what is a Sphinx?
>
>> and it is generated into
>> Marvell documentation along with all other projects. If I align it to
>> edk2 coding style, I'd have to maintain 2 copies of this file... If
>> you insist, I'll change it, but I would be very grateful for accepting
>> this one exception, if possible:) Please let know your decision
>> (looking at my branch, there won't be much updates of this file).
>
> Well, I don't see how we can keep a file that makes PatchCheck.py
> barf whenever we touch it. If you want to keep this format, please
> send a proposal to exclude files of this type from said checks by
> PatchCheck.py to edk2-devel.
>
> Is there a default "Sphinx source" file extension that can be used to
> describe this format, rather than changing the rule for anything
> called ".txt"? (If not, should we make one?)
>

Ok, never mind - I'll change it.

Thanks,
Marcin


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  2017-10-09 16:25         ` Marcin Wojtas
@ 2017-10-09 16:43           ` Leif Lindholm
  0 siblings, 0 replies; 11+ messages in thread
From: Leif Lindholm @ 2017-10-09 16:43 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel-01, Ard Biesheuvel, nadavh, Neta Zur Hershkovits,
	Kostya Porotchkin, Hua Jing, semihalf-dabros-jan

On Mon, Oct 09, 2017 at 06:25:18PM +0200, Marcin Wojtas wrote:
> 2017-10-09 18:23 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> > On Mon, Oct 09, 2017 at 06:06:53PM +0200, Marcin Wojtas wrote:
> >> 2017-10-09 18:00 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> >> > On Sun, Oct 08, 2017 at 12:56:50PM +0200, Marcin Wojtas wrote:
> >> >> This patch introduces UTMI description, using the new structures
> >> >> and template in MvHwDescLib. This change enables more flexible
> >> >> addition of multiple CP with UTMI PHY's and also significantly
> >> >> reduces amount of used PCD's for that purpose. Update PortingGuide
> >> >> documentation accordingly.
> >> >>
> >> >> This patch replaces string-based description of Utmi on
> >> >> Armada 70x0 DB with new, reduced format, which uses macros
> >> >> in Armada.dsc.inc file for better readability.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> >> ---
> >> >>  Platform/Marvell/Armada/Armada.dsc.inc             |   5 +
> >> >>  Platform/Marvell/Armada/Armada70x0.dsc             |   7 +-
> >> >>  Platform/Marvell/Include/Library/MvHwDescLib.h     |  47 ++++++
> >> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++++++++++----------
> >> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
> >> >>  Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
> >> >>  Platform/Marvell/Marvell.dec                       |   7 +-
> >> >>  Silicon/Marvell/Documentation/PortingGuide.txt     |  30 ++--
> >> >>  8 files changed, 148 insertions(+), 110 deletions(-)
> >> >>
> >
> >> > This indentation does not appear to follow any of the patterns
> >> > permitted by the coding style. Please address here and in the two
> >> > instances below (calls to UtmiPhyConfig and UtmiPhyPowerUp).
> >> >
> >> > No need to resubmit the whole series - just the single patch.
> >>
> >> Sure, will send right away.
> >
> > Thx.
> 
> Rebased patches are available here:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/parsepcd-upstream-r20171009

Thanks - for the series: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 7f0f07968b..a1b8b22b97

/
    Leif


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-09 16:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-08 10:56 [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal Marcin Wojtas
2017-10-08 10:56 ` [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing Marcin Wojtas
2017-10-08 10:56 ` [platforms: PATCH v2 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib Marcin Wojtas
2017-10-08 10:56 ` [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: " Marcin Wojtas
2017-10-09 16:00   ` Leif Lindholm
2017-10-09 16:06     ` Marcin Wojtas
2017-10-09 16:23       ` Leif Lindholm
2017-10-09 16:25         ` Marcin Wojtas
2017-10-09 16:43           ` Leif Lindholm
2017-10-08 10:56 ` [platforms: PATCH v2 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling Marcin Wojtas
2017-10-08 10:56 ` [platforms: PATCH v2 5/5] Platform/Marvell/Armada: Remove ParsePcdLib Marcin Wojtas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox