> -----Original Message-----
> From: Leif Lindholm <leif.lindh...@linaro.org>
> Sent: Wednesday, December 19, 2018 6:56 PM
> To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
> Cc: ard.biesheu...@linaro.org; michael.d.kin...@intel.com; edk2-
> de...@lists.01.org; Udit Kumar <udit.ku...@nxp.com>; Varun Sethi
> <v.se...@nxp.com>
> Subject: Re: [PATCH edk2-platforms 13/41] Silicon/NXP : Add support of IfcLib
>
> On Wed, Nov 28, 2018 at 08:31:27PM +0530, Meenakshi Aggarwal wrote:
> > Add support of IfcLib, it will be used to perform any operation on IFC
> > controller.
>
> Expand acronym.
>
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>
> > ---
> > Silicon/NXP/Include/Library/IfcLib.h | 26 +++++
> > Silicon/NXP/Library/IfcLib/IfcLib.c | 150 +++++++++++++++++++++++++++
> > Silicon/NXP/Library/IfcLib/IfcLib.h | 190
> ++++++++++++++++++++++++++++++++++
> > Silicon/NXP/Library/IfcLib/IfcLib.inf | 38 +++++++
>
> Names Ifc -> NxpIfc please.
>
Is this renaming really needed for IfcLib, I can change the guard and header
file name.
Please suggest?
> > Silicon/NXP/NxpQoriqLs.dec | 1 +
> > 5 files changed, 405 insertions(+)
> > create mode 100644 Silicon/NXP/Include/Library/IfcLib.h
> > create mode 100644 Silicon/NXP/Library/IfcLib/IfcLib.c
> > create mode 100644 Silicon/NXP/Library/IfcLib/IfcLib.h
> > create mode 100644 Silicon/NXP/Library/IfcLib/IfcLib.inf
> >
> > diff --git a/Silicon/NXP/Include/Library/IfcLib.h
> > b/Silicon/NXP/Include/Library/IfcLib.h
> > new file mode 100644
> > index 0000000..8d2c151
> > --- /dev/null
> > +++ b/Silicon/NXP/Include/Library/IfcLib.h
> > @@ -0,0 +1,26 @@
> > +/** @IfcLib.h
> > +
> > + The integrated flash controller (IFC) is used to interface with
> > + external asynchronous NAND flash, asynchronous NOR flash, SRAM, generic
> ASIC memories and EPROM.
> > +
> > + Copyright 2018 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%2Fop
> > + ensource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenaks
> > +
> hi.aggarwal%40nxp.com%7C62c0401c65ac481e650508d665b57ea9%7C686ea1
> d3b
> > +
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636808227519182943&sdata=4
> UMgN
> > + 7laz86jDwTlvJrHWkqdum8qmZPGWrhESvjHBMQ%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 __IFC_LIB_H__
> > +#define __IFC_LIB_H__
>
> Header guard NXP_ (and/or QORIQ_) prefix.
>
> > +
> > +VOID
> > +IfcInit (
> > + VOID
> > + );
> > +
> > +#endif //__IFC_LIB_H__
> > diff --git a/Silicon/NXP/Library/IfcLib/IfcLib.c
> > b/Silicon/NXP/Library/IfcLib/IfcLib.c
> > new file mode 100644
> > index 0000000..8cf02ae
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/IfcLib/IfcLib.c
> > @@ -0,0 +1,150 @@
> > +/** @IfcLib.c
> > +
> > + The integrated flash controller (IFC) is used to interface with
> > + external asynchronous/ synchronous NAND flash, asynchronous NOR
> > + flash, SRAM, generic ASIC memory and EPROM.
> > + It has eight chip-selects, to which a maximum of eight flash
> > + devices can be attached, although only one of these can be accessed at
> > any
> given time.
> > +
> > + Copyright 2018 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%2Fop
> > + ensource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenaks
> > +
> hi.aggarwal%40nxp.com%7C62c0401c65ac481e650508d665b57ea9%7C686ea1
> d3b
> > +
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636808227519182943&sdata=4
> UMgN
> > + 7laz86jDwTlvJrHWkqdum8qmZPGWrhESvjHBMQ%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 <Library/IoAccessLib.h>
> > +#include "IfcLib.h"
> > +
> > +STATIC MMIO_OPERATIONS_32 *mMmioOps;
> > +
> > +STATIC UINT8 mNandCS;
> > +STATIC UINT8 mNorCS;
> > +STATIC UINT8 mFpgaCS;
> > +
> > +VOID
>
> Local only?
> If so, STATIC please.
>
> > +SetTimings (
> > + IN UINT8 CS,
> > + IN IFC_TIMINGS IfcTimings
> > + )
> > +{
> > + IFC_REGS* IfcRegs;
> > +
> > + IfcRegs = (IFC_REGS*)PcdGet64 (PcdIfcBaseAddr);
> > +
> > + // Configure Extended chip select property registers
> > + mMmioOps->Write ((UINTN)&IfcRegs->CsprCs[CS].CsprExt,
> > + IfcTimings.CsprExt);
> > +
> > + // Configure Fpga timing registers
> > + mMmioOps->Write ((UINTN)&IfcRegs->FtimCs[CS].Ftim[IFC_FTIM0],
> > + IfcTimings.Ftim[0]); mMmioOps->Write
> > + ((UINTN)&IfcRegs->FtimCs[CS].Ftim[IFC_FTIM1], IfcTimings.Ftim[1]);
> > + mMmioOps->Write ((UINTN)&IfcRegs->FtimCs[CS].Ftim[IFC_FTIM2],
> > + IfcTimings.Ftim[2]); mMmioOps->Write
> > + ((UINTN)&IfcRegs->FtimCs[CS].Ftim[IFC_FTIM3], IfcTimings.Ftim[3]);
> > +
> > + // Configure chip select option registers mMmioOps->Write
> > + ((UINTN)&IfcRegs->CsprCs[CS].Cspr, IfcTimings.Cspr);
> > +
> > + // Configure address mask registers mMmioOps->Write
> > + ((UINTN)&IfcRegs->AmaskCs[CS].Amask, IfcTimings.Amask);
> > +
> > + // Configure chip select property registers mMmioOps->Write
> > + ((UINTN)&IfcRegs->CsorCs[CS].Csor, IfcTimings.Csor);
> > +
> > + return;
> > +}
> > +
> > +VOID
>
> Local only?
> If so, STATIC please.
>
> > +NandInit(
> > + VOID
> > + )
> > +{
> > + IFC_REGS* IfcRegs;
> > + IFC_TIMINGS NandIfcTimings;
> > +
> > + IfcRegs = (IFC_REGS*)PcdGet64 (PcdIfcBaseAddr);
> > +
> > + // Get Nand Flash Timings
> > + GetIfcNandFlashTimings (&NandIfcTimings);
> > +
> > + // Validate chip select
> > + if (NandIfcTimings.CS < IFC_CS_MAX) {
> > + mNandCS = NandIfcTimings.CS;
> > +
> > + // clear event registers
> > + mMmioOps->Write ((UINTN)&IfcRegs->IfcNand.PgrdcmplEvtStat, ~0U);
> > +
> > + mMmioOps->Write ((UINTN)&IfcRegs->IfcNand.NandEvterStat, ~0U);
> > +
> > + // Enable error and event for any detected errors
> > + mMmioOps->Write ((UINTN)&IfcRegs->IfcNand.NandEvterEn,
> > + IFC_NAND_EVTER_EN_OPC_EN |
>
> Indentation should be to function name, not struct name.
> (Please address throughout.)
>
> > + IFC_NAND_EVTER_EN_PGRDCMPL_EN |
> > + IFC_NAND_EVTER_EN_FTOER_EN |
> > + IFC_NAND_EVTER_EN_WPER_EN);
> > + mMmioOps->Write ((UINTN)&IfcRegs->IfcNand.Ncfgr, 0x0);
> > +
> > + SetTimings (mNandCS, NandIfcTimings); }
> > +
> > + return;
> > +}
> > +
> > +VOID
>
> Local only?
> If so, STATIC please.
>
> > +FpgaInit (
> > + VOID
> > + )
> > +{
> > + IFC_TIMINGS FpgaIfcTimings;
> > +
> > + // Get Fpga Flash Timings
> > + GetIfcFpgaTimings (&FpgaIfcTimings);
> > +
> > + // Validate chip select
> > + if (FpgaIfcTimings.CS < IFC_CS_MAX) {
> > + mFpgaCS = FpgaIfcTimings.CS;
> > + SetTimings (mFpgaCS, FpgaIfcTimings); }
> > +
> > + return;
> > +}
> > +
> > +VOID
> > +NorInit (
> > + VOID
> > + )
> > +{
> > + IFC_TIMINGS NorIfcTimings;
> > +
> > + // Get NOR Flash Timings
> > + GetIfcNorFlashTimings (&NorIfcTimings);
> > +
> > + // Validate chip select
> > + if (NorIfcTimings.CS < IFC_CS_MAX) {
> > + mNorCS = NorIfcTimings.CS;
> > + SetTimings (mNorCS, NorIfcTimings); }
> > +
> > + return;
> > +}
> > +
> > +//
> > +// IFC has NOR , NAND and FPGA
> > +//
> > +VOID
> > +IfcInit (
> > + VOID
> > + )
> > +{
> > + mMmioOps = GetMmioOperations32 (FixedPcdGetBool (PcdIfcBigEndian));
> > +
> > + NorInit();
> > + NandInit();
> > + FpgaInit();
> > +
> > + return;
> > +}
> > diff --git a/Silicon/NXP/Library/IfcLib/IfcLib.h
> > b/Silicon/NXP/Library/IfcLib/IfcLib.h
> > new file mode 100644
> > index 0000000..38ce247
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/IfcLib/IfcLib.h
> > @@ -0,0 +1,190 @@
> > +/** @IfcLib.h
> > +
> > + The integrated flash controller (IFC) is used to interface with
> > + external asynchronous/ synchronous NAND flash, asynchronous NOR
> > + flash, SRAM, generic ASIC memory and EPROM.
> > + It has eight chip-selects, to which a maximum of eight flash
> > + devices can be attached, although only one of these can be accessed at
> > any
> given time.
> > +
> > + 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%2Fop
> > + ensource.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenaks
> > +
> hi.aggarwal%40nxp.com%7C62c0401c65ac481e650508d665b57ea9%7C686ea1
> d3b
> > +
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636808227519182943&sdata=4
> UMgN
> > + 7laz86jDwTlvJrHWkqdum8qmZPGWrhESvjHBMQ%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 __IFC_LIB_H__
> > +#define __IFC_LIB_H__
>
> NXP_ and/or QORIQ_ prefix?
>
> > +
> > +#include <Ifc.h>
> > +#include <Uefi.h>
> > +
> > +#define IFC_NAND_RESERVED_SIZE FixedPcdGet32
> (PcdIfcNandReservedSize)
> > +
> > +typedef enum {
> > + IFC_FTIM0 = 0,
> > + IFC_FTIM1,
> > + IFC_FTIM2,
> > + IFC_FTIM3,
>
> CamelCase member names please, throughout.
>
> > +} IFC_FTIMS;
> > +
> > +typedef struct {
> > + UINT32 CsprExt;
> > + UINT32 Cspr;
> > + UINT32 Res;
> > +} IFC_CSPR;
> > +
> > +typedef struct {
> > + UINT32 Amask;
>
> AddressMask?
>
> > + UINT32 Res[0x2];
>
> Is this "Reserved"?
> If so, please write out in full.
> Also please drop the hex prefix.
> Apples throughout.
>
> > +} IFC_AMASK;
> > +
> > +typedef struct {
> > + UINT32 Csor;
> > + UINT32 CsorExt;
> > + UINT32 Res;
> > +} IFC_CSOR;
> > +
> > +typedef struct {
> > + UINT32 Ftim[4];
> > + UINT32 Res[0x8];
> > +}IFC_FTIM ;
> > +
> > +typedef struct {
> > + UINT32 Ncfgr;
> > + UINT32 Res1[0x4];
> > + UINT32 NandFcr0;
> > + UINT32 NandFcr1;
> > + UINT32 Res2[0x8];
> > + UINT32 Row0;
> > + UINT32 Res3;
> > + UINT32 Col0;
> > + UINT32 Res4;
> > + UINT32 Row1;
> > + UINT32 Res5;
> > + UINT32 Col1;
> > + UINT32 Res6;
> > + UINT32 Row2;
> > + UINT32 Res7;
> > + UINT32 Col2;
> > + UINT32 Res8;
> > + UINT32 Row3;
> > + UINT32 Res9;
> > + UINT32 Col3;
> > + UINT32 Res10[0x24];
> > + UINT32 NandFbcr;
> > + UINT32 Res11;
> > + UINT32 NandFir0;
> > + UINT32 NandFir1;
> > + UINT32 nandFir2;
> > + UINT32 Res12[0x10];
> > + UINT32 NandCsel;
> > + UINT32 Res13;
> > + UINT32 NandSeqStrt;
> > + UINT32 Res14;
> > + UINT32 NandEvterStat;
> > + UINT32 Res15;
> > + UINT32 PgrdcmplEvtStat;
> > + UINT32 Res16[0x2];
> > + UINT32 NandEvterEn;
> > + UINT32 Res17[0x2];
> > + UINT32 NandEvterIntrEn;
> > + UINT32 Res18[0x2];
> > + UINT32 NandErattr0;
> > + UINT32 NandErattr1;
> > + UINT32 Res19[0x10];
> > + UINT32 NandFsr;
> > + UINT32 Res20;
> > + UINT32 NandEccstat[4];
> > + UINT32 Res21[0x20];
> > + UINT32 NanNdcr;
> > + UINT32 Res22[0x2];
> > + UINT32 NandAutobootTrgr;
> > + UINT32 Res23;
> > + UINT32 NandMdr;
> > + UINT32 Res24[0x5C];
> > +} IFC_NAND;
> > +
> > +/*
> > + * IFC controller NOR Machine registers */ typedef struct {
> > + UINT32 NorEvterStat;
> > + UINT32 Res1[0x2];
> > + UINT32 NorEvterEn;
> > + UINT32 Res2[0x2];
> > + UINT32 NorEvterIntrEn;
> > + UINT32 Res3[0x2];
> > + UINT32 NorErattr0;
> > + UINT32 NorErattr1;
> > + UINT32 NorErattr2;
> > + UINT32 Res4[0x4];
> > + UINT32 NorCr;
> > + UINT32 Res5[0xEF];
> > +} IFC_NOR;
> > +
> > +/*
> > + * IFC controller GPCM Machine registers
> > + */
> > +typedef struct {
>
> extra space
>
> > + UINT32 GpcmEvterStat;
> > + UINT32 Res1[0x2];
> > + UINT32 GpcmEvterEn;
> > + UINT32 Res2[0x2];
> > + UINT32 gpcmEvterIntrEn;
> > + UINT32 Res3[0x2];
> > + UINT32 GpcmErattr0;
> > + UINT32 GpcmErattr1;
> > + UINT32 GcmErattr2;
> > + UINT32 GpcmStat;
> > +} IFC_GPCM;
> > +
> > +/*
> > + * IFC Controller Registers
> > + */
> > +typedef struct {
> > + UINT32 IfcRev;
> > + UINT32 Res1[0x2];
> > + IFC_CSPR CsprCs[IFC_BANK_COUNT];
> > + UINT8 Res2[IFC_CSPR_REG_LEN - IFC_CSPR_USED_LEN];
> > + IFC_AMASK AmaskCs[IFC_BANK_COUNT];
> > + UINT8 Res3[IFC_AMASK_REG_LEN - IFC_AMASK_USED_LEN];
> > + IFC_CSOR CsorCs[IFC_BANK_COUNT];
> > + UINT8 Res4[IFC_CSOR_REG_LEN - IFC_CSOR_USED_LEN];
> > + IFC_FTIM FtimCs[IFC_BANK_COUNT];
> > + UINT8 Res5[IFC_FTIM_REG_LEN - IFC_FTIM_USED_LEN];
> > + UINT32 RbStat;
> > + UINT32 RbMap;
> > + UINT32 WpMap;
> > + UINT32 IfcGcr;
> > + UINT32 Res7[0x2];
> > + UINT32 CmEvter_stat;
> > + UINT32 Res8[0x2];
> > + UINT32 CmEvterEn;
> > + UINT32 Res9[0x2];
> > + UINT32 CmEvterIntrEn;
> > + UINT32 Res10[0x2];
> > + UINT32 CmErattr0;
> > + UINT32 CmErattr1;
> > + UINT32 Res11[0x2];
> > + UINT32 IfcCcr;
> > + UINT32 IfcCsr;
> > + UINT32 DdrCcrLow;
> > + UINT32 Res12[IFC_NAND_RESERVED_SIZE];
> > + IFC_NAND IfcNand;
> > + IFC_NOR IfcNor;
> > + IFC_GPCM IfcGpcm;
> > +} IFC_REGS;
> > +
> > +extern VOID GetIfcNorFlashTimings (IFC_TIMINGS * NorIfcTimings);
> > +
> > +extern VOID GetIfcFpgaTimings (IFC_TIMINGS *FpgaIfcTimings);
> > +
> > +extern VOID GetIfcNandFlashTimings (IFC_TIMINGS * NandIfcTimings);
>
> Please move these function declarations to the (first) patch that adds
> implementations of these functions.
>
> > +
> > +#endif //__IFC_LIB_H__
> > diff --git a/Silicon/NXP/Library/IfcLib/IfcLib.inf
> b/Silicon/NXP/Library/IfcLib/IfcLib.inf
> > new file mode 100644
> > index 0000000..989eb44
> > --- /dev/null
> > +++ b/Silicon/NXP/Library/IfcLib/IfcLib.inf
> > @@ -0,0 +1,38 @@
> > +# IfcLib.inf
> > +#
> > +# Component description file for IFC Library
> > +#
> > +# Copyright 2018 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%2Fopenso
> urce.org%2Flicenses%2Fbsd-
> license.php&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C62c0
> 401c65ac481e650508d665b57ea9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636808227519182943&sdata=4UMgN7laz86jDwTlvJrHWkqdu
> m8qmZPGWrhESvjHBMQ%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 = IfcLib
>
> Nxp
>
> > + FILE_GUID = a465d76c-0785-4ee7-bd72-767983d575a2
> > + MODULE_TYPE = BASE
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = IfcLib
>
> Nxp
>
> /
> Leif
>
> > +
> > +[Sources.common]
> > + IfcLib.c
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + Silicon/NXP/NxpQoriqLs.dec
> > +
> > +[LibraryClasses]
> > + BoardLib
> > + IoAccessLib
> > +
> > +[FixedPcd]
> > + gNxpQoriqLsTokenSpaceGuid.PcdIfcBaseAddr
> > + gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian
> > + gNxpQoriqLsTokenSpaceGuid.PcdIfcNandReservedSize
> > diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> > index df64ad6..bd89da4 100644
> > --- a/Silicon/NXP/NxpQoriqLs.dec
> > +++ b/Silicon/NXP/NxpQoriqLs.dec
> > @@ -77,6 +77,7 @@
> > gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr|0x0|UINT64|0x00000128
> > gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize|0x0|UINT64|0x00000129
> > gNxpQoriqLsTokenSpaceGuid.PcdDramMemSize|0x0|UINT64|0x0000012A
> > + gNxpQoriqLsTokenSpaceGuid.PcdIfcBaseAddr|0x0|UINT64|0x0000012B
> >
> > #
> > # IFC PCDs
> > --
> > 1.9.1
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
