From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4864:20::143; helo=mail-it1-x143.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7EA62211B7F73 for ; Mon, 21 Jan 2019 07:47:43 -0800 (PST) Received: by mail-it1-x143.google.com with SMTP id a6so15763069itl.4 for ; Mon, 21 Jan 2019 07:47:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=qUOFmahW2zG7TfC72xUJcZx17mlPD3uUIVH3NB8Hkro=; b=KFRe2aSYdCtUCIEKSmPbyd4vAK6q6iUWtwJumq32fY7vWFyTKd7qjzQcj3L7NiwsP9 WmulPXSiFQ5BYB2aNkzCuwsbIF63LsdsepDg/fsoXEyVcU+vDcPn8M1HW0fydyRtKUZn BENJletKRxmkS4jRDni7V1eJD+HDKruptoIfd1pK9ciLbCQMagIEiYzideWp3Zgcft9L 8emsWXs+9S84ursbFO3tGPWiaJhDRjgxjhsLLUsXfxcevdLsOFM9DhDw/dtDiPjBnwM4 hm2p1UIPU0KVYAVEfUM9xwv6/5670c4t5A83q1vswpTrIJ5jDvHATH4ZWAyvH6z/nrOG skqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=qUOFmahW2zG7TfC72xUJcZx17mlPD3uUIVH3NB8Hkro=; b=kB6rhBn4IOqm6oWZB+cEDdhigz2G8NPP8ZTeSHKB8H9SZZnaU7gy6k1XKY2gOAIZJn qQgwQ/4U60XjuTOORXt3XPjnIoZQ4K2lfEGmEp8B+2NWMl4SyL/Bych1y0BRoxb8lLBw GICl9NARarLX3i8xvz/USLVVLuDZ5nBVU6XRohJ0Vb/LJX3xllthbVsx7KOT/Nb6RlmK 7WccpRhLBuWC6Tl8ZMvPPBvI5643dUwpzxMmim34piGYkc+ThE0wssmokqmJ8hPSFFqx fVrEegexqZfqpy5ILGhc8k/h2KqwUFk8aAa553fw+ZWz7zgZ3d0uXfNYFo9fS4LW2OXD e4Bw== X-Gm-Message-State: AJcUukdDXl+WFHjrn38RhRf4ZID7SQfaVgMpYXqx/BWZ1nL4A3PBvm4p AGCPa8ZZ5mjoM/sUPwIZ3eYaEtI6//bGOzAbxYOlyA== X-Google-Smtp-Source: ALg8bN65JCdANfMHT/Tn+/FmxnQsWrc6pCZnmFLo7caQxTdcxCrREfDwp5TYtWtoziSZMCu2btfgA269KN8Ulgeu3yA= X-Received: by 2002:a02:9d0:: with SMTP id 77mr17444435jam.14.1548085662744; Mon, 21 Jan 2019 07:47:42 -0800 (PST) MIME-Version: 1.0 References: <1548067931-18618-1-git-send-email-mw@semihalf.com> <1548067931-18618-4-git-send-email-mw@semihalf.com> <20190121115140.qqva7s3ldq62k35w@bivouac.eciton.net> In-Reply-To: <20190121115140.qqva7s3ldq62k35w@bivouac.eciton.net> From: Marcin Wojtas Date: Mon, 21 Jan 2019 16:47:31 +0100 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, "jsd@semihalf.com" , Grzegorz Jaszczyk , Kostya Porotchkin Subject: Re: [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jan 2019 15:47:43 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Leif, pon., 21 sty 2019 o 12:51 Leif Lindholm napisa= =C5=82(a): > > On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote: > > From: Grzegorz Jaszczyk > > > > The memory controller registers are marked as secure in the latest > > ARM-TF for Armada SoCs. It is available however get the DRAM > > information via SiP services in the EL3, so use it instead > > of accessing the registers directly. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | = 3 ++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | = 27 ++-------- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | = 55 +++++--------------- > > 3 files changed, 21 insertions(+), 64 deletions(-) > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8= kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.i= nf > > index e888566..0c7f320 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.in= f > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.in= f > > @@ -41,12 +41,15 @@ > > [Packages] > > ArmPkg/ArmPkg.dec > > ArmPlatformPkg/ArmPlatformPkg.dec > > + EmbeddedPkg/EmbeddedPkg.dec > > MdeModulePkg/MdeModulePkg.dec > > MdePkg/MdePkg.dec > > Silicon/Marvell/Marvell.dec > > > > [LibraryClasses] > > + ArmadaSoCDescLib > > ArmLib > > + ArmSmcLib > > DebugLib > > MemoryAllocationLib > > MppLib > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8= kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibM= em.h > > index cc30e4a..8a46df6 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem= .h > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem= .h > > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH= DAMAGE. > > #define DRAM_REMAP_TARGET \ > > (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) > > > > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) > > -#define DRAM_CS_VALID_ENABLED_MASK 0x1 > > -#define DRAM_AREA_LENGTH_OFFS 16 > > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS= ) > > -#define DRAM_START_ADDRESS_L_OFFS 23 > > -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L= _OFFS) > > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) > > -#define DRAM_START_ADDR_HTOL_OFFS 32 > > +/* Armada7k8k North Bridge index */ > > +#define ARMADA7K8K_AP806_INDEX 0 > > > > -#define DRAM_MAX_CS_NUM 8 > > - > > -#define DRAM_CS_ENABLED(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENA= BLED_MASK) > > -#define GET_DRAM_REGION_BASE(Cs) \ > > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ > > - DRAM_START_ADDR_HTOL_OFFS) | \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRES= S_L_MASK); > > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ > > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS > > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >=3D 7) && ((C) <=3D 26)= ) > > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <=3D 4) > > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C)) > > +/* Firmware related definition used for SMC calls */ > > +#define MV_SIP_DRAM_SIZE 0x82000010 > > Hmm... > This would end us up with having Marvell SMC calls spread across > multiple files. Could you insert an intermediate patch which breaks out > the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a > separate (MarvellSmc.h?) include file? How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially when adding new code. > > If you could further add _SMC_ID to the #defines (like in edk2 > ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very > happy. (I'd be happy for you to drop _SIP, or keep it either side of > the addition, as per your preference - we don't seem to have precedent > here yet.) > How about below: #define MV_SMC_ID_COMPHY_POWER_ON 0x82000001 #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002 #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003 ? Thanks, Marcin > Regards, > > Leif > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8= kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibM= em.c > > index 2a4f5ad..62e8467 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem= .c > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem= .c > > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUC= H DAMAGE. > > **********************************************************************= *********/ > > > > #include > > +#include > > #include > > +#include > > #include > > #include > > #include > > #include > > +#include > > > > #include "Armada7k8kLibMem.h" > > > > @@ -57,49 +60,19 @@ GetDramSize ( > > IN OUT UINT64 *MemSize > > ) > > { > > - UINT64 BaseAddr; > > - UINT8 RegionCode; > > - UINT8 Cs; > > - > > - *MemSize =3D 0; > > - > > - for (Cs =3D 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > > - > > - /* Exit loop on first disabled DRAM CS */ > > - if (!DRAM_CS_ENABLED (Cs)) { > > - break; > > - } > > - > > - /* > > - * Sanity check for base address of next DRAM block. > > - * Only continuous space will be used. > > - */ > > - BaseAddr =3D GET_DRAM_REGION_BASE (Cs); > > - if (BaseAddr !=3D *MemSize) { > > - DEBUG ((DEBUG_ERROR, > > - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", > > - __FUNCTION__, > > - *MemSize)); > > - return EFI_SUCCESS; > > - } > > - > > - /* Decode area length for current CS from register value */ > > - RegionCode =3D GET_DRAM_REGION_SIZE_CODE (Cs); > > - > > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > > - *MemSize +=3D GET_DRAM_REGION_SIZE_EVEN (RegionCode); > > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > > - *MemSize +=3D GET_DRAM_REGION_SIZE_ODD (RegionCode); > > - } else { > > - DEBUG ((DEBUG_ERROR, > > - "%a: Invalid memory region code (0x%x) for CS#%d\n", > > - __FUNCTION__, > > - RegionCode, > > - Cs)); > > - return EFI_INVALID_PARAMETER; > > - } > > + ARM_SMC_ARGS SmcRegs =3D {0}; > > + EFI_STATUS Status; > > + > > + SmcRegs.Arg0 =3D MV_SIP_DRAM_SIZE; > > + Status =3D ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_IN= DEX); > > + if (EFI_ERROR (Status)) { > > + return Status; > > } > > > > + ArmCallSmc (&SmcRegs); > > + > > + *MemSize =3D SmcRegs.Arg0; > > + > > return EFI_SUCCESS; > > } > > > > -- > > 2.7.4 > >