* [edk2-platforms][PATCH v1 0/2] MinPlatformPkg: Improve NULL ACPI table lib maintainability
@ 2021-04-07 18:33 Michael Kubacki
2021-04-07 18:33 ` [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability Michael Kubacki
2021-04-07 18:33 ` [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: " Michael Kubacki
0 siblings, 2 replies; 6+ messages in thread
From: Michael Kubacki @ 2021-04-07 18:33 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
From: Michael Kubacki <michael.kubacki@microsoft.com>
This series makes some simple changes to the organization of
BoardAcpiEnableLibNull and BoardAcpiTableLibNull to better align
the library instances with patterns typically used so they are
easier to integrate into platforms.
The following issues are resolved:
1. Sharing of a directory with another unrelated library instance.
2. The directory name "BoardAcpiLibNull" is not directly related to
either library instance name in the directory.
3. The library instances have unnecessary dependencies.
4. The BASE_NAME does not indicate the library instance is the NULL
instance.
5. The C source file names do not match the INF file name making
finding the C source by search more cumbersome than needed.
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Michael Kubacki (2):
MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability
MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability
Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiEnableLib.c => BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c} | 4 +---
Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiTableLib.c => BoardAcpiTableLibNull/BoardAcpiTableLibNull.c} | 4 +---
Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiEnableLibNull}/BoardAcpiEnableLibNull.inf | 12 ++++--------
Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiTableLibNull}/BoardAcpiTableLibNull.inf | 12 ++++--------
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 8 ++++----
5 files changed, 14 insertions(+), 26 deletions(-)
rename Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiEnableLib.c => BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c} (73%)
rename Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiTableLib.c => BoardAcpiTableLibNull/BoardAcpiTableLibNull.c} (73%)
rename Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiEnableLibNull}/BoardAcpiEnableLibNull.inf (67%)
rename Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiTableLibNull}/BoardAcpiTableLibNull.inf (67%)
--
2.28.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability 2021-04-07 18:33 [edk2-platforms][PATCH v1 0/2] MinPlatformPkg: Improve NULL ACPI table lib maintainability Michael Kubacki @ 2021-04-07 18:33 ` Michael Kubacki 2021-04-08 0:29 ` [edk2-devel] " Nate DeSimone 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: " Michael Kubacki 1 sibling, 1 reply; 6+ messages in thread From: Michael Kubacki @ 2021-04-07 18:33 UTC (permalink / raw) To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong From: Michael Kubacki <michael.kubacki@microsoft.com> The NULL instance of BoardAcpiEnableLib in MinPlatformPkg currently has a few organization issues that make it more difficult to find and use than a typical NULL library instance. 1. It shares a directory with another unrelated library instance. 2. The directory name "BoardAcpiLibNull" is not directly related to either library instance name in the directory. 3. The library instance has unnecessary dependencies. 4. The BASE_NAME does not indicate the library instance is the NULL instance. 5. The C source file name does not match the INF file name making finding the C source by search more cumbersome than needed. This change resolves the above issues to improve maintainability. Cc: Chasel Chiu <chasel.chiu@intel.com> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Eric Dong <eric.dong@intel.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> --- Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiEnableLib.c => BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c} | 4 +--- Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiEnableLibNull}/BoardAcpiEnableLibNull.inf | 12 ++++-------- Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLib.c b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c similarity index 73% rename from Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLib.c rename to Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c index 334cb21714ac..494389ae5dc8 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLib.c +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c @@ -5,10 +5,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ -#include <Library/BoardAcpiTableLib.h> +#include <Base.h> #include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> -#include <Library/DebugLib.h> EFI_STATUS EFIAPI diff --git a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf similarity index 67% rename from Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf rename to Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf index 54985ee50d5f..90bed3760e2a 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf @@ -1,7 +1,8 @@ ## @file -# Component information file for Board Acpi Library +# Component information file for NULL instance of the Board ACPI Enable library # # Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> +# Copyright (c) Microsoft Corporation.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -9,20 +10,15 @@ [Defines] INF_VERSION = 0x00010005 - BASE_NAME = BoardAcpiEnableLib + BASE_NAME = BoardAcpiEnableLibNull FILE_GUID = EAF8AA6D-549A-40CC-8FB2-3A1FFEC41601 MODULE_TYPE = BASE VERSION_STRING = 1.0 LIBRARY_CLASS = BoardAcpiEnableLib -[LibraryClasses] - BaseLib - PcdLib - DebugLib - [Packages] MinPlatformPkg/MinPlatformPkg.dec MdePkg/MdePkg.dec [Sources] - BoardAcpiEnableLib.c + BoardAcpiEnableLibNull.c diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc index 998ee7909568..da27aa1c4227 100644 --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc @@ -79,7 +79,7 @@ [LibraryClasses.common] BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInitLibNull.inf BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf - BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf + BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibNull/SiliconPolicyInitLibNull.inf SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUpdateLibNull/SiliconPolicyUpdateLibNull.inf @@ -150,7 +150,7 @@ [Components] MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf - MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf + MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpiSupportLib.inf MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAcpiSupportLib.inf -- 2.28.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability Michael Kubacki @ 2021-04-08 0:29 ` Nate DeSimone 0 siblings, 0 replies; 6+ messages in thread From: Nate DeSimone @ 2021-04-08 0:29 UTC (permalink / raw) To: devel@edk2.groups.io, mikuback@linux.microsoft.com Cc: Chiu, Chasel, Liming Gao, Dong, Eric Hi Michael, Feedback is line. Thanks, Nate > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael > Kubacki > Sent: Wednesday, April 7, 2021 11:33 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desimone@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com> > Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/2] > MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability > > From: Michael Kubacki <michael.kubacki@microsoft.com> > > The NULL instance of BoardAcpiEnableLib in MinPlatformPkg currently has a > few organization issues that make it more difficult to find and use than a > typical NULL library instance. > > 1. It shares a directory with another unrelated library instance. > 2. The directory name "BoardAcpiLibNull" is not directly related to > either library instance name in the directory. > 3. The library instance has unnecessary dependencies. > 4. The BASE_NAME does not indicate the library instance is the NULL > instance. > 5. The C source file name does not match the INF file name making > finding the C source by search more cumbersome than needed. > > This change resolves the above issues to improve maintainability. > > Cc: Chasel Chiu <chasel.chiu@intel.com> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Eric Dong <eric.dong@intel.com> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > > Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiEn > ableLib.c => BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.c} | 4 +--- > Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => > BoardAcpiEnableLibNull}/BoardAcpiEnableLibNull.inf | 12 ++++------ > -- > Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > | 4 ++-- > 3 files changed, 7 insertions(+), 13 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiE > nableLib.c > b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/Boar > dAcpiEnableLibNull.c > similarity index 73% > rename from > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEna > bleLib.c > rename to > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/Board > AcpiEnableLibNull.c > index 334cb21714ac..494389ae5dc8 100644 > --- > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiE > nableLib.c > +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/ > +++ BoardAcpiEnableLibNull.c > @@ -5,10 +5,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > -#include <Library/BoardAcpiTableLib.h> > +#include <Base.h> Any reason for choosing Base.h? From what I see this file uses types that require Uefi/UefiBaseType.h like EFI_STATUS for example. It will still compile fine since BoardAcpiEnableLib.h includes Uefi.h (which includes Uefi/UefiBaseType.h). So I wonder what the value of including only Base.h is here since the file will still have implicit dependencies. > #include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> - > #include <Library/DebugLib.h> > > EFI_STATUS > EFIAPI > diff --git > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiE > nableLibNull.inf > b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/Boar > dAcpiEnableLibNull.inf > similarity index 67% > rename from > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEna > bleLibNull.inf > rename to > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/Board > AcpiEnableLibNull.inf > index 54985ee50d5f..90bed3760e2a 100644 > --- > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiE > nableLibNull.inf > +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/ > +++ BoardAcpiEnableLibNull.inf > @@ -1,7 +1,8 @@ > ## @file > -# Component information file for Board Acpi Library > +# Component information file for NULL instance of the Board ACPI Enable > +library > # > # Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) Microsoft Corporation.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -9,20 +10,15 @@ > > [Defines] > INF_VERSION = 0x00010005 > - BASE_NAME = BoardAcpiEnableLib > + BASE_NAME = BoardAcpiEnableLibNull > FILE_GUID = EAF8AA6D-549A-40CC-8FB2-3A1FFEC41601 > MODULE_TYPE = BASE > VERSION_STRING = 1.0 > LIBRARY_CLASS = BoardAcpiEnableLib > > -[LibraryClasses] > - BaseLib > - PcdLib > - DebugLib > - > [Packages] > MinPlatformPkg/MinPlatformPkg.dec > MdePkg/MdePkg.dec > > [Sources] > - BoardAcpiEnableLib.c > + BoardAcpiEnableLibNull.c > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > index 998ee7909568..da27aa1c4227 100644 > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > @@ -79,7 +79,7 @@ [LibraryClasses.common] > > > BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInit > LibNull.inf > > BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAc > piTableLibNull.inf > - > BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardA > cpiEnableLibNull.inf > + > + > BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/ > + BoardAcpiEnableLibNull.inf > > SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibN > ull/SiliconPolicyInitLibNull.inf > > SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUp > dateLibNull/SiliconPolicyUpdateLibNull.inf > > @@ -150,7 +150,7 @@ [Components] > MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf > - MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf > + > + > MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibN > + ull.inf > MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf > > MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpi > SupportLib.inf > > MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAc > piSupportLib.inf > -- > 2.28.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#73791): https://edk2.groups.io/g/devel/message/73791 > Mute This Topic: https://groups.io/mt/81923537/1767664 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [nathaniel.l.desimone@intel.com] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability 2021-04-07 18:33 [edk2-platforms][PATCH v1 0/2] MinPlatformPkg: Improve NULL ACPI table lib maintainability Michael Kubacki 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability Michael Kubacki @ 2021-04-07 18:33 ` Michael Kubacki 2021-04-08 0:29 ` Nate DeSimone 1 sibling, 1 reply; 6+ messages in thread From: Michael Kubacki @ 2021-04-07 18:33 UTC (permalink / raw) To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong From: Michael Kubacki <michael.kubacki@microsoft.com> The NULL instance of BoardAcpiTableLib in MinPlatformPkg currently has a few organization issues that make it more difficult to find and use than a typical NULL library instance. 1. It shares a directory with another unrelated library instance. 2. The directory name "BoardAcpiLibNull" is not directly related to either library instance name in the directory. 3. The library instance has unnecessary dependencies. 4. The BASE_NAME does not indicate the library instance is the NULL instance. 5. The C source file name does not match the INF file name making finding the C source by search more cumbersome than needed. This change resolves the above issues to improve use and maintainability. Cc: Chasel Chiu <chasel.chiu@intel.com> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Eric Dong <eric.dong@intel.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> --- Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiTableLib.c => BoardAcpiTableLibNull/BoardAcpiTableLibNull.c} | 4 +--- Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => BoardAcpiTableLibNull}/BoardAcpiTableLibNull.inf | 12 ++++-------- Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLib.c b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.c similarity index 73% rename from Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLib.c rename to Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.c index e49e6ad44162..0f871b6f07ef 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLib.c +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.c @@ -5,10 +5,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include <Base.h> #include <Library/BoardAcpiTableLib.h> -#include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> -#include <Library/DebugLib.h> EFI_STATUS EFIAPI diff --git a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf similarity index 67% rename from Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf rename to Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf index 04f55b49d5a1..6102897ab67b 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf @@ -1,7 +1,8 @@ ## @file -# Component information file for Board Acpi Library +# Component information file for NULL instance of the Board ACPI Enable library # # Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> +# Copyright (c) Microsoft Corporation.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -9,20 +10,15 @@ [Defines] INF_VERSION = 0x00010005 - BASE_NAME = BoardAcpiTableLib + BASE_NAME = BoardAcpiTableLibNull FILE_GUID = F220FAB7-F8E4-4E7A-A599-D47E2D547956 MODULE_TYPE = BASE VERSION_STRING = 1.0 LIBRARY_CLASS = BoardAcpiTableLib -[LibraryClasses] - BaseLib - PcdLib - DebugLib - [Packages] MinPlatformPkg/MinPlatformPkg.dec MdePkg/MdePkg.dec [Sources] - BoardAcpiTableLib.c + BoardAcpiTableLibNull.c diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc index da27aa1c4227..cf3ff13e7b29 100644 --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc @@ -78,7 +78,7 @@ [LibraryClasses.common] FspWrapperPlatformLib|MinPlatformPkg/FspWrapper/Library/PeiFspWrapperPlatformLib/PeiFspWrapperPlatformLib.inf BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInitLibNull.inf - BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf + BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibNull/SiliconPolicyInitLibNull.inf SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUpdateLibNull/SiliconPolicyUpdateLibNull.inf @@ -151,7 +151,7 @@ [Components] MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf - MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf + MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpiSupportLib.inf MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAcpiSupportLib.inf -- 2.28.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: " Michael Kubacki @ 2021-04-08 0:29 ` Nate DeSimone 2021-04-08 21:26 ` [edk2-devel] " Michael Kubacki 0 siblings, 1 reply; 6+ messages in thread From: Nate DeSimone @ 2021-04-08 0:29 UTC (permalink / raw) To: mikuback@linux.microsoft.com, devel@edk2.groups.io Cc: Chiu, Chasel, Liming Gao, Dong, Eric Hi Michael, Feedback is line. Thanks, Nate > -----Original Message----- > From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com> > Sent: Wednesday, April 7, 2021 11:33 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desimone@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com> > Subject: [edk2-platforms][PATCH v1 2/2] > MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability > > From: Michael Kubacki <michael.kubacki@microsoft.com> > > The NULL instance of BoardAcpiTableLib in MinPlatformPkg currently has a > few organization issues that make it more difficult to find and use than a > typical NULL library instance. > > 1. It shares a directory with another unrelated library instance. > 2. The directory name "BoardAcpiLibNull" is not directly related to > either library instance name in the directory. > 3. The library instance has unnecessary dependencies. > 4. The BASE_NAME does not indicate the library instance is the NULL > instance. > 5. The C source file name does not match the INF file name making > finding the C source by search more cumbersome than needed. > > This change resolves the above issues to improve use and maintainability. > > Cc: Chasel Chiu <chasel.chiu@intel.com> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Eric Dong <eric.dong@intel.com> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > > Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiTa > bleLib.c => BoardAcpiTableLibNull/BoardAcpiTableLibNull.c} | 4 +--- > Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => > BoardAcpiTableLibNull}/BoardAcpiTableLibNull.inf | 12 ++++-------- > Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > | 4 ++-- > 3 files changed, 7 insertions(+), 13 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT > ableLib.c > b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Board > AcpiTableLibNull.c > similarity index 73% > rename from > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTab > leLib.c > rename to > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAc > piTableLibNull.c > index e49e6ad44162..0f871b6f07ef 100644 > --- > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT > ableLib.c > +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/B > +++ oardAcpiTableLibNull.c > @@ -5,10 +5,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > +#include <Base.h> Any reason for choosing Base.h? From what I see this file uses types that require Uefi/UefiBaseType.h like EFI_STATUS for example. It will still compile fine since BoardAcpiEnableLib.h includes Uefi.h (which includes Uefi/UefiBaseType.h). So I wonder what the value of including only Base.h is here since the file will still have implicit dependencies. > #include <Library/BoardAcpiTableLib.h> > -#include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> - > #include <Library/DebugLib.h> > > EFI_STATUS > EFIAPI > diff --git > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT > ableLibNull.inf > b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Board > AcpiTableLibNull.inf > similarity index 67% > rename from > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTab > leLibNull.inf > rename to > Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAc > piTableLibNull.inf > index 04f55b49d5a1..6102897ab67b 100644 > --- > a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT > ableLibNull.inf > +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/B > +++ oardAcpiTableLibNull.inf > @@ -1,7 +1,8 @@ > ## @file > -# Component information file for Board Acpi Library > +# Component information file for NULL instance of the Board ACPI Enable > +library > # > # Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) Microsoft Corporation.<BR> > # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -9,20 +10,15 @@ > > [Defines] > INF_VERSION = 0x00010005 > - BASE_NAME = BoardAcpiTableLib > + BASE_NAME = BoardAcpiTableLibNull > FILE_GUID = F220FAB7-F8E4-4E7A-A599-D47E2D547956 > MODULE_TYPE = BASE > VERSION_STRING = 1.0 > LIBRARY_CLASS = BoardAcpiTableLib > > -[LibraryClasses] > - BaseLib > - PcdLib > - DebugLib > - > [Packages] > MinPlatformPkg/MinPlatformPkg.dec > MdePkg/MdePkg.dec > > [Sources] > - BoardAcpiTableLib.c > + BoardAcpiTableLibNull.c > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > index da27aa1c4227..cf3ff13e7b29 100644 > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > @@ -78,7 +78,7 @@ [LibraryClasses.common] > > FspWrapperPlatformLib|MinPlatformPkg/FspWrapper/Library/PeiFspWrapp > erPlatformLib/PeiFspWrapperPlatformLib.inf > > > BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInit > LibNull.inf > - > BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAc > piTableLibNull.inf > + > + > BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Bo > + ardAcpiTableLibNull.inf > > BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/B > oardAcpiEnableLibNull.inf > > SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibN > ull/SiliconPolicyInitLibNull.inf > > SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUp > dateLibNull/SiliconPolicyUpdateLibNull.inf > @@ -151,7 +151,7 @@ [Components] > MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf > > MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibN > ull.inf > - MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf > + > + MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNul > + l.inf > > MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpi > SupportLib.inf > > MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAc > piSupportLib.inf > > -- > 2.28.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability 2021-04-08 0:29 ` Nate DeSimone @ 2021-04-08 21:26 ` Michael Kubacki 0 siblings, 0 replies; 6+ messages in thread From: Michael Kubacki @ 2021-04-08 21:26 UTC (permalink / raw) To: devel, nathaniel.l.desimone; +Cc: Chiu, Chasel, Liming Gao, Dong, Eric Thanks for the feedback. I removed Base.h in a v2 series just sent. On 4/7/2021 5:29 PM, Nate DeSimone wrote: > Hi Michael, > > Feedback is line. > > Thanks, > Nate > >> -----Original Message----- >> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com> >> Sent: Wednesday, April 7, 2021 11:33 AM >> To: devel@edk2.groups.io >> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L >> <nathaniel.l.desimone@intel.com>; Liming Gao >> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com> >> Subject: [edk2-platforms][PATCH v1 2/2] >> MinPlatformPkg/BoardAcpiTableLibNull: Improve maintainability >> >> From: Michael Kubacki <michael.kubacki@microsoft.com> >> >> The NULL instance of BoardAcpiTableLib in MinPlatformPkg currently has a >> few organization issues that make it more difficult to find and use than a >> typical NULL library instance. >> >> 1. It shares a directory with another unrelated library instance. >> 2. The directory name "BoardAcpiLibNull" is not directly related to >> either library instance name in the directory. >> 3. The library instance has unnecessary dependencies. >> 4. The BASE_NAME does not indicate the library instance is the NULL >> instance. >> 5. The C source file name does not match the INF file name making >> finding the C source by search more cumbersome than needed. >> >> This change resolves the above issues to improve use and maintainability. >> >> Cc: Chasel Chiu <chasel.chiu@intel.com> >> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com> >> Cc: Liming Gao <gaoliming@byosoft.com.cn> >> Cc: Eric Dong <eric.dong@intel.com> >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> >> --- >> >> Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull/BoardAcpiTa >> bleLib.c => BoardAcpiTableLibNull/BoardAcpiTableLibNull.c} | 4 +--- >> Platform/Intel/MinPlatformPkg/Acpi/Library/{BoardAcpiLibNull => >> BoardAcpiTableLibNull}/BoardAcpiTableLibNull.inf | 12 ++++-------- >> Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc >> | 4 ++-- >> 3 files changed, 7 insertions(+), 13 deletions(-) >> >> diff --git >> a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT >> ableLib.c >> b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Board >> AcpiTableLibNull.c >> similarity index 73% >> rename from >> Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTab >> leLib.c >> rename to >> Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAc >> piTableLibNull.c >> index e49e6ad44162..0f871b6f07ef 100644 >> --- >> a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT >> ableLib.c >> +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/B >> +++ oardAcpiTableLibNull.c >> @@ -5,10 +5,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >> >> **/ >> >> +#include <Base.h> > > Any reason for choosing Base.h? From what I see this file uses types that require Uefi/UefiBaseType.h like EFI_STATUS for example. It will still compile fine since BoardAcpiEnableLib.h includes Uefi.h (which includes Uefi/UefiBaseType.h). So I wonder what the value of including only Base.h is here since the file will still have implicit dependencies. > >> #include <Library/BoardAcpiTableLib.h> >> -#include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> - >> #include <Library/DebugLib.h> >> >> EFI_STATUS >> EFIAPI >> diff --git >> a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT >> ableLibNull.inf >> b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Board >> AcpiTableLibNull.inf >> similarity index 67% >> rename from >> Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTab >> leLibNull.inf >> rename to >> Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAc >> piTableLibNull.inf >> index 04f55b49d5a1..6102897ab67b 100644 >> --- >> a/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiT >> ableLibNull.inf >> +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/B >> +++ oardAcpiTableLibNull.inf >> @@ -1,7 +1,8 @@ >> ## @file >> -# Component information file for Board Acpi Library >> +# Component information file for NULL instance of the Board ACPI Enable >> +library >> # >> # Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) Microsoft Corporation.<BR> >> # >> # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -9,20 +10,15 @@ >> >> [Defines] >> INF_VERSION = 0x00010005 >> - BASE_NAME = BoardAcpiTableLib >> + BASE_NAME = BoardAcpiTableLibNull >> FILE_GUID = F220FAB7-F8E4-4E7A-A599-D47E2D547956 >> MODULE_TYPE = BASE >> VERSION_STRING = 1.0 >> LIBRARY_CLASS = BoardAcpiTableLib >> >> -[LibraryClasses] >> - BaseLib >> - PcdLib >> - DebugLib >> - >> [Packages] >> MinPlatformPkg/MinPlatformPkg.dec >> MdePkg/MdePkg.dec >> >> [Sources] >> - BoardAcpiTableLib.c >> + BoardAcpiTableLibNull.c >> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc >> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc >> index da27aa1c4227..cf3ff13e7b29 100644 >> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc >> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc >> @@ -78,7 +78,7 @@ [LibraryClasses.common] >> >> FspWrapperPlatformLib|MinPlatformPkg/FspWrapper/Library/PeiFspWrapp >> erPlatformLib/PeiFspWrapperPlatformLib.inf >> >> >> BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInit >> LibNull.inf >> - >> BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAc >> piTableLibNull.inf >> + >> + >> BoardAcpiTableLib|MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/Bo >> + ardAcpiTableLibNull.inf >> >> BoardAcpiEnableLib|MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/B >> oardAcpiEnableLibNull.inf >> >> SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibN >> ull/SiliconPolicyInitLibNull.inf >> >> SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUp >> dateLibNull/SiliconPolicyUpdateLibNull.inf >> @@ -151,7 +151,7 @@ [Components] >> MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf >> MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf >> >> MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibN >> ull.inf >> - MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf >> + >> + MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNul >> + l.inf >> >> MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpi >> SupportLib.inf >> >> MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAc >> piSupportLib.inf >> >> -- >> 2.28.0.windows.1 > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-08 21:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-07 18:33 [edk2-platforms][PATCH v1 0/2] MinPlatformPkg: Improve NULL ACPI table lib maintainability Michael Kubacki 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 1/2] MinPlatformPkg/BoardAcpiEnableLibNull: Improve maintainability Michael Kubacki 2021-04-08 0:29 ` [edk2-devel] " Nate DeSimone 2021-04-07 18:33 ` [edk2-platforms][PATCH v1 2/2] MinPlatformPkg/BoardAcpiTableLibNull: " Michael Kubacki 2021-04-08 0:29 ` Nate DeSimone 2021-04-08 21:26 ` [edk2-devel] " Michael Kubacki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox