public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Wasim Khan <wasim.khan@oss.nxp.com>
Cc: devel@edk2.groups.io, meenakshi.aggarwal@nxp.com,
	V.Sethi@nxp.com, ard.biesheuvel@arm.com,
	Wasim Khan <wasim.khan@nxp.com>
Subject: Re: [PATCH edk2-platforms 1/3] Silicon/NXP/Library: Implement SerDesHelperLib
Date: Fri, 5 Jun 2020 13:26:12 +0100	[thread overview]
Message-ID: <20200605122612.GD28566@vanye> (raw)
In-Reply-To: <1591039658-18541-2-git-send-email-wasim.khan@oss.nxp.com>

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
> 

  reply	other threads:[~2020-06-05 12:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200605122612.GD28566@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox