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.
Thanks.
>
> /
> Leif
>
>> +}
>> +
>> --
>> 2.9.5
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
