From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::342; helo=mail-wm1-x342.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) (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 8E7552194D3AE for ; Mon, 21 Jan 2019 08:05:32 -0800 (PST) Received: by mail-wm1-x342.google.com with SMTP id y8so11288765wmi.4 for ; Mon, 21 Jan 2019 08:05:32 -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=b0mX8axwxFjnp709Kc6fbFszxmNuapjV0xZQv2V9xyg=; b=hPzqK/+lqxByex89ZbKbwL8Xatn/75X3Lb6nt9ye4uoknw3rCg6XPi+s3w54ginoNy q0AD+bXTWlHThYm2YF0xExMMA4DNuIRm9bNe3XGWqjL+u2OHV3yDxvc5zhuZNk0Oe6JK CmtJjWB31SyhIE6IcBl8ZgSPk74IPR36zcsMs= 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=b0mX8axwxFjnp709Kc6fbFszxmNuapjV0xZQv2V9xyg=; b=Fo8NfN4W79Nq/1gi5qsV+IyCub8RBC9Hi5VM4upvsRvpAgqjciFgdjaTD2FG5h9gai lvFJAhY0iBpgDgddqmM/ZKNt/BUnHQ/UvioE/n4uBmLlmYdNMAN25gnNIbXJ0ETbq6zV OMCCgpEDL+3pVii2lTzeOh9gdbEnoFXLIzPorpppFGBeoDD7zNf6MUG/HtyxBUkrgp5v lm9yLIQj2rwmvshQ0U6g7OStmWdigeiq0SqIqHA5ybCHQqbYxYrdZvu2eDJoVciWmHj3 QaRfbRBOEAi3+HGAjwdzIGT2KQ67Gxw9kBkB4RAT9cAQSYd65ikWVesci3FlrUiUOmCE vrRQ== X-Gm-Message-State: AJcUukfr45KHhqI6fy6rVIE/QR9iSSiGkcz1Cq7zl/M+Dpe4DgE31AJw 1A0M+/lgGa54HkqKZRa/1z1KawBLRZk= X-Google-Smtp-Source: ALg8bN4j8V2k1D9waukRU0W4kQB2krvRhJGtDnF7fs2WV66IFOMVZ1JY9vlZBUylj5k1EKGrmjFVdQ== X-Received: by 2002:a1c:9a0d:: with SMTP id c13mr9588wme.41.1548086730949; Mon, 21 Jan 2019 08:05:30 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id s8sm107095561wrn.44.2019.01.21.08.05.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 21 Jan 2019 08:05:29 -0800 (PST) Date: Mon, 21 Jan 2019 16:05:28 +0000 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, "jsd@semihalf.com" , Grzegorz Jaszczyk , Kostya Porotchkin Message-ID: <20190121160528.lmlronj3segvudg4@bivouac.eciton.net> References: <1548067931-18618-1-git-send-email-mw@semihalf.com> <1548067931-18618-4-git-send-email-mw@semihalf.com> <20190121115140.qqva7s3ldq62k35w@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) 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 16:05:32 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jan 21, 2019 at 04:47:31PM +0100, Marcin Wojtas wrote: > > > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26)) > > > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) > > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 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. Sure. > > > > 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 > ? Yeah, that works. Regards, Leif > Thanks, > Marcin > > > > Regards, > > > > Leif > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.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 SUCH 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 = 0; > > > - > > > - for (Cs = 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 = GET_DRAM_REGION_BASE (Cs); > > > - if (BaseAddr != *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 = GET_DRAM_REGION_SIZE_CODE (Cs); > > > - > > > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > > > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); > > > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > > > - *MemSize += 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 = {0}; > > > + EFI_STATUS Status; > > > + > > > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; > > > + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX); > > > + if (EFI_ERROR (Status)) { > > > + return Status; > > > } > > > > > > + ArmCallSmc (&SmcRegs); > > > + > > > + *MemSize = SmcRegs.Arg0; > > > + > > > return EFI_SUCCESS; > > > } > > > > > > -- > > > 2.7.4 > > >