Re: [PATCH edk2-platforms 03/39] SocLib : Add support for initialization of peripherals

[Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>]

Hi Leif,

Thanks for review, responses inlined.

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Wednesday, April 18, 2018 8:42 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Cc: ard.biesheuvel@linaro.org; edk2-devel@lists.01.org; Udit Kumar
> <udit.kumar@nxp.com>; Varun Sethi <V.Sethi@nxp.com>
> Subject: Re: [PATCH edk2-platforms 03/39] SocLib : Add support for
> initialization of peripherals
> 
> On Fri, Feb 16, 2018 at 02:19:59PM +0530, Meenakshi wrote:
> > From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> >
> > Add SocInit function that initializes peripherals
> > and print board and soc information.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > ---
> >  Silicon/NXP/Chassis/Chassis.c             | 388
> ++++++++++++++++++++++++++++++
> >  Silicon/NXP/Chassis/Chassis.h             | 144 +++++++++++
> >  Silicon/NXP/Chassis/Chassis2/Chassis2.dec |  19 ++
> >  Silicon/NXP/Chassis/Chassis2/SerDes.h     |  68 ++++++
> >  Silicon/NXP/Chassis/Chassis2/Soc.c        | 172 +++++++++++++
> >  Silicon/NXP/Chassis/Chassis2/Soc.h        | 367
> ++++++++++++++++++++++++++++
> >  Silicon/NXP/Chassis/LS1043aSocLib.inf     |  47 ++++
> >  Silicon/NXP/Chassis/SerDes.c              | 271 +++++++++++++++++++++
> >  Silicon/NXP/Include/Bitops.h              | 179 ++++++++++++++
> >  Silicon/NXP/LS1043A/Include/SocSerDes.h   |  55 +++++
> >  10 files changed, 1710 insertions(+)
> >  create mode 100644 Silicon/NXP/Chassis/Chassis.c
> >  create mode 100644 Silicon/NXP/Chassis/Chassis.h
> >  create mode 100644 Silicon/NXP/Chassis/Chassis2/Chassis2.dec
> >  create mode 100644 Silicon/NXP/Chassis/Chassis2/SerDes.h
> >  create mode 100644 Silicon/NXP/Chassis/Chassis2/Soc.c
> >  create mode 100644 Silicon/NXP/Chassis/Chassis2/Soc.h
> >  create mode 100644 Silicon/NXP/Chassis/LS1043aSocLib.inf
> >  create mode 100644 Silicon/NXP/Chassis/SerDes.c
> >  create mode 100644 Silicon/NXP/Include/Bitops.h
> >  create mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h
> >
> > diff --git a/Silicon/NXP/Chassis/Chassis.c b/Silicon/NXP/Chassis/Chassis.c
> > new file mode 100644
> > index 0000000..9f2928b
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis.c
> > @@ -0,0 +1,388 @@
> > +/** @file
> > +  SoC specific Library containg functions to initialize various SoC
> components
> > +
> > +  Copyright 2017 NXP
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the
> BSD License
> > +  which accompanies this distribution. The full text of the license may be
> found at
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include <Base.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/BeIoLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/SerialPortLib.h>
> > +
> > +#include <Soc.h>
> > +
> > +#include "Chassis.h"
> > +
> > +UINT32
> > +EFIAPI
> > +GurRead (
> > +  IN  UINTN     Address
> > +  )
> > +{
> > +  if (FixedPcdGetBool (PcdGurBigEndian)) {
> > +    return BeMmioRead32 (Address);
> > +  } else {
> > +    return MmioRead32 (Address);
> > +  }
> > +}
> 
> So, since this pattern is being repeated in multiple modules, I think
> it would make sense to have (for now) an NXP-specific helper library
> to return a struct of function pointers accessing either
> 
> I.e. something like (in a header)
> 
> typedef struct _MMIO_OPERATIONS {
>   MMIO_WRITE_8 Write8;
>   ...
> } MMIO_OPERATIONS;
> 
> and then in the .c file:
> 
> STATIC MMIO_OPERATIONS SwappingFunctions = {
>   SwapMmioWrite8,
>   ...
> };
> 
> STATIC MMIO_OPERATIONS NonSwappingFunctions = {
>   MmioWrite8,
>   ...
> };
> 
> MMIO_OPERATIONS *GetMmioOperationsStructure (BOOL BigEndian)
> {
>   if (BigEndian) {
>     return &SwappingFunctions;
>   else {
>     return &NonSwappingFunctions;
>   }
> }
> 
> To be used in _this_ file as
> 
> STATIC MMIO_OPERATIONS mGurOps;
> 
> Initialized in some sort of Initialize() function as
>   mGurOps = GetMmioOperationsStructure (FixedPcdGetBool
> (PcdGurBigEndian));
> 
> This will then let us fix up whatever the final core function names
> end up being in a single place.
> 
> This feedback applies also to the resubmitted watchdog driver (but
> should be a very minor change there).
> 
Okay, sounds a good idea to me.

> > +
> > +/*
> > + *  Structure to list available SOCs.
> > + */
> > +STATIC CPU_TYPE CpuTypeList[] = {
> > +  CPU_TYPE_ENTRY (LS1043A, LS1043A, 4),
> > +};
> > +
> > +/*
> > + * Return the number of bits set
> > + */
> > +STATIC
> > +inline
> > +UINTN
> > +CountSetBits (
> 
> This helper function is only used in one location.
> If there is enough in this set to break out into a generically useful
> helper library (for later consideration for inclusion in edk2 for
> example), please do so. Otherwise, please move this inline in the
> (otherwise near-empty) calling function.
>
Okay, will see what is more approproate.
 
> > +  IN  UINTN  Num
> > +  )
> > +{
> > +  UINTN Count;
> > +
> > +  Count = 0;
> > +
> > +  while (Num) {
> > +    Count += Num & 1;
> > +    Num >>= 1;
> > +  }
> > +
> > +  return Count;
> > +}
> > +
> > +/*
> > + * Return the type of initiator (core or hardware accelerator)
> > + */
> > +UINT32
> > +InitiatorType (
> > +  IN UINT32 Cluster,
> > +  IN UINTN  InitId
> > +  )
> > +{
> > +  CCSR_GUR *GurBase;
> > +  UINT32   Idx;
> > +  UINT32   Type;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +  Idx = (Cluster >> (InitId * 8)) & TP_CLUSTER_INIT_MASK;
> > +  Type = GurRead ((UINTN)&GurBase->TpItyp[Idx]);
> > +
> > +  if (Type & TP_ITYP_AV_MASK) {
> > +    return Type;
> > +  }
> > +
> > +  return 0;
> > +}
> > +
> > +/*
> > + *  Return the mask for number of cores on this SOC.
> > + */
> > +UINT32
> > +CpuMask (
> > +  VOID
> > +  )
> > +{
> > +  CCSR_GUR  *GurBase;
> > +  UINTN     ClusterIndex;
> > +  UINTN     Count;
> > +  UINT32    Cluster;
> > +  UINT32    Type;
> > +  UINT32    Mask;
> > +  UINTN     InitiatorIndex;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +  ClusterIndex = 0;
> > +  Count = 0;
> > +  Mask = 0;
> > +
> > +  do {
> > +    Cluster = GurRead ((UINTN)&GurBase-
> >TpCluster[ClusterIndex].Lower);
> > +    for (InitiatorIndex = 0; InitiatorIndex < TP_INIT_PER_CLUSTER;
> InitiatorIndex++) {
> > +      Type = InitiatorType (Cluster, InitiatorIndex);
> > +      if (Type) {
> > +        if (TP_ITYP_TYPE_MASK (Type) == TP_ITYP_TYPE_ARM)
> > +          Mask |= 1 << Count;
> 
> Always use braces {} with if.
> 
yes
> > +        Count++;
> > +      }
> > +    }
> > +    ClusterIndex++;
> > +  } while (CHECK_CLUSTER (Cluster));
> > +
> > +  return Mask;
> > +}
> > +
> > +/*
> > + *  Return the number of cores on this SOC.
> > + */
> > +UINTN
> > +CpuNumCores (
> > +  VOID
> > +  )
> > +{
> > +    return CountSetBits (CpuMask ());
> 
> Spurious indentation. (Should be 2 spaces.)
> 
> > +}
> > +
> > +/*
> > + *  Return the type of core i.e. A53, A57 etc of inputted
> > + *  core number.
> > + */
> > +UINT32
> > +QoriqCoreToType (
> > +  IN UINTN Core
> > +  )
> > +{
> > +  CCSR_GUR  *GurBase;
> > +  UINTN     ClusterIndex;
> > +  UINTN     Count;
> > +  UINT32    Cluster;
> > +  UINT32    Type;
> > +  UINTN     InitiatorIndex;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +  ClusterIndex = 0;
> > +  Count = 0;
> > +
> > +  do {
> > +    Cluster = GurRead ((UINTN)&GurBase-
> >TpCluster[ClusterIndex].Lower);
> > +    for (InitiatorIndex = 0; InitiatorIndex < TP_INIT_PER_CLUSTER;
> InitiatorIndex++) {
> > +      Type = InitiatorType (Cluster, InitiatorIndex);
> > +      if (Type) {
> > +        if (Count == Core)
> > +          return Type;
> 
> Always braces with if.
> 
> > +        Count++;
> > +      }
> > +    }
> > +    ClusterIndex++;
> > +  } while (CHECK_CLUSTER (Cluster));
> > +
> > +  return -1;      /* cannot identify the cluster */
> 
> Please use a #define for return value.
> 
ok
> > +}
> > +
> > +/*
> > + * Print CPU information
> > + */
> > +VOID
> > +PrintCpuInfo (
> > +  VOID
> > +  )
> > +{
> > +  SYS_INFO SysInfo;
> > +  UINTN    CoreIndex;
> > +  UINTN    Core;
> > +  UINT32   Type;
> > +  CHAR8    Buffer[100];
> > +  UINTN    CharCount;
> > +
> > +  GetSysInfo (&SysInfo);
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Clock Configuration:");
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> 
> Why SerialPortWrite instead of Print? (Question for throughout patch.)
> 
No particular reason, will replace it.
> > +
> > +  ForEachCpu (CoreIndex, Core, CpuNumCores (), CpuMask ()) {
> > +    if (!(CoreIndex % 3)) {
> > +      CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n      ");
> > +      SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +    }
> > +
> > +    Type = TP_ITYP_VERSION (QoriqCoreToType (Core));
> > +    CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "CPU%d(%a):%-4d MHz
> ", Core,
> > +        Type == TY_ITYP_VERSION_A7 ? "A7 " :
> > +        (Type == TY_ITYP_VERSION_A53 ? "A53" :
> > +         (Type == TY_ITYP_VERSION_A57 ? "A57" :
> > +          (Type == TY_ITYP_VERSION_A72 ? "A72" : " Unknown Core "))),
> 
> That's a lot more nested than I like my ternaries.
> Can you rewrite as a switch statement that sets a pointer.
> 
Ok
> > +        SysInfo.FreqProcessor[Core] / MEGA_HZ);
> 
> Please use either MEGAHERTZ or MHZ.
> 
ok
> > +    SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +  }
> > +
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n      Bus:      %-4d MHz
> ",
> > +                           SysInfo.FreqSystemBus / MEGA_HZ);
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "DDR:      %-4d MT/s",
> > +                           SysInfo.FreqDdrBus / MEGA_HZ);
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +
> > +  if (SysInfo.FreqFman[0] != 0) {
> > +    CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n      FMAN:     %-4d
> MHz  ",
> > +                             SysInfo.FreqFman[0] / MEGA_HZ);
> > +    SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +  }
> > +
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n");
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +}
> > +
> > +/*
> > + * Return system bus frequency
> > + */
> > +UINT64
> > +GetBusFrequency (
> > +   VOID
> > +  )
> > +{
> > +  SYS_INFO SocSysInfo;
> > +
> > +  GetSysInfo (&SocSysInfo);
> > +
> > +  return SocSysInfo.FreqSystemBus;
> > +}
> > +
> > +/*
> > + * Return SDXC bus frequency
> > + */
> > +UINT64
> > +GetSdxcFrequency (
> > +   VOID
> > +  )
> > +{
> > +  SYS_INFO SocSysInfo;
> > +
> > +  GetSysInfo (&SocSysInfo);
> > +
> > +  return SocSysInfo.FreqSdhc;
> > +}
> > +
> > +/*
> > + * Print Soc information
> > + */
> > +VOID
> > +PrintSoc (
> > +  VOID
> > +  )
> > +{
> > +  CHAR8    Buf[16];
> > +  CCSR_GUR *GurBase;
> > +  UINTN    Count;
> > +  UINTN    Svr;
> > +  UINTN    Ver;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +
> > +  Buf[0] = L'\0';
> > +  Svr = GurRead ((UINTN)&GurBase->Svr);
> > +  Ver = SVR_SOC_VER (Svr);
> > +
> > +  for (Count = 0; Count < ARRAY_SIZE (CpuTypeList); Count++)
> > +    if ((CpuTypeList[Count].SocVer & SVR_WO_E) == Ver) {
> > +      AsciiStrCpy (Buf, (CONST CHAR8 *)CpuTypeList[Count].Name);
> > +
> > +      if (IS_E_PROCESSOR (Svr)) {
> > +        AsciiStrCat (Buf, (CONST CHAR8 *)"E");
> > +      }
> > +      break;
> > +    }
> > +
> > +  if (Count == ARRAY_SIZE (CpuTypeList)) {
> > +    AsciiStrCpy (Buf, (CONST CHAR8 *)"unknown");
> > +  }
> > +
> > +  DEBUG ((DEBUG_INFO, "SoC: %a (0x%x); Rev %d.%d\n",
> > +         Buf, Svr, SVR_MAJOR (Svr), SVR_MINOR (Svr)));
> > +
> > +  return;
> > +}
> > +
> > +/*
> > + * Dump RCW (Reset Control Word) on console
> > + */
> > +VOID
> > +PrintRCW (
> > +  VOID
> > +  )
> > +{
> > +  CCSR_GUR *Base;
> > +  UINTN    Count;
> > +  CHAR8    Buffer[100];
> > +  UINTN    CharCount;
> > +
> > +  Base = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +
> > +  /*
> > +   * Display the RCW, so that no one gets confused as to what RCW
> > +   * we're actually using for this boot.
> > +   */
> > +
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer),
> > +               "Reset Configuration Word (RCW):");
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +  for (Count = 0; Count < ARRAY_SIZE(Base->RcwSr); Count++) {
> > +    UINT32 Rcw = BeMmioRead32((UINTN)&Base->RcwSr[Count]);
> > +
> > +    if ((Count % 4) == 0) {
> > +      CharCount = AsciiSPrint (Buffer, sizeof (Buffer),
> > +                   "\n       %08x:", Count * 4);
> > +      SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +    }
> > +
> > +    CharCount = AsciiSPrint (Buffer, sizeof (Buffer), " %08x", Rcw);
> > +    SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +  }
> > +
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n");
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +}
> > +
> > +/*
> > + * Setup SMMU in bypass mode
> > + * and also set its pagesize
> > + */
> > +VOID
> > +SmmuInit (
> > +  VOID
> > +  )
> > +{
> > +  UINT32 Value;
> > +
> > +  /* set pagesize as 64K and ssmu-500 in bypass mode */
> > +  Value = (MmioRead32 ((UINTN)SMMU_REG_SACR) |
> SACR_PAGESIZE_MASK);
> > +  MmioWrite32 ((UINTN)SMMU_REG_SACR, Value);
> > +
> > +  Value = (MmioRead32 ((UINTN)SMMU_REG_SCR0) |
> SCR0_CLIENTPD_MASK) & ~SCR0_USFCFG_MASK;
> > +  MmioWrite32 ((UINTN)SMMU_REG_SCR0, Value);
> > +
> > +  Value = (MmioRead32 ((UINTN)SMMU_REG_NSCR0) |
> SCR0_CLIENTPD_MASK) & ~SCR0_USFCFG_MASK;
> > +  MmioWrite32 ((UINTN)SMMU_REG_NSCR0, Value);
> > +}
> > +
> > +/*
> > + * Return current Soc Name form CpuTypeList
> > + */
> > +CHAR8 *
> > +GetSocName (
> > +  VOID
> > +  )
> > +{
> > +  UINT8     Count;
> > +  UINTN     Svr;
> > +  UINTN     Ver;
> > +  CCSR_GUR  *GurBase;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +
> > +  Svr = GurRead ((UINTN)&GurBase->Svr);
> > +  Ver = SVR_SOC_VER (Svr);
> > +
> > +  for (Count = 0; Count < ARRAY_SIZE (CpuTypeList); Count++) {
> > +    if ((CpuTypeList[Count].SocVer & SVR_WO_E) == Ver) {
> > +      return (CHAR8 *)CpuTypeList[Count].Name;
> > +    }
> > +  }
> > +
> > +  return NULL;
> > +}
> > diff --git a/Silicon/NXP/Chassis/Chassis.h b/Silicon/NXP/Chassis/Chassis.h
> > new file mode 100644
> > index 0000000..4bdb4d0
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis.h
> > @@ -0,0 +1,144 @@
> > +/** @file
> > +*  Header defining the Base addresses, sizes, flags etc for chassis 1
> > +*
> > +*  Copyright 2017 NXP
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of the
> BSD License
> > +*  which accompanies this distribution.  The full text of the license may be
> found at
> > +*
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +*
> > +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#ifndef __CHASSIS_H__
> > +#define __CHASSIS_H__
> > +
> > +#define TP_ITYP_AV_MASK            0x00000001  /* Initiator available */
> > +#define TP_ITYP_TYPE_MASK(x)       (((x) & 0x6) >> 1) /* Initiator Type */
> > +#define TP_ITYP_TYPE_ARM           0x0
> > +#define TP_ITYP_TYPE_PPC           0x1
> > +#define TP_ITYP_TYPE_OTHER         0x2  /* StarCore DSP */
> > +#define TP_ITYP_TYPE_HA            0x3  /* HW Accelerator */
> > +#define TP_ITYP_THDS(x)            (((x) & 0x18) >> 3)  /* # threads */
> > +#define TP_ITYP_VERSION(x)         (((x) & 0xe0) >> 5)  /* Initiator Version
> */
> > +#define TP_CLUSTER_INIT_MASK       0x0000003f  /* initiator mask */
> > +#define TP_INIT_PER_CLUSTER        4
> > +
> > +#define TY_ITYP_VERSION_A7         0x1
> > +#define TY_ITYP_VERSION_A53        0x2
> > +#define TY_ITYP_VERSION_A57        0x3
> > +#define TY_ITYP_VERSION_A72        0x4
> > +
> > +STATIC
> > +inline
> > +UINTN
> > +CpuMaskNext (
> > +  IN  UINTN  Cpu,
> > +  IN  UINTN  Mask
> > +  )
> > +{
> > +  for (Cpu++; !((1 << Cpu) & Mask); Cpu++)
> > +    ;
> > +
> > +  return Cpu;
> > +}
> > +
> > +#define ForEachCpu(Iter, Cpu, NumCpus, Mask) \
> > +  for (Iter = 0, Cpu = CpuMaskNext(-1, Mask); \
> > +    Iter < NumCpus; \
> > +    Iter++, Cpu = CpuMaskNext(Cpu, Mask)) \
> > +
> > +#define CPU_TYPE_ENTRY(N, V, NC) \
> > +           { .Name = #N, .SocVer = SVR_##V, .NumCores = (NC)}
> > +
> > +#define SVR_WO_E                    0xFFFFFE
> > +#define SVR_LS1043A                 0x879200
> > +
> > +#define SVR_MAJOR(svr)              (((svr) >> 4) & 0xf)
> > +#define SVR_MINOR(svr)              (((svr) >> 0) & 0xf)
> > +#define SVR_SOC_VER(svr)            (((svr) >> 8) & SVR_WO_E)
> > +#define IS_E_PROCESSOR(svr)         (!((svr >> 8) & 0x1))
> > +
> > +#define MEGA_HZ                     1000000
> > +
> > +typedef struct {
> > +  CHAR8  Name[16];
> > +  UINT32 SocVer;
> > +  UINT32 NumCores;
> > +} CPU_TYPE;
> > +
> > +typedef struct {
> > +  UINTN CpuClk;  /* CPU clock in Hz! */
> > +  UINTN BusClk;
> > +  UINTN MemClk;
> > +  UINTN PciClk;
> > +  UINTN SdhcClk;
> > +} SOC_CLOCK_INFO;
> > +
> > +/*
> > + * Print Soc information
> > + */
> > +VOID
> > +PrintSoc (
> > +  VOID
> > +  );
> > +
> > +/*
> > + * Initialize Clock structure
> > + */
> > +VOID
> > +ClockInit (
> > +  VOID
> > +  );
> > +
> > +/*
> > + * Setup SMMU in bypass mode
> > + * and also set its pagesize
> > + */
> > +VOID
> > +SmmuInit (
> > +  VOID
> > +  );
> > +
> > +/*
> > + * Print CPU information
> > + */
> > +VOID
> > +PrintCpuInfo (
> > +  VOID
> > +  );
> > +
> > +/*
> > + * Dump RCW (Reset Control Word) on console
> > + */
> > +VOID
> > +PrintRCW (
> > +  VOID
> > +  );
> > +
> > +UINT32
> > +InitiatorType (
> > +  IN UINT32 Cluster,
> > +  IN UINTN InitId
> > +  );
> > +
> > +/*
> > + *  Return the mask for number of cores on this SOC.
> > + */
> > +UINT32
> > +CpuMask (
> > +  VOID
> > +  );
> > +
> > +/*
> > + *  Return the number of cores on this SOC.
> > + */
> > +UINTN
> > +CpuNumCores (
> > +  VOID
> > +  );
> > +
> > +#endif /* __CHASSIS_H__ */
> > diff --git a/Silicon/NXP/Chassis/Chassis2/Chassis2.dec
> b/Silicon/NXP/Chassis/Chassis2/Chassis2.dec
> > new file mode 100644
> > index 0000000..cf41b3c
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis2/Chassis2.dec
> > @@ -0,0 +1,19 @@
> > +# @file
> > +#
> > +# Copyright 2017 NXP
> > +#
> > +# This program and the accompanying materials are licensed and made
> available under
> > +# the terms and conditions of the BSD License which accompanies this
> distribution.
> > +# The full text of the license may be found at
> > +#
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +#
> > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +#
> > +#
> > +
> > +[Defines]
> > +  DEC_SPECIFICATION              = 0x00010005
> 
> 0x0001001a
> 
OK
> > +
> > +[Includes]
> > +  .
> 
> Hmm. In general, this appears to be using the .dec file simply to
> tweak which "Soc.h" and "SerDes.h" file gets included in
> Silicon/NXP/Chassis/Chassis.c and Silicon/NXP/Chassis/SerDes.c
> 
> First of all, a .dec is a package declaration file, and that is not
> what is being created here. So this doesn't follow expected TianoCore
> layout.
> 
> Secondly, there appears to be some quite spurious differences between
> Chassis2/Soc.h and Chassis3/Soc.h: difference in indentation,
> differences in comment style, use of macros for array sizes in struct
> definitions in one and hardcoded in the other.
> Can this be cleaned up so that a
> diff -u Silicon/NXP/Chassis/Chassis2/Soc.h
> Silicon/NXP/Chassis/Chassis3/Soc.h
> describes the differences between the platforms rather than the
> differences in coding style?
> 
> Finally, can Silicon/NXP/Chassis be moved across to
> Silicon/NXP/Library/SocLib, with include files under
> Silicon/NXP/Include/{Chassis2|Chassis3}?
> The different .inf files can then set a -D CHASSIS_MODEL=Chassis2 or
> -D CHASSIS_MODEL=Chassis3 in GCC:*_*_*_CC_FLAGS [BuildOptions]
> with affected source files referring to them as
> #include <Library/CHASSIS_MODEL/Soc.h>
> and
> #include <Library/CHASSIS_MODEL/SerDes.h>
> ?
> 
Hmm, will try to implement same.

> > diff --git a/Silicon/NXP/Chassis/Chassis2/SerDes.h
> b/Silicon/NXP/Chassis/Chassis2/SerDes.h
> > new file mode 100644
> > index 0000000..4c874aa
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis2/SerDes.h
> > @@ -0,0 +1,68 @@
> > +/** SerDes.h
> > + The Header file of SerDes Module for Chassis 2
> > +
> > + Copyright 2017 NXP
> > +
> > + This program and the accompanying materials
> > + are licensed and made available under the terms and conditions of the
> BSD License
> > + which accompanies this distribution. The full text of the license may be
> found
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef __SERDES_H__
> > +#define __SERDES_H__
> > +
> > +#include <Uefi/UefiBaseType.h>
> > +
> > +#define SRDS_MAX_LANES     4
> > +
> > +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;
> > +
> > +typedef enum {
> > +  SRDS_1  = 0,
> > +  SRDS_2,
> > +  SRDS_MAX_NUM
> > +} SERDES_NUMBER;
> > +
> > +typedef struct {
> > +  UINT16 Protocol;
> > +  UINT8  SrdsLane[SRDS_MAX_LANES];
> > +} SERDES_CONFIG;
> > +
> > +typedef VOID
> > +(*SERDES_PROBE_LANES_CALLBACK) (
> > +  IN SERDES_PROTOCOL LaneProtocol,
> > +  IN VOID *Arg
> > +  );
> > +
> > +VOID
> > +SerDesProbeLanes(
> > +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> > +  IN VOID *Arg
> > +  );
> > +
> > +#endif /* __SERDES_H */
> > diff --git a/Silicon/NXP/Chassis/Chassis2/Soc.c
> b/Silicon/NXP/Chassis/Chassis2/Soc.c
> > new file mode 100644
> > index 0000000..7f9f963
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis2/Soc.c
> > @@ -0,0 +1,172 @@
> > +/** @Soc.c
> > +  SoC specific Library containg functions to initialize various SoC
> components
> > +
> > +  Copyright 2017 NXP
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the
> BSD License
> > +  which accompanies this distribution. The full text of the license may be
> found at
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include <Base.h>
> > +#include <Chassis.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/BaseMemoryLib/MemLibInternals.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/SerialPortLib.h>
> > +
> > +#include "Soc.h"
> > +
> > +/**
> > +  Calculate the frequency of various controllers and
> > +  populate the passed structure with frequuencies.
> > +
> > +  @param  PtrSysInfo            Input structure to populate with
> > +                                frequencies.
> > +**/
> > +VOID
> > +GetSysInfo (
> > +  OUT SYS_INFO *PtrSysInfo
> > +  )
> > +{
> > +  CCSR_GUR     *GurBase;
> > +  CCSR_CLOCK   *ClkBase;
> > +  UINTN        CpuIndex;
> > +  UINT32       TempRcw;
> > +  UINT32       CPllSel;
> > +  UINT32       CplxPll;
> > +  CONST UINT8  CoreCplxPll[8] = {
> > +    [0] = 0,    /* CC1 PPL / 1 */
> > +    [1] = 0,    /* CC1 PPL / 2 */
> > +    [4] = 1,    /* CC2 PPL / 1 */
> > +    [5] = 1,    /* CC2 PPL / 2 */
> > +  };
> > +
> > +  CONST UINT8  CoreCplxPllDivisor[8] = {
> > +    [0] = 1,    /* CC1 PPL / 1 */
> > +    [1] = 2,    /* CC1 PPL / 2 */
> > +    [4] = 1,    /* CC2 PPL / 1 */
> > +    [5] = 2,    /* CC2 PPL / 2 */
> > +  };
> > +
> > +  UINTN        PllCount;
> > +  UINTN        FreqCPll[NUM_CC_PLLS];
> > +  UINTN        PllRatio[NUM_CC_PLLS];
> > +  UINTN        SysClk;
> > +
> > +  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +  ClkBase = (VOID *)PcdGet64 (PcdClkBaseAddr);
> > +  SysClk = CLK_FREQ;
> > +
> > +  SetMem (PtrSysInfo, sizeof (SYS_INFO), 0);
> > +
> > +  PtrSysInfo->FreqSystemBus = SysClk;
> > +  PtrSysInfo->FreqDdrBus = SysClk;
> > +
> > +  //
> > +  // selects the platform clock:SYSCLK ratio and calculate
> > +  // system frequency
> > +  //
> > +  PtrSysInfo->FreqSystemBus *= (GurRead ((UINTN)&GurBase->RcwSr[0])
> >>
> > +                CHASSIS2_RCWSR0_SYS_PLL_RAT_SHIFT) &
> > +                CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK;
> > +  //
> > +  // selects the DDR PLL:SYSCLK Ratio and calculate DDR frequency
> > +  //
> > +  PtrSysInfo->FreqDdrBus *= (GurRead ((UINTN)&GurBase->RcwSr[0]) >>
> > +                CHASSIS2_RCWSR0_MEM_PLL_RAT_SHIFT) &
> > +                CHASSIS2_RCWSR0_MEM_PLL_RAT_MASK;
> > +
> > +  for (PllCount = 0; PllCount < NUM_CC_PLLS; PllCount++) {
> > +    PllRatio[PllCount] = (GurRead ((UINTN)&ClkBase-
> >PllCgSr[PllCount].PllCnGSr) >> 1) & 0xff;
> > +    if (PllRatio[PllCount] > 4) {
> > +      FreqCPll[PllCount] = SysClk * PllRatio[PllCount];
> > +    } else {
> > +      FreqCPll[PllCount] = PtrSysInfo->FreqSystemBus * PllRatio[PllCount];
> > +    }
> > +  }
> > +
> > +  //
> > +  // Calculate Core frequency
> > +  //
> > +  for (CpuIndex = 0; CpuIndex < MAX_CPUS; CpuIndex++) {
> > +    CPllSel = (GurRead ((UINTN)&ClkBase->ClkcSr[CpuIndex].ClkCnCSr) >>
> 27) & 0xf;
> > +    CplxPll = CoreCplxPll[CPllSel];
> > +
> > +    PtrSysInfo->FreqProcessor[CpuIndex] = FreqCPll[CplxPll] /
> CoreCplxPllDivisor[CPllSel];
> > +  }
> > +
> > +  //
> > +  // Calculate FMAN frequency
> > +  //
> > +  TempRcw = GurRead ((UINTN)&GurBase->RcwSr[7]);
> > +  switch ((TempRcw & HWA_CGA_M1_CLK_SEL) >>
> HWA_CGA_M1_CLK_SHIFT) {
> > +  case 2:
> > +    PtrSysInfo->FreqFman[0] = FreqCPll[0] / 2;
> > +    break;
> > +  case 3:
> > +    PtrSysInfo->FreqFman[0] = FreqCPll[0] / 3;
> > +    break;
> > +  case 4:
> > +    PtrSysInfo->FreqFman[0] = FreqCPll[0] / 4;
> > +    break;
> > +  case 5:
> > +    PtrSysInfo->FreqFman[0] = PtrSysInfo->FreqSystemBus;
> > +    break;
> > +  case 6:
> > +    PtrSysInfo->FreqFman[0] = FreqCPll[1] / 2;
> > +    break;
> > +  case 7:
> > +    PtrSysInfo->FreqFman[0] = FreqCPll[1] / 3;
> > +    break;
> > +  default:
> > +    DEBUG ((DEBUG_WARN, "Error: Unknown FMan1 clock select!\n"));
> > +    break;
> > +  }
> > +  PtrSysInfo->FreqSdhc = PtrSysInfo->FreqSystemBus/PcdGet32
> (PcdPlatformFreqDiv);
> > +  PtrSysInfo->FreqQman = PtrSysInfo->FreqSystemBus/PcdGet32
> (PcdPlatformFreqDiv);
> > +}
> > +
> > +/**
> > +  Function to initialize SoC specific constructs
> > +  CPU Info
> > +  SoC Personality
> > +  Board Personality
> > +  RCW prints
> > + **/
> > +VOID
> > +SocInit (
> > +  VOID
> > +  )
> > +{
> > +  CHAR8 Buffer[100];
> > +  UINTN CharCount;
> > +
> > +  SmmuInit ();
> > +
> > +  //
> > +  // Early init serial Port to get board information.
> > +  //
> > +  SerialPortInitialize ();
> > +  CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\nUEFI firmware
> (version %s built at %a on %a)\n\r",
> > +    (CHAR16*)PcdGetPtr (PcdFirmwareVersionString), __TIME__,
> __DATE__);
> > +  SerialPortWrite ((UINT8 *) Buffer, CharCount);
> > +
> > +  PrintCpuInfo ();
> > +
> > +  //
> > +  // Print Reset control Word
> > +  //
> > +  PrintRCW ();
> > +  PrintSoc ();
> > +
> > +  return;
> > +}
> > diff --git a/Silicon/NXP/Chassis/Chassis2/Soc.h
> b/Silicon/NXP/Chassis/Chassis2/Soc.h
> > new file mode 100644
> > index 0000000..10e99ab
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/Chassis2/Soc.h
> > @@ -0,0 +1,367 @@
> > +/** Soc.h
> > +*  Header defining the Base addresses, sizes, flags etc for chassis 1
> > +*
> > +*  Copyright 2017 NXP
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of the
> BSD License
> > +*  which accompanies this distribution.  The full text of the license may be
> found at
> > +*
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +*
> > +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#ifndef __SOC_H__
> > +#define __SOC_H__
> > +
> > +#define HWA_CGA_M1_CLK_SEL         0xe0000000
> > +#define HWA_CGA_M1_CLK_SHIFT       29
> > +
> > +#define TP_CLUSTER_EOC_MASK        0xc0000000  /* end of clusters mask
> */
> > +#define NUM_CC_PLLS                2
> > +#define CLK_FREQ                   100000000
> > +#define MAX_CPUS                   4
> > +#define NUM_FMAN                   1
> > +#define CHECK_CLUSTER(Cluster)    ((Cluster & TP_CLUSTER_EOC_MASK)
> == 0x0)
> > +
> > +/* RCW SERDES MACRO */
> > +#define RCWSR_INDEX                4
> > +#define RCWSR_SRDS1_PRTCL_MASK     0xffff0000
> > +#define RCWSR_SRDS1_PRTCL_SHIFT    16
> > +#define RCWSR_SRDS2_PRTCL_MASK     0x0000ffff
> > +#define RCWSR_SRDS2_PRTCL_SHIFT    0
> > +
> > +/* SMMU Defintions */
> > +#define SMMU_BASE_ADDR             0x09000000
> > +#define SMMU_REG_SCR0              (SMMU_BASE_ADDR + 0x0)
> > +#define SMMU_REG_SACR              (SMMU_BASE_ADDR + 0x10)
> > +#define SMMU_REG_IDR1              (SMMU_BASE_ADDR + 0x24)
> > +#define SMMU_REG_NSCR0             (SMMU_BASE_ADDR + 0x400)
> > +#define SMMU_REG_NSACR             (SMMU_BASE_ADDR + 0x410)
> > +
> > +#define SCR0_USFCFG_MASK           0x00000400
> > +#define SCR0_CLIENTPD_MASK         0x00000001
> > +#define SACR_PAGESIZE_MASK         0x00010000
> > +#define IDR1_PAGESIZE_MASK         0x80000000
> > +
> > +typedef struct {
> > +  UINTN FreqProcessor[MAX_CPUS];
> > +  UINTN FreqSystemBus;
> > +  UINTN FreqDdrBus;
> > +  UINTN FreqLocalBus;
> > +  UINTN FreqSdhc;
> > +  UINTN FreqFman[NUM_FMAN];
> > +  UINTN FreqQman;
> > +} SYS_INFO;
> > +
> > +/* Device Configuration and Pin Control */
> > +typedef struct {
> > +  UINT32   PorSr1;         /* POR status 1 */
> > +#define CHASSIS2_CCSR_PORSR1_RCW_MASK  0xFF800000
> > +  UINT32   PorSr2;         /* POR status 2 */
> > +  UINT8    Res008[0x20-0x8];
> > +  UINT32   GppOrCr1;       /* General-purpose POR configuration */
> > +  UINT32   GppOrCr2;
> > +  UINT32   DcfgFuseSr;    /* Fuse status register */
> > +  UINT8    Res02c[0x70-0x2c];
> > +  UINT32   DevDisr;        /* Device disable control */
> > +  UINT32   DevDisr2;       /* Device disable control 2 */
> > +  UINT32   DevDisr3;       /* Device disable control 3 */
> > +  UINT32   DevDisr4;       /* Device disable control 4 */
> > +  UINT32   DevDisr5;       /* Device disable control 5 */
> > +  UINT32   DevDisr6;       /* Device disable control 6 */
> > +  UINT32   DevDisr7;       /* Device disable control 7 */
> > +  UINT8    Res08c[0x94-0x8c];
> > +  UINT32   CoreDisrU;      /* uppper portion for support of 64 cores */
> > +  UINT32   CoreDisrL;      /* lower portion for support of 64 cores */
> > +  UINT8    Res09c[0xa0-0x9c];
> > +  UINT32   Pvr;            /* Processor version */
> > +  UINT32   Svr;            /* System version */
> > +  UINT32   Mvr;            /* Manufacturing version */
> > +  UINT8    Res0ac[0xb0-0xac];
> > +  UINT32   RstCr;          /* Reset control */
> > +  UINT32   RstRqPblSr;     /* Reset request preboot loader status */
> > +  UINT8    Res0b8[0xc0-0xb8];
> > +  UINT32   RstRqMr1;       /* Reset request mask */
> > +  UINT8    Res0c4[0xc8-0xc4];
> > +  UINT32   RstRqSr1;       /* Reset request status */
> > +  UINT8    Res0cc[0xd4-0xcc];
> > +  UINT32   RstRqWdTmrL;     /* Reset request WDT mask */
> > +  UINT8    Res0d8[0xdc-0xd8];
> > +  UINT32   RstRqWdtSrL;     /* Reset request WDT status */
> > +  UINT8    Res0e0[0xe4-0xe0];
> > +  UINT32   BrrL;            /* Boot release */
> > +  UINT8    Res0e8[0x100-0xe8];
> > +  UINT32   RcwSr[16];      /* Reset control word status */
> > +#define CHASSIS2_RCWSR0_SYS_PLL_RAT_SHIFT  25
> > +#define CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK  0x1f
> > +#define CHASSIS2_RCWSR0_MEM_PLL_RAT_SHIFT  16
> > +#define CHASSIS2_RCWSR0_MEM_PLL_RAT_MASK  0x3f
> > +  UINT8    Res140[0x200-0x140];
> > +  UINT32   ScratchRw[4];   /* Scratch Read/Write */
> > +  UINT8    Res210[0x300-0x210];
> > +  UINT32   ScratcHw1R[4];  /* Scratch Read (Write once) */
> > +  UINT8    Res310[0x400-0x310];
> > +  UINT32   CrstSr[12];
> > +  UINT8    Res430[0x500-0x430];
> > +  /* PCI Express n Logical I/O Device Number register */
> > +  UINT32   DcfgCcsrPex1LiodNr;
> > +  UINT32   DcfgCcsrPex2LiodNr;
> > +  UINT32   DcfgCcsrPex3LiodNr;
> > +  UINT32   DcfgCcsrPex4LiodNr;
> > +  /* RIO n Logical I/O Device Number register */
> > +  UINT32   DcfgCcsrRio1LiodNr;
> > +  UINT32   DcfgCcsrRio2LiodNr;
> > +  UINT32   DcfgCcsrRio3LiodNr;
> > +  UINT32   DcfgCcsrRio4LiodNr;
> > +  /* USB Logical I/O Device Number register */
> > +  UINT32   DcfgCcsrUsb1LiodNr;
> > +  UINT32   DcfgCcsrUsb2LiodNr;
> > +  UINT32   DcfgCcsrUsb3LiodNr;
> > +  UINT32   DcfgCcsrUsb4LiodNr;
> > +  /* SD/MMC Logical I/O Device Number register */
> > +  UINT32   DcfgCcsrSdMmc1LiodNr;
> > +  UINT32   DcfgCcsrSdMmc2LiodNr;
> > +  UINT32   DcfgCcsrSdMmc3LiodNr;
> > +  UINT32   DcfgCcsrSdMmc4LiodNr;
> > +  /* RIO Message Unit Logical I/O Device Number register */
> > +  UINT32   DcfgCcsrRiomaintLiodNr;
> > +  UINT8    Res544[0x550-0x544];
> > +  UINT32   SataLiodNr[4];
> > +  UINT8    Res560[0x570-0x560];
> > +  UINT32   DcfgCcsrMisc1LiodNr;
> > +  UINT32   DcfgCcsrMisc2LiodNr;
> > +  UINT32   DcfgCcsrMisc3LiodNr;
> > +  UINT32   DcfgCcsrMisc4LiodNr;
> > +  UINT32   DcfgCcsrDma1LiodNr;
> > +  UINT32   DcfgCcsrDma2LiodNr;
> > +  UINT32   DcfgCcsrDma3LiodNr;
> > +  UINT32   DcfgCcsrDma4LiodNr;
> > +  UINT32   DcfgCcsrSpare1LiodNr;
> > +  UINT32   DcfgCcsrSpare2LiodNr;
> > +  UINT32   DcfgCcsrSpare3LiodNr;
> > +  UINT32   DcfgCcsrSpare4LiodNr;
> > +  UINT8    Res5a0[0x600-0x5a0];
> > +  UINT32   DcfgCcsrPblSr;
> > +  UINT32   PamuBypENr;
> > +  UINT32   DmaCr1;
> > +  UINT8    Res60c[0x610-0x60c];
> > +  UINT32   DcfgCcsrGenSr1;
> > +  UINT32   DcfgCcsrGenSr2;
> > +  UINT32   DcfgCcsrGenSr3;
> > +  UINT32   DcfgCcsrGenSr4;
> > +  UINT32   DcfgCcsrGenCr1;
> > +  UINT32   DcfgCcsrGenCr2;
> > +  UINT32   DcfgCcsrGenCr3;
> > +  UINT32   DcfgCcsrGenCr4;
> > +  UINT32   DcfgCcsrGenCr5;
> > +  UINT32   DcfgCcsrGenCr6;
> > +  UINT32   DcfgCcsrGenCr7;
> > +  UINT8    Res63c[0x658-0x63c];
> > +  UINT32   DcfgCcsrcGenSr1;
> > +  UINT32   DcfgCcsrcGenSr0;
> > +  UINT8    Res660[0x678-0x660];
> > +  UINT32   DcfgCcsrcGenCr1;
> > +  UINT32   DcfgCcsrcGenCr0;
> > +  UINT8    Res680[0x700-0x680];
> > +  UINT32   DcfgCcsrSrIoPstecr;
> > +  UINT32   DcfgCcsrDcsrCr;
> > +  UINT8    Res708[0x740-0x708]; /* add more registers when needed */
> > +  UINT32   TpItyp[64];          /* Topology Initiator Type Register */
> > +  struct {
> > +    UINT32 Upper;
> > +    UINT32 Lower;
> > +  } TpCluster[16];
> > +  UINT8    Res8c0[0xa00-0x8c0]; /* add more registers when needed */
> > +  UINT32   DcfgCcsrQmBmWarmRst;
> > +  UINT8    Resa04[0xa20-0xa04]; /* add more registers when needed */
> > +  UINT32   DcfgCcsrReserved0;
> > +  UINT32   DcfgCcsrReserved1;
> > +} CCSR_GUR;
> > +
> > +/* Supplemental Configuration Unit */
> > +typedef struct {
> > +  UINT8  Res000[0x070-0x000];
> > +  UINT32 Usb1Prm1Cr;
> > +  UINT32 Usb1Prm2Cr;
> > +  UINT32 Usb1Prm3Cr;
> > +  UINT32 Usb2Prm1Cr;
> > +  UINT32 Usb2Prm2Cr;
> > +  UINT32 Usb2Prm3Cr;
> > +  UINT32 Usb3Prm1Cr;
> > +  UINT32 Usb3Prm2Cr;
> > +  UINT32 Usb3Prm3Cr;
> > +  UINT8  Res094[0x100-0x094];
> > +  UINT32 Usb2Icid;
> > +  UINT32 Usb3Icid;
> > +  UINT8  Res108[0x114-0x108];
> > +  UINT32 DmaIcid;
> > +  UINT32 SataIcid;
> > +  UINT32 Usb1Icid;
> > +  UINT32 QeIcid;
> > +  UINT32 SdhcIcid;
> > +  UINT32 EdmaIcid;
> > +  UINT32 EtrIcid;
> > +  UINT32 Core0SftRst;
> > +  UINT32 Core1SftRst;
> > +  UINT32 Core2SftRst;
> > +  UINT32 Core3SftRst;
> > +  UINT8  Res140[0x158-0x140];
> > +  UINT32 AltCBar;
> > +  UINT32 QspiCfg;
> > +  UINT8  Res160[0x180-0x160];
> > +  UINT32 DmaMcr;
> > +  UINT8  Res184[0x188-0x184];
> > +  UINT32 GicAlign;
> > +  UINT32 DebugIcid;
> > +  UINT8  Res190[0x1a4-0x190];
> > +  UINT32 SnpCnfGcr;
> > +#define CCSR_SCFG_SNPCNFGCR_SECRDSNP         BIT31
> > +#define CCSR_SCFG_SNPCNFGCR_SECWRSNP         BIT30
> > +#define CCSR_SCFG_SNPCNFGCR_SATARDSNP        BIT23
> > +#define CCSR_SCFG_SNPCNFGCR_SATAWRSNP        BIT22
> > +#define CCSR_SCFG_SNPCNFGCR_USB1RDSNP        BIT21
> > +#define CCSR_SCFG_SNPCNFGCR_USB1WRSNP        BIT20
> > +#define CCSR_SCFG_SNPCNFGCR_USB2RDSNP        BIT15
> > +#define CCSR_SCFG_SNPCNFGCR_USB2WRSNP        BIT16
> > +#define CCSR_SCFG_SNPCNFGCR_USB3RDSNP        BIT13
> > +#define CCSR_SCFG_SNPCNFGCR_USB3WRSNP        BIT14
> > +  UINT8  Res1a8[0x1ac-0x1a8];
> > +  UINT32 IntpCr;
> > +  UINT8  Res1b0[0x204-0x1b0];
> > +  UINT32 CoreSrEnCr;
> > +  UINT8  Res208[0x220-0x208];
> > +  UINT32 RvBar00;
> > +  UINT32 RvBar01;
> > +  UINT32 RvBar10;
> > +  UINT32 RvBar11;
> > +  UINT32 RvBar20;
> > +  UINT32 RvBar21;
> > +  UINT32 RvBar30;
> > +  UINT32 RvBar31;
> > +  UINT32 LpmCsr;
> > +  UINT8  Res244[0x400-0x244];
> > +  UINT32 QspIdQScr;
> > +  UINT32 EcgTxcMcr;
> > +  UINT32 SdhcIoVSelCr;
> > +  UINT32 RcwPMuxCr0;
> > +  /**Setting RCW PinMux Register bits 17-19 to select USB2_DRVVBUS
> > +  *Setting RCW PinMux Register bits 21-23 to select USB2_PWRFAULT
> > +  *Setting RCW PinMux Register bits 25-27 to select USB3_DRVVBUS
> > +  Setting RCW PinMux Register bits 29-31 to select USB3_DRVVBUS*/
> > +#define CCSR_SCFG_RCWPMUXCRO_SELCR_USB      0x3333
> > +  /**Setting RCW PinMux Register bits 17-19 to select USB2_DRVVBUS
> > +  *Setting RCW PinMux Register bits 21-23 to select USB2_PWRFAULT
> > +  *Setting RCW PinMux Register bits 25-27 to select IIC4_SCL
> > +  Setting RCW PinMux Register bits 29-31 to select IIC4_SDA*/
> > +#define CCSR_SCFG_RCWPMUXCRO_NOT_SELCR_USB  0x3300
> > +  UINT32 UsbDrvVBusSelCr;
> > +#define CCSR_SCFG_USBDRVVBUS_SELCR_USB1      0x00000000
> > +#define CCSR_SCFG_USBDRVVBUS_SELCR_USB2      0x00000001
> > +#define CCSR_SCFG_USBDRVVBUS_SELCR_USB3      0x00000003
> > +  UINT32 UsbPwrFaultSelCr;
> > +#define CCSR_SCFG_USBPWRFAULT_INACTIVE       0x00000000
> > +#define CCSR_SCFG_USBPWRFAULT_SHARED         0x00000001
> > +#define CCSR_SCFG_USBPWRFAULT_DEDICATED      0x00000002
> > +#define CCSR_SCFG_USBPWRFAULT_USB3_SHIFT     4
> > +#define CCSR_SCFG_USBPWRFAULT_USB2_SHIFT     2
> > +#define CCSR_SCFG_USBPWRFAULT_USB1_SHIFT     0
> > +  UINT32 UsbRefclkSelcr1;
> > +  UINT32 UsbRefclkSelcr2;
> > +  UINT32 UsbRefclkSelcr3;
> > +  UINT8  Res424[0x600-0x424];
> > +  UINT32 ScratchRw[4];
> > +  UINT8  Res610[0x680-0x610];
> > +  UINT32 CoreBCr;
> > +  UINT8  Res684[0x1000-0x684];
> > +  UINT32 Pex1MsiIr;
> > +  UINT32 Pex1MsiR;
> > +  UINT8  Res1008[0x2000-0x1008];
> > +  UINT32 Pex2;
> > +  UINT32 Pex2MsiR;
> > +  UINT8  Res2008[0x3000-0x2008];
> > +  UINT32 Pex3MsiIr;
> > +  UINT32 Pex3MsiR;
> > +} CCSR_SCFG;
> > +
> > +#define USB_TXVREFTUNE        0x9
> > +#define USB_SQRXTUNE          0xFC7FFFFF
> > +#define USB_PCSTXSWINGFULL    0x47
> > +#define USB_PHY_RX_EQ_VAL_1   0x0000
> > +#define USB_PHY_RX_EQ_VAL_2   0x8000
> > +#define USB_PHY_RX_EQ_VAL_3   0x8003
> > +#define USB_PHY_RX_EQ_VAL_4   0x800b
> > +
> > +/*USB_PHY_SS memory map*/
> > +typedef struct {
> > +  UINT16 IpIdcodeLo;
> > +  UINT16 SupIdcodeHi;
> > +  UINT8  Res4[0x0006-0x0004];
> > +  UINT16 RtuneDebug;
> > +  UINT16 RtuneStat;
> > +  UINT16 SupSsPhase;
> > +  UINT16 SsFreq;
> > +  UINT8  ResE[0x0020-0x000e];
> > +  UINT16 Ateovrd;
> > +  UINT16 MpllOvrdInLo;
> > +  UINT8  Res24[0x0026-0x0024];
> > +  UINT16 SscOvrdIn;
> > +  UINT8  Res28[0x002A-0x0028];
> > +  UINT16 LevelOvrdIn;
> > +  UINT8  Res2C[0x0044-0x002C];
> > +  UINT16 ScopeCount;
> > +  UINT8  Res46[0x0060-0x0046];
> > +  UINT16 MpllLoopCtl;
> > +  UINT8  Res62[0x006C-0x0062];
> > +  UINT16 SscClkCntrl;
> > +  UINT8  Res6E[0x2002-0x006E];
> > +  UINT16 Lane0TxOvrdInHi;
> > +  UINT16 Lane0TxOvrdDrvLo;
> > +  UINT8  Res2006[0x200C-0x2006];
> > +  UINT16 Lane0RxOvrdInHi;
> > +  UINT8  Res200E[0x2022-0x200E];
> > +  UINT16 Lane0TxCmWaitTimeOvrd;
> > +  UINT8  Res2024[0x202A-0x2024];
> > +  UINT16 Lane0TxLbertCtl;
> > +  UINT16 Lane0RxLbertCtl;
> > +  UINT16 Lane0RxLbertErr;
> > +  UINT8  Res2030[0x205A-0x2030];
> > +  UINT16 Lane0TxAltBlock;
> > +} CCSR_USB_PHY;
> > +
> > +/* Clocking */
> > +typedef struct {
> > +  struct {
> > +    UINT32 ClkCnCSr;    /* core cluster n clock control status */
> > +    UINT8  Res004[0x0c];
> > +    UINT32 ClkcGHwAcSr; /* Clock generator n hardware accelerator */
> > +    UINT8 Res014[0x0c];
> > +  } ClkcSr[4];
> > +  UINT8  Res040[0x780]; /* 0x100 */
> > +  struct {
> > +    UINT32 PllCnGSr;
> > +    UINT8  Res804[0x1c];
> > +  } PllCgSr[NUM_CC_PLLS];
> > +  UINT8  Res840[0x1c0];
> > +  UINT32 ClkPCSr;  /* 0xa00 Platform clock domain control/status */
> > +  UINT8  Resa04[0x1fc];
> > +  UINT32 PllPGSr;  /* 0xc00 Platform PLL General Status */
> > +  UINT8  Resc04[0x1c];
> > +  UINT32 PllDGSr;  /* 0xc20 DDR PLL General Status */
> > +  UINT8  Resc24[0x3dc];
> > +} CCSR_CLOCK;
> > +
> > +VOID
> > +GetSysInfo (
> > +  OUT SYS_INFO *
> > +  );
> > +
> > +UINT32
> > +EFIAPI
> > +GurRead (
> > +  IN  UINTN     Address
> > +  );
> > +
> > +#endif /* __SOC_H__ */
> > diff --git a/Silicon/NXP/Chassis/LS1043aSocLib.inf
> b/Silicon/NXP/Chassis/LS1043aSocLib.inf
> > new file mode 100644
> > index 0000000..1b2f9c4
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/LS1043aSocLib.inf
> > @@ -0,0 +1,47 @@
> > +#  @file
> > +#
> > +#  Copyright 2017 NXP
> > +#
> > +#  This program and the accompanying materials
> > +#  are licensed and made available under the terms and conditions of the
> BSD License
> > +#  which accompanies this distribution.  The full text of the license may be
> found at
> > +#
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +#
> > +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +#
> > +#
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x0001001A
> > +  BASE_NAME                      = SocLib
> > +  FILE_GUID                      = e868c5ca-9729-43ae-bff4-438c67de8c68
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = SocLib
> > +
> > +[Packages]
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  MdePkg/MdePkg.dec
> > +  Silicon/NXP/NxpQoriqLs.dec
> > +  Silicon/NXP/Chassis/Chassis2/Chassis2.dec
> > +  Silicon/NXP/LS1043A/LS1043A.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  BeIoLib
> > +  DebugLib
> > +  SerialPortLib
> > +
> > +[Sources.common]
> > +  Chassis.c
> > +  Chassis2/Soc.c
> > +  SerDes.c
> > +
> > +[FixedPcd]
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> > +  gNxpQoriqLsTokenSpaceGuid.PcdGutsBaseAddr
> > +  gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv
> > +  gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled
> > +  gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian
> > +  gNxpQoriqLsTokenSpaceGuid.PcdClkBaseAddr
> > diff --git a/Silicon/NXP/Chassis/SerDes.c b/Silicon/NXP/Chassis/SerDes.c
> > new file mode 100644
> > index 0000000..e4578c3
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis/SerDes.c
> > @@ -0,0 +1,271 @@
> > +/** SerDes.c
> > +  Provides the basic interfaces for SerDes Module
> > +
> > +  Copyright 2017 NXP
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the
> BSD License
> > +  which accompanies this distribution. The full text of the license may be
> found
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include <Bitops.h>
> > +#include <Library/DebugLib.h>
> > +#include <SerDes.h>
> > +#include <SocSerDes.h>
> > +#include <Soc.h>
> > +#include <Uefi.h>
> > +
> > +/**
> > +  Function to get serdes Lane protocol corresponding to
> > +  serdes protocol.
> > +
> > +  @param  SerDes    Serdes number.
> > +  @param  Cfg       Serdes Protocol.
> > +  @param  Lane      Serdes Lane number.
> > +
> > +  @return           Serdes Lane protocol.
> > +
> > +**/
> > +STATIC
> > +SERDES_PROTOCOL
> > +GetSerDesPrtcl (
> > +  IN  INTN          SerDes,
> > +  IN  INTN          Cfg,
> > +  IN  INTN          Lane
> > +  )
> > +{
> > +  SERDES_CONFIG     *Config;
> > +
> > +  if (SerDes >= ARRAY_SIZE (SerDesConfigTbl)) {
> > +    return 0;
> > +  }
> > +
> > +  Config = SerDesConfigTbl[SerDes];
> > +  while (Config->Protocol) {
> > +    if (Config->Protocol == Cfg) {
> > +      return Config->SrdsLane[Lane];
> > +    }
> > +    Config++;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  Function to check if inputted protocol is a valid serdes protocol.
> > +
> > +  @param  SerDes                   Serdes number.
> > +  @param  Prtcl                    Serdes Protocol to be verified.
> > +
> > +  @return EFI_INVALID_PARAMETER    Input parameter in invalid.
> > +  @return EFI_NOT_FOUND            Serdes Protocol not a valid protocol.
> > +  @return EFI_SUCCESS              Serdes Protocol is a valid protocol.
> > +
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +CheckSerDesPrtclValid (
> > +  IN  INTN      SerDes,
> > +  IN  UINT32    Prtcl
> > +  )
> > +{
> > +  SERDES_CONFIG *Config;
> > +  INTN          Cnt;
> > +
> > +  if (SerDes >= ARRAY_SIZE (SerDesConfigTbl)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Config = SerDesConfigTbl[SerDes];
> > +  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 < SRDS_MAX_LANES; Cnt++) {
> > +    if (Config->SrdsLane[Cnt] != NONE) {
> > +      return EFI_SUCCESS;
> > +    }
> > +  }
> > +
> > +  return EFI_NOT_FOUND;
> > +}
> > +
> > +/**
> > +  Function to fill serdes map information.
> > +
> > +  @param  Srds                  Serdes number.
> > +  @param  SerdesProtocolMask    Serdes Protocol Mask.
> > +  @param  SerdesProtocolShift   Serdes Protocol shift value.
> > +  @param  SerDesPrtclMap        Pointer to Serdes Protocol map.
> > +
> > +**/
> > +STATIC
> > +VOID
> > +LSSerDesMap (
> > +  IN  UINT32                    Srds,
> > +  IN  UINT32                    SerdesProtocolMask,
> > +  IN  UINT32                    SerdesProtocolShift,
> > +  OUT UINT64                    *SerDesPrtclMap
> > +  )
> > +{
> > +  CCSR_GUR                      *Gur;
> > +  UINT32                        SrdsProt;
> > +  INTN                          Lane;
> > +  UINT32                        Flag;
> > +
> > +  Gur = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> > +  *SerDesPrtclMap = 0x0;
> > +  Flag = 0;
> > +
> > +  SrdsProt = GurRead ((UINTN)&Gur->RcwSr[RCWSR_INDEX]) &
> SerdesProtocolMask;
> > +  SrdsProt >>= SerdesProtocolShift;
> > +
> > +  DEBUG ((DEBUG_INFO, "Using SERDES%d Protocol: %d (0x%x)\n",
> > +                                   Srds + 1, SrdsProt, SrdsProt));
> 
> Spurious indentation.
> 
Will correct
> > +
> > +  if (EFI_SUCCESS != CheckSerDesPrtclValid (Srds, SrdsProt)) {
> > +    DEBUG ((DEBUG_ERROR, "SERDES%d[PRTCL] = 0x%x is not valid\n",
> > +                                   Srds + 1, SrdsProt));
> 
> Spurious indentation.
> 
Will correct
> > +    Flag++;
> > +  }
> > +
> > +  for (Lane = 0; Lane < SRDS_MAX_LANES; Lane++) {
> > +    SERDES_PROTOCOL LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane);
> > +    if (LanePrtcl >= SERDES_PRTCL_COUNT) {
> > +      DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol %d\n",
> LanePrtcl));
> > +      Flag++;
> > +    } else {
> > +      *SerDesPrtclMap |= BIT (LanePrtcl);
> > +    }
> > +  }
> > +
> > +  if (Flag) {
> > +    DEBUG ((DEBUG_ERROR, "Could not configure SerDes module!!\n"));
> > +  } else {
> > +    DEBUG ((DEBUG_INFO, "Successfully configured SerDes module!!\n"));
> > +  }
> > +}
> > +
> > +/**
> > +  Get lane protocol on provided serdes lane and execute callback function.
> > +
> > +  @param  Srds                    Serdes number.
> > +  @param  SerdesProtocolMask      Mask to get Serdes Protocol for Srds
> > +  @param  SerdesProtocolShift     Shift value to get Serdes Protocol for
> Srds.
> > +  @param  SerDesLaneProbeCallback Pointer Callback function to be called
> for Lane protocol
> > +  @param  Arg                     Pointer to Arguments to be passed to callback
> function.
> > +
> > +**/
> > +STATIC
> > +VOID
> > +SerDesInstanceProbeLanes (
> > +  IN  UINT32                      Srds,
> > +  IN  UINT32                      SerdesProtocolMask,
> > +  IN  UINT32                      SerdesProtocolShift,
> > +  IN  SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> > +  IN  VOID                        *Arg
> > +  )
> > +{
> > +
> > +  CCSR_GUR                        *Gur;
> > +  UINT32                          SrdsProt;
> > +  INTN                            Lane;
> > +
> > +  Gur = (VOID *)PcdGet64 (PcdGutsBaseAddr);;
> > +
> > +  SrdsProt = GurRead ((UINTN)&Gur->RcwSr[RCWSR_INDEX]) &
> SerdesProtocolMask;
> > +  SrdsProt >>= SerdesProtocolShift;
> > +
> > +  /*
> > +   * Invoke callback for all lanes in the SerDes instance:
> > +   */
> > +  for (Lane = 0; Lane < SRDS_MAX_LANES; Lane++) {
> > +    SERDES_PROTOCOL LanePrtcl = GetSerDesPrtcl (Srds, SrdsProt, Lane);
> > +    if (LanePrtcl >= SERDES_PRTCL_COUNT || LanePrtcl < NONE) {
> 
> Please use parentheses rather than relying on operator precedence.
> 
Ok
> > +      DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol %d\n",
> LanePrtcl));
> > +    }
> > +    else if (LanePrtcl != NONE) {
> 
> else on same line as }
> 
OK
> > +      SerDesLaneProbeCallback (LanePrtcl, Arg);
> > +    }
> > +  }
> > +}
> > +
> > +/**
> > +  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
> > +  )
> > +{
> > +  SerDesInstanceProbeLanes (SRDS_1,
> > +                            RCWSR_SRDS1_PRTCL_MASK,
> > +                            RCWSR_SRDS1_PRTCL_SHIFT,
> > +                            SerDesLaneProbeCallback,
> > +                            Arg);
> > +
> > +  if (PcdGetBool (PcdSerdes2Enabled)) {
> > +   SerDesInstanceProbeLanes (SRDS_2,
> > +                             RCWSR_SRDS2_PRTCL_MASK,
> > +                             RCWSR_SRDS2_PRTCL_SHIFT,
> > +                             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
> > +  )
> > +{
> > +  LSSerDesMap (SRDS_1,
> > +               RCWSR_SRDS1_PRTCL_MASK,
> > +               RCWSR_SRDS1_PRTCL_SHIFT,
> > +               SerDesPrtclMap);
> > +
> > +  if (PcdGetBool (PcdSerdes2Enabled)) {
> > +    LSSerDesMap (SRDS_2,
> > +                 RCWSR_SRDS2_PRTCL_MASK,
> > +                 RCWSR_SRDS2_PRTCL_SHIFT,
> > +                 SerDesPrtclMap);
> > +  }
> > +
> > +}
> > +
> > +BOOLEAN
> > +IsSerDesLaneProtocolConfigured (
> > +  IN UINT64          SerDesPrtclMap,
> > +  IN SERDES_PROTOCOL Device
> > +  )
> > +{
> > +  if (Device >= SERDES_PRTCL_COUNT || Device < NONE) {
> 
> Please use parentheses rather than relying on operator precedence.
> 
Sure

> > +    ASSERT (Device > NONE && Device < SERDES_PRTCL_COUNT);
> 
> (Here as well.)
> 
OK
> > +    DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol Device
> %d\n", Device));
> > +  }
> > +
> > +  return (SerDesPrtclMap & BIT (Device)) != 0 ;
> > +}
> > diff --git a/Silicon/NXP/Include/Bitops.h b/Silicon/NXP/Include/Bitops.h
> > new file mode 100644
> > index 0000000..beddb4e
> > --- /dev/null
> > +++ b/Silicon/NXP/Include/Bitops.h
> > @@ -0,0 +1,179 @@
> > +/** Bitops.h
> > +  Header defining the general bitwise operations
> > +
> > +  Copyright 2017 NXP
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the
> BSD License
> > +  which accompanies this distribution.  The full text of the license may be
> found at
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef __BITOPS_H__
> > +#define __BITOPS_H__
> > +
> > +#include <Library/DebugLib.h>
> > +
> > +#define MASK_LOWER_16              0xFFFF0000
> > +#define MASK_UPPER_16              0x0000FFFF
> > +#define MASK_LOWER_8               0xFF000000
> > +#define MASK_UPPER_8               0x000000FF
> 
> These appear unused by the set.
> 
Will check.
> > +
> > +/*
> > + * Returns the bit mask for a bit index from 0 to 31
> > + */
> > +#define BIT(_BitIndex)         (0x1u << (_BitIndex))
> 
> I don't see these being used for anything other than setting up BIT1
> BIT2 BIT3 and so on. We already have those in Base.h.
> 
In Base.h, we have BIT1, BIT2 defined, here the requirement is to get set bit on basis of bit number, bit is not known in advance.

> > +
> > +/**
> > + * Upper32Bits - return bits 32-63 of a number
> > + * @N: the number we're accessing
> > + *
> > + * A basic shift-right of a 64- or 32-bit quantity.  Use this to suppress
> > + * the "right shift count >= width of type" warning when that quantity is
> > + * 32-bits.
> > + */
> > +#define Upper32Bits(N) ((UINT32)(((N) >> 16) >> 16))
> 
> 1) Compiler warnings are there for a reason.
> 2) This does not appear to be actually used.
> 
> Can we just drop it?
> 
Yes, we can
> > +
> > +/**
> > + * Lower32Bits - return bits 0-31 of a number
> > + * @N: the number we're accessing
> > + */
> > +#define Lower32Bits(N) ((UINT32)(N))
> 
> Same here.
> 
sure
> > +
> > +
> > +/*
> > + * Stores a value for a given bit field in 32-bit '_Container'
> > + */
> > +
> > +#define SET_BIT_FIELD32(_Container, _BitShift, _BitWidth, _Value) \
> > +  __SET_BIT_FIELD32(_Container,                                   \
> > +      __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth),               \
> > +      _BitShift,                                                  \
> > +      _Value)
> > +
> > +#define __SET_BIT_FIELD32(_Container, _BitMask, _BitShift, _Value)      \
> > +  do {                                                                  \
> > +    (_Container) &= ~(_BitMask);                                        \
> > +    if ((_Value) != 0) {                                                \
> > +      ASSERT(((UINT32)(_Value) << (_BitShift)) <= (_BitMask));          \
> > +      (_Container) |=                                                   \
> > +      ((UINT32)(_Value) << (_BitShift)) & (_BitMask);                   \
> > +    }                                                                   \
> > +  } while (0)
> > +
> > +/*
> > + * Extracts the value for a given bit field in 32-bit _Container
> > + */
> > +
> > +#define GET_BIT_FIELD32(_Container, _BitShift, _BitWidth) \
> > +  __GET_BIT_FIELD32(_Container,                           \
> > +      __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth),       \
> > +      _BitShift)
> > +
> > +#define __GET_BIT_FIELD32(_Container, _BitMask, _BitShift)  \
> > +  (((UINT32)(_Container) & (_BitMask)) >> (_BitShift))
> > +
> > +#define __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth)        \
> > +  ((_BitWidth) < 32 ?                                       \
> > +   (((UINT32)1 << (_BitWidth)) - 1) << (_BitShift) :        \
> > +   ~(UINT32)0)
> > +
> > +/*
> > + *Stores a value for a given bit field in 64-bit '_Container'
> > + */
> > +#define SET_BIT_FIELD64(_Container, _BitShift, _BitWidth, _Value) \
> > +  __SET_BIT_FIELD64(_Container,                                   \
> > +      __GEN_BIT_FIELD_MASK64(_BitShift, _BitWidth),               \
> > +      _BitShift,                                                  \
> > +      _Value)
> > +
> > +#define __SET_BIT_FIELD64(_Container, _BitMask, _BitShift, _Value)  \
> > +  do {                                                              \
> > +    (_Container) &= ~(_BitMask);                                    \
> > +    if ((_Value) != 0) {                                            \
> > +      ASSERT(((UINT64)(_Value) << (_BitShift)) <= (_BitMask));      \
> > +      (_Container) |=                                               \
> > +      ((UINT64)(_Value) << (_BitShift)) & (_BitMask);               \
> > +    }                                                               \
> > +  } while (0)
> > +
> > +/*
> > + * Extracts the value for a given bit field in 64-bit _Container
> > + */
> > +#define GET_BIT_FIELD64(_Container, _BitShift, _BitWidth) \
> > +  __GET_BIT_FIELD64(_Container,                           \
> > +      __GEN_BIT_FIELD_MASK64(_BitShift, _BitWidth),       \
> > +      _BitShift)
> > +
> > +#define __GET_BIT_FIELD64(_Container, _BitMask, _BitShift) \
> > +  (((UINT64)(_Container) & (_BitMask)) >> (_BitShift))
> > +
> > +#define __GEN_BIT_FIELD_MASK64(_BitShift, _BitWidth)       \
> > +  ((_BitWidth) < 64 ?                                      \
> > +   (((UINT64)1 << (_BitWidth)) - 1) << (_BitShift) :       \
> > +   ~(UINT64)0)
> > +
> 
> These all appear unused.
> Also, there are BitField operations in edk2 BaseLib.
> 
Will remove these.

> > +/**
> > +
> > + Test If the Destination buffer sets (0->1) or clears (1->0) any bit in Source
> buffer ?
> > +
> > + @param[in]  Source       Source Buffer Pointer
> > + @param[in]  Destination  Destination Buffer Pointer
> > + @param[in]  NumBytes     Bytes to Compare
> > + @param[in]  Set          True : Test Weather Destination buffer sets any bit
> in Source buffer ?
> > +                          False : Test Weather Destination buffer clears any bit in
> Source buffer ?
> > +
> > + @retval     TRUE         Destination buffer sets/clear a bit in source buffer.
> > + @retval     FALSE        Destination buffer doesn't sets/clear bit in source
> buffer.
> > +
> > +**/
> > +STATIC
> > +inline
> > +BOOLEAN
> > +TestBitSetClear (
> 
> This one is used, but only once, in NorFlashDxe.
> Coding Style also bans function definitions in header files.
> Can this move directly to NorFlashDxe.c?
> 
Will check if needed by other modules [in later set of patches], else will move it to NorFlashDxe.

> > +  IN  VOID    *Source,
> > +  IN  VOID    *Destination,
> > +  IN  UINTN   NumBytes,
> > +  IN  BOOLEAN Set
> > +  )
> > +{
> > +  UINTN Index = 0;
> > +  VOID* Buffer;
> > +
> > +  if (Set) {
> > +    Buffer = Destination;
> > +  } else {
> > +    Buffer = Source;
> > +  }
> > +
> > +  while (Index < NumBytes) {
> > +    if ((NumBytes - Index) >= 8) {
> > +      if ((*((UINT64*)(Source+Index)) ^ *((UINT64*)(Destination+Index))) &
> *((UINT64*)(Buffer+Index))) {
> > +        return TRUE;
> > +      }
> > +      Index += 8;
> > +    } else if ((NumBytes - Index) >= 4) {
> > +      if ((*((UINT32*)(Source+Index)) ^ *((UINT32*)(Destination+Index))) &
> *((UINT32*)(Buffer+Index))) {
> > +        return TRUE;
> > +      }
> > +      Index += 4;
> > +    } else if ((NumBytes - Index) >= 2) {
> > +      if ((*((UINT16*)(Source+Index)) ^ *((UINT16*)(Destination+Index))) &
> *((UINT16*)(Buffer+Index))) {
> > +        return TRUE;
> > +      }
> > +      Index += 2;
> > +    } else if ((NumBytes - Index) >= 1) {
> > +      if ((*((UINT8*)(Source+Index)) ^ *((UINT8*)(Destination+Index))) &
> *((UINT8*)(Buffer+Index))) {
> > +        return TRUE;
> > +      }
> > +      Index += 1;
> > +    }
> > +  }
> > +  return FALSE;
> > +}
> > +
> > +#endif
> 
> On the whole, it looks like this header file can go.
> 
> /
>     Leif
> 
> > diff --git a/Silicon/NXP/LS1043A/Include/SocSerDes.h
> b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> > new file mode 100644
> > index 0000000..90e165f
> > --- /dev/null
> > +++ b/Silicon/NXP/LS1043A/Include/SocSerDes.h
> > @@ -0,0 +1,55 @@
> > +/** @file
> > + The Header file of SerDes Module for LS1043A
> > +
> > + Copyright 2017 NXP
> > +
> > + This program and the accompanying materials
> > + are licensed and made available under the terms and conditions of the
> BSD License
> > + which accompanies this distribution. The full text of the license may be
> found
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9a50d6
> e105bc4080493208d5a53ed014%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636596611590418850&sdata=jZ0%2B2I7ul%2F8Te5ASAsABuk4O0LR
> 6%2Bn8G8avnxAvcF9k%3D&reserved=0
> > +
> > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef __SOC_SERDES_H__
> > +#define __SOC_SERDES_H__
> > +
> > +#include <SerDes.h>
> > +
> > +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 /* __SOC_SERDES_H */
> > --
> > 1.9.1
> >