From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: "Agyeman, Prince" <prince.agyeman@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Sinha, Ankit" <ankit.sinha@intel.com>,
"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-platforms] [PATCH v2] SimicsOpenBoardPkg: Add CmosAccessLib to BoardX58Ich10
Date: Tue, 5 Nov 2019 22:47:21 +0000 [thread overview]
Message-ID: <BY5PR11MB44843B0CB1D5BB605F39A3D4B57E0@BY5PR11MB4484.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20191101234335.42520-1-prince.agyeman@intel.com>
Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>
> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Friday, November 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Sinha, Ankit <ankit.sinha@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@intel.com>
> Subject: [edk2-platforms] [PATCH v2] 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 <ankit.sinha@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Kubacki Michael A <michael.a.kubacki@intel.com>
>
> Signed-off-by: Prince Agyeman <prince.agyeman@intel.com>
> ---
> .../BoardInitLib/PeiX58Ich10InitPreMemLib.c | 46 ---------------
> .../BoardX58Ich10/OpenBoardPkg.dsc | 2 +
> .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.c | 57 -------------------
> .../Intel/SimicsOpenBoardPkg/SimicsPei/Cmos.h | 50 ----------------
> .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c | 19 +------
> .../SimicsOpenBoardPkg/SimicsPei/Platform.c | 4 +-
> .../SimicsPei/SimicsPei.inf | 3 +-
> .../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 38 +------------
> .../SmbiosPlatformDxe/SmbiosPlatformDxe.h | 1 +
> .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 2 +
> 10 files changed, 14 insertions(+), 208 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/Library/BoardInitLib/
> PeiX58Ich10InitPreMemLib.c
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/Library/BoardInitLib/
> PeiX58Ich10InitPreMemLib.c
> index c3a31ed426..325a341cdf 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/Library/BoardInitLib/
> PeiX58Ich10InitPreMemLib.c
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/Library/BoardInitL
> +++ ib/PeiX58Ich10InitPreMemLib.c
> @@ -20,52 +20,6 @@
>
> #include "PeiX58Ich10InitLib.h"
> #include <Register/X58Ich10.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;
> -}
> -
>
> EFI_STATUS
> EFIAPI
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> index 78f1e80990..9b9e088cbe 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> @@ -116,6 +116,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..0000000000
> --- 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. <BR>
> -
> - 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..0000000000
> --- 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. <BR>
> -
> - 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..e547de0045 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -22,11 +22,11 @@
> #include <Library/PeimEntryPoint.h>
> #include <Library/ResourcePublicationLib.h>
> #include <Library/MtrrLib.h>
> +#include <Library/CmosAccessLib.h>
> #include <SimicsPlatforms.h>
> #include <Guid/SmramMemoryReserve.h>
>
> #include "Platform.h"
> -#include "Cmos.h"
>
> UINT8 mPhysMemAddressWidth;
>
> @@ -80,9 +80,6 @@ GetSystemMemorySizeBelow4gb (
> VOID
> )
> {
> - UINT8 Cmos0x34;
> - UINT8 Cmos0x35;
> -
> //
> // CMOS 0x34/0x35 specifies the system memory above 16 MB.
> // * CMOS(0x35) is the high byte
> @@ -91,11 +88,7 @@ GetSystemMemorySizeBelow4gb (
> // * Since this is memory above 16MB, the 16MB must be added
> // into the calculation to get the total memory size.
> //
> -
> - Cmos0x34 = (UINT8) CmosRead8 (0x34);
> - Cmos0x35 = (UINT8) CmosRead8 (0x35);
> -
> - return (UINT32) (((UINTN)((Cmos0x35 << 8) + Cmos0x34) << 16) +
> SIZE_16MB);
> + return (UINT32) (((UINTN)CmosRead16 (0x34) << 16) + SIZE_16MB);
> }
>
>
> @@ -105,8 +98,6 @@ GetSystemMemorySizeAbove4gb (
> )
> {
> UINT32 Size;
> - UINTN CmosIndex;
> -
> //
> // CMOS 0x5b-0x5d specifies the system memory above 4GB MB.
> // * CMOS(0x5d) is the most significant size byte @@ -114,11 +105,7 @@
> GetSystemMemorySizeAbove4gb (
> // * CMOS(0x5b) is the least significant size byte
> // * The size is specified in 64kb chunks
> //
> -
> - Size = 0;
> - for (CmosIndex = 0x5d; CmosIndex >= 0x5b; CmosIndex--) {
> - Size = (UINT32) (Size << 8) + (UINT32) CmosRead8 (CmosIndex);
> - }
> + Size = (CmosRead16 (0x5c) << 8) + CmosRead8 (0x5b);
>
> return LShiftU64 (Size, 16);
> }
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
> index 7568d25c4e..0bec76e496 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/Platform.c
> @@ -22,13 +22,13 @@
> #include <Library/PeimEntryPoint.h>
> #include <Library/PeiServicesLib.h>
> #include <Library/ResourcePublicationLib.h>
> +#include <Library/CmosAccessLib.h>
> #include <Guid/MemoryTypeInformation.h> #include
> <Ppi/MasterBootMode.h> #include <IndustryStandard/Pci22.h> #include
> <SimicsPlatforms.h>
>
> #include "Platform.h"
> -#include "Cmos.h"
>
> EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {
> { EfiACPIMemoryNVS, 0x004 },
> @@ -524,7 +524,7 @@ DebugDumpCmos (
> VOID
> )
> {
> - UINT32 Loop;
> + UINT8 Loop;
>
> DEBUG ((EFI_D_INFO, "CMOS:\n"));
>
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> index 9499d2aad5..710fa680be 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> @@ -23,7 +23,6 @@
> #
>
> [Sources]
> - Cmos.c
> FeatureControl.c
> MemDetect.c
> Platform.c
> @@ -36,6 +35,7 @@
> MinPlatformPkg/MinPlatformPkg.dec
> SimicsX58SktPkg/SktPkg.dec
> SimicsIch10Pkg/Ich10Pkg.dec
> + BoardModulePkg/BoardModulePkg.dec
>
> [Guids]
> gEfiMemoryTypeInformationGuid
> @@ -53,6 +53,7 @@
> PeimEntryPoint
> MtrrLib
> PcdLib
> + CmosAccessLib
>
> [Pcd]
> gSimicsOpenBoardPkgTokenSpaceGuid.PcdSimicsPeiMemFvBase
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.c
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.c
> index 7165c0a0c3..37c659e275 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.c
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> +++ Dxe.c
> @@ -10,35 +10,11 @@
> #include "SmbiosPlatformDxe.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);
> -}
> -
> UINT32
> GetSystemMemorySizeBelow4gb(
> VOID
> )
> {
> - UINT8 Cmos0x34;
> - UINT8 Cmos0x35;
> -
> //
> // CMOS 0x34/0x35 specifies the system memory above 16 MB.
> // * CMOS(0x35) is the high byte
> @@ -47,11 +23,7 @@ GetSystemMemorySizeBelow4gb(
> // * Since this is memory above 16MB, the 16MB must be added
> // into the calculation to get the total memory size.
> //
> -
> - Cmos0x34 = (UINT8)CmosRead8(0x34);
> - Cmos0x35 = (UINT8)CmosRead8(0x35);
> -
> - return (UINT32)(((UINTN)((Cmos0x35 << 8) + Cmos0x34) << 16) +
> SIZE_16MB);
> + return (UINT32) (((UINTN) CmosRead16 (0x34) << 16) + SIZE_16MB);
> }
>
> STATIC
> @@ -61,8 +33,6 @@ GetSystemMemorySizeAbove4gb(
> )
> {
> UINT32 Size;
> - UINTN CmosIndex;
> -
> //
> // CMOS 0x5b-0x5d specifies the system memory above 4GB MB.
> // * CMOS(0x5d) is the most significant size byte @@ -70,11 +40,7 @@
> GetSystemMemorySizeAbove4gb(
> // * CMOS(0x5b) is the least significant size byte
> // * The size is specified in 64kb chunks
> //
> -
> - Size = 0;
> - for (CmosIndex = 0x5d; CmosIndex >= 0x5b; CmosIndex--) {
> - Size = (UINT32)(Size << 8) + (UINT32)CmosRead8(CmosIndex);
> - }
> + Size = (CmosRead16 (0x5c) << 8) + CmosRead8 (0x5b);
>
> return LShiftU64(Size, 16);
> }
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.h
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.h
> index f9c641845c..0dc174421c 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.h
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> +++ Dxe.h
> @@ -20,6 +20,7 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/MemoryAllocationLib.h> #include <Library/IoLib.h>
> +#include <Library/CmosAccessLib.h>
>
> /**
> Validates the SMBIOS entry point structure diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.inf
> index 1420a315cf..3cc6a03564 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> Dxe.inf
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/SmbiosPlatformDxe/SmbiosPlatform
> +++ Dxe.inf
> @@ -32,6 +32,7 @@
> MdeModulePkg/MdeModulePkg.dec
> SimicsOpenBoardPkg/OpenBoardPkg.dec
> AdvancedFeaturePkg/AdvancedFeaturePkg.dec
> + BoardModulePkg/BoardModulePkg.dec
>
> [LibraryClasses]
> UefiBootServicesTableLib
> @@ -42,6 +43,7 @@
> HobLib
> MemoryAllocationLib
> IoLib
> + CmosAccessLib
>
> [Protocols]
> gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
> --
> 2.19.1.windows.1
next prev parent reply other threads:[~2019-11-05 22:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-01 23:43 [edk2-platforms] [PATCH v2] SimicsOpenBoardPkg: Add CmosAccessLib to BoardX58Ich10 Agyeman, Prince
2019-11-05 22:47 ` Kubacki, Michael A [this message]
2019-11-06 1:31 ` Nate DeSimone
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BY5PR11MB44843B0CB1D5BB605F39A3D4B57E0@BY5PR11MB4484.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox