[edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib

Marcin Wojtas posted 5 patches 7 years, 2 months ago
[edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Marcin Wojtas 7 years, 2 months ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Leif Lindholm 7 years, 2 months ago
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
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Marcin Wojtas 7 years, 2 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Leif Lindholm 7 years, 2 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Marcin Wojtas 7 years, 2 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
Posted by Leif Lindholm 7 years, 2 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel