Re: [PATCH edk2-platforms v2 2/3] Silicon/NXP: LS1043A: Add SerDes Support

["Leif Lindholm" <leif@nuviainc.com>]

On Sun, Jun 07, 2020 at 18:45:49 +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>
> ---
> 
> Notes:
>     Changes in V2:
>     - Addressed review comments for structure, variable and function names
>     - Removed SocSerDes.h and moved data definitions to SerDes.c
> 
>  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/Library/SocLib/SerDes.c   | 132 ++++++++++++++++++++
>  4 files changed, 168 insertions(+)
> 
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index e023bfbc7c04..1ac7c6da7092 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..728e8f0c8ee1 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]
> +  SerDes.c
>    SocLib.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..f015b40403c5
> --- /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
> +GetSerDesProtocolMap (
> +  OUT UINT64               *SerDesProtocolMap
> +  );
> +
> +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/Library/SocLib/SerDes.c b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> new file mode 100644
> index 000000000000..90fa7146faa7
> --- /dev/null
> +++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
> @@ -0,0 +1,132 @@
> +/** 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 <Uefi.h>
> +
> +// SerDes1 Protocol Mask in Reset Configuration Word (RCW) Status Register
> +#define SERDES1_PROTOCOL_MASK      0xffff0000
> +
> +// SerDes1 Protocol Shift in Reset Configuration Word (RCW) Status Register
> +#define SERDES1_PROTOCOL_SHIFT     16
> +
> +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_PROTOCOL_COUNT
> +} SERDES_PROTOCOL;

Thanks for moving the static structs over here.
The typedef was fine in the .h though - are you not going to need it
there at some point?

> +
> +SERDES_CONFIG gSerDes1ConfigTable[] = {
> +  {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 *gSerDesConfig[] = {
> +  gSerDes1ConfigTable
> +};
> +
> +/**
> +  Probe all SerDes 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                 SerDesProtocol;
> +  LS1043A_DEVICE_CONFIG  *DeviceConfig;
> +
> +  DeviceConfig = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
> +  SerDesProtocol = DcfgRead32 ((UINTN)&DeviceConfig->RcwSr[4]) & SERDES1_PROTOCOL_MASK;
> +  SerDesProtocol >>= SERDES1_PROTOCOL_SHIFT;
> +
> +  SerDesInstanceProbeLanes (SERDES_1, SerDesProtocol,
> +                                    FixedPcdGet8 (PcdSerDesLanes),
> +                                    SERDES_PROTOCOL_COUNT,
> +                                    gSerDesConfig[SERDES_1],
> +                                    SerDesLaneProbeCallback,
> +                                    Arg);

And this indentation is a bit funky.

> +}
> +
> +/**
> +  Function to return SerDes protocol map for all SerDes available on board.
> +
> +  @param  SerDesProtocolMap   Pointer to SerDes protocl map.
> +
> +**/
> +VOID
> +GetSerDesProtocolMap (
> +  OUT UINT64   *SerDesProtocolMap
> +  )
> +{
> +  UINT32                 SerDesProtocol;
> +  LS1043A_DEVICE_CONFIG  *DeviceConfig;
> +  EFI_STATUS             Status;
> +
> +  *SerDesProtocolMap = 0;
> +  DeviceConfig = (LS1043A_DEVICE_CONFIG  *)LS1043A_DCFG_ADDRESS;
> +  SerDesProtocol = DcfgRead32 ((UINTN)&DeviceConfig->RcwSr[4]) & SERDES1_PROTOCOL_MASK;
> +  SerDesProtocol >>= SERDES1_PROTOCOL_SHIFT;
> +
> +  Status = GetSerDesMap (SERDES_1, SerDesProtocol, FixedPcdGet8 (PcdSerDesLanes),
> +                          SERDES_PROTOCOL_COUNT, gSerDesConfig[SERDES_1],
> +                          SerDesProtocolMap);

As is this one, in a different way to the one above.

/
    Leif

> +
> +  if (Status != EFI_SUCCESS) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed for SerDes1 \n",__FUNCTION__));
> +    *SerDesProtocolMap = 0;
> +  }
> +}
> -- 
> 2.7.4
>