This dependency existed prior to this change (and still does exist). It was obfuscated in such a way that contributed to this problem.
See the previous library header path: \Silicon\Intel\KabylakeSiliconPkg\SampleCode\MdeModulePkg\Include\Library\ResetSystemLib.h The fact KabylakeSiliconPkg implements a MdeModulePkg library API introduces the dependency on MdeModulePkg. Hiding a redundant definition of the API locally does not eliminate the dependency in any meaningful way. I think the practice of "freezing" an API with a local copy only works if the codebase is locked onto a specific stable tag in which the upstream API is not expected to change. Zhichao rightfully added the new function definition to the KabylakeSiliconPkg library class implementation because a board package consumer would expect a ResetSystemLib library class instance to be compliant with the API defined in MdeModulePkg and link the ResetSystem () function. The only problem was a set of circumstances that led to the duplicate symbol definition for ResetSystem () with PchResetLib. So I view the task of eliminating the package dependency as a larger separate effort outside the scope of this change. But I do not agree with maintaining redundant local copies of edk2 APIs in packages in edk2-platforms. Thanks, Michael > -----Original Message----- > From: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com> > Sent: Tuesday, November 26, 2019 10:43 PM > To: Kubacki, Michael A <michael.a.kuba...@intel.com>; > devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Gao, Zhichao <zhichao....@intel.com> > Subject: RE: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > ResetSystemLib.h override > > This change is introducing SiliconPkg's dependency on MdeModulePkg. > SiliconPkg dependencies should be limited to a selected few packages and > this seems to be an unnecessary addition to the dependency list. > The reset interfaces are providing generic reset services and perhaps better > suited in packages like MdePkg. > > -----Original Message----- > From: Kubacki, Michael A > Sent: Tuesday, November 26, 2019 6:57 PM > To: devel@edk2.groups.io > Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Chiu, Chasel > <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Gao, Zhichao <zhichao....@intel.com> > Subject: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > ResetSystemLib.h override > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2375 > > Removes a stale ResetSystemLib.h override in KabylakeSiliconPkg that does > not contain the prototype for ResetSystem () and ResetPlatformSpecific (). > > The ResetSystemLib.h file from MdeModulePkg will be used. Any INF files > that did not include the MdeModulePkg.dec under [Packages] were updated > to do so. > > Cc: Sai Chaganty <rangasai.v.chaga...@intel.com> > Cc: Chasel Chiu <chasel.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Zhichao Gao <zhichao....@intel.com> > Signed-off-by: Michael Kubacki <michael.a.kuba...@intel.com> > --- > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeResetS > ystemLib.inf | 3 +- > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/Dx > eRuntimeResetSystemLib.inf | 3 +- > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetLib. > inf | 3 +- > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSys > temLib.inf | 3 +- > > Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Libra > ry/ResetSystemLib.h | 62 -------------------- > 5 files changed, 8 insertions(+), 66 deletions(-) > > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > index aa8877140a..46313bf35f 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/Dxe > +++ ResetSystemLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for Intel Ich7 Reset System Library. > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -35,6 +35,7 @@ > PchCycleDecodingLib > > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec > > > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > DxeRuntimeResetSystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > DxeRuntimeResetSystemLib.inf > index 6b27661603..c7fad31c71 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > DxeRuntimeResetSystemLib.inf > +++ > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystem > +++ Lib/DxeRuntimeResetSystemLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for Intel Ich7 Reset System Library. > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -36,6 +36,7 @@ > PchCycleDecodingLib > > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec > > > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetL > ib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetL > ib.inf > index b04f4006ef..29f69078a4 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetL > ib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPch > +++ ResetLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for PCH Reset Lib Pei Phase # -# Copyright (c) > 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -28,6 +28,7 @@ > ResetSystemLib > > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec > > [Sources] > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetS > ystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiReset > SystemLib.inf > index 18a92a6f18..3c6ff78863 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetS > ystemLib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/Pei > +++ ResetSystemLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for Intel Ich7 Reset System Library. > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -32,6 +32,7 @@ > PchCycleDecodingLib > > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec > > > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > rary/ResetSystemLib.h > b/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > rary/ResetSystemLib.h > deleted file mode 100644 > index 75d3e15ed7..0000000000 > --- > a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > rary/ResetSystemLib.h > +++ /dev/null > @@ -1,62 +0,0 @@ > -/** @file > - System reset Library Services. This library class defines a set of > - methods that reset the whole system. > - > -Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.<BR> > -SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#ifndef __RESET_SYSTEM_LIB_H__ > -#define __RESET_SYSTEM_LIB_H__ > - > -/** > - This function causes a system-wide reset (cold reset), in which > - all circuitry within the system returns to its initial state. This type of > reset > - is asynchronous to system operation and operates without regard to > - cycle boundaries. > - > - If this function returns, it means that the system does not support cold > reset. > -**/ > -VOID > -EFIAPI > -ResetCold ( > - VOID > - ); > - > -/** > - This function causes a system-wide initialization (warm reset), in which > all > processors > - are set to their initial state. Pending cycles are not corrupted. > - > - If this function returns, it means that the system does not support warm > reset. > -**/ > -VOID > -EFIAPI > -ResetWarm ( > - VOID > - ); > - > -/** > - This function causes the system to enter a power state equivalent > - to the ACPI G2/S5 or G3 states. > - > - If this function returns, it means that the system does not support > shutdown reset. > -**/ > -VOID > -EFIAPI > -ResetShutdown ( > - VOID > - ); > - > -/** > - This function causes the system to enter S3 and then wake up immediately. > - > - If this function returns, it means that the system does not support S3 > feature. > -**/ > -VOID > -EFIAPI > -EnterS3WithImmediateWake ( > - VOID > - ); > - > -#endif > -- > 2.16.2.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51362): https://edk2.groups.io/g/devel/message/51362 Mute This Topic: https://groups.io/mt/62194086/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
