On Fri, Feb 16, 2018 at 05:35:26PM +0100, Marcin Wojtas wrote: > From: Igal Liberman <[email protected]> > > 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 <[email protected]> > Signed-off-by: Marcin Wojtas <[email protected]> > --- > 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 <Uefi.h> > + > +#include <Library/DebugLib.h> > +#include <Library/IoLib.h> > +#include <Library/MvSARLib.h> > + > +#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 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

