public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/3] Add SerDes Support
@ 2020-06-01 19:27 Wasim Khan
  2020-06-01 19:27 ` [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Wasim Khan
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Wasim Khan @ 2020-06-01 19:27 UTC (permalink / raw)
  To: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, leif; +Cc: Wasim Khan

From: Wasim Khan <wasim.khan@nxp.com>

NXP SoCs supports different serdes protocols using reset
configuration word (RCW). 
Based on serdes protocol value in reset configuration word (RCW)
different IP blocks gets enabled in HW.

This patch series implements SerDesHelperLib and provide SoC specific
serdes configuration for LS1043A, which can be used by different IPs
(Ex: PCIe) to know the enabled interfaces and perform the required
initialization.

Wasim Khan (3):
  Silicon/NXP/Library: Implement SerDesHelperLib
  Silicon/NXP: LS1043A: Add Serdes Support
  Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe
    controllers

 Silicon/NXP/NxpQoriqLs.dec                                |   1 +
 Silicon/NXP/LS1043A/LS1043A.dsc.inc                       |   2 +
 Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf             |   6 +
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf |   1 +
 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf   |  28 ++++
 Silicon/NXP/Include/Library/SerDes.h                      |  28 ++++
 Silicon/NXP/Include/Library/SerDesHelperLib.h             |  64 ++++++++
 Silicon/NXP/LS1043A/Include/SocSerDes.h                   |  68 ++++++++
 Silicon/NXP/LS1043A/Library/SocLib/SerDes.c               |  75 +++++++++
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c   |  35 ++++-
 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c     | 165 ++++++++++++++++++++
 11 files changed, 472 insertions(+), 1 deletion(-)
 create mode 100644 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
 create mode 100644 Silicon/NXP/Include/Library/SerDes.h
 create mode 100644 Silicon/NXP/Include/Library/SerDesHelperLib.h
 create mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h
 create mode 100644 Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
 create mode 100644 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c

-- 
2.7.4


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

* [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib
  2020-06-01 19:27 [PATCH edk2-platforms 0/3] Add SerDes Support Wasim Khan
@ 2020-06-01 19:27 ` Wasim Khan
  2020-06-05 12:26   ` Leif Lindholm
  2020-06-01 19:27 ` [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Wasim Khan
  2020-06-01 19:27 ` [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers Wasim Khan
  2 siblings, 1 reply; 8+ messages in thread
From: Wasim Khan @ 2020-06-01 19:27 UTC (permalink / raw)
  To: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, leif; +Cc: Wasim Khan

From: Wasim Khan <wasim.khan@nxp.com>

Implement SerDesHelperLib to provide helper functions which
can be used for SoC specific serdes configuration.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/NxpQoriqLs.dec                              |   1 +
 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf |  28 ++++
 Silicon/NXP/Include/Library/SerDesHelperLib.h           |  64 ++++++++
 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c   | 165 ++++++++++++++++++++
 4 files changed, 258 insertions(+)

diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
index d4d3057af509..720bb5794960 100644
--- a/Silicon/NXP/NxpQoriqLs.dec
+++ b/Silicon/NXP/NxpQoriqLs.dec
@@ -35,6 +35,7 @@ [PcdsFixedAtBuild.common]
   gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|0|UINT32|0x00000501
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x00000502
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x0|UINT32|0x00000503
+  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x0|UINT8|0x00000504
 
 [PcdsDynamic.common]
   gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x00000600
diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
new file mode 100644
index 000000000000..05814c986393
--- /dev/null
+++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
@@ -0,0 +1,28 @@
+## @file
+#
+#  Copyright 2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = SocHelperLib
+  FILE_GUID                      = 2930e932-a700-41e8-80f9-f1a2dedd2c4f
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SerDesHelperLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  Silicon/NXP/NxpQoriqLs.dec
+
+[LibraryClasses]
+  DebugLib
+  PcdLib
+
+[Sources.common]
+  SerDesHelperLib.c
+
+[FixedPcd]
+  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes
diff --git a/Silicon/NXP/Include/Library/SerDesHelperLib.h b/Silicon/NXP/Include/Library/SerDesHelperLib.h
new file mode 100644
index 000000000000..ef08c3edc30a
--- /dev/null
+++ b/Silicon/NXP/Include/Library/SerDesHelperLib.h
@@ -0,0 +1,64 @@
+/** SerDesHelperLib.h
+  The Header file for SerdesHelperLib
+
+  Copyright 2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SERDES_HELPER_LIB_H
+#define SERDES_HELPER_LIB_H
+
+#include <Uefi.h>
+#include <Library/SerDes.h>
+
+typedef struct {
+  UINT16 Protocol;
+  UINT8  SrdsLane[FixedPcdGet8 (PcdSerdesLanes)];
+} SERDES_CONFIG;
+
+typedef enum {
+  SRDS_1  = 0,
+  SRDS_2,
+  SRDS_3,
+  SRDS_MAX_NUM
+} SERDES_NUMBER;
+
+UINT32
+GetSerDesPrtcl (
+  IN  INTN            SerDes,
+  IN  INTN            Cfg,
+  IN  INTN            Lane,
+  IN  UINT32          SerdesMaxProtocol,
+  IN  SERDES_CONFIG   *Config
+ );
+
+EFI_STATUS
+CheckSerDesPrtclValid (
+  IN  INTN           SerDes,
+  IN  UINT32         Prtcl,
+  IN  UINT8          SerdesLanes,
+  IN  SERDES_CONFIG  *Config
+  );
+
+EFI_STATUS
+GetSerDesMap (
+  IN  UINT32                    Srds,
+  IN  UINT32                    SrdsProt,
+  IN  UINT8                     SerdesLanes,
+  IN  UINT32                    SerdesMaxProtocol,
+  IN  SERDES_CONFIG             *Config,
+  OUT UINT64                    *SerDesPrtclMap
+  );
+
+VOID
+SerDesInstanceProbeLanes (
+  IN  UINT32                      Srds,
+  IN  UINT32                      SrdsProt,
+  IN  UINT8                       SerdesLanes,
+  IN  UINT32                      SerdesMaxProtocol,
+  IN  SERDES_CONFIG               *Config,
+  IN  SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
+  IN  VOID                        *Arg
+  );
+#endif
diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
new file mode 100644
index 000000000000..54d0a6181707
--- /dev/null
+++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
@@ -0,0 +1,165 @@
+/** SerDes.c
+  Provides SoC specific serdes interface
+
+  Copyright 2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/SerDesHelperLib.h>
+
+/**
+  Function to get serdes Lane protocol corresponding to
+  serdes protocol.
+
+  @param  SerDes              Serdes number.
+  @param  Cfg                 Serdes Protocol.
+  @param  Lane                Serdes Lane number.
+  @param  SerdesMaxProtocol   Max Serdes protocol number.
+  @param  Config              Serdes Configuration.
+
+  @return Serdes Lane protocol.
+
+**/
+UINT32
+GetSerDesPrtcl (
+  IN  INTN            SerDes,
+  IN  INTN            Cfg,
+  IN  INTN            Lane,
+  IN  UINT32          SerdesMaxProtocol,
+  IN  SERDES_CONFIG   *Config
+  )
+{
+  while (Config->Protocol) {
+    if (Config->Protocol == Cfg) {
+      return Config->SrdsLane[Lane];
+    }
+    Config++;
+  }
+
+  return SerdesMaxProtocol;
+}
+
+/**
+  Function to validate input serdes protocol.
+
+  @param  SerDes            Serdes number.
+  @param  Prtcl             Serdes Protocol to be verified.
+  @param  SerdesLanes       Number of Serdes Lanes
+  @param  Config            Serdes Configuration.
+
+  @return EFI_NOT_FOUND     Serdes Protocol not a valid protocol.
+  @return EFI_SUCCESS       Serdes Protocol is a valid protocol.
+
+**/
+EFI_STATUS
+CheckSerDesPrtclValid (
+  IN  INTN           SerDes,
+  IN  UINT32         Prtcl,
+  IN  UINT8          SerdesLanes,
+  IN  SERDES_CONFIG  *Config
+  )
+{
+  UINT8 Cnt;
+
+  while (Config->Protocol) {
+    if (Config->Protocol == Prtcl) {
+      DEBUG ((DEBUG_INFO, "Protocol: %x Matched with the one in Table\n", Prtcl));
+      break;
+    }
+    Config++;
+  }
+
+  if (!Config->Protocol) {
+    return EFI_NOT_FOUND;
+  }
+
+  for (Cnt = 0; Cnt < SerdesLanes; Cnt++) {
+    if (Config->SrdsLane[Cnt] != 0) {
+      return EFI_SUCCESS;
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
+/**
+  Get lane protocol on provided serdes lane and execute callback function.
+
+  @param  Srds                    Serdes number.
+  @param  SrdsProt                Serdes protocol number.
+  @param  SerdesLanes             Number of Serdes Lanes
+  @param  SerdesMaxProtocol       Max Serdes protocol number.
+  @param  Config                  Serdes Configuration.
+  @param  SerDesLaneProbeCallback Pointer Callback function to be called for Lane protocol
+  @param  Arg                     Pointer to Arguments to be passed to callback function.
+
+**/
+VOID
+SerDesInstanceProbeLanes (
+  IN  UINT32                      Srds,
+  IN  UINT32                      SrdsProt,
+  IN  UINT8                       SerdesLanes,
+  IN  UINT32                      SerdesMaxProtocol,
+  IN  SERDES_CONFIG               *Config,
+  IN  SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
+  IN  VOID                        *Arg
+  )
+{
+  INT8    Lane;
+  UINT32  LanePrtcl;
+
+  // Invoke callback for all lanes in the SerDes instance:
+  for (Lane = 0; Lane < SerdesLanes; Lane++) {
+    LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane, SerdesMaxProtocol, Config);
+    ASSERT (LanePrtcl < SerdesMaxProtocol);
+    if (LanePrtcl != 0x0) {
+      SerDesLaneProbeCallback (LanePrtcl, Arg);
+    }
+  }
+}
+
+/**
+  Function to fill serdes map information.
+
+  @param  Srds                Serdes number.
+  @param  SrdsProt            Serdes protocol number.
+  @param  SerdesLanes         Number of Serdes Lanes.
+  @param  SerdesMaxProtocol   Max Serdes protocol number.
+  @param  Config              Serdes Configuration.
+  @param  SerDesPrtclMap      Output Serdes protocol map of enabled devices.
+
+**/
+EFI_STATUS
+GetSerDesMap (
+  IN  UINT32                    Srds,
+  IN  UINT32                    SrdsProt,
+  IN  UINT8                     SerdesLanes,
+  IN  UINT32                    SerdesMaxProtocol,
+  IN  SERDES_CONFIG             *Config,
+  OUT UINT64                    *SerDesPrtclMap
+  )
+{
+  INTN                   Lane;
+  EFI_STATUS             Status;
+  UINT32                 LanePrtcl;
+
+  Status = CheckSerDesPrtclValid (Srds, SrdsProt, SerdesLanes, Config);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: SERDES%d[PRTCL] = 0x%x is not valid, Status = %r \n",
+            __FUNCTION__, Srds + 1, SrdsProt, Status));
+    return Status;
+  }
+
+  for (Lane = 0; Lane < SerdesLanes; Lane++) {
+    LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane, SerdesMaxProtocol, Config);
+    if (LanePrtcl >= SerdesMaxProtocol) {
+      DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol %d\n", LanePrtcl));
+      return EFI_NO_MAPPING;
+    }
+    *SerDesPrtclMap |= (0x1u << (LanePrtcl));
+  }
+
+  return EFI_SUCCESS;
+}
-- 
2.7.4


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

* [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support
  2020-06-01 19:27 [PATCH edk2-platforms 0/3] Add SerDes Support Wasim Khan
  2020-06-01 19:27 ` [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Wasim Khan
@ 2020-06-01 19:27 ` Wasim Khan
  2020-06-05 12:36   ` Leif Lindholm
  2020-06-01 19:27 ` [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers Wasim Khan
  2 siblings, 1 reply; 8+ messages in thread
From: Wasim Khan @ 2020-06-01 19:27 UTC (permalink / raw)
  To: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, leif; +Cc: Wasim Khan

From: Wasim Khan <wasim.khan@nxp.com>

Based on serdes protocol value in reset configuration word (RCW)
different IP blocks gets enabled in HW.
Add SoC specific serdes configuration for LS1043A, which can be
used by different IPs to know the enabled interfaces and perform
the required initialization.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/LS1043A/LS1043A.dsc.inc           |  2 +
 Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf |  6 ++
 Silicon/NXP/Include/Library/SerDes.h          | 28 ++++++++
 Silicon/NXP/LS1043A/Include/SocSerDes.h       | 68 ++++++++++++++++++
 Silicon/NXP/LS1043A/Library/SocLib/SerDes.c   | 75 ++++++++++++++++++++
 5 files changed, 179 insertions(+)

diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
index e023bfbc7c04..b598b5790b65 100644
--- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
+++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
@@ -12,6 +12,7 @@
 [LibraryClasses.common]
   SocLib|Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
   SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
+  SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
 
 ################################################################################
 #
@@ -37,4 +38,5 @@ [PcdsFixedAtBuild.common]
   gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|3
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x10000
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC
+  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x4
 ##
diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
index 3d0f988e1c67..4731bea8b92e 100644
--- a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
+++ b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
@@ -22,6 +22,12 @@ [Packages]
 [LibraryClasses]
   ChassisLib
   DebugLib
+  PcdLib
+  SerDesHelperLib
 
 [Sources.common]
   SocLib.c
+  SerDes.c
+
+[FixedPcd]
+  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes
diff --git a/Silicon/NXP/Include/Library/SerDes.h b/Silicon/NXP/Include/Library/SerDes.h
new file mode 100644
index 000000000000..d846589401c6
--- /dev/null
+++ b/Silicon/NXP/Include/Library/SerDes.h
@@ -0,0 +1,28 @@
+/** SerDes.h
+  Header file for SoC specific Serdes routines
+
+  Copyright 2017-2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SERDES_H
+#define SERDES_H
+
+VOID
+GetSerdesProtocolMaps (
+  OUT UINT64               *SerDesPrtclMap
+  );
+
+typedef VOID
+(*SERDES_PROBE_LANES_CALLBACK) (
+  IN UINT32 LaneProtocol,
+  IN VOID *Arg
+  );
+
+VOID
+SerDesProbeLanes (
+  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
+  IN VOID                        *Arg
+  );
+#endif
diff --git a/Silicon/NXP/LS1043A/Include/SocSerDes.h b/Silicon/NXP/LS1043A/Include/SocSerDes.h
new file mode 100644
index 000000000000..5bf93cb32df6
--- /dev/null
+++ b/Silicon/NXP/LS1043A/Include/SocSerDes.h
@@ -0,0 +1,68 @@
+/** @file
+  SoC Specific header file for Serdes mapping
+
+  Copyright 2017-2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SOC_SERDES_H
+#define SOC_SERDES_H
+
+typedef enum {
+  NONE = 0,
+  PCIE1,
+  PCIE2,
+  PCIE3,
+  SATA,
+  SGMII_FM1_DTSEC1,
+  SGMII_FM1_DTSEC2,
+  SGMII_FM1_DTSEC5,
+  SGMII_FM1_DTSEC6,
+  SGMII_FM1_DTSEC9,
+  SGMII_FM1_DTSEC10,
+  QSGMII_FM1_A,
+  XFI_FM1_MAC9,
+  XFI_FM1_MAC10,
+  SGMII_2500_FM1_DTSEC2,
+  SGMII_2500_FM1_DTSEC5,
+  SGMII_2500_FM1_DTSEC9,
+  SGMII_2500_FM1_DTSEC10,
+  SERDES_PRTCL_COUNT
+} SERDES_PROTOCOL;
+
+SERDES_CONFIG SerDes1ConfigTbl[] = {
+        /* SerDes 1 */
+  {0x1555, {XFI_FM1_MAC9, PCIE1, PCIE2, PCIE3 } },
+  {0x2555, {SGMII_2500_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
+  {0x4555, {QSGMII_FM1_A, PCIE1, PCIE2, PCIE3 } },
+  {0x4558, {QSGMII_FM1_A,  PCIE1, PCIE2, SATA } },
+  {0x1355, {XFI_FM1_MAC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
+  {0x2355, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
+  {0x3335, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, PCIE3 } },
+  {0x3355, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
+  {0x3358, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, SATA } },
+  {0x3555, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
+  {0x3558, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, SATA } },
+  {0x7000, {PCIE1, PCIE1, PCIE1, PCIE1 } },
+  {0x9998, {PCIE1, PCIE2, PCIE3, SATA } },
+  {0x6058, {PCIE1, PCIE1, PCIE2, SATA } },
+  {0x1455, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
+  {0x2455, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
+  {0x2255, {SGMII_2500_FM1_DTSEC9, SGMII_2500_FM1_DTSEC2, PCIE2, PCIE3 } },
+  {0x3333, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
+  {0x1460, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
+  {0x2460, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
+  {0x3460, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
+  {0x3455, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
+  {0x9960, {PCIE1, PCIE2, PCIE3, PCIE3 } },
+  {0x2233, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
+  {0x2533, {SGMII_2500_FM1_DTSEC9, PCIE1, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
+  {}
+};
+
+SERDES_CONFIG *SerDesConfigTbl[] = {
+  SerDes1ConfigTbl
+};
+#endif
diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
new file mode 100644
index 000000000000..c27367621168
--- /dev/null
+++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
@@ -0,0 +1,75 @@
+/** SerDes.c
+  Provides SoC specific serdes interface
+
+  Copyright 2017-2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/ChassisLib.h>
+#include <Library/DebugLib.h>
+#include <Library/SerDesHelperLib.h>
+#include <Soc.h>
+#include <SocSerDes.h>
+#include <Uefi.h>
+
+#define RCWSR_SRDS1_PRTCL_MASK     0xffff0000
+#define RCWSR_SRDS1_PRTCL_SHIFT    16
+
+/**
+  Probe all serdes lanes for lane protocol and execute provided callback function.
+
+  @param  SerDesLaneProbeCallback Pointer Callback function to be called for Lane protocol
+  @param  Arg                     Pointer to Arguments to be passed to callback function.
+
+**/
+VOID
+SerDesProbeLanes (
+  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
+  IN VOID                        *Arg
+  )
+{
+  UINT32                 SrdsProt;
+  LS1043A_DEVICE_CONFIG  *Dcfg;
+
+  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
+  SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
+  SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;
+
+  SerDesInstanceProbeLanes (SRDS_1, SrdsProt,
+                                    FixedPcdGet8 (PcdSerdesLanes),
+                                    SERDES_PRTCL_COUNT,
+                                    SerDesConfigTbl[SRDS_1],
+                                    SerDesLaneProbeCallback,
+                                    Arg);
+}
+
+/**
+  Function to return Serdes protocol map for all serdes available on board.
+
+  @param  SerDesPrtclMap   Pointer to Serdes protocl map.
+
+**/
+VOID
+GetSerdesProtocolMaps (
+  OUT UINT64   *SerDesPrtclMap
+  )
+{
+  UINT32                 SrdsProt;
+  LS1043A_DEVICE_CONFIG  *Dcfg;
+  EFI_STATUS             Status;
+
+  *SerDesPrtclMap = 0x0;
+  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
+  SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
+  SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;
+
+  Status = GetSerDesMap (SRDS_1, SrdsProt, FixedPcdGet8 (PcdSerdesLanes),
+                          SERDES_PRTCL_COUNT, SerDesConfigTbl[SRDS_1],
+                          SerDesPrtclMap);
+
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "%a: failed for Serdes1 \n",__FUNCTION__));
+    *SerDesPrtclMap = 0x0;
+  }
+}
-- 
2.7.4


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

* [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers
  2020-06-01 19:27 [PATCH edk2-platforms 0/3] Add SerDes Support Wasim Khan
  2020-06-01 19:27 ` [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Wasim Khan
  2020-06-01 19:27 ` [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Wasim Khan
@ 2020-06-01 19:27 ` Wasim Khan
  2020-06-05 12:38   ` Leif Lindholm
  2 siblings, 1 reply; 8+ messages in thread
From: Wasim Khan @ 2020-06-01 19:27 UTC (permalink / raw)
  To: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, leif; +Cc: Wasim Khan

From: Wasim Khan <wasim.khan@nxp.com>

Based on the serdes protocol value in reset configuration
word (RCW), different PCIe controllers are enabled.
Get serdes protocol map and initialize only enabled PCIe
controllers.

Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf |  1 +
 Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c   | 35 +++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
index aa5a9dec7c34..6003da708698 100644
--- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
+++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
@@ -28,6 +28,7 @@ [LibraryClasses]
   IoAccessLib
   MemoryAllocationLib
   PcdLib
+  SocLib
 
 [FeaturePcd]
   gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian
diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
index 549f4fa133fb..323afc2015ae 100644
--- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -15,6 +15,7 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/PciHostBridgeLib.h>
+#include <Library/SerDes.h>
 #include <Pcie.h>
 #include <Protocol/PciHostBridgeResourceAllocation.h>
 #include <Protocol/PciRootBridgeIo.h>
@@ -719,6 +720,32 @@ PcieSetupCntrl (
 }
 
 /**
+   This function checks whether PCIe is enabled or not
+   depending upon SoC serdes protocol map
+
+   @param  PcieNum PCIe number.
+
+   @return The     PCIe number enabled in map.
+   @return FALSE   PCIe number is disabled in map.
+
+**/
+STATIC
+BOOLEAN
+IsPcieNumEnabled(
+  IN UINTN PcieNum
+  )
+{
+  UINT64 SerDesProtocolMap;
+
+  SerDesProtocolMap = 0x0;
+
+  // Reading serdes protocol map
+  GetSerdesProtocolMaps (&SerDesProtocolMap);
+
+  return (SerDesProtocolMap & (0x1u << (PcieNum))) != 0 ;
+}
+
+/**
   Return all the root bridge instances in an array.
 
   @param Count  Return the count of root bridge instances.
@@ -750,13 +777,19 @@ PciHostBridgeGetRootBridges (
     PciPhyIoAddr [Idx] =  PCI_SEG0_PHY_IO_BASE + (PCI_BASE_DIFF * Idx);
     Regs[Idx] =  PCI_SEG0_DBI_BASE + (PCI_DBI_SIZE_DIFF * Idx);
 
+    // Check is the PCIe controller is enabled
+    if (IsPcieNumEnabled (Idx + 1) == 0) {
+      DEBUG ((DEBUG_INFO, "PCIE%d reg @ 0x%lx is disabled \n", Idx + 1, Regs[Idx]));
+      continue;
+    }
+
     // Check PCIe Link
     LinkUp = PcieLinkUp(Regs[Idx], Idx);
 
     if (!LinkUp) {
       continue;
     }
-    DEBUG ((DEBUG_INFO, "PCIE%d Passed Linkup Phase\n", Idx + 1));
+    DEBUG ((DEBUG_INFO, "PCIE%d reg @ 0x%lx :Passed Linkup Phase\n", Idx + 1, Regs[Idx]));
     // Set up PCIe Controller and ATU windows
     PcieSetupCntrl (Regs[Idx],
                     PciPhyCfg0Addr[Idx],
-- 
2.7.4


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

* Re: [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib
  2020-06-01 19:27 ` [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Wasim Khan
@ 2020-06-05 12:26   ` Leif Lindholm
  0 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2020-06-05 12:26 UTC (permalink / raw)
  To: Wasim Khan; +Cc: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, Wasim Khan

On Tue, Jun 02, 2020 at 00:57:36 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Implement SerDesHelperLib to provide helper functions which
> can be used for SoC specific serdes configuration.
> 
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Silicon/NXP/NxpQoriqLs.dec                              |   1 +
>  Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf |  28 ++++
>  Silicon/NXP/Include/Library/SerDesHelperLib.h           |  64 ++++++++
>  Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c   | 165 ++++++++++++++++++++
>  4 files changed, 258 insertions(+)
> 
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index d4d3057af509..720bb5794960 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -35,6 +35,7 @@ [PcdsFixedAtBuild.common]
>    gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|0|UINT32|0x00000501
>    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x00000502
>    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x0|UINT32|0x00000503
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x0|UINT8|0x00000504
>  
>  [PcdsDynamic.common]
>    gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x00000600
> diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
> new file mode 100644
> index 000000000000..05814c986393
> --- /dev/null
> +++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
> @@ -0,0 +1,28 @@
> +## @file
> +#
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = SocHelperLib

I'm not saying BASE_NAME needs to fully match LIBRARY_CLASS or the
name of the directory the file is in, but there is usually a
connection with one of them...

> +  FILE_GUID                      = 2930e932-a700-41e8-80f9-f1a2dedd2c4f
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SerDesHelperLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  PcdLib
> +
> +[Sources.common]
> +  SerDesHelperLib.c
> +
> +[FixedPcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes
> diff --git a/Silicon/NXP/Include/Library/SerDesHelperLib.h b/Silicon/NXP/Include/Library/SerDesHelperLib.h
> new file mode 100644
> index 000000000000..ef08c3edc30a
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/SerDesHelperLib.h
> @@ -0,0 +1,64 @@
> +/** SerDesHelperLib.h
> +  The Header file for SerdesHelperLib
> +
> +  Copyright 2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef SERDES_HELPER_LIB_H
> +#define SERDES_HELPER_LIB_H
> +
> +#include <Uefi.h>
> +#include <Library/SerDes.h>
> +
> +typedef struct {
> +  UINT16 Protocol;
> +  UINT8  SrdsLane[FixedPcdGet8 (PcdSerdesLanes)];

Please write out Serdes fully. Also, please use consistent
capitalisation (including file names) - this patch has a mix of Serdes
and SerDes.

> +} SERDES_CONFIG;
> +
> +typedef enum {
> +  SRDS_1  = 0,

SERDES

> +  SRDS_2,
> +  SRDS_3,
> +  SRDS_MAX_NUM
> +} SERDES_NUMBER;
> +
> +UINT32
> +GetSerDesPrtcl (
> +  IN  INTN            SerDes,
> +  IN  INTN            Cfg,
> +  IN  INTN            Lane,
> +  IN  UINT32          SerdesMaxProtocol,
> +  IN  SERDES_CONFIG   *Config
> + );
> +
> +EFI_STATUS
> +CheckSerDesPrtclValid (

Protocol (search/replace globally)

> +  IN  INTN           SerDes,
> +  IN  UINT32         Prtcl,
> +  IN  UINT8          SerdesLanes,
> +  IN  SERDES_CONFIG  *Config
> +  );
> +
> +EFI_STATUS
> +GetSerDesMap (
> +  IN  UINT32                    Srds,
> +  IN  UINT32                    SrdsProt,
> +  IN  UINT8                     SerdesLanes,
> +  IN  UINT32                    SerdesMaxProtocol,
> +  IN  SERDES_CONFIG             *Config,
> +  OUT UINT64                    *SerDesPrtclMap
> +  );
> +
> +VOID
> +SerDesInstanceProbeLanes (
> +  IN  UINT32                      Srds,
> +  IN  UINT32                      SrdsProt,
> +  IN  UINT8                       SerdesLanes,
> +  IN  UINT32                      SerdesMaxProtocol,
> +  IN  SERDES_CONFIG               *Config,
> +  IN  SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> +  IN  VOID                        *Arg
> +  );
> +#endif
> diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
> new file mode 100644
> index 000000000000..54d0a6181707
> --- /dev/null
> +++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
> @@ -0,0 +1,165 @@
> +/** SerDes.c
> +  Provides SoC specific serdes interface
> +
> +  Copyright 2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/SerDesHelperLib.h>
> +
> +/**
> +  Function to get serdes Lane protocol corresponding to
> +  serdes protocol.
> +
> +  @param  SerDes              Serdes number.
> +  @param  Cfg                 Serdes Protocol.
> +  @param  Lane                Serdes Lane number.
> +  @param  SerdesMaxProtocol   Max Serdes protocol number.
> +  @param  Config              Serdes Configuration.
> +
> +  @return Serdes Lane protocol.
> +
> +**/
> +UINT32
> +GetSerDesPrtcl (
> +  IN  INTN            SerDes,
> +  IN  INTN            Cfg,
> +  IN  INTN            Lane,
> +  IN  UINT32          SerdesMaxProtocol,
> +  IN  SERDES_CONFIG   *Config

This function has one input called Cfg and one called Config.
Cfg is an abbreviation that should be avoided, and when expanded it
becomes Config.

Please rename the variables in a way that describes what they contain.

> +  )
> +{
> +  while (Config->Protocol) {
> +    if (Config->Protocol == Cfg) {
> +      return Config->SrdsLane[Lane];
> +    }
> +    Config++;
> +  }
> +
> +  return SerdesMaxProtocol;
> +}
> +
> +/**
> +  Function to validate input serdes protocol.
> +
> +  @param  SerDes            Serdes number.
> +  @param  Prtcl             Serdes Protocol to be verified.
> +  @param  SerdesLanes       Number of Serdes Lanes
> +  @param  Config            Serdes Configuration.
> +
> +  @return EFI_NOT_FOUND     Serdes Protocol not a valid protocol.
> +  @return EFI_SUCCESS       Serdes Protocol is a valid protocol.
> +
> +**/
> +EFI_STATUS
> +CheckSerDesPrtclValid (
> +  IN  INTN           SerDes,
> +  IN  UINT32         Prtcl,
> +  IN  UINT8          SerdesLanes,
> +  IN  SERDES_CONFIG  *Config
> +  )
> +{
> +  UINT8 Cnt;

Count

> +
> +  while (Config->Protocol) {
> +    if (Config->Protocol == Prtcl) {
> +      DEBUG ((DEBUG_INFO, "Protocol: %x Matched with the one in Table\n", Prtcl));
> +      break;
> +    }
> +    Config++;
> +  }
> +
> +  if (!Config->Protocol) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  for (Cnt = 0; Cnt < SerdesLanes; Cnt++) {
> +    if (Config->SrdsLane[Cnt] != 0) {
> +      return EFI_SUCCESS;
> +    }
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> +/**
> +  Get lane protocol on provided serdes lane and execute callback function.
> +
> +  @param  Srds                    Serdes number.
> +  @param  SrdsProt                Serdes protocol number.
> +  @param  SerdesLanes             Number of Serdes Lanes
> +  @param  SerdesMaxProtocol       Max Serdes protocol number.
> +  @param  Config                  Serdes Configuration.
> +  @param  SerDesLaneProbeCallback Pointer Callback function to be called for Lane protocol
> +  @param  Arg                     Pointer to Arguments to be passed to callback function.
> +
> +**/
> +VOID
> +SerDesInstanceProbeLanes (
> +  IN  UINT32                      Srds,
> +  IN  UINT32                      SrdsProt,
> +  IN  UINT8                       SerdesLanes,
> +  IN  UINT32                      SerdesMaxProtocol,
> +  IN  SERDES_CONFIG               *Config,
> +  IN  SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> +  IN  VOID                        *Arg
> +  )
> +{
> +  INT8    Lane;
> +  UINT32  LanePrtcl;
> +
> +  // Invoke callback for all lanes in the SerDes instance:
> +  for (Lane = 0; Lane < SerdesLanes; Lane++) {
> +    LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane, SerdesMaxProtocol, Config);
> +    ASSERT (LanePrtcl < SerdesMaxProtocol);
> +    if (LanePrtcl != 0x0) {
> +      SerDesLaneProbeCallback (LanePrtcl, Arg);
> +    }
> +  }
> +}
> +
> +/**
> +  Function to fill serdes map information.
> +
> +  @param  Srds                Serdes number.
> +  @param  SrdsProt            Serdes protocol number.
> +  @param  SerdesLanes         Number of Serdes Lanes.
> +  @param  SerdesMaxProtocol   Max Serdes protocol number.
> +  @param  Config              Serdes Configuration.
> +  @param  SerDesPrtclMap      Output Serdes protocol map of enabled devices.
> +
> +**/
> +EFI_STATUS
> +GetSerDesMap (
> +  IN  UINT32                    Srds,
> +  IN  UINT32                    SrdsProt,
> +  IN  UINT8                     SerdesLanes,
> +  IN  UINT32                    SerdesMaxProtocol,
> +  IN  SERDES_CONFIG             *Config,
> +  OUT UINT64                    *SerDesPrtclMap
> +  )
> +{
> +  INTN                   Lane;
> +  EFI_STATUS             Status;
> +  UINT32                 LanePrtcl;
> +
> +  Status = CheckSerDesPrtclValid (Srds, SrdsProt, SerdesLanes, Config);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: SERDES%d[PRTCL] = 0x%x is not valid, Status = %r \n",
> +            __FUNCTION__, Srds + 1, SrdsProt, Status));
> +    return Status;
> +  }
> +
> +  for (Lane = 0; Lane < SerdesLanes; Lane++) {
> +    LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane, SerdesMaxProtocol, Config);
> +    if (LanePrtcl >= SerdesMaxProtocol) {
> +      DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol %d\n", LanePrtcl));
> +      return EFI_NO_MAPPING;
> +    }
> +    *SerDesPrtclMap |= (0x1u << (LanePrtcl));

BIT0, or just 1.

/
    Leif

> +  }
> +
> +  return EFI_SUCCESS;
> +}
> -- 
> 2.7.4
> 

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

* Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support
  2020-06-01 19:27 ` [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Wasim Khan
@ 2020-06-05 12:36   ` Leif Lindholm
  2020-06-07 11:14     ` Wasim Khan
  0 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2020-06-05 12:36 UTC (permalink / raw)
  To: Wasim Khan; +Cc: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, Wasim Khan

On Tue, Jun 02, 2020 at 00:57:37 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Based on serdes protocol value in reset configuration word (RCW)
> different IP blocks gets enabled in HW.
> Add SoC specific serdes configuration for LS1043A, which can be
> used by different IPs to know the enabled interfaces and perform
> the required initialization.
> 
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc           |  2 +
>  Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf |  6 ++
>  Silicon/NXP/Include/Library/SerDes.h          | 28 ++++++++
>  Silicon/NXP/LS1043A/Include/SocSerDes.h       | 68 ++++++++++++++++++
>  Silicon/NXP/LS1043A/Library/SocLib/SerDes.c   | 75 ++++++++++++++++++++
>  5 files changed, 179 insertions(+)
> 
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index e023bfbc7c04..b598b5790b65 100644
> --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> @@ -12,6 +12,7 @@
>  [LibraryClasses.common]
>    SocLib|Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
>    SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> +  SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
>  
>  ################################################################################
>  #
> @@ -37,4 +38,5 @@ [PcdsFixedAtBuild.common]
>    gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|3
>    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x10000
>    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x4
>  ##
> diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> index 3d0f988e1c67..4731bea8b92e 100644
> --- a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> +++ b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> @@ -22,6 +22,12 @@ [Packages]
>  [LibraryClasses]
>    ChassisLib
>    DebugLib
> +  PcdLib
> +  SerDesHelperLib
>  
>  [Sources.common]
>    SocLib.c
> +  SerDes.c

Insert alphabetically sorted.

> +
> +[FixedPcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes
> diff --git a/Silicon/NXP/Include/Library/SerDes.h b/Silicon/NXP/Include/Library/SerDes.h
> new file mode 100644
> index 000000000000..d846589401c6
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/SerDes.h
> @@ -0,0 +1,28 @@
> +/** SerDes.h
> +  Header file for SoC specific Serdes routines
> +
> +  Copyright 2017-2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef SERDES_H
> +#define SERDES_H
> +
> +VOID
> +GetSerdesProtocolMaps (
> +  OUT UINT64               *SerDesPrtclMap
> +  );
> +
> +typedef VOID
> +(*SERDES_PROBE_LANES_CALLBACK) (
> +  IN UINT32 LaneProtocol,
> +  IN VOID *Arg
> +  );
> +
> +VOID
> +SerDesProbeLanes (
> +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> +  IN VOID                        *Arg
> +  );
> +#endif
> diff --git a/Silicon/NXP/LS1043A/Include/SocSerDes.h b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> new file mode 100644
> index 000000000000..5bf93cb32df6
> --- /dev/null
> +++ b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> @@ -0,0 +1,68 @@
> +/** @file
> +  SoC Specific header file for Serdes mapping
> +
> +  Copyright 2017-2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SOC_SERDES_H
> +#define SOC_SERDES_H
> +
> +typedef enum {
> +  NONE = 0,
> +  PCIE1,
> +  PCIE2,
> +  PCIE3,
> +  SATA,
> +  SGMII_FM1_DTSEC1,
> +  SGMII_FM1_DTSEC2,
> +  SGMII_FM1_DTSEC5,
> +  SGMII_FM1_DTSEC6,
> +  SGMII_FM1_DTSEC9,
> +  SGMII_FM1_DTSEC10,
> +  QSGMII_FM1_A,
> +  XFI_FM1_MAC9,
> +  XFI_FM1_MAC10,
> +  SGMII_2500_FM1_DTSEC2,
> +  SGMII_2500_FM1_DTSEC5,
> +  SGMII_2500_FM1_DTSEC9,
> +  SGMII_2500_FM1_DTSEC10,
> +  SERDES_PRTCL_COUNT
> +} SERDES_PROTOCOL;
> +
> +SERDES_CONFIG SerDes1ConfigTbl[] = {

No data definitions in header files please.
Also, global variables should have 'm' or 'g' prefix.

> +        /* SerDes 1 */
> +  {0x1555, {XFI_FM1_MAC9, PCIE1, PCIE2, PCIE3 } },
> +  {0x2555, {SGMII_2500_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
> +  {0x4555, {QSGMII_FM1_A, PCIE1, PCIE2, PCIE3 } },
> +  {0x4558, {QSGMII_FM1_A,  PCIE1, PCIE2, SATA } },
> +  {0x1355, {XFI_FM1_MAC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
> +  {0x2355, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
> +  {0x3335, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, PCIE3 } },
> +  {0x3355, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
> +  {0x3358, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, SATA } },
> +  {0x3555, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
> +  {0x3558, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, SATA } },
> +  {0x7000, {PCIE1, PCIE1, PCIE1, PCIE1 } },
> +  {0x9998, {PCIE1, PCIE2, PCIE3, SATA } },
> +  {0x6058, {PCIE1, PCIE1, PCIE2, SATA } },
> +  {0x1455, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> +  {0x2455, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> +  {0x2255, {SGMII_2500_FM1_DTSEC9, SGMII_2500_FM1_DTSEC2, PCIE2, PCIE3 } },
> +  {0x3333, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
> +  {0x1460, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> +  {0x2460, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> +  {0x3460, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> +  {0x3455, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> +  {0x9960, {PCIE1, PCIE2, PCIE3, PCIE3 } },
> +  {0x2233, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
> +  {0x2533, {SGMII_2500_FM1_DTSEC9, PCIE1, SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
> +  {}
> +};
> +
> +SERDES_CONFIG *SerDesConfigTbl[] = {

No data definitions in header files please.
Also, global variables should have 'm' or 'g' prefix.

Finally, consider naming. I usually let the abbreviation "Tbl" slip,
but in this case it is inapproproate. I assume for the previous data
definition it is intended to mean "Table", and here it is supposed to
mean "Tables"? The reader is not supposed to have to figure that out.

> +  SerDes1ConfigTbl
> +};
> +#endif
> diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> new file mode 100644
> index 000000000000..c27367621168
> --- /dev/null
> +++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> @@ -0,0 +1,75 @@
> +/** SerDes.c
> +  Provides SoC specific serdes interface
> +
> +  Copyright 2017-2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/ChassisLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/SerDesHelperLib.h>
> +#include <Soc.h>
> +#include <SocSerDes.h>
> +#include <Uefi.h>
> +
> +#define RCWSR_SRDS1_PRTCL_MASK     0xffff0000
> +#define RCWSR_SRDS1_PRTCL_SHIFT    16
> +
> +/**
> +  Probe all serdes lanes for lane protocol and execute provided callback function.
> +
> +  @param  SerDesLaneProbeCallback Pointer Callback function to be called for Lane protocol
> +  @param  Arg                     Pointer to Arguments to be passed to callback function.
> +
> +**/
> +VOID
> +SerDesProbeLanes (
> +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> +  IN VOID                        *Arg
> +  )
> +{
> +  UINT32                 SrdsProt;

SerdesProtocol (or SerDesProtocol, depending on which variant you
choose).

> +  LS1043A_DEVICE_CONFIG  *Dcfg;

DeviceConfig

> +
> +  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
> +  SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
> +  SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;

RCWSR is not a generally understood abbreviation. Please either add a
glossary section to the start of the file, or expand the abbreviation
at every point of use.

> +
> +  SerDesInstanceProbeLanes (SRDS_1, SrdsProt,
> +                                    FixedPcdGet8 (PcdSerdesLanes),
> +                                    SERDES_PRTCL_COUNT,
> +                                    SerDesConfigTbl[SRDS_1],
> +                                    SerDesLaneProbeCallback,
> +                                    Arg);
> +}
> +
> +/**
> +  Function to return Serdes protocol map for all serdes available on board.
> +
> +  @param  SerDesPrtclMap   Pointer to Serdes protocl map.
> +
> +**/
> +VOID
> +GetSerdesProtocolMaps (
> +  OUT UINT64   *SerDesPrtclMap
> +  )
> +{
> +  UINT32                 SrdsProt;
> +  LS1043A_DEVICE_CONFIG  *Dcfg;

Both comments for previous function apply here too.

/
    Leif

> +  EFI_STATUS             Status;
> +
> +  *SerDesPrtclMap = 0x0;
> +  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
> +  SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
> +  SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;
> +
> +  Status = GetSerDesMap (SRDS_1, SrdsProt, FixedPcdGet8 (PcdSerdesLanes),
> +                          SERDES_PRTCL_COUNT, SerDesConfigTbl[SRDS_1],
> +                          SerDesPrtclMap);
> +
> +  if (Status != EFI_SUCCESS) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed for Serdes1 \n",__FUNCTION__));
> +    *SerDesPrtclMap = 0x0;
> +  }
> +}
> -- 
> 2.7.4
> 

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

* Re: [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers
  2020-06-01 19:27 ` [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers Wasim Khan
@ 2020-06-05 12:38   ` Leif Lindholm
  0 siblings, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2020-06-05 12:38 UTC (permalink / raw)
  To: Wasim Khan; +Cc: devel, meenakshi.aggarwal, V.Sethi, ard.biesheuvel, Wasim Khan

On Tue, Jun 02, 2020 at 00:57:38 +0530, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
> 
> Based on the serdes protocol value in reset configuration
> word (RCW), different PCIe controllers are enabled.
> Get serdes protocol map and initialize only enabled PCIe
> controllers.
> 
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
>  Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf |  1 +
>  Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c   | 35 +++++++++++++++++++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> index aa5a9dec7c34..6003da708698 100644
> --- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> +++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> @@ -28,6 +28,7 @@ [LibraryClasses]
>    IoAccessLib
>    MemoryAllocationLib
>    PcdLib
> +  SocLib
>  
>  [FeaturePcd]
>    gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian
> diff --git a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
> index 549f4fa133fb..323afc2015ae 100644
> --- a/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
> +++ b/Silicon/NXP/Library/PciHostBridgeLib/PciHostBridgeLib.c
> @@ -15,6 +15,7 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/PciHostBridgeLib.h>
> +#include <Library/SerDes.h>
>  #include <Pcie.h>
>  #include <Protocol/PciHostBridgeResourceAllocation.h>
>  #include <Protocol/PciRootBridgeIo.h>
> @@ -719,6 +720,32 @@ PcieSetupCntrl (
>  }
>  
>  /**
> +   This function checks whether PCIe is enabled or not
> +   depending upon SoC serdes protocol map
> +
> +   @param  PcieNum PCIe number.
> +
> +   @return The     PCIe number enabled in map.
> +   @return FALSE   PCIe number is disabled in map.
> +
> +**/
> +STATIC
> +BOOLEAN
> +IsPcieNumEnabled(
> +  IN UINTN PcieNum
> +  )
> +{
> +  UINT64 SerDesProtocolMap;
> +
> +  SerDesProtocolMap = 0x0;

There is nothing magical about hexadecimal zero. 0 is sufficient.

> +
> +  // Reading serdes protocol map
> +  GetSerdesProtocolMaps (&SerDesProtocolMap);
> +
> +  return (SerDesProtocolMap & (0x1u << (PcieNum))) != 0 ;

BIT0, or simply 1.
No space before ;.

/
    Leif

> +}
> +
> +/**
>    Return all the root bridge instances in an array.
>  
>    @param Count  Return the count of root bridge instances.
> @@ -750,13 +777,19 @@ PciHostBridgeGetRootBridges (
>      PciPhyIoAddr [Idx] =  PCI_SEG0_PHY_IO_BASE + (PCI_BASE_DIFF * Idx);
>      Regs[Idx] =  PCI_SEG0_DBI_BASE + (PCI_DBI_SIZE_DIFF * Idx);
>  
> +    // Check is the PCIe controller is enabled
> +    if (IsPcieNumEnabled (Idx + 1) == 0) {
> +      DEBUG ((DEBUG_INFO, "PCIE%d reg @ 0x%lx is disabled \n", Idx + 1, Regs[Idx]));
> +      continue;
> +    }
> +
>      // Check PCIe Link
>      LinkUp = PcieLinkUp(Regs[Idx], Idx);
>  
>      if (!LinkUp) {
>        continue;
>      }
> -    DEBUG ((DEBUG_INFO, "PCIE%d Passed Linkup Phase\n", Idx + 1));
> +    DEBUG ((DEBUG_INFO, "PCIE%d reg @ 0x%lx :Passed Linkup Phase\n", Idx + 1, Regs[Idx]));
>      // Set up PCIe Controller and ATU windows
>      PcieSetupCntrl (Regs[Idx],
>                      PciPhyCfg0Addr[Idx],
> -- 
> 2.7.4
> 

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

* Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support
  2020-06-05 12:36   ` Leif Lindholm
@ 2020-06-07 11:14     ` Wasim Khan
  0 siblings, 0 replies; 8+ messages in thread
From: Wasim Khan @ 2020-06-07 11:14 UTC (permalink / raw)
  To: Leif Lindholm, Wasim Khan (OSS)
  Cc: devel@edk2.groups.io, Meenakshi Aggarwal, Varun Sethi,
	ard.biesheuvel@arm.com



> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Friday, June 5, 2020 6:06 PM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>;
> ard.biesheuvel@arm.com; Wasim Khan <wasim.khan@nxp.com>
> Subject: Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes
> Support
> 
> On Tue, Jun 02, 2020 at 00:57:37 +0530, Wasim Khan wrote:
> > From: Wasim Khan <wasim.khan@nxp.com>
> >
> > Based on serdes protocol value in reset configuration word (RCW)
> > different IP blocks gets enabled in HW.
> > Add SoC specific serdes configuration for LS1043A, which can be used
> > by different IPs to know the enabled interfaces and perform the
> > required initialization.
> >
> > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > ---
> >  Silicon/NXP/LS1043A/LS1043A.dsc.inc           |  2 +
> >  Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf |  6 ++
> >  Silicon/NXP/Include/Library/SerDes.h          | 28 ++++++++
> >  Silicon/NXP/LS1043A/Include/SocSerDes.h       | 68 ++++++++++++++++++
> >  Silicon/NXP/LS1043A/Library/SocLib/SerDes.c   | 75 ++++++++++++++++++++
> >  5 files changed, 179 insertions(+)
> >
> > diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > index e023bfbc7c04..b598b5790b65 100644
> > --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> > @@ -12,6 +12,7 @@
> >  [LibraryClasses.common]
> >    SocLib|Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> >    SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> > +
> > + SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.
> > + inf
> >
> >
> >
> #################################################################
> #####
> > ##########
> >  #
> > @@ -37,4 +38,5 @@ [PcdsFixedAtBuild.common]
> >    gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|3
> >    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x10000
> >    gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC
> > +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x4
> >  ##
> > diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> > b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> > index 3d0f988e1c67..4731bea8b92e 100644
> > --- a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> > +++ b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> > @@ -22,6 +22,12 @@ [Packages]
> >  [LibraryClasses]
> >    ChassisLib
> >    DebugLib
> > +  PcdLib
> > +  SerDesHelperLib
> >
> >  [Sources.common]
> >    SocLib.c
> > +  SerDes.c
> 
> Insert alphabetically sorted.
> 
> > +
> > +[FixedPcd]
> > +  gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes
> > diff --git a/Silicon/NXP/Include/Library/SerDes.h
> > b/Silicon/NXP/Include/Library/SerDes.h
> > new file mode 100644
> > index 000000000000..d846589401c6
> > --- /dev/null
> > +++ b/Silicon/NXP/Include/Library/SerDes.h
> > @@ -0,0 +1,28 @@
> > +/** SerDes.h
> > +  Header file for SoC specific Serdes routines
> > +
> > +  Copyright 2017-2020 NXP
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > +
> > +#ifndef SERDES_H
> > +#define SERDES_H
> > +
> > +VOID
> > +GetSerdesProtocolMaps (
> > +  OUT UINT64               *SerDesPrtclMap
> > +  );
> > +
> > +typedef VOID
> > +(*SERDES_PROBE_LANES_CALLBACK) (
> > +  IN UINT32 LaneProtocol,
> > +  IN VOID *Arg
> > +  );
> > +
> > +VOID
> > +SerDesProbeLanes (
> > +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> > +  IN VOID                        *Arg
> > +  );
> > +#endif
> > diff --git a/Silicon/NXP/LS1043A/Include/SocSerDes.h
> > b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> > new file mode 100644
> > index 000000000000..5bf93cb32df6
> > --- /dev/null
> > +++ b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> > @@ -0,0 +1,68 @@
> > +/** @file
> > +  SoC Specific header file for Serdes mapping
> > +
> > +  Copyright 2017-2020 NXP
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef SOC_SERDES_H
> > +#define SOC_SERDES_H
> > +
> > +typedef enum {
> > +  NONE = 0,
> > +  PCIE1,
> > +  PCIE2,
> > +  PCIE3,
> > +  SATA,
> > +  SGMII_FM1_DTSEC1,
> > +  SGMII_FM1_DTSEC2,
> > +  SGMII_FM1_DTSEC5,
> > +  SGMII_FM1_DTSEC6,
> > +  SGMII_FM1_DTSEC9,
> > +  SGMII_FM1_DTSEC10,
> > +  QSGMII_FM1_A,
> > +  XFI_FM1_MAC9,
> > +  XFI_FM1_MAC10,
> > +  SGMII_2500_FM1_DTSEC2,
> > +  SGMII_2500_FM1_DTSEC5,
> > +  SGMII_2500_FM1_DTSEC9,
> > +  SGMII_2500_FM1_DTSEC10,
> > +  SERDES_PRTCL_COUNT
> > +} SERDES_PROTOCOL;
> > +
> > +SERDES_CONFIG SerDes1ConfigTbl[] = {
> 
> No data definitions in header files please.
> Also, global variables should have 'm' or 'g' prefix.
> 
> > +        /* SerDes 1 */
> > +  {0x1555, {XFI_FM1_MAC9, PCIE1, PCIE2, PCIE3 } },
> > +  {0x2555, {SGMII_2500_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
> > +  {0x4555, {QSGMII_FM1_A, PCIE1, PCIE2, PCIE3 } },
> > +  {0x4558, {QSGMII_FM1_A,  PCIE1, PCIE2, SATA } },
> > +  {0x1355, {XFI_FM1_MAC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
> > +  {0x2355, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 }
> > +},
> > +  {0x3335, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5,
> > +PCIE3 } },
> > +  {0x3355, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } },
> > +  {0x3358, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, SATA } },
> > +  {0x3555, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
> > +  {0x3558, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, SATA } },
> > +  {0x7000, {PCIE1, PCIE1, PCIE1, PCIE1 } },
> > +  {0x9998, {PCIE1, PCIE2, PCIE3, SATA } },
> > +  {0x6058, {PCIE1, PCIE1, PCIE2, SATA } },
> > +  {0x1455, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> > +  {0x2455, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> > +  {0x2255, {SGMII_2500_FM1_DTSEC9, SGMII_2500_FM1_DTSEC2, PCIE2,
> > +PCIE3 } },
> > +  {0x3333, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5,
> > +SGMII_FM1_DTSEC6 } },
> > +  {0x1460, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> > +  {0x2460, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> > +  {0x3460, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } },
> > +  {0x3455, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } },
> > +  {0x9960, {PCIE1, PCIE2, PCIE3, PCIE3 } },
> > +  {0x2233, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2,
> > +SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } },
> > +  {0x2533, {SGMII_2500_FM1_DTSEC9, PCIE1, SGMII_FM1_DTSEC5,
> > +SGMII_FM1_DTSEC6 } },
> > +  {}
> > +};
> > +
> > +SERDES_CONFIG *SerDesConfigTbl[] = {
> 
> No data definitions in header files please.
> Also, global variables should have 'm' or 'g' prefix.
> 
> Finally, consider naming. I usually let the abbreviation "Tbl" slip, but in this case
> it is inapproproate. I assume for the previous data definition it is intended to
> mean "Table", and here it is supposed to mean "Tables"? The reader is not
> supposed to have to figure that out.
> 
> > +  SerDes1ConfigTbl
> > +};
> > +#endif
> > diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> > b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> > new file mode 100644
> > index 000000000000..c27367621168
> > --- /dev/null
> > +++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> > @@ -0,0 +1,75 @@
> > +/** SerDes.c
> > +  Provides SoC specific serdes interface
> > +
> > +  Copyright 2017-2020 NXP
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > +
> > +#include <Library/ChassisLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/SerDesHelperLib.h>
> > +#include <Soc.h>
> > +#include <SocSerDes.h>
> > +#include <Uefi.h>
> > +
> > +#define RCWSR_SRDS1_PRTCL_MASK     0xffff0000
> > +#define RCWSR_SRDS1_PRTCL_SHIFT    16
> > +
> > +/**
> > +  Probe all serdes lanes for lane protocol and execute provided callback
> function.
> > +
> > +  @param  SerDesLaneProbeCallback Pointer Callback function to be called
> for Lane protocol
> > +  @param  Arg                     Pointer to Arguments to be passed to callback
> function.
> > +
> > +**/
> > +VOID
> > +SerDesProbeLanes (
> > +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> > +  IN VOID                        *Arg
> > +  )
> > +{
> > +  UINT32                 SrdsProt;
> 
> SerdesProtocol (or SerDesProtocol, depending on which variant you choose).
> 
> > +  LS1043A_DEVICE_CONFIG  *Dcfg;
> 
> DeviceConfig
> 
> > +
> > +  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;  SrdsProt =
> > + DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
> > + SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;
> 
> RCWSR is not a generally understood abbreviation. Please either add a glossary
> section to the start of the file, or expand the abbreviation at every point of use.
> 
> > +
> > +  SerDesInstanceProbeLanes (SRDS_1, SrdsProt,
> > +                                    FixedPcdGet8 (PcdSerdesLanes),
> > +                                    SERDES_PRTCL_COUNT,
> > +                                    SerDesConfigTbl[SRDS_1],
> > +                                    SerDesLaneProbeCallback,
> > +                                    Arg); }
> > +
> > +/**
> > +  Function to return Serdes protocol map for all serdes available on board.
> > +
> > +  @param  SerDesPrtclMap   Pointer to Serdes protocl map.
> > +
> > +**/
> > +VOID
> > +GetSerdesProtocolMaps (
> > +  OUT UINT64   *SerDesPrtclMap
> > +  )
> > +{
> > +  UINT32                 SrdsProt;
> > +  LS1043A_DEVICE_CONFIG  *Dcfg;
> 
> Both comments for previous function apply here too.
> 
> /
>     Leif


Thank you Leif for the review. I am addressing your review comments.


> 
> > +  EFI_STATUS             Status;
> > +
> > +  *SerDesPrtclMap = 0x0;
> > +  Dcfg = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;  SrdsProt =
> > + DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK;
> > + SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT;
> > +
> > +  Status = GetSerDesMap (SRDS_1, SrdsProt, FixedPcdGet8 (PcdSerdesLanes),
> > +                          SERDES_PRTCL_COUNT, SerDesConfigTbl[SRDS_1],
> > +                          SerDesPrtclMap);
> > +
> > +  if (Status != EFI_SUCCESS) {
> > +    DEBUG ((DEBUG_ERROR, "%a: failed for Serdes1 \n",__FUNCTION__));
> > +    *SerDesPrtclMap = 0x0;
> > +  }
> > +}
> > --
> > 2.7.4
> >

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

end of thread, other threads:[~2020-06-07 11:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-01 19:27 [PATCH edk2-platforms 0/3] Add SerDes Support Wasim Khan
2020-06-01 19:27 ` [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Wasim Khan
2020-06-05 12:26   ` Leif Lindholm
2020-06-01 19:27 ` [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Wasim Khan
2020-06-05 12:36   ` Leif Lindholm
2020-06-07 11:14     ` Wasim Khan
2020-06-01 19:27 ` [PATCH edk2-platforms 3/3] Silicon/NXP: PciHostBridgeLib: Initialize only enabled PCIe controllers Wasim Khan
2020-06-05 12:38   ` Leif Lindholm

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