From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web09.14615.1574786622797859206 for ; Tue, 26 Nov 2019 08:43:43 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=dKcc2mJ7; spf=pass (domain: linaro.org, ip: 209.85.128.65, mailfrom: leif.lindholm@linaro.org) Received: by mail-wm1-f65.google.com with SMTP id g206so3955317wme.1 for ; Tue, 26 Nov 2019 08:43:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=T5XOn4qLvRuOiMfng0iCZe3hQec33FbO1s+R5PhOTpA=; b=dKcc2mJ7FmTU++vVKqRFhjbIHXozZFGsGU40vQYPtoI87SXbjLzEBViwoJs4qYMttr 44t2dbG3ChxVADdBwU1WC7Vb11JPnPdAW13TLSn8NlLdNbo3w/B5abaJxPhkISKatg4l tfFazNazErl/ablOEro9jj2n92A47sieUwKD00cksSSDPjiR77lJ+f3aBuWdXKC4sE6O bFT3ccbW+ZT35AgjGqD6Z9hBBz8NfKKNk9NDcsAwfsK4fUcQDlvDionUnHxl/cR286Em FQO+72TU+P7HC66gqRG0aDUI+nzrFwOVz0olaSJ5PTirgrvO06Zh+kns3rKUw45Nm2Dq sTZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=T5XOn4qLvRuOiMfng0iCZe3hQec33FbO1s+R5PhOTpA=; b=BmXS4TrixA0ub9ggXncr/4T2mF5I3cdKrQnQIzowy8t1MIqInqb9iVTY4+pNxxo5Dh KoImzbgr4i4kDwAMMft/pM1b5z9z8QN4TWl4Fk0Nn7qdeNROA4Az6Thk8e6m1sUsWSDR 8jf2X/+5MxX5I/32DZZXhwLEyd57iwjx781GQKOueYbP/RB/kFQltxMMhDe2GGFsjuiQ bYejtFeOOKN0M/QFACOSLZ3kOOfQnvLQZR/oKVy6ZoocwuQeXtft55eKuEAOeo6WTl8o KBjwL7FbEC7dxwvCQygSZ0RXNBVUp7Vpjtndpqzi074DBs1nwzL8L9Ew0pQKjHhqgmQl VzEQ== X-Gm-Message-State: APjAAAVbEnDO+GFd9iRHG1asPQViT+efQTzrlI6HSbE1oFCDD79gHcxv mtd6FUJmQatPqV0a5iYR+1qR2g== X-Google-Smtp-Source: APXvYqwDR8/TfDSnisq/v5QLyyZeWz7qSRqSqp+FYRsTGIS4/AS2n+UAnlMTMzDwzK6TTsPHiUBIHA== X-Received: by 2002:a7b:cf26:: with SMTP id m6mr5236839wmg.17.1574786621154; Tue, 26 Nov 2019 08:43:41 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id z4sm3874707wmf.36.2019.11.26.08.43.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Nov 2019 08:43:40 -0800 (PST) Date: Tue, 26 Nov 2019 16:43:38 +0000 From: "Leif Lindholm" To: Meenakshi Aggarwal Cc: ard.biesheuvel@linaro.org, michael.d.kinney@intel.com, devel@edk2.groups.io, v.sethi@nxp.com Subject: Re: [edk2-platforms] [PATCH v2 03/11] SocLib : Add support for initialization of peripherals Message-ID: <20191126164338.GD7359@bivouac.eciton.net> References: <1570639758-30355-1-git-send-email-meenakshi.aggarwal@nxp.com> <1574353514-23986-1-git-send-email-meenakshi.aggarwal@nxp.com> <1574353514-23986-4-git-send-email-meenakshi.aggarwal@nxp.com> MIME-Version: 1.0 In-Reply-To: <1574353514-23986-4-git-send-email-meenakshi.aggarwal@nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline I am a bit confused - none of the feedback from last review seems to have been addressed. Comments below. On Thu, Nov 21, 2019 at 21:55:06 +0530, Meenakshi Aggarwal wrote: > Add SocInit function that initializes peripherals > and print board and soc information. > > Signed-off-by: Meenakshi Aggarwal > --- > Silicon/NXP/Library/SocLib/LS1043aSocLib.inf | 45 ++ > Silicon/NXP/Include/Chassis2/LsSerDes.h | 62 +++ > Silicon/NXP/Include/Chassis2/NxpSoc.h | 361 ++++++++++++++ > Silicon/NXP/Include/DramInfo.h | 38 ++ > Silicon/NXP/LS1043A/Include/SocSerDes.h | 51 ++ > Silicon/NXP/Library/SocLib/NxpChassis.h | 136 ++++++ > Silicon/NXP/Library/SocLib/Chassis.c | 498 ++++++++++++++++++++ > Silicon/NXP/Library/SocLib/Chassis2/Soc.c | 162 +++++++ > Silicon/NXP/Library/SocLib/SerDes.c | 268 +++++++++++ > 9 files changed, 1621 insertions(+) > diff --git a/Silicon/NXP/Library/SocLib/Chassis.c b/Silicon/NXP/Library/SocLib/Chassis.c > new file mode 100644 > index 000000000000..5dda6f8c2662 > --- /dev/null > +++ b/Silicon/NXP/Library/SocLib/Chassis.c > @@ -0,0 +1,498 @@ > +/** @file > + SoC specific Library containg functions to initialize various SoC components > + > + Copyright 2017-2019 NXP > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#ifdef CHASSIS2 > +#include > +#elif CHASSIS3 > +#include > +#endif > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include "NxpChassis.h" > + > +/* > + * Structure to list available SOCs. > + * Name, Soc Version, Number of Cores > + */ > +STATIC CPU_TYPE mCpuTypeList[] = { > + CPU_TYPE_ENTRY (LS1043A, LS1043A, 4), > + CPU_TYPE_ENTRY (LS1046A, LS1046A, 4), > + CPU_TYPE_ENTRY (LS2088A, LS2088A, 8), > +}; > + > +UINT32 > +EFIAPI > +GurRead ( > + IN UINTN Address > + ) > +{ > + if (FixedPcdGetBool (PcdGurBigEndian)) { > + return SwapMmioRead32 (Address); > + } else { > + return MmioRead32 (Address); > + } > +} > + > +/* > + * 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; > + } > + Count++; > + } > + } > + ClusterIndex++; > + } while (CHECK_CLUSTER (Cluster)); > + > + return Mask; > +} > + > +/* > + * Return the number of cores on this SOC. > + */ > +UINTN > +CpuNumCores ( > + VOID > + ) > +{ > + UINTN Count; > + UINTN Num; > + > + Count = 0; > + Num = CpuMask (); > + > + while (Num) { > + Count += Num & 1; > + Num >>= 1; > + } > + > + return Count; > +} > + > +/* > + * Return core's cluster > + */ > +INT32 > +QoriqCoreToCluster ( > + 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 ClusterIndex; > + } > + Count++; > + } > + } > + ClusterIndex++; > + } while (CHECK_CLUSTER (Cluster)); > + > + return -1; // cannot identify the cluster > +} > + > +/* > + * Return the type of core i.e. A53, A57 etc of inputted > + * core number. > + */ > +UINTN > +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; > + } > + Count++; > + } > + } > + ClusterIndex++; > + } while (CHECK_CLUSTER (Cluster)); > + > + return EFI_NOT_FOUND; /* cannot identify the cluster */ > +} > + > +STATIC > +UINTN > +CpuMaskNext ( > + IN UINTN Cpu, > + IN UINTN Mask > + ) > +{ > + for (Cpu++; !((1 << Cpu) & Mask); Cpu++); > + > + return Cpu; > +} > + > +/* > + * Print CPU information > + */ > +VOID > +PrintCpuInfo ( > + VOID > + ) > +{ > + SYS_INFO SysInfo; > + UINTN CoreIndex; > + UINTN Core; > + UINT32 Type; > + UINT32 NumCpus; > + UINT32 Mask; > + CHAR8 *CoreName; > + > + GetSysInfo (&SysInfo); > + DEBUG ((DEBUG_INIT, "Clock Configuration:")); > + > + NumCpus = CpuNumCores (); > + Mask = CpuMask (); > + > + for (CoreIndex = 0, Core = CpuMaskNext(-1, Mask); > + CoreIndex < NumCpus; > + CoreIndex++, Core = CpuMaskNext(Core, Mask)) > + { > + if (!(CoreIndex % 3)) { > + DEBUG ((DEBUG_INIT, "\n ")); > + } > + > + Type = TP_ITYP_VERSION (QoriqCoreToType (Core)); > + switch (Type) { > + case TY_ITYP_VERSION_A7: > + CoreName = "A7"; > + break; > + case TY_ITYP_VERSION_A53: > + CoreName = "A53"; > + break; > + case TY_ITYP_VERSION_A57: > + CoreName = "A57"; > + break; > + case TY_ITYP_VERSION_A72: > + CoreName = "A72"; > + break; > + default: > + CoreName = " Unknown Core "; > + } > + DEBUG ((DEBUG_INIT, "CPU%d(%a):%-4d MHz ", > + Core, CoreName, SysInfo.FreqProcessor[Core] / MHZ)); > + } > + > + DEBUG ((DEBUG_INIT, "\n Bus: %-4d MHz ", SysInfo.FreqSystemBus / MHZ)); > + DEBUG ((DEBUG_INIT, "DDR: %-4d MT/s", SysInfo.FreqDdrBus / MHZ)); > + > + if (SysInfo.FreqFman[0] != 0) { > + DEBUG ((DEBUG_INIT, "\n FMAN: %-4d MHz ", SysInfo.FreqFman[0] / MHZ)); > + } > + > + DEBUG ((DEBUG_INIT, "\n")); > +} > + > +/* > + * 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[20]; > + CCSR_GUR *GurBase; > + UINTN Count; > + // > + // Svr : System Version Register > + // > + UINTN Svr; > + UINTN Ver; > + > + GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr); > + > + Svr = GurRead ((UINTN)&GurBase->Svr); > + Ver = SVR_SOC_VER (Svr); > + > + for (Count = 0; Count < ARRAY_SIZE (mCpuTypeList); Count++) { > + if ((mCpuTypeList[Count].SocVer & SVR_WO_E) == Ver) { > + AsciiStrCpyS (Buf, AsciiStrnLenS (mCpuTypeList[Count].Name, 7) + 1, As I said in previous review - the second parameter should be sizeof (Buf). > + (CONST CHAR8 *)mCpuTypeList[Count].Name); > + > + if (IS_E_PROCESSOR (Svr)) { > + AsciiStrCatS (Buf, > + (AsciiStrLen (Buf) + AsciiStrLen ((CONST CHAR8 *)"E") + 1), And here too. > + (CONST CHAR8 *)"E"); And please drop all casts to CHAR8 * in this function (as requested in previous review) - they are not needed. > + } > + break; > + } > + } > + > + DEBUG ((DEBUG_INFO, "SoC: %a (0x%x); Rev %d.%d\n", > + Buf, Svr, SVR_MAJOR (Svr), SVR_MINOR (Svr))); > + > + return; > +}