From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (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 44C9A21FD73E7 for ; Sun, 18 Feb 2018 04:20:19 -0800 (PST) Received: by mail-wm0-x241.google.com with SMTP id v10so10388011wmh.5 for ; Sun, 18 Feb 2018 04:26:15 -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=n7B1e6GSyTB6iTJqf2cRsHBHh0VgdU/m1zicluj8W+8=; b=g3jeFzlxqz6qe7fa2GzeDYatC5MvK5pQ5NKEfjxa08BEgNQ+oUj8ebCmQVa5LTlv4a JNfyilGVY4QOuCGqFWCt44+/8kyVYww0Xl2lTzWn2ry/c43L2EI7tIJqBFDajnPWq405 gAV0C3eoIGv4WyKZvIJ8pq1/DbIc3aRpVOEWA= 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=n7B1e6GSyTB6iTJqf2cRsHBHh0VgdU/m1zicluj8W+8=; b=q+TiCMh1aPdJoTxIHXnaaNd6jBbzAKB3mMEDb1smnmONNvthGf+l6HMUuWo9I5cqE1 CLAGZoychiXT9fiXxa2oC/oH+A8Kb/rDbAfnxs9Og5lKMV4wnqLoUlEDAVz6iEYHrcad aXTcwq+DeZjGjB2QyjmMHTVb+nDzpmchPh65qnsH+q40xG+iQrGa1cm+H7v7ypS+pXqK KNTX8wdk8zOHdaws/dp9DozofDGw5OXHd0hrG2YOnaVEiyhpA9UiBTBZgSBskrK/CsN9 blJLbyZdsSnR2cLLm9nXdBvcCcA3Qohnbn6G6U1R+aoiHAAJ6Tr37YBRb6qCBl/8XTw5 dYdg== X-Gm-Message-State: APf1xPAhlTYQJlVonqT8KvaXhUeTscuhAV0nK4wOtsC2lBHpyZCS1PcE GAqMUWKuBNW5o0ShvA2yDHe3rA== X-Google-Smtp-Source: AH8x224qFYwj9A3PXbMA5Efq1bITumC3wNoIW3Ugxkgif7XxkvrNnThgUF4gOxICCLqOO1QV/gI0yA== X-Received: by 10.28.91.17 with SMTP id p17mr8877542wmb.151.1518956773026; Sun, 18 Feb 2018 04:26:13 -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 11sm24338522wmp.36.2018.02.18.04.26.10 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 18 Feb 2018 04:26:11 -0800 (PST) Date: Sun, 18 Feb 2018 12:26:09 +0000 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, nadavh@marvell.com, neta@marvell.com, jinghua@marvell.com, xswang@marvell.com, igall@marvell.com, jsd@semihalf.com Message-ID: <20180218122609.xwxdd2h5dqqmd4g5@bivouac.eciton.net> References: <1518798927-8248-1-git-send-email-mw@semihalf.com> <1518798927-8248-2-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1518798927-8248-2-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 1/2] Marvell/Armada7k8k: Add basic sample at reset library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Feb 2018 12:20:20 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Feb 16, 2018 at 05:35:26PM +0100, Marcin Wojtas wrote: > From: Igal Liberman > > The sample at reset library adds the following functionalities: > - MvSARGetCpuFreq - Get the CPU frequency > - MvSARGetDramFreq - Get the DRAM frequency > - MvSARGetPcieClkDirection - Determine the PCIe clock direction > for two types specified in CP110 HW block. It will be needed > for proper configuration during the PCIE SerDes training. Please add an explanation of what a SAR is. Since it appears to be a specific thing referred to under that name, it is correct to use the abbreviation throughout, but it needs to be introduced. Similarly, from coding standards, section 4.1.3.2 --- Any abbreviation used, which is not documented in this specification, or in the UEFI Specification shall be placed into a Glossary section of the File header as specified in See "File Heading". --- (search for "File Heading" in https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/52_spacing.html) I would appreciate if you could include this in .c and .h files of this patch. > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Igal Liberman > Signed-off-by: Marcin Wojtas > --- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.c | 166 ++++++++++++++++++++ > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.inf | 54 +++++++ > Silicon/Marvell/Include/Library/MvSARLib.h | 57 +++++++ > Silicon/Marvell/Marvell.dec | 3 + > 4 files changed, 280 insertions(+) > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.c > create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.inf > create mode 100644 Silicon/Marvell/Include/Library/MvSARLib.h > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.c > new file mode 100644 > index 0000000..27ada6f > --- /dev/null > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.c > @@ -0,0 +1,166 @@ > +/******************************************************************************** > +Copyright (C) 2018 Marvell International Ltd. > + > +Marvell BSD License Option > + > +If you received this File from Marvell, you may opt to use, redistribute and/or > +modify this File under the following licensing terms. > +Redistribution and use in source and binary forms, with or without modification, > +are permitted provided that the following conditions are met: > + > +* Redistributions of source code must Retain the above copyright notice, > + this list of conditions and the following disclaimer. > + > +* Redistributions in binary form must reproduce the above copyright > + notice, this list of conditions and the following disclaimer in the > + documentation and/or other materials provided with the distribution. > + > +* Neither the name of Marvell nor the names of its contributors may be > + used to endorse or promote products derived from this software without > + specific prior written permission. > + > +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND > +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE > +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR > +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + > +*******************************************************************************/ > + > +#include > + > +#include > +#include > +#include > + > +#define SAR_MAX_OPTIONS 16 > + > +#define CPU_CLOCK_ID 0 > +#define DDR_CLOCK_ID 1 > +#define RING_CLOCK_ID 2 > + > +#define MV_AP_SAR_BASE 0xf06f8200 AP? > +#define SAR_CLOCK_FREQ_MODE_MASK 0x1f > + > +#define MV_CP_SAR_BASE(_CpIndex) (0xf2000000 + (0x2000000 * _CpIndex) + 0x400200) CP? Missing parantheses in macro. If CP and AP are also common terms used in documentation for this component, please add it to the glossary (but not commit message). > + > +#define MAX_CP_COUNT 2 > +#define MAX_PCIE_CLK_TYPE_COUNT 2 > + > +#define CP0_PCIE0_CLK_MASK 0x4 > +#define CP0_PCIE1_CLK_MASK 0x8 > +#define CP1_PCIE0_CLK_MASK 0x1 > +#define CP1_PCIE1_CLK_MASK 0x2 > +#define CP0_PCIE0_CLK_OFFSET 2 > +#define CP0_PCIE1_CLK_OFFSET 3 > +#define CP1_PCIE0_CLK_OFFSET 0 > +#define CP1_PCIE1_CLK_OFFSET 1 Are these not just derivatives of each other? I.e. CP0_PCIE0_CLK_MASK is the same as (1 << CP0_PCIE0_CLK_OFFSET)? If so, can they be defined as such? > + > +typedef enum { > + CPU_2000_DDR_1200_RCLK_1200 = 0x0, > + CPU_2000_DDR_1050_RCLK_1050 = 0x1, > + CPU_1600_DDR_800_RCLK_800 = 0x4, > + CPU_1800_DDR_1200_RCLK_1200 = 0x6, > + CPU_1800_DDR_1050_RCLK_1050 = 0x7, > + CPU_1600_DDR_1050_RCLK_1050 = 0x0d, > + CPU_1000_DDR_650_RCLK_650 = 0x13, > + CPU_1300_DDR_800_RCLK_800 = 0x14, > + CPU_1300_DDR_650_RCLK_650 = 0x17, > + CPU_1200_DDR_800_RCLK_800 = 0x19, > + CPU_1400_DDR_800_RCLK_800 = 0x1a, > + CPU_600_DDR_800_RCLK_800 = 0x1b, > + CPU_800_DDR_800_RCLK_800 = 0x1c, > + CPU_1000_DDR_800_RCLK_800 = 0x1d, > +} ClockingOptions; TianoCore coding style suggests name should be CLOCKING_OPTIONS. And that Also, I would prefer to see all of the above definitions broken out into a local include. > + > +static const UINT32 PllFreqTbl[SAR_MAX_OPTIONS][4] = { STATIC CONST (throughout). Ideally, this table would be named PllFrequencyTable. But really, this needs be an array of struct objects, rather than using live-coded offsets in the code. > + /* CPU DDR Ring [MHz] */ > + {2000, 1200, 1200, CPU_2000_DDR_1200_RCLK_1200}, > + {2000, 1050, 1050, CPU_2000_DDR_1050_RCLK_1050}, > + {1800, 1200, 1200, CPU_1800_DDR_1200_RCLK_1200}, > + {1800, 1050, 1050, CPU_1800_DDR_1050_RCLK_1050}, > + {1600, 1050, 1050, CPU_1600_DDR_1050_RCLK_1050}, > + {1300, 800 , 800 , CPU_1300_DDR_800_RCLK_800}, > + {1300, 650 , 650 , CPU_1300_DDR_650_RCLK_650}, > + {1600, 800 , 800 , CPU_1600_DDR_800_RCLK_800}, > + {1000, 650 , 650 , CPU_1000_DDR_650_RCLK_650}, > + {1200, 800 , 800 , CPU_1200_DDR_800_RCLK_800}, > + {1400, 800 , 800 , CPU_1400_DDR_800_RCLK_800}, > + {600 , 800 , 800 , CPU_600_DDR_800_RCLK_800}, > + {800 , 800 , 800 , CPU_800_DDR_800_RCLK_800}, > + {1000, 800 , 800 , CPU_1000_DDR_800_RCLK_800} > +}; > + > +static const UINT32 PcieClkMask[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT] = { PcieClockMask. > + {CP0_PCIE0_CLK_MASK, CP0_PCIE1_CLK_MASK}, > + {CP1_PCIE0_CLK_MASK, CP1_PCIE1_CLK_MASK} > +}; > + > +static const UINT32 PcieClkOffset[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT] = { PcieClockOffset > + {CP0_PCIE0_CLK_OFFSET, CP0_PCIE1_CLK_OFFSET}, > + {CP1_PCIE0_CLK_OFFSET, CP1_PCIE1_CLK_OFFSET} > +}; > + > +UINT32 > +EFIAPI > +MvSARGetCpuFreq ( Frequency > + VOID > + ) > +{ > + UINT32 ClkVal; ClockValue > + UINT32 Index; > + > + ClkVal = MmioAnd32 (MV_AP_SAR_BASE, SAR_CLOCK_FREQ_MODE_MASK); > + > + for (Index = 0; Index < SAR_MAX_OPTIONS; Index++) { > + if (PllFreqTbl[Index][3] == ClkVal) { > + break; > + } > + } > + > + return PllFreqTbl[Index][CPU_CLOCK_ID]; > +} > + > +UINT32 > +EFIAPI > +MvSARGetDramFreq ( Frequency > + VOID > + ) > +{ > + UINT32 ClkVal; > + UINT32 Index; > + > + ClkVal = MmioAnd32 (MV_AP_SAR_BASE, SAR_CLOCK_FREQ_MODE_MASK); > + > + for (Index = 0; Index < SAR_MAX_OPTIONS; Index++) { > + if (PllFreqTbl[Index][3] == ClkVal) { > + break; > + } > + } > + > + return PllFreqTbl[Index][DDR_CLOCK_ID]; > +} > + > +UINT32 > +EFIAPI > +MvSARGetPcieClkDirection ( Clock > + IN UINT32 CpIndex, > + IN UINT32 PcieIndex > + ) > +{ > + UINT32 ClkDir; ClockDirection. / Leif > + > + ASSERT (CpIndex < MAX_CP_COUNT); > + ASSERT (PcieIndex < MAX_PCIE_CLK_TYPE_COUNT); > + > + ClkDir = MmioAnd32 (MV_CP_SAR_BASE (CpIndex), > + PcieClkMask[CpIndex][PcieIndex] >> > + PcieClkOffset[CpIndex][PcieIndex]); > + > + return ClkDir; > +} > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.inf > new file mode 100644 > index 0000000..32b3fec > --- /dev/null > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSARLib/Armada7k8kSARLib.inf > @@ -0,0 +1,54 @@ > +# Copyright (C) 2018 Marvell International Ltd. > +# > +# Marvell BSD License Option > +# > +# If you received this File from Marvell, you may opt to use, redistribute and/or > +# modify this File under the following licensing terms. > +# Redistribution and use in source and binary forms, with or without modification, > +# are permitted provided that the following conditions are met: > +# > +# * Redistributions of source code must retain the above copyright notice, > +# this list of conditions and the following disclaimer. > +# > +# * Redistributions in binary form must reproduce the above copyright > +# notice, this list of conditions and the following disclaimer in the > +# documentation and/or other materials provided with the distribution. > +# > +# * Neither the name of Marvell nor the names of its contributors may be > +# used to endorse or promote products derived from this software without > +# specific prior written permission. > +# > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE > +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR > +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > +# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > +# > + > +[Defines] > + INF_VERSION = 0x00010019 > + BASE_NAME = Armada7k8kSARLib > + FILE_GUID = 03e022c7-9bd7-4608-aa21-379deaac2430 > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = MvSARLib > + > +[Sources] > + Armada7k8kSARLib.c > + > +[Packages] > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Silicon/Marvell/Marvell.dec > + > +[LibraryClasses] > + DebugLib > + IoLib > + > +[Depex] > + TRUE > diff --git a/Silicon/Marvell/Include/Library/MvSARLib.h b/Silicon/Marvell/Include/Library/MvSARLib.h > new file mode 100644 > index 0000000..1985a84 > --- /dev/null > +++ b/Silicon/Marvell/Include/Library/MvSARLib.h > @@ -0,0 +1,57 @@ > +/******************************************************************************** > +Copyright (C) 2018 Marvell International Ltd. > + > +Marvell BSD License Option > + > +If you received this File from Marvell, you may opt to use, redistribute and/or > +modify this File under the following licensing terms. > +Redistribution and use in source and binary forms, with or without modification, > +are permitted provided that the following conditions are met: > + > +* Redistributions of source code must Retain the above copyright notice, > + this list of conditions and the following disclaimer. > + > +* Redistributions in binary form must reproduce the above copyright > + notice, this list of conditions and the following disclaimer in the > + documentation and/or other materials provided with the distribution. > + > +* Neither the name of Marvell nor the names of its contributors may be > + used to endorse or promote products derived from this software without > + specific prior written permission. > + > +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND > +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE > +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR > +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + > +*******************************************************************************/ > + > +#ifndef __MV_SAR_LIB_H__ > +#define __MV_SAR_LIB_H__ > + > +UINT32 > +EFIAPI > +MvSARGetCpuFreq ( > + VOID > + ); > + > +UINT32 > +EFIAPI > +MvSARGetDramFreq ( > + VOID > + ); > + > +UINT32 > +EFIAPI > +MvSARGetPcieClkDirection ( > + IN UINT32 CpIndex, > + IN UINT32 PcieIndex > + ); > + > +#endif /* __MV_SAR_LIB_H__ */ > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > index 2eb6238..b188882 100644 > --- a/Silicon/Marvell/Marvell.dec > +++ b/Silicon/Marvell/Marvell.dec > @@ -59,6 +59,9 @@ > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > +[LibraryClasses] > + MvSARLib|Include/Library/MvSARLib.h > + > [Protocols] > # installed as a protocol by PlatInitDxe to force ordering between DXE drivers > # that depend on the lowlevel platform initialization having been completed > -- > 2.7.4 >