[edk2,edk2-platforms,v1,06/16] Hisilicon/D06: Add OemGetCpuFreq to encapsulate difference

Message ID 20190201133436.10500-7-ming.huang@linaro.org
State New
Headers show
Series
  • Fix issues and improve D0x
Related show

Commit Message

Ming Huang Feb. 1, 2019, 1:34 p.m.
From: xingjiang tang <tangxingjiang@huawei.com>


Implementation OemGetCpuFreq() to get cpu frequency from cpld to
encapsulate project difference, for some projects don't support
get cpu frequency by this way.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>

---
 Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++
 Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++
 Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++
 3 files changed, 22 insertions(+)

-- 
2.9.5

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 11, 2019, 5:15 p.m. | #1
On Fri, Feb 01, 2019 at 09:34:26PM +0800, Ming Huang wrote:
> From: xingjiang tang <tangxingjiang@huawei.com>

> 

> Implementation OemGetCpuFreq() to get cpu frequency from cpld to

> encapsulate project difference, for some projects don't support

> get cpu frequency by this way.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> ---

>  Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++

>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++

>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++

>  3 files changed, 22 insertions(+)

> 

> diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h

> index ec9b49f4e70d..4d07a8ab3741 100644

> --- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h

> +++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h

> @@ -36,4 +36,8 @@

>  #define CPLD_X8_X8_X8_BOARD_ID            0x92

>  #define CPLD_X16_X8_BOARD_ID              0x93

>  

> +#define CPLD_CLOCK_FLAG      0xFD

> +#define CPLD_BOM_VER_FLAG    0x0B

> +#define BRD_VER_4TH          0x4


What is BRD_VER_4TH? Please write out full words.
Also, this macro needs a CPLD_ prefix.

> +

>  #endif /* __CPLDD06_H__ */

> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h

> index 86ea6a1b3deb..dfac87d635d9 100644

> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h

> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h

> @@ -53,4 +53,6 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);

>  

>  extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];

>  EFI_HII_HANDLE EFIAPI OemGetPackages ();

> +UINTN OemGetCpuFreq (UINT8 Socket);

> +

>  #endif

> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

> index 2a9db46d1ff9..8f2ac308c7b9 100644

> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

> @@ -207,3 +207,19 @@ OemIsNeedDisableExpanderBuffer (

>  {

>    return TRUE;

>  }

> +

> +UINTN OemGetCpuFreq (UINT8 Socket)

> +{

> +  UINT8 BrdVerData;


Write out full words.

> +

> +  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);


Space before (.

> +

> +  if (BrdVerData >= BRD_VER_4TH){  //2.5G


What is the comment saying? The number below?
The number below is also saying the number below.
A useful comment would be
"// Board revision 4 and higher run at 2.5GHz
 // Earlier revisions run at 2GHz"

At that point you don't even need the #define.
And not really the temporary variable either.

> +    return 2500000000;

> +  }

> +  else

> +  {


} else {

/
    Leif

> +     return 2000000000;

> +  }

> +}

> +

> -- 

> 2.9.5

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 13, 2019, 2:29 a.m. | #2
On 2/12/2019 1:15 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:26PM +0800, Ming Huang wrote:

>> From: xingjiang tang <tangxingjiang@huawei.com>

>>

>> Implementation OemGetCpuFreq() to get cpu frequency from cpld to

>> encapsulate project difference, for some projects don't support

>> get cpu frequency by this way.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>> ---

>>  Platform/Hisilicon/D06/Include/Library/CpldD06.h             |  4 ++++

>>  Silicon/Hisilicon/Include/Library/OemMiscLib.h               |  2 ++

>>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c | 16 ++++++++++++++++

>>  3 files changed, 22 insertions(+)

>>

>> diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h

>> index ec9b49f4e70d..4d07a8ab3741 100644

>> --- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h

>> +++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h

>> @@ -36,4 +36,8 @@

>>  #define CPLD_X8_X8_X8_BOARD_ID            0x92

>>  #define CPLD_X16_X8_BOARD_ID              0x93

>>  

>> +#define CPLD_CLOCK_FLAG      0xFD

>> +#define CPLD_BOM_VER_FLAG    0x0B

>> +#define BRD_VER_4TH          0x4

> 

> What is BRD_VER_4TH? Please write out full words.

> Also, this macro needs a CPLD_ prefix.


BRD_VER_4TH: BOARD_REVISION_4TH
Modify in v2.

> 

>> +

>>  #endif /* __CPLDD06_H__ */

>> diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h

>> index 86ea6a1b3deb..dfac87d635d9 100644

>> --- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h

>> +++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h

>> @@ -53,4 +53,6 @@ BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);

>>  

>>  extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];

>>  EFI_HII_HANDLE EFIAPI OemGetPackages ();

>> +UINTN OemGetCpuFreq (UINT8 Socket);

>> +

>>  #endif

>> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

>> index 2a9db46d1ff9..8f2ac308c7b9 100644

>> --- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

>> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c

>> @@ -207,3 +207,19 @@ OemIsNeedDisableExpanderBuffer (

>>  {

>>    return TRUE;

>>  }

>> +

>> +UINTN OemGetCpuFreq (UINT8 Socket)

>> +{

>> +  UINT8 BrdVerData;

> 

> Write out full words.

> 

>> +

>> +  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);

> 

> Space before (.

> 

>> +

>> +  if (BrdVerData >= BRD_VER_4TH){  //2.5G

> 

> What is the comment saying? The number below?

> The number below is also saying the number below.

> A useful comment would be

> "// Board revision 4 and higher run at 2.5GHz

>  // Earlier revisions run at 2GHz"

> 

> At that point you don't even need the #define.

> And not really the temporary variable either.

> 

>> +    return 2500000000;

>> +  }

>> +  else

>> +  {

> 

> } else {


Modify in v2.

Thanks

> 

> /

>     Leif

> 

>> +     return 2000000000;

>> +  }

>> +}

>> +

>> -- 

>> 2.9.5

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Platform/Hisilicon/D06/Include/Library/CpldD06.h b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
index ec9b49f4e70d..4d07a8ab3741 100644
--- a/Platform/Hisilicon/D06/Include/Library/CpldD06.h
+++ b/Platform/Hisilicon/D06/Include/Library/CpldD06.h
@@ -36,4 +36,8 @@ 
 #define CPLD_X8_X8_X8_BOARD_ID            0x92
 #define CPLD_X16_X8_BOARD_ID              0x93
 
+#define CPLD_CLOCK_FLAG      0xFD
+#define CPLD_BOM_VER_FLAG    0x0B
+#define BRD_VER_4TH          0x4
+
 #endif /* __CPLDD06_H__ */
diff --git a/Silicon/Hisilicon/Include/Library/OemMiscLib.h b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
index 86ea6a1b3deb..dfac87d635d9 100644
--- a/Silicon/Hisilicon/Include/Library/OemMiscLib.h
+++ b/Silicon/Hisilicon/Include/Library/OemMiscLib.h
@@ -53,4 +53,6 @@  BOOLEAN OemIsNeedDisableExpanderBuffer(VOID);
 
 extern EFI_STRING_ID gDimmToDevLocator[MAX_SOCKET][MAX_CHANNEL][MAX_DIMM];
 EFI_HII_HANDLE EFIAPI OemGetPackages ();
+UINTN OemGetCpuFreq (UINT8 Socket);
+
 #endif
diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
index 2a9db46d1ff9..8f2ac308c7b9 100644
--- a/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
+++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c
@@ -207,3 +207,19 @@  OemIsNeedDisableExpanderBuffer (
 {
   return TRUE;
 }
+
+UINTN OemGetCpuFreq (UINT8 Socket)
+{
+  UINT8 BrdVerData;
+
+  BrdVerData = MmioRead8(CPLD_BASE_ADDRESS + CPLD_BOM_VER_FLAG);
+
+  if (BrdVerData >= BRD_VER_4TH){  //2.5G
+    return 2500000000;
+  }
+  else
+  {
+     return 2000000000;
+  }
+}
+