On Tue, Feb 12, 2019 at 10:45:05PM +0800, Ming Huang wrote: > > > On 2/12/2019 2:36 AM, Leif Lindholm wrote: > > On Fri, Feb 01, 2019 at 09:34:28PM +0800, Ming Huang wrote: > >> Follow chip team suggestion to change HCCS(Huawei Cache-Coherent > >> System) speed from 30G to 26G, this modification can avoid some > >> unstable stress issue. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ming Huang <ming.hu...@linaro.org> > >> --- > >> Silicon/Hisilicon/Include/Library/OemMiscLib.h | 10 > >> ++++++++++ > >> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 8 ++++++++ > >> 2 files changed, 18 insertions(+) > >> > >> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h > >> b/Silicon/Hisilicon/Include/Library/OemMiscLib.h > >> index dfac87d635d9..3c0cd0319122 100644 > >> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h > >> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h > >> @@ -22,6 +22,11 @@ > >> #include <PlatformArch.h> > >> #include <Library/I2CLib.h> > >> > >> +#define HCCS_PLL_VALUE_3000 0x52240781 > >> +#define HCCS_PLL_VALUE_2600 0x52240681 > >> +#define HCCS_PLL_VALUE_2800 0x52240701 > > > > Could these be described by a proper macro instead of just values? > > A cursory glance suggests that an increase of 0x80 in the lower half > > means 200MHz. > > > > If not, please sort them by frequency, ascending. > > As the macros have use in other files, I prefer sort them by frequency. > > > > >> + > >> + > >> #define PCIEDEVICE_REPORT_MAX 8 > >> #define MAX_PROCESSOR_SOCKETS MAX_SOCKET > >> #define MAX_MEMORY_CHANNELS MAX_CHANNEL > >> @@ -55,4 +60,9 @@ extern EFI_STRING_ID > >> gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM]; > >> EFI_HII_HANDLE EFIAPI OemGetPackages (); > >> UINTN OemGetCpuFreq (UINT8 Socket); > >> > >> +UINTN > >> +OemGetHccsFreq ( > >> + VOID > >> + ); > >> + > >> #endif > >> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c > >> b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c > >> index 8f2ac308c7b9..83e53cfeb5dd 100644 > >> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c > >> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c > >> @@ -223,3 +223,11 @@ UINTN OemGetCpuFreq (UINT8 Socket) > >> } > >> } > >> > >> +UINTN > >> +OemGetHccsFreq ( > > > > The commit message describes this patch as changing the frequency. > > The actual code simply returns a value. > > The name of the function returning this value suggests the value is a > > frequency> > >> + VOID > >> + ) > >> +{ > >> + return HCCS_PLL_VALUE_2600; > > > > But the constant returned is named suggesting a PLL configuration > > value. And the frequency suggested by the name is many orders of > > magnitude below that described by the commit message. > > Yes, the macros and function name are not very matched. > I plan to modify the commit title and message: > Hisilicon/D06: Use HCCS speed with 2.6G > > Follow chip team suggestion, HCCS(Huawei Cache-Coherent System) > may be unstable while speed is 3.0G, so use 2.6G to avoid some > unstable stress issue.
This all sounds good, thanks. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel