[edk2-devel] [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove redundant ResetSystem() implementation

Kubacki, Michael A posted 2 patches 6 years, 2 months ago
[edk2-devel] [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove redundant ResetSystem() implementation
Posted by Kubacki, Michael A 6 years, 2 months ago
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2375

The following commits introduced a new implementation of
ResetSystem () in ResetLib instances in KabylakeSiliconPkg:
 * 2393cff0d4
 * 07fd95c9ca
 * 0ad2f057d8
 * a6e61d43c9

The PchResetLib instance PeiPchResetLib links against ResetLib and
already contained a definition of ResetSystem (). This resulted in
a build failure in VS2017 due to multiple definitions of a symbol
being provided to the linker. The failure did not reproduce in
VS2015 so it persisted until now.

This change removes the pre-existing implementation of
ResetSystem () in PchReset.c as it is now linked by the
implementation in the ResetLib instance.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c             | 55 --------------------
 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c |  5 ++
 2 files changed, 5 insertions(+), 55 deletions(-)

diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
index 20e5d36892..c3963164c1 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
@@ -18,61 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/ResetSystemLib.h>
 #include <PchResetPlatformSpecific.h>
 
-VOID
-EFIAPI
-ResetPlatformSpecific (
-  IN UINTN            DataSize,
-  IN VOID             *ResetData OPTIONAL
-  );
-
-
-/**
-  Resets the entire platform.
-
-  @param[in] ResetType            UEFI defined reset type.
-  @param[in] ResetStatus          The status code for the reset.
-  @param[in] DataSize             The size of ResetData in bytes.
-  @param[in] ResetData            Optional element used to introduce a platform specific reset.
-                                  The exact type of the reset is defined by the EFI_GUID that follows
-                                  the Null-terminated Unicode string.
-
-**/
-VOID
-EFIAPI
-ResetSystem (
-  IN EFI_RESET_TYPE     ResetType,
-  IN EFI_STATUS         ResetStatus,
-  IN UINTN              DataSize,
-  IN VOID               *ResetData OPTIONAL
-  )
-{
-  switch (ResetType) {
-  case EfiResetWarm:
-    ResetWarm ();
-    break;
-
-  case EfiResetCold:
-    ResetCold ();
-    break;
-
-  case EfiResetShutdown:
-    ResetShutdown ();
-    return;
-
-  case EfiResetPlatformSpecific:
-    ResetPlatformSpecific (DataSize, ResetData);
-    return;
-
-  default:
-    return;
-  }
-
-  //
-  // Given we should have reset getting here would be bad
-  //
-  ASSERT (FALSE);
-}
-
 /**
   Execute Pch Reset from the host controller.
 
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c
index 6f12abf111..a3db100619 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c
@@ -317,5 +317,10 @@ ResetSystem (
   default:
     return;
   }
+
+  //
+  // Given we should have reset getting here would be bad
+  //
+  ASSERT (FALSE);
 }
 
-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51321): https://edk2.groups.io/g/devel/message/51321
Mute This Topic: https://groups.io/mt/62194129/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove redundant ResetSystem() implementation
Posted by Chiu, Chasel 6 years, 2 months ago
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: Kubacki, Michael A <michael.a.kubacki@intel.com>
> Sent: Wednesday, November 27, 2019 10:57 AM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove
> redundant ResetSystem() implementation
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2375
> 
> The following commits introduced a new implementation of ResetSystem ()
> in ResetLib instances in KabylakeSiliconPkg:
>  * 2393cff0d4
>  * 07fd95c9ca
>  * 0ad2f057d8
>  * a6e61d43c9
> 
> The PchResetLib instance PeiPchResetLib links against ResetLib and already
> contained a definition of ResetSystem (). This resulted in a build failure in
> VS2017 due to multiple definitions of a symbol being provided to the linker.
> The failure did not reproduce in
> VS2015 so it persisted until now.
> 
> This change removes the pre-existing implementation of ResetSystem () in
> PchReset.c as it is now linked by the implementation in the ResetLib
> instance.
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
>  Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
> | 55 --------------------
> 
> Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSyst
> emLib.c |  5 ++
>  2 files changed, 5 insertions(+), 55 deletions(-)
> 
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
> index 20e5d36892..c3963164c1 100644
> --- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchRes
> +++ et.c
> @@ -18,61 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/ResetSystemLib.h>  #include
> <PchResetPlatformSpecific.h>
> 
> -VOID
> -EFIAPI
> -ResetPlatformSpecific (
> -  IN UINTN            DataSize,
> -  IN VOID             *ResetData OPTIONAL
> -  );
> -
> -
> -/**
> -  Resets the entire platform.
> -
> -  @param[in] ResetType            UEFI defined reset type.
> -  @param[in] ResetStatus          The status code for the reset.
> -  @param[in] DataSize             The size of ResetData in bytes.
> -  @param[in] ResetData            Optional element used to introduce
> a platform specific reset.
> -                                  The exact type of the reset is
> defined by the EFI_GUID that follows
> -                                  the Null-terminated Unicode string.
> -
> -**/
> -VOID
> -EFIAPI
> -ResetSystem (
> -  IN EFI_RESET_TYPE     ResetType,
> -  IN EFI_STATUS         ResetStatus,
> -  IN UINTN              DataSize,
> -  IN VOID               *ResetData OPTIONAL
> -  )
> -{
> -  switch (ResetType) {
> -  case EfiResetWarm:
> -    ResetWarm ();
> -    break;
> -
> -  case EfiResetCold:
> -    ResetCold ();
> -    break;
> -
> -  case EfiResetShutdown:
> -    ResetShutdown ();
> -    return;
> -
> -  case EfiResetPlatformSpecific:
> -    ResetPlatformSpecific (DataSize, ResetData);
> -    return;
> -
> -  default:
> -    return;
> -  }
> -
> -  //
> -  // Given we should have reset getting here would be bad
> -  //
> -  ASSERT (FALSE);
> -}
> -
>  /**
>    Execute Pch Reset from the host controller.
> 
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSy
> stemLib.c
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSy
> stemLib.c
> index 6f12abf111..a3db100619 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSy
> stemLib.c
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/Pei
> +++ ResetSystemLib.c
> @@ -317,5 +317,10 @@ ResetSystem (
>    default:
>      return;
>    }
> +
> +  //
> +  // Given we should have reset getting here would be bad  //  ASSERT
> + (FALSE);
>  }
> 
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51322): https://edk2.groups.io/g/devel/message/51322
Mute This Topic: https://groups.io/mt/62194129/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove redundant ResetSystem() implementation
Posted by Nate DeSimone 6 years, 2 months ago
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Kubacki, Michael A <michael.a.kubacki@intel.com> 
Sent: Tuesday, November 26, 2019 6:57 PM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: [edk2-platforms][PATCH V1 2/2] KabylakeSiliconPkg: Remove redundant ResetSystem() implementation

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

The following commits introduced a new implementation of ResetSystem () in ResetLib instances in KabylakeSiliconPkg:
 * 2393cff0d4
 * 07fd95c9ca
 * 0ad2f057d8
 * a6e61d43c9

The PchResetLib instance PeiPchResetLib links against ResetLib and already contained a definition of ResetSystem (). This resulted in a build failure in VS2017 due to multiple definitions of a symbol being provided to the linker. The failure did not reproduce in
VS2015 so it persisted until now.

This change removes the pre-existing implementation of ResetSystem () in PchReset.c as it is now linked by the implementation in the ResetLib instance.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c             | 55 --------------------
 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c |  5 ++
 2 files changed, 5 insertions(+), 55 deletions(-)

diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
index 20e5d36892..c3963164c1 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchReset.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PchRes
+++ et.c
@@ -18,61 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include <Library/ResetSystemLib.h>  #include <PchResetPlatformSpecific.h>
 
-VOID
-EFIAPI
-ResetPlatformSpecific (
-  IN UINTN            DataSize,
-  IN VOID             *ResetData OPTIONAL
-  );
-
-
-/**
-  Resets the entire platform.
-
-  @param[in] ResetType            UEFI defined reset type.
-  @param[in] ResetStatus          The status code for the reset.
-  @param[in] DataSize             The size of ResetData in bytes.
-  @param[in] ResetData            Optional element used to introduce a platform specific reset.
-                                  The exact type of the reset is defined by the EFI_GUID that follows
-                                  the Null-terminated Unicode string.
-
-**/
-VOID
-EFIAPI
-ResetSystem (
-  IN EFI_RESET_TYPE     ResetType,
-  IN EFI_STATUS         ResetStatus,
-  IN UINTN              DataSize,
-  IN VOID               *ResetData OPTIONAL
-  )
-{
-  switch (ResetType) {
-  case EfiResetWarm:
-    ResetWarm ();
-    break;
-
-  case EfiResetCold:
-    ResetCold ();
-    break;
-
-  case EfiResetShutdown:
-    ResetShutdown ();
-    return;
-
-  case EfiResetPlatformSpecific:
-    ResetPlatformSpecific (DataSize, ResetData);
-    return;
-
-  default:
-    return;
-  }
-
-  //
-  // Given we should have reset getting here would be bad
-  //
-  ASSERT (FALSE);
-}
-
 /**
   Execute Pch Reset from the host controller.
 
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c
index 6f12abf111..a3db100619 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSystemLib.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/Pei
+++ ResetSystemLib.c
@@ -317,5 +317,10 @@ ResetSystem (
   default:
     return;
   }
+
+  //
+  // Given we should have reset getting here would be bad  //  ASSERT 
+ (FALSE);
 }
 
--
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51329): https://edk2.groups.io/g/devel/message/51329
Mute This Topic: https://groups.io/mt/62194129/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-