Re: [edk2-devel] [edk2-platforms] [PATCH] SimicsOpenBoardPkg: Add CmosAccessLib to BoardX58Ich10

2019-10-31 Thread Kubacki, Michael A
I still see custom implementation of CMOS functions and usage of those
functions in SimicsOpenBoardPkg. Examples:

1. 
Platform\Intel\SimicsOpenBoardPkg\BoardX58Ich10\Library\BoardInitLib\PeiX58Ich10InitPreMemLib.c:
   * CmosRead8 ()
   * CmosWrite8 ()
2. Platform\Intel\SimicsOpenBoardPkg\SimicsPei\MemDetect.c:
   * CmosRead8 ()
3. Platform\Intel\SimicsOpenBoardPkg\SimicsPei\Platform.c:
   * CmosRead8 ()
4. Platform\Intel\SimicsOpenBoardPkg\SmbiosPlatformDxe\SmbiosPlatformDxe.c:
  * CmosRead8 ()

Can you please check these and clean any other CMOS usage that is handled by 
the library?

Thanks,
Michael

> -Original Message-
> From: Agyeman, Prince 
> Sent: Tuesday, October 29, 2019 3:27 PM
> To: devel@edk2.groups.io
> Cc: Sinha, Ankit ; Desimone, Nathaniel L
> ; Kubacki, Michael A
> 
> Subject: [edk2-platforms] [PATCH] SimicsOpenBoardPkg: Add CmosAccessLib
> to BoardX58Ich10
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2164
> 
> Replaced Cmos.c and Cmos.h with BoardModulePkg's Cmos library
> CmosAccessLib
> 
> Cc: Ankit Sinha 
> Cc: Nate DeSimone 
> Cc: Kubacki Michael A 
> 
> Signed-off-by: Prince Agyeman 
> ---
>  .../BoardX58Ich10/OpenBoardPkg.dsc|  2 +
>  .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c | 57 ---
> .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h | 50 
> .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  |  4 +-
>  .../SimicsOpenBoardPkg/SimicsPei/Platform.c   |  4 +-
>  .../SimicsPei/SimicsPei.inf   |  3 +-
>  6 files changed, 8 insertions(+), 112 deletions(-)  delete mode 100644
> Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
>  delete mode 100644 Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> index 4f8ab4170d..67f1680a4f 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> @@ -115,6 +115,8 @@
> 
> SerializeVariablesLib|$(BOARD_PKG)/Library/SerializeVariablesLib/SerializeV
> ariablesLib.inf
> 
> SiliconPolicyInitLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyInitLib/SiliconP
> olicyInitLib.inf
> 
> SiliconPolicyUpdateLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyUpdateLib
> /SiliconPolicyUpdateLib.inf
> +
> +
> PlatformCmosAccessLib|BoardModulePkg/Library/PlatformCmosAccessLibN
> ull
> + /PlatformCmosAccessLibNull.inf
> +
> CmosAccessLib|BoardModulePkg/Library/CmosAccessLib/CmosAccessLib.inf
> 
>  [LibraryClasses.common.SEC]
>###
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
> deleted file mode 100644
> index b34ba9283b..00
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -/** @file
> -  PC/AT CMOS access routines
> -
> -  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
> -
> -#include "Cmos.h"
> -#include "Library/IoLib.h"
> -
> -/**
> -  Reads 8-bits of CMOS data.
> -
> -  Reads the 8-bits of CMOS data at the location specified by Index.
> -  The 8-bit read value is returned.
> -
> -  @param  Index  The CMOS location to read.
> -
> -  @return The value read.
> -
> -**/
> -UINT8
> -EFIAPI
> -CmosRead8 (
> -  IN  UINTN Index
> -  )
> -{
> -  IoWrite8 (0x70, (UINT8) Index);
> -  return IoRead8 (0x71);
> -}
> -
> -
> -/**
> -  Writes 8-bits of CMOS data.
> -
> -  Writes 8-bits of CMOS data to the location specified by Index
> -  with the value specified by Value and returns Value.
> -
> -  @param  Index  The CMOS location to write.
> -  @param  Value  The value to write to CMOS.
> -
> -  @return The value written to CMOS.
> -
> -**/
> -UINT8
> -EFIAPI
> -CmosWrite8 (
> -  IN  UINTN Index,
> -  IN  UINT8 Value
> -  )
> -{
> -  IoWrite8 (0x70, (UINT8) Index);
> -  IoWrite8 (0x71, Value);
> -  return Value;
> -}
> -
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
> deleted file mode 100644
> index 07fa2e2d11..00
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/** @file
> -  PC/AT CMOS access routines
> -
> -  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
> -
> -#ifndef __CMOS_H__
> -#define __CMOS_H__
> -
> -/**
> -  Reads 8-bits of CMOS data.
> -
> -  Reads the 8-bits of CMOS data at the location specified by Index.
> -  The 8-bit read value is returned.
> -
> -  @param  Index  The CMOS location to read.
> -
> -  @return The value read.
> -
> -**/
> -UINT8
> -EFIAPI
> -CmosRead8 (
> -  IN  UINTN Index
> -  );
> -

Re: [edk2-devel] [edk2-platforms] [PATCH] SimicsOpenBoardPkg: Add CmosAccessLib to BoardX58Ich10

2019-10-30 Thread Nate DeSimone
Reviewed-by: Nate DeSimone 

-Original Message-
From: Agyeman, Prince  
Sent: Tuesday, October 29, 2019 3:27 PM
To: devel@edk2.groups.io
Cc: Sinha, Ankit ; Desimone, Nathaniel L 
; Kubacki, Michael A 

Subject: [edk2-platforms] [PATCH] SimicsOpenBoardPkg: Add CmosAccessLib to 
BoardX58Ich10

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2164

Replaced Cmos.c and Cmos.h with BoardModulePkg's Cmos library CmosAccessLib

Cc: Ankit Sinha 
Cc: Nate DeSimone 
Cc: Kubacki Michael A 

Signed-off-by: Prince Agyeman 
---
 .../BoardX58Ich10/OpenBoardPkg.dsc|  2 +
 .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c | 57 ---  
.../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h | 50   
.../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  |  4 +-
 .../SimicsOpenBoardPkg/SimicsPei/Platform.c   |  4 +-
 .../SimicsPei/SimicsPei.inf   |  3 +-
 6 files changed, 8 insertions(+), 112 deletions(-)  delete mode 100644 
Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
 delete mode 100644 Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h

diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc 
b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
index 4f8ab4170d..67f1680a4f 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
@@ -115,6 +115,8 @@
   
SerializeVariablesLib|$(BOARD_PKG)/Library/SerializeVariablesLib/SerializeVariablesLib.inf
   
SiliconPolicyInitLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyInitLib/SiliconPolicyInitLib.inf
   
SiliconPolicyUpdateLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyUpdateLib/SiliconPolicyUpdateLib.inf
+
+ PlatformCmosAccessLib|BoardModulePkg/Library/PlatformCmosAccessLibNull
+ /PlatformCmosAccessLibNull.inf
+ CmosAccessLib|BoardModulePkg/Library/CmosAccessLib/CmosAccessLib.inf
 
 [LibraryClasses.common.SEC]
   ###
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
deleted file mode 100644
index b34ba9283b..00
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/** @file
-  PC/AT CMOS access routines
-
-  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include "Cmos.h"
-#include "Library/IoLib.h"
-
-/**
-  Reads 8-bits of CMOS data.
-
-  Reads the 8-bits of CMOS data at the location specified by Index.
-  The 8-bit read value is returned.
-
-  @param  Index  The CMOS location to read.
-
-  @return The value read.
-
-**/
-UINT8
-EFIAPI
-CmosRead8 (
-  IN  UINTN Index
-  )
-{
-  IoWrite8 (0x70, (UINT8) Index);
-  return IoRead8 (0x71);
-}
-
-
-/**
-  Writes 8-bits of CMOS data.
-
-  Writes 8-bits of CMOS data to the location specified by Index
-  with the value specified by Value and returns Value.
-
-  @param  Index  The CMOS location to write.
-  @param  Value  The value to write to CMOS.
-
-  @return The value written to CMOS.
-
-**/
-UINT8
-EFIAPI
-CmosWrite8 (
-  IN  UINTN Index,
-  IN  UINT8 Value
-  )
-{
-  IoWrite8 (0x70, (UINT8) Index);
-  IoWrite8 (0x71, Value);
-  return Value;
-}
-
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
deleted file mode 100644
index 07fa2e2d11..00
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/** @file
-  PC/AT CMOS access routines
-
-  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#ifndef __CMOS_H__
-#define __CMOS_H__
-
-/**
-  Reads 8-bits of CMOS data.
-
-  Reads the 8-bits of CMOS data at the location specified by Index.
-  The 8-bit read value is returned.
-
-  @param  Index  The CMOS location to read.
-
-  @return The value read.
-
-**/
-UINT8
-EFIAPI
-CmosRead8 (
-  IN  UINTN Index
-  );
-
-/**
-  Writes 8-bits of CMOS data.
-
-  Writes 8-bits of CMOS data to the location specified by Index
-  with the value specified by Value and returns Value.
-
-  @param  Index  The CMOS location to write.
-  @param  Value  The value to write to CMOS.
-
-  @return The value written to CMOS.
-
-**/
-UINT8
-EFIAPI
-CmosWrite8 (
-  IN  UINTN Index,
-  IN  UINT8 Value
-  );
-
-
-#endif
-
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index ee0eead5a8..4d16fd2fac 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -22,11 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "Platform.h"
-#include "Cmos.h"
 
 UINT8 

[edk2-devel] [edk2-platforms] [PATCH] SimicsOpenBoardPkg: Add CmosAccessLib to BoardX58Ich10

2019-10-29 Thread Agyeman, Prince
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2164

Replaced Cmos.c and Cmos.h with BoardModulePkg's
Cmos library CmosAccessLib

Cc: Ankit Sinha 
Cc: Nate DeSimone 
Cc: Kubacki Michael A 

Signed-off-by: Prince Agyeman 
---
 .../BoardX58Ich10/OpenBoardPkg.dsc|  2 +
 .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c | 57 ---
 .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h | 50 
 .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  |  4 +-
 .../SimicsOpenBoardPkg/SimicsPei/Platform.c   |  4 +-
 .../SimicsPei/SimicsPei.inf   |  3 +-
 6 files changed, 8 insertions(+), 112 deletions(-)
 delete mode 100644 Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
 delete mode 100644 Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h

diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc 
b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
index 4f8ab4170d..67f1680a4f 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
@@ -115,6 +115,8 @@
   
SerializeVariablesLib|$(BOARD_PKG)/Library/SerializeVariablesLib/SerializeVariablesLib.inf
   
SiliconPolicyInitLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyInitLib/SiliconPolicyInitLib.inf
   
SiliconPolicyUpdateLib|$(BOARD_PKG)/Policy/Library/SiliconPolicyUpdateLib/SiliconPolicyUpdateLib.inf
+  
PlatformCmosAccessLib|BoardModulePkg/Library/PlatformCmosAccessLibNull/PlatformCmosAccessLibNull.inf
+  CmosAccessLib|BoardModulePkg/Library/CmosAccessLib/CmosAccessLib.inf
 
 [LibraryClasses.common.SEC]
   ###
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
deleted file mode 100644
index b34ba9283b..00
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/** @file
-  PC/AT CMOS access routines
-
-  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include "Cmos.h"
-#include "Library/IoLib.h"
-
-/**
-  Reads 8-bits of CMOS data.
-
-  Reads the 8-bits of CMOS data at the location specified by Index.
-  The 8-bit read value is returned.
-
-  @param  Index  The CMOS location to read.
-
-  @return The value read.
-
-**/
-UINT8
-EFIAPI
-CmosRead8 (
-  IN  UINTN Index
-  )
-{
-  IoWrite8 (0x70, (UINT8) Index);
-  return IoRead8 (0x71);
-}
-
-
-/**
-  Writes 8-bits of CMOS data.
-
-  Writes 8-bits of CMOS data to the location specified by Index
-  with the value specified by Value and returns Value.
-
-  @param  Index  The CMOS location to write.
-  @param  Value  The value to write to CMOS.
-
-  @return The value written to CMOS.
-
-**/
-UINT8
-EFIAPI
-CmosWrite8 (
-  IN  UINTN Index,
-  IN  UINT8 Value
-  )
-{
-  IoWrite8 (0x70, (UINT8) Index);
-  IoWrite8 (0x71, Value);
-  return Value;
-}
-
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
deleted file mode 100644
index 07fa2e2d11..00
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/** @file
-  PC/AT CMOS access routines
-
-  Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved. 
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#ifndef __CMOS_H__
-#define __CMOS_H__
-
-/**
-  Reads 8-bits of CMOS data.
-
-  Reads the 8-bits of CMOS data at the location specified by Index.
-  The 8-bit read value is returned.
-
-  @param  Index  The CMOS location to read.
-
-  @return The value read.
-
-**/
-UINT8
-EFIAPI
-CmosRead8 (
-  IN  UINTN Index
-  );
-
-/**
-  Writes 8-bits of CMOS data.
-
-  Writes 8-bits of CMOS data to the location specified by Index
-  with the value specified by Value and returns Value.
-
-  @param  Index  The CMOS location to write.
-  @param  Value  The value to write to CMOS.
-
-  @return The value written to CMOS.
-
-**/
-UINT8
-EFIAPI
-CmosWrite8 (
-  IN  UINTN Index,
-  IN  UINT8 Value
-  );
-
-
-#endif
-
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c 
b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index ee0eead5a8..4d16fd2fac 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -22,11 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "Platform.h"
-#include "Cmos.h"
 
 UINT8 mPhysMemAddressWidth;
 
@@ -105,7 +105,7 @@ GetSystemMemorySizeAbove4gb (
   )
 {
   UINT32 Size;
-  UINTN  CmosIndex;
+  UINT8  CmosIndex;
 
   //
   // CMOS 0x5b-0x5d specifies the system memory above 4GB MB.
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c