public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Pankaj Bansal <pankaj.bansal@nxp.com>
Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Varun Sethi <V.Sethi@nxp.com>,
	devel@edk2.groups.io
Subject: Re: [PATCH 17/19] Silicon/NXP: Add Chassis3V2
Date: Wed, 12 Feb 2020 20:33:39 +0000	[thread overview]
Message-ID: <20200212203339.GF23627@bivouac.eciton.net> (raw)
In-Reply-To: <20200207124328.8723-18-pankaj.bansal@nxp.com>

On Fri, Feb 07, 2020 at 18:13:26 +0530, Pankaj Bansal wrote:
> Add Chassis3V2

Insufficient as a commit message.

> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  Silicon/NXP/Chassis3V2/Chassis3V2.dec         |  23 +++
>  Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc     |  10 +
>  Silicon/NXP/Chassis3V2/Include/Chassis.h      |  42 ++++
>  .../Library/ChassisLib/ChassisLib.c           | 186 ++++++++++++++++++
>  .../Library/ChassisLib/ChassisLib.inf         |  41 ++++
>  5 files changed, 302 insertions(+)
>  create mode 100644 Silicon/NXP/Chassis3V2/Chassis3V2.dec
>  create mode 100644 Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc
>  create mode 100644 Silicon/NXP/Chassis3V2/Include/Chassis.h
>  create mode 100644 Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c
>  create mode 100644 Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf
> 
> diff --git a/Silicon/NXP/Chassis3V2/Chassis3V2.dec b/Silicon/NXP/Chassis3V2/Chassis3V2.dec
> new file mode 100644
> index 0000000000..106b118188
> --- /dev/null
> +++ b/Silicon/NXP/Chassis3V2/Chassis3V2.dec
> @@ -0,0 +1,23 @@
> +#/** @file
> +# NXP Layerscape processor package.
> +#
> +# Copyright 2020 NXP
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 1.27
> +  PACKAGE_VERSION                = 0.1
> +
> +################################################################################
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#                   Comments are used for Keywords and Module Types.
> +#
> +#
> +################################################################################
> +[Includes.common]
> +  Include                        # Root include for the package
> +

Skip the blank line at EOF.

> diff --git a/Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc b/Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc
> new file mode 100644
> index 0000000000..dabe2ae230
> --- /dev/null
> +++ b/Silicon/NXP/Chassis3V2/Chassis3V2.dsc.inc
> @@ -0,0 +1,10 @@
> +#  @file
> +#
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +[LibraryClasses.common]
> +  ChassisLib|Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf
> diff --git a/Silicon/NXP/Chassis3V2/Include/Chassis.h b/Silicon/NXP/Chassis3V2/Include/Chassis.h
> new file mode 100644
> index 0000000000..2771f26fe3
> --- /dev/null
> +++ b/Silicon/NXP/Chassis3V2/Include/Chassis.h
> @@ -0,0 +1,42 @@
> +/** @file
> +
> +  Copyright 2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#ifndef __CHASSIS_H__
> +#define __CHASSIS_H__

Please drop leading __ from include guards.

> +
> +#define  NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS  0x1E00000
> +
> +#define  TP_CLUSTER_ITYPE_IDX  0x3f
> +#define  TP_CLUSTER_EOC        BIT31
> +#define  TP_ITYPE_AVAILABLE    BIT0
> +#define  TP_ITYPE_TYPE(x)      (((x) & 0x06) >> 1)
> +#define  TP_ITYPE_ARM          0x0
> +#define  TP_ITYPE_VERSION(x)   (((x) & 0xe0) >> 5)
> +
> +#define  TP_ITYPE_VERSION_A53   0x2
> +#define  TP_ITYPE_VERSION_A72   0x4
> +
> +/**
> +  The Device Configuration Unit provides general purpose configuration and status for the
> +  device. These registers only support 32-bit accesses.
> +**/
> +#pragma pack(1)
> +typedef struct {
> +  UINT8   Reserved0[0x100 - 0x0];
> +  UINT32  RcwSr[32]; // Reset Control Word Status Register
> +  UINT8   Reserved180[0x200 - 0x180];
> +  UINT32  ScratchRw[16]; /// Scratch Read / Write Register
> +  UINT8   Reserved240[0x740-0x240];
> +  UINT32  TpItyp[65]; /// Topology Initiator Type Register
> +  struct {
> +    UINT32  Lower;
> +    UINT32  Upper;
> +  }TpCluster[8];
> +} NXP_LAYERSCAPE_CHASSIS3V2_DEVICE_CONFIG;
> +#pragma pack()
> +
> +#endif
> diff --git a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c
> new file mode 100644
> index 0000000000..99567bb76f
> --- /dev/null
> +++ b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.c
> @@ -0,0 +1,186 @@
> +/** @file
> +  Chassis specific functions common to all SOCs based on a specific Chessis
> +
> +  Copyright 2020 NXP
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Chassis.h>
> +#include <Uefi.h>
> +#include <Library/IoLib.h>
> +#include <Library/IoAccessLib.h>

Please flip above two lines, which will order the include files alphabetically.

> +#include <Library/PcdLib.h>
> +#include <Library/SerialPortLib.h>
> +#include <Ppi/ArmMpCoreInfo.h>
> +
> +UINT32
> +EFIAPI
> +DcfgRead32 (
> +  IN  UINTN     Address
> +  )
> +{
> +  if (FeaturePcdGet (PcdDcfgBigEndian)) {
> +    return SwapMmioRead32 (Address);

Same as for previous patches, please rewrite to use GetMmioOperations*.

> +  } else {
> +    return MmioRead32 (Address);
> +  }
> +}
> +
> +UINT32
> +EFIAPI
> +DcfgWrite32 (
> +  IN      UINTN                     Address,
> +  IN      UINT32                    Value
> +  )
> +{
> +  if (FeaturePcdGet (PcdDcfgBigEndian)) {
> +    return SwapMmioWrite32 (Address, Value);
> +  } else {
> +    return MmioWrite32 (Address, Value);
> +  }
> +}
> +
> +/**
> +  Get the type of core in cluster
> +
> +  The core can be of type ARM or PowerPC or Hardware Accelerator.
> +  If the core is enabled and of type ARM EFI_SUCCESS is returned and a code for type of ARM core is returned

Please wrap long lines, throughout.

> +
> +  @param[in]    TpItypeIdx  Index of Core to be searched in TpItyp in Device Config Registers.
> +  @param[out]   CoreType    If the core is ARM core then the type of core i.e. A53/A72 etc.
> +                            These cores are identified based on their codes like TP_ITYPE_VERSION_A72
> +
> +  @return  EFI_NOT_FOUND  No enabled ARM core found
> +  @return  EFI_SUCCESS     An enabled ARM core found
> +**/
> +STATIC
> +EFI_STATUS
> +SocGetCoreType (
> +  IN  UINT8  TpItypeIdx,
> +  OUT UINT8  *CoreType
> +  )
> +{
> +  NXP_LAYERSCAPE_CHASSIS3V2_DEVICE_CONFIG *Dcfg;
> +  UINT32                                  TpItype;

Same comment about TpItype as for previous chassis library.

> +
> +  Dcfg = (NXP_LAYERSCAPE_CHASSIS3V2_DEVICE_CONFIG *)NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS;
> +  TpItype = MmioRead32 ((UINTN)&Dcfg->TpItyp[TpItypeIdx]);
> +  if (TpItype & TP_ITYPE_AVAILABLE) {
> +    if (TP_ITYPE_TYPE (TpItype) == TP_ITYPE_ARM) {
> +      *CoreType = TP_ITYPE_VERSION (TpItype);
> +    } else {
> +      return EFI_NOT_FOUND;
> +    }
> +  } else {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Return the number of cores present in SOC
> +
> +  This function returns the number of cores present in SOC.
> +  and also their position (cluster number and core number) in the form of ARM_CORE_INFO array
> +  and NxpCoreTable array.
> +  NxpCoreTable array can be used to find out the type of core. it's values are of type
> +  TP_ITYPE_VERSION_*.
> +  The number of cores present in SOC can vary depending on which flavour of SOC is being used.
> +  This function doesn't allocte any memory and must be provided memory for array of ARM_CORE_INFO
> +  and NxpCoreTable for maximum number of cores the SOC can have.
> +
> +  @param[out]  NxpCoreTable        array of UINT8 for maximum number of cores the SOC can have.
> +  @param[out]  ArmCoreTable        array of ARM_CORE_INFO for maximum number of cores the SOC can have.
> +  @param[in]   ArmCoreTableSize    Size of ArmCoreTable
> +
> +  @return         Actual number of cores present in SOC. After calling this function only the returned
> +                  value number of entries in ArmCoreTable are valid entries.
> +**/
> +UINTN
> +__attribute__((weak))

Same question about weak symbol linkage as for previous chassis library.

> +SocGetMpCoreInfo (
> +  OUT UINT8           *NxpCoreTable,
> +  OUT ARM_CORE_INFO   *ArmCoreTable,
> +  IN  UINTN           CoreTableSize
> +  )
> +{
> +  NXP_LAYERSCAPE_CHASSIS3V2_DEVICE_CONFIG  *Dcfg;
> +  UINT32                                   TpClusterLower;
> +  UINT8                                    TpClusterParser;

Same ClusterLower/ClusterParser comments as for previous chassis library.

> +  UINT8                                    ClusterIndex;
> +  UINT8                                    CoreIndex;
> +  UINTN                                    CoreCount;
> +  UINT8                                    CoreType;
> +  EFI_STATUS                               Status;
> +
> +  Dcfg = (NXP_LAYERSCAPE_CHASSIS3V2_DEVICE_CONFIG *)NXP_LAYERSCAPE_CHASSIS3V2_DCFG_ADDRESS;
> +  ClusterIndex = 0;
> +  CoreCount = 0;
> +  while (TRUE) {
> +    TpClusterLower = MmioRead32 ((UINTN)&Dcfg->TpCluster[ClusterIndex].Lower);
> +    for (CoreIndex = 0; CoreIndex < (sizeof (TpClusterLower) / sizeof (TpClusterParser)); CoreIndex++) {

And here.

> +      TpClusterParser = (TpClusterLower >> (8 * CoreIndex));

And here.

> +      Status = SocGetCoreType (TpClusterParser & TP_CLUSTER_ITYPE_IDX, &CoreType);
> +      if (Status != EFI_NOT_FOUND) {
> +        ArmCoreTable[CoreCount].ClusterId = ClusterIndex;
> +        ArmCoreTable[CoreCount].CoreId = CoreIndex;
> +        ArmCoreTable[CoreCount].MailboxSetAddress = 0;
> +        ArmCoreTable[CoreCount].MailboxGetAddress = 0;
> +        ArmCoreTable[CoreCount].MailboxClearAddress = 0;
> +        ArmCoreTable[CoreCount].MailboxClearValue = ~0;
> +
> +        NxpCoreTable[CoreCount] = CoreType;
> +        CoreCount++;
> +        if (CoreCount == CoreTableSize) {
> +          break;
> +        }
> +      }
> +    }
> +    if (TpClusterLower & TP_CLUSTER_EOC) {
> +      break;
> +    }
> +    if (CoreCount == CoreTableSize) {
> +      break;
> +    }
> +    ClusterIndex++;
> +  }
> +
> +  return CoreCount;
> +}
> +
> +/**
> +  Function to initialize Chassis Specific functions
> + **/
> +VOID
> +ChassisInit (
> +  VOID
> +  )
> +{
> +  UINT64              BaudRate;
> +  UINT32              ReceiveFifoDepth;
> +  EFI_PARITY_TYPE     Parity;
> +  UINT8               DataBits;
> +  EFI_STOP_BITS_TYPE  StopBits;
> +  UINT32              Timeout;
> +
> +  BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
> +  ReceiveFifoDepth = FixedPcdGet16 (PcdUartDefaultReceiveFifoDepth);
> +  Timeout = 0;
> +  Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
> +  DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
> +  StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
> +
> +  //
> +  // Early init serial Port to get board information.
> +  //
> +  SerialPortSetAttributes (
> +    &BaudRate,
> +    &ReceiveFifoDepth,
> +    &Timeout,
> +    &Parity,
> +    &DataBits,
> +    &StopBits
> +  );
> +}
> diff --git a/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf
> new file mode 100644
> index 0000000000..302296bf65
> --- /dev/null
> +++ b/Silicon/NXP/Chassis3V2/Library/ChassisLib/ChassisLib.inf
> @@ -0,0 +1,41 @@
> +#/**  @file
> +#
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = Chassis3V2Lib
> +  FILE_GUID                      = fae0d077-5fc2-494f-b8e1-c51a3023ee3e
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ChassisLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  ArmPkg/ArmPkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +  Silicon/NXP/Chassis3V2/Chassis3V2.dec

Please sort packages alphabetically.
> +
> +[LibraryClasses]
> +  IoLib
> +  IoAccessLib

Please flip above two lines.

/
    Leif

> +  PcdLib
> +  SerialPortLib
> +
> +[Sources.common]
> +  ChassisLib.c
> +
> +[FeaturePcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian
> +
> +[FixedPcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth
> +
> -- 
> 2.17.1
> 

  reply	other threads:[~2020-02-12 20:33 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 12:43 [PATCH 00/19] ADD LX2160ARDB Platform Support Pankaj Bansal
2020-02-07 12:43 ` [PATCH 01/19] Silicon/NXP: Add I2c lib Pankaj Bansal
2020-02-08 17:13   ` Leif Lindholm
2020-02-09 11:49     ` [edk2-devel] " Ard Biesheuvel
2020-02-07 12:43 ` [PATCH 02/19] Silicon/NXP: changes to use I2clib in i2cdxe Pankaj Bansal
2020-02-08 17:23   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 03/19] NXP/LS1043aRdb: Move Soc specific components to soc files Pankaj Bansal
2020-02-08 17:27   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 04/19] Silicon/NXP: Remove DuartLib and use BaseSerialPortLib16550 Pankaj Bansal
2020-02-08 17:46   ` Leif Lindholm
2020-02-10  5:48     ` Pankaj Bansal
2020-02-12 23:27       ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 05/19] NXP/BaseSerialPortLib16550: remove SerialPortInitalize functionality Pankaj Bansal
2020-02-07 12:43 ` [PATCH 06/19] Silicon/NXP: remove print information from Soc lib Pankaj Bansal
2020-02-10 17:09   ` [EXTERNAL] " Leif Lindholm
2020-02-07 12:43 ` [PATCH 07/19] Silicon/NXP: remove not needed components Pankaj Bansal
2020-02-10 17:11   ` Leif Lindholm
2020-02-11  7:24     ` Pankaj Bansal
2020-02-20 19:05       ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 08/19] Silicon/NXP: Remove unnecessary PCDs Pankaj Bansal
2020-02-10 17:32   ` Leif Lindholm
2020-02-11  8:45     ` Pankaj Bansal
2020-02-20 18:56       ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 09/19] Silicon/NXP: Move dsc file Pankaj Bansal
2020-02-11 11:35   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 10/19] Platform/NXP: rename the ArmPlatformLib as per ArmPlatformPkg Pankaj Bansal
2020-02-11 11:40   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 11/19] Silicon/NXP: Add Chassis Lib for Chassis2 Pankaj Bansal
2020-02-11 12:28   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 12/19] Silicon/NXP/LS1043A: Add SocLib Pankaj Bansal
2020-02-11 12:38   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 13/19] Silicon/NXP: Move RAM retrieval from SocLib Pankaj Bansal
2020-02-11 13:28   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 14/19] Silicon/NXP/LS1043A: Replce SocLib Pankaj Bansal
2020-02-11 13:35   ` Leif Lindholm
2020-02-12  9:37     ` Pankaj Bansal
2020-02-12 22:50       ` Leif Lindholm
2020-02-13 11:00         ` Pankaj Bansal
2020-02-20 18:45           ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 15/19] Platform/NXP/LS1043ARDB: introduce PEI Phase Pankaj Bansal
2020-02-12 20:24   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 16/19] Silicon/NXP: Add Pl011 Serial port lib Pankaj Bansal
2020-02-12 20:26   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 17/19] Silicon/NXP: Add Chassis3V2 Pankaj Bansal
2020-02-12 20:33   ` Leif Lindholm [this message]
2020-02-07 12:43 ` [PATCH 18/19] Silicon/NXP: Add LX2160A SocLib Pankaj Bansal
2020-02-12 21:39   ` Leif Lindholm
2020-02-07 12:43 ` [PATCH 19/19] Platform/NXP: Add LX2160ARDBPKG Pankaj Bansal
2020-02-12 21:36   ` 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=20200212203339.GF23627@bivouac.eciton.net \
    --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