> -----Original Message-----
> From: Leif Lindholm <l...@nuviainc.com>
> Sent: Wednesday, April 1, 2020 7:48 PM
> To: Pankaj Bansal (OSS) <pankaj.ban...@oss.nxp.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; Michael D Kinney
> <michael.d.kin...@intel.com>; devel@edk2.groups.io; Varun Sethi
> <v.se...@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> mahm...@arm.com>; Jon Nettleton <j...@solid-run.com>
> Subject: Re: [PATCH v2 18/28] Silicon/NXP: Add Chassis2 Package
>
> On Fri, Mar 20, 2020 at 20:05:33 +0530, Pankaj Bansal wrote:
> > From: Pankaj Bansal <pankaj.ban...@nxp.com>
> >
> > A Chassis is a base framework used for building SoCs.
> > We can think of Chassis/Soc/Platform(a.k.a Borad) in Oops terms.
>
> Board?
> "Oops" is not a term I am familiar with other than as an exclamation -
> can you elaborate please?
Oops: Object-Oriented Programming System
Basically concept of base classes and derived classes.
>
> > Chassis is base. Soc is based on some Chassis.
> > Platform is based on some Soc.
> >
> > SOCs that are designed around same chassis, reuse most of the components.
> >
> > Therefore, add the package for Chassis2. LS1043A and LS1046A SOCs belong
> > to Chassis2.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.ban...@nxp.com>
> > ---
> > Silicon/NXP/Chassis2/Chassis2.dec | 23 +++++
> > Silicon/NXP/Chassis2/Chassis2.dsc.inc | 10 ++
> > Silicon/NXP/Chassis2/Include/Chassis.h | 34 +++++++
> > .../Chassis2/Library/ChassisLib/ChassisLib.c | 97 +++++++++++++++++++
> > .../Library/ChassisLib/ChassisLib.inf | 34 +++++++
> > Silicon/NXP/Include/Library/ChassisLib.h | 51 ++++++++++
> > Silicon/NXP/NxpQoriqLs.dec | 4 +
> > 7 files changed, 253 insertions(+)
> > create mode 100644 Silicon/NXP/Chassis2/Chassis2.dec
> > create mode 100644 Silicon/NXP/Chassis2/Chassis2.dsc.inc
> > create mode 100644 Silicon/NXP/Chassis2/Include/Chassis.h
> > create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> > create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> > create mode 100644 Silicon/NXP/Include/Library/ChassisLib.h
> >
> > diff --git a/Silicon/NXP/Chassis2/Chassis2.dec
> b/Silicon/NXP/Chassis2/Chassis2.dec
> > new file mode 100644
> > index 000000000000..a0048bd784ea
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis2/Chassis2.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
> > +
> > diff --git a/Silicon/NXP/Chassis2/Chassis2.dsc.inc
> b/Silicon/NXP/Chassis2/Chassis2.dsc.inc
> > new file mode 100644
> > index 000000000000..db8e5a92eacb
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis2/Chassis2.dsc.inc
> > @@ -0,0 +1,10 @@
> > +# @file
> > +#
> > +# Copyright 2020 NXP
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +#
> > +
> > +[LibraryClasses.common]
> > + ChassisLib|Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> > diff --git a/Silicon/NXP/Chassis2/Include/Chassis.h
> b/Silicon/NXP/Chassis2/Include/Chassis.h
> > new file mode 100644
> > index 000000000000..72bd97efd004
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis2/Include/Chassis.h
> > @@ -0,0 +1,34 @@
> > +/** @file
> > +
> > + Copyright 2020 NXP
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +#ifndef CHASSIS_H__
> > +#define CHASSIS_H__
> > +
> > +#define NXP_LAYERSCAPE_CHASSIS2_DCFG_ADDRESS 0x1EE0000
> > +
> > +/* 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_NSCR0 (SMMU_BASE_ADDR + 0x400)
> > +
> > +#define SCR0_USFCFG_MASK 0x00000400
> > +#define SCR0_CLIENTPD_MASK 0x00000001
> > +#define SACR_PAGESIZE_MASK 0x00010000
> > +
> > +/**
> > + 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[16]; // Reset Control Word Status Register
>
> If this is the formal word as used in official documentation, it still
> needs listing in a glossary in the file comment header.
This is register in SOC.
I thought the explanation in structure definition is fine for registers.
In
https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/6_documenting_software/64_what_you_must_comment.html
6.4.4 Comment data structure declarations and #define statements.
The structure data members won't be used standalone in code without including
the structure definition.
So any abbreviation can be referred from structure definition right ?
Also I referred
https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/appendix_a_common_examples.html
Type declarations.
/// Structure without forward reference.
typedef struct {
UINT32 Signature; ///< Signature description.
EFI_HANDLE Handle; ///< Handle description.
EFI_PROD_PROT1_PROTOCOL ProdProt1; ///< ProdProt1 description.
EFI_PROD_PROT2_PROTOCOL ProdProt2; ///< ProdProt2 description.
} DRIVER_NAME_INSTANCE;
>
> > +} NXP_LAYERSCAPE_CHASSIS2_DEVICE_CONFIG;
> > +#pragma pack()
> > +
> > +#endif // CHASSIS_H__
> > diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> > new file mode 100644
> > index 000000000000..816e0fa29c4a
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> > @@ -0,0 +1,97 @@
> > +/** @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>
>
> Can you flip the two above lines around to get them in order?
OK
>
> > +#include <Library/PcdLib.h>
> > +#include <Library/SerialPortLib.h>
> > +
> > +/**
> > + Read Dcfg register
> > +
> > + @param Address The MMIO register to read.
> > +
> > + @return The value read.
> > +**/
> > +UINT32
> > +EFIAPI
> > +DcfgRead32 (
> > + IN UINTN Address
> > + )
> > +{
>
> Please replace these with the GetMmioOperations call, as designed.
>
> In fact, please provide an additional patch deleting all of the direct
> SwapMmio* functions from IoAccessLib.h and changing all of the
> SwapMmio* function definitions in IoAccessLib.c to STATIC to prevent
> future accidental direct use.
ok
>
> /
> Leif
>
> > + if (FeaturePcdGet (PcdDcfgBigEndian)) {
> > + return SwapMmioRead32 (Address);
> > + } else {
> > + return MmioRead32 (Address);
> > + }
> > +}
> > +
> > +/**
> > + Write Dcfg register
> > +
> > + @param Address The MMIO register to write.
> > + @param Value The value to write to the MMIO register.
> > +
> > + @return Value.
> > +**/
> > +UINT32
> > +EFIAPI
> > +DcfgWrite32 (
> > + IN UINTN Address,
> > + IN UINT32 Value
> > + )
> > +{
> > + if (FeaturePcdGet (PcdDcfgBigEndian)) {
> > + return SwapMmioWrite32 (Address, Value);
> > + } else {
> > + return MmioWrite32 (Address, Value);
> > + }
> > +}
> > +
> > +/*
> > + * Setup SMMU in bypass mode
> > + * and also set its pagesize
> > + */
> > +STATIC
> > +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);
> > + Value &= ~SCR0_USFCFG_MASK;
> > + MmioWrite32 ((UINTN)SMMU_REG_SCR0, Value);
> > +
> > + Value = (MmioRead32 ((UINTN)SMMU_REG_NSCR0) |
> SCR0_CLIENTPD_MASK);
> > + Value &= ~SCR0_USFCFG_MASK;
> > + MmioWrite32 ((UINTN)SMMU_REG_NSCR0, Value);
> > +}
> > +
> > +/**
> > + Function to initialize Chassis Specific functions
> > + **/
> > +VOID
> > +ChassisInit (
> > + VOID
> > + )
> > +{
> > + //
> > + // Early init serial Port to get board information.
> > + //
> > + SerialPortInitialize ();
> > +
> > + SmmuInit ();
> > +}
> > diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> > new file mode 100644
> > index 000000000000..2bb16af53134
> > --- /dev/null
> > +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> > @@ -0,0 +1,34 @@
> > +# @file
> > +#
> > +# Copyright 2020 NXP
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +#
> > +#
> > +
> > +[Defines]
> > + INF_VERSION = 1.27
> > + BASE_NAME = Chassis2Lib
> > + FILE_GUID = fae0d077-5fc2-494f-b8e1-c51a3023ee3e
> > + MODULE_TYPE = BASE
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = ChassisLib
> > +
> > +[Packages]
> > + ArmPkg/ArmPkg.dec
> > + MdePkg/MdePkg.dec
> > + Silicon/NXP/Chassis2/Chassis2.dec
> > + Silicon/NXP/NxpQoriqLs.dec
> > +
> > +[LibraryClasses]
> > + IoAccessLib
> > + IoLib
> > + PcdLib
> > + SerialPortLib
> > +
> > +[Sources.common]
> > + ChassisLib.c
> > +
> > +[FeaturePcd]
> > + gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian
> > +
> > diff --git a/Silicon/NXP/Include/Library/ChassisLib.h
> b/Silicon/NXP/Include/Library/ChassisLib.h
> > new file mode 100644
> > index 000000000000..89992a4b6fd5
> > --- /dev/null
> > +++ b/Silicon/NXP/Include/Library/ChassisLib.h
> > @@ -0,0 +1,51 @@
> > +/** @file
> > + Chassis Lib to provide Chessis specific functionality to all SOCs in
> > + a Chassis.
> > +
> > + Copyright 2020 NXP
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +**/
> > +
> > +#ifndef CHASSIS_LIB_H__
> > +#define CHASSIS_LIB_H__
> > +
> > +#include <Chassis.h>
> > +
> > +/**
> > + Read Dcfg register
> > +
> > + @param Address The MMIO register to read.
> > +
> > + @return The value read.
> > +**/
> > +UINT32
> > +EFIAPI
> > +DcfgRead32 (
> > + IN UINTN Address
> > + );
> > +
> > +/**
> > + Write Dcfg register
> > +
> > + @param Address The MMIO register to write.
> > + @param Value The value to write to the MMIO register.
> > +
> > + @return Value.
> > +**/
> > +UINT32
> > +EFIAPI
> > +DcfgWrite32 (
> > + IN UINTN Address,
> > + IN UINT32 Value
> > + );
> > +
> > +/**
> > + Function to initialize Chassis Specific functions
> > + **/
> > +VOID
> > +ChassisInit (
> > + VOID
> > + );
> > +
> > +#endif // CHASSIS_LIB_H__
> > diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> > index 2ac047a89274..3e79f502c127 100644
> > --- a/Silicon/NXP/NxpQoriqLs.dec
> > +++ b/Silicon/NXP/NxpQoriqLs.dec
> > @@ -14,6 +14,9 @@ [Includes]
> > Include
> >
> > [LibraryClasses]
> > + ## @libraryclass Provides Chassis specific functions to other modules
> > + ChassisLib|Include/Library/ChassisLib.h
> > +
> > ## @libraryclass Provides services to read/write to I2c devices
> > I2cLib|Include/Library/I2cLib.h
> >
> > @@ -29,4 +32,5 @@ [PcdsFixedAtBuild.common]
> >
> > [PcdsFeatureFlag]
> >
> gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x0000
> 0315
> > +
> gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000316
> >
> > --
> > 2.17.1
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56977): https://edk2.groups.io/g/devel/message/56977
Mute This Topic: https://groups.io/mt/72077451/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
