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.huang@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