[PATCH v2 0/3] ArmPkg: Fix several issues in OemMiscLib

[PATCH v2 0/3] ArmPkg: Fix several issues in OemMiscLib

[Rebecca Cran]

Update OemMiscLib with the following changes:

o Fixed ordering of return type and EFIAPI specifier.
o Renamed 'Offset' parameter in OemUpdateSmbiosInfo to 'Field'.
o Renamed OemGetProcessorMaxSockets to OemGetMaxProcessors.
o Renamed OemIsSocketPresent to OemIsProcessorPresent.
o Updated OemGetChassisType to return MISC_CHASSIS_TYPE instead of
  EFI_STATUS, which matches other OemMiscLib functions.

Github pull request (personal build): https://github.com/tianocore/edk2/pull/1435

Changes between v1 and v2:

Split changes out into three patches.

Rebecca Cran (3):
  ArmPkg: Fix ordering of return type and EFIAPI specifier in OemMiscLib
  ArmPkg: Rename some functions and parameters in OemMiscLib
  ArmPkg: Update OemGetChassisType function to return MISC_CHASSIS_TYPE

 ArmPkg/Include/Library/OemMiscLib.h                                            | 32 ++++++++--------
 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c                            | 36 ++++++++----------
 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c               | 40 ++++++++++----------
 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c | 28 +-------------
 4 files changed, 52 insertions(+), 84 deletions(-)

-- 
2.26.2

[PATCH v2 1/3] ArmPkg: Fix ordering of return type and EFIAPI specifier in OemMiscLib

[Rebecca Cran]

The return type should be on the line before any EFIAPI specifier.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 ArmPkg/Include/Library/OemMiscLib.h                 | 14 +++++++-------
 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 12 ++++++------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
index e70019d05f15..ad0e77685dbe 100644
--- a/ArmPkg/Include/Library/OemMiscLib.h
+++ b/ArmPkg/Include/Library/OemMiscLib.h
@@ -71,8 +71,8 @@ typedef enum
 
   @return               CPU frequency in Hz
 **/
-EFIAPI
 UINTN
+EFIAPI
 OemGetCpuFreq (
   IN UINT8 ProcessorIndex
   );
@@ -87,8 +87,8 @@ OemGetCpuFreq (
 
   @return  TRUE on success, FALSE on failure.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemGetProcessorInformation (
   IN UINTN ProcessorIndex,
   IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
@@ -106,8 +106,8 @@ OemGetProcessorInformation (
 
   @return TRUE on success, FALSE on failure.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemGetCacheInformation (
   IN UINT8   ProcessorIndex,
   IN UINT8   CacheLevel,
@@ -120,8 +120,8 @@ OemGetCacheInformation (
 
   @return The maximum number of sockets.
 **/
-EFIAPI
 UINT8
+EFIAPI
 OemGetProcessorMaxSockets (
   VOID
   );
@@ -132,8 +132,8 @@ OemGetProcessorMaxSockets (
 
   @retval EFI_SUCCESS The chassis type was fetched successfully.
 **/
-EFIAPI
 EFI_STATUS
+EFIAPI
 OemGetChassisType (
   OUT UINT8 *ChassisType
   );
@@ -144,8 +144,8 @@ OemGetChassisType (
 
   @return TRUE is the processor is present, FALSE otherwise.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemIsSocketPresent (
   IN UINTN ProcessorIndex
   );
@@ -156,8 +156,8 @@ OemIsSocketPresent (
   @param TokenToUpdate The string to update.
   @param Offset        The field to get information about.
 **/
-EFIAPI
 VOID
+EFIAPI
 OemUpdateSmbiosInfo (
   IN EFI_HII_HANDLE    HiiHandle,
   IN EFI_STRING_ID     TokenToUpdate,
diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
index 73cebef2d1b9..6b179941e414 100644
--- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
+++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
@@ -23,8 +23,8 @@
 
   @return               CPU frequency in Hz
 **/
-EFIAPI
 UINTN
+EFIAPI
 OemGetCpuFreq (
   IN UINT8 ProcessorIndex
   )
@@ -43,8 +43,8 @@ OemGetCpuFreq (
 
   @return  TRUE on success, FALSE on failure.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemGetProcessorInformation (
   IN UINTN ProcessorIndex,
   IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
@@ -66,8 +66,8 @@ OemGetProcessorInformation (
 
   @return TRUE on success, FALSE on failure.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemGetCacheInformation (
   IN UINT8   ProcessorIndex,
   IN UINT8   CacheLevel,
@@ -84,8 +84,8 @@ OemGetCacheInformation (
 
   @return The maximum number of sockets.
 **/
-EFIAPI
 UINT8
+EFIAPI
 OemGetProcessorMaxSockets (
   VOID
   )
@@ -117,8 +117,8 @@ OemGetChassisType (
 
   @return TRUE is the processor is present, FALSE otherwise.
 **/
-EFIAPI
 BOOLEAN
+EFIAPI
 OemIsSocketPresent (
   IN UINTN ProcessorIndex
   )
@@ -133,8 +133,8 @@ OemIsSocketPresent (
   @param TokenToUpdate The string to update.
   @param Offset        The field to get information about.
 **/
-EFIAPI
 VOID
+EFIAPI
 OemUpdateSmbiosInfo (
   IN EFI_HII_HANDLE mHiiHandle,
   IN EFI_STRING_ID TokenToUpdate,
-- 
2.26.2

[PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

[Rebecca Cran]

o Rename 'Offset' parameter in OemUpdateSmbiosInfo to 'Field'.
o Rename OemGetProcessorMaxSockets to OemGetMaxProcessors.
o Rename OemIsSocketPresent to OemIsProcessorPresent.
o Update Universal/Smbios to follow the changes to OemMiscLib.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 ArmPkg/Include/Library/OemMiscLib.h                                            | 12 +++---
 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c                            | 13 +++----
 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c               | 40 ++++++++++----------
 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c | 28 +-------------
 4 files changed, 33 insertions(+), 60 deletions(-)

diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
index ad0e77685dbe..31dfe7dac2a6 100644
--- a/ArmPkg/Include/Library/OemMiscLib.h
+++ b/ArmPkg/Include/Library/OemMiscLib.h
@@ -116,13 +116,13 @@ OemGetCacheInformation (
   IN OUT SMBIOS_TABLE_TYPE7 *SmbiosCacheTable
   );
 
-/** Gets the maximum number of sockets supported by the platform.
+/** Gets the maximum number of processors supported by the platform.
 
-  @return The maximum number of sockets.
+  @return The maximum number of processors.
 **/
 UINT8
 EFIAPI
-OemGetProcessorMaxSockets (
+OemGetMaxProcessors (
   VOID
   );
 
@@ -146,7 +146,7 @@ OemGetChassisType (
 **/
 BOOLEAN
 EFIAPI
-OemIsSocketPresent (
+OemIsProcessorPresent (
   IN UINTN ProcessorIndex
   );
 
@@ -154,14 +154,14 @@ OemIsSocketPresent (
 
   @param mHiiHandle    The HII handle.
   @param TokenToUpdate The string to update.
-  @param Offset        The field to get information about.
+  @param Field         The field to get information about.
 **/
 VOID
 EFIAPI
 OemUpdateSmbiosInfo (
   IN EFI_HII_HANDLE    HiiHandle,
   IN EFI_STRING_ID     TokenToUpdate,
-  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
+  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
   );
 
 #endif // OEM_MISC_LIB_H_
diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
index 6b179941e414..b02a568426dd 100644
--- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
+++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
@@ -13,7 +13,6 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HiiLib.h>
-
 #include <Library/OemMiscLib.h>
 
 
@@ -80,13 +79,13 @@ OemGetCacheInformation (
   return TRUE;
 }
 
-/** Gets the maximum number of sockets supported by the platform.
+/** Gets the maximum number of processors supported by the platform.
 
-  @return The maximum number of sockets.
+  @return The maximum number of processors.
 **/
 UINT8
 EFIAPI
-OemGetProcessorMaxSockets (
+OemGetMaxProcessors (
   VOID
   )
 {
@@ -119,7 +118,7 @@ OemGetChassisType (
 **/
 BOOLEAN
 EFIAPI
-OemIsSocketPresent (
+OemIsProcessorPresent (
   IN UINTN ProcessorIndex
   )
 {
@@ -131,14 +130,14 @@ OemIsSocketPresent (
 
   @param mHiiHandle    The HII handle.
   @param TokenToUpdate The string to update.
-  @param Offset        The field to get information about.
+  @param Field         The field to get information about.
 **/
 VOID
 EFIAPI
 OemUpdateSmbiosInfo (
   IN EFI_HII_HANDLE mHiiHandle,
   IN EFI_STRING_ID TokenToUpdate,
-  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
+  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
   )
 {
   ASSERT (FALSE);
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
index d03de12a820e..0cb56c53975e 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
@@ -439,8 +439,8 @@ AddSmbiosCacheTypeTable (
     strings following the data fields.
 
   @param[out] Type4Record    The Type 4 structure to allocate and initialize
-  @param[in]  ProcessorIndex The index of the processor socket
-  @param[in]  Populated      Whether the specified processor socket is
+  @param[in]  ProcessorIndex The index of the processor
+  @param[in]  Populated      Whether the specified processor is
                              populated.
 
   @retval EFI_SUCCESS          The Type 4 structure was successfully
@@ -460,7 +460,7 @@ AllocateType4AndSetProcessorInformationStrings (
   EFI_STRING_ID   SerialNumber;
   EFI_STRING_ID   AssetTag;
   EFI_STRING_ID   PartNumber;
-  EFI_STRING      ProcessorSocketStr;
+  EFI_STRING      ProcessorStr;
   EFI_STRING      ProcessorManuStr;
   EFI_STRING      ProcessorVersionStr;
   EFI_STRING      SerialNumberStr;
@@ -468,7 +468,7 @@ AllocateType4AndSetProcessorInformationStrings (
   EFI_STRING      PartNumberStr;
   CHAR8           *OptionalStrStart;
   CHAR8           *StrStart;
-  UINTN           ProcessorSocketStrLen;
+  UINTN           ProcessorStrLen;
   UINTN           ProcessorManuStrLen;
   UINTN           ProcessorVersionStrLen;
   UINTN           SerialNumberStrLen;
@@ -497,14 +497,14 @@ AllocateType4AndSetProcessorInformationStrings (
   SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorAssetTag, AssetTag);
   SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorPartNumber, PartNumber);
 
-  // Processor Socket Designation
+  // Processor Designation
   StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
-  ProcessorSocketStr = AllocateZeroPool (StringBufferSize);
-  if (ProcessorSocketStr == NULL) {
+  ProcessorStr = AllocateZeroPool (StringBufferSize);
+  if (ProcessorStr == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  ProcessorSocketStrLen = UnicodeSPrint (ProcessorSocketStr, StringBufferSize,
+  ProcessorStrLen = UnicodeSPrint (ProcessorStr, StringBufferSize,
                                          L"CPU%02d", ProcessorIndex + 1);
 
   // Processor Manufacture
@@ -528,7 +528,7 @@ AllocateType4AndSetProcessorInformationStrings (
   PartNumberStrLen = StrLen (PartNumberStr);
 
   TotalSize = sizeof (SMBIOS_TABLE_TYPE4) +
-              ProcessorSocketStrLen  + 1 +
+              ProcessorStrLen        + 1 +
               ProcessorManuStrLen    + 1 +
               ProcessorVersionStrLen + 1 +
               SerialNumberStrLen     + 1 +
@@ -545,12 +545,12 @@ AllocateType4AndSetProcessorInformationStrings (
 
   OptionalStrStart = (CHAR8 *)(*Type4Record + 1);
   UnicodeStrToAsciiStrS (
-    ProcessorSocketStr,
+    ProcessorStr,
     OptionalStrStart,
-    ProcessorSocketStrLen + 1
+    ProcessorStrLen + 1
     );
 
-  StrStart = OptionalStrStart + ProcessorSocketStrLen + 1;
+  StrStart = OptionalStrStart + ProcessorStrLen + 1;
   UnicodeStrToAsciiStrS (
     ProcessorManuStr,
     StrStart,
@@ -586,7 +586,7 @@ AllocateType4AndSetProcessorInformationStrings (
     );
 
 Exit:
-  FreePool (ProcessorSocketStr);
+  FreePool (ProcessorStr);
   FreePool (ProcessorManuStr);
   FreePool (ProcessorVersionStr);
   FreePool (SerialNumberStr);
@@ -618,7 +618,7 @@ AddSmbiosProcessorTypeTable (
   UINT64                      *ProcessorId;
   PROCESSOR_CHARACTERISTIC_FLAGS ProcessorCharacteristics;
   OEM_MISC_PROCESSOR_DATA     MiscProcessorData;
-  BOOLEAN                     SocketPopulated;
+  BOOLEAN                     ProcessorPopulated;
 
   Type4Record         = NULL;
 
@@ -632,12 +632,12 @@ AddSmbiosProcessorTypeTable (
   L2CacheHandle       = 0xFFFF;
   L3CacheHandle       = 0xFFFF;
 
-  SocketPopulated = OemIsSocketPresent(ProcessorIndex);
+  ProcessorPopulated = OemIsProcessorPresent (ProcessorIndex);
 
   Status = AllocateType4AndSetProcessorInformationStrings (
              &Type4Record,
              ProcessorIndex,
-             SocketPopulated
+             ProcessorPopulated
              );
   if (EFI_ERROR (Status)) {
     return Status;
@@ -649,7 +649,7 @@ AddSmbiosProcessorTypeTable (
                                 &Type4Record->ProcessorCharacteristics,
                               &MiscProcessorData);
 
-  if (SocketPopulated) {
+  if (ProcessorPopulated) {
     AddSmbiosCacheTypeTable (ProcessorIndex, &L1CacheHandle,
                              &L2CacheHandle, &L3CacheHandle);
   }
@@ -713,7 +713,7 @@ ProcessorSubClassEntryPoint(
   )
 {
   EFI_STATUS    Status;
-  UINT32        SocketIndex;
+  UINT32        ProcessorIndex;
 
   //
   // Locate dependent protocols
@@ -740,8 +740,8 @@ ProcessorSubClassEntryPoint(
   //
   // Add SMBIOS tables for populated sockets.
   //
-  for (SocketIndex = 0; SocketIndex < OemGetProcessorMaxSockets(); SocketIndex++) {
-    Status = AddSmbiosProcessorTypeTable (SocketIndex);
+  for (ProcessorIndex = 0; ProcessorIndex < OemGetMaxProcessors (); ProcessorIndex++) {
+    Status = AddSmbiosProcessorTypeTable (ProcessorIndex);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "Add Processor Type Table Failed!  %r.\n", Status));
       return Status;
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
index e6adbceba2d5..fc4dba319aad 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
@@ -23,27 +23,6 @@
 
 #include "SmbiosMisc.h"
 
-/**
- * Returns the chassis type in SMBIOS format.
- *
- * @return Chassis type
-**/
-UINT8
-GetChassisType (
-  VOID
-  )
-{
-  EFI_STATUS                      Status;
-  UINT8                           ChassisType;
-
-  Status = OemGetChassisType (&ChassisType);
-  if (EFI_ERROR (Status)) {
-    return 0;
-  }
-
-  return ChassisType;
-}
-
 /**
   This function makes boot time changes to the contents of the
   MiscChassisManufacturer (Type 3) record.
@@ -80,8 +59,6 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
   CONTAINED_ELEMENT               ContainedElements;
   UINT8                           ExtendLength;
 
-  UINT8                           ChassisType;
-
   ExtendLength = 0;
 
   //
@@ -165,10 +142,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
 
   SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE3) + ExtendLength + 1;
 
-  ChassisType = GetChassisType ();
-  if (ChassisType != 0) {
-    SmbiosRecord->Type  = ChassisType;
-  }
+  SmbiosRecord->Type = OemGetChassisType ();
 
   //ContainedElements
   ASSERT (ContainedElementCount < 2);
-- 
2.26.2

[PATCH v2 3/3] ArmPkg: Update OemGetChassisType function to return MISC_CHASSIS_TYPE

[Rebecca Cran]

Update OemGetChassisType in OemMiscLib to return MISC_CHASSIS_TYPE
instead of EFI_STATUS, which matches other OemMiscLib functions.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 ArmPkg/Include/Library/OemMiscLib.h                 |  8 +++-----
 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 11 ++++-------
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
index 31dfe7dac2a6..5f71b2ad48c7 100644
--- a/ArmPkg/Include/Library/OemMiscLib.h
+++ b/ArmPkg/Include/Library/OemMiscLib.h
@@ -128,14 +128,12 @@ OemGetMaxProcessors (
 
 /** Gets the type of chassis for the system.
 
-  @param ChassisType The type of the chassis.
-
-  @retval EFI_SUCCESS The chassis type was fetched successfully.
+  @retval The type of the chassis.
 **/
-EFI_STATUS
+MISC_CHASSIS_TYPE
 EFIAPI
 OemGetChassisType (
-  OUT UINT8 *ChassisType
+  VOID
   );
 
 /** Returns whether the specified processor is present or not.
diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
index b02a568426dd..6b233742feb0 100644
--- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
+++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
@@ -95,19 +95,16 @@ OemGetMaxProcessors (
 
 /** Gets the type of chassis for the system.
 
-  @param ChassisType The type of the chassis.
-
-  @retval EFI_SUCCESS The chassis type was fetched successfully.
+  @retval The type of the chassis.
 **/
-EFI_STATUS
+MISC_CHASSIS_TYPE
 EFIAPI
 OemGetChassisType (
-  UINT8 *ChassisType
+  VOID
   )
 {
   ASSERT (FALSE);
-  *ChassisType = MiscChassisTypeUnknown;
-  return EFI_SUCCESS;
+  return MiscChassisTypeUnknown;
 }
 
 /** Returns whether the specified processor is present or not.
-- 
2.26.2

Re: [PATCH v2 1/3] ArmPkg: Fix ordering of return type and EFIAPI specifier in OemMiscLib

[Leif Lindholm]

On Sat, Feb 20, 2021 at 20:28:58 -0700, Rebecca Cran wrote:
> The return type should be on the line before any EFIAPI specifier.
> 
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  ArmPkg/Include/Library/OemMiscLib.h                 | 14 +++++++-------
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 12 ++++++------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index e70019d05f15..ad0e77685dbe 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -71,8 +71,8 @@ typedef enum
>  
>    @return               CPU frequency in Hz
>  **/
> -EFIAPI
>  UINTN
> +EFIAPI
>  OemGetCpuFreq (
>    IN UINT8 ProcessorIndex
>    );
> @@ -87,8 +87,8 @@ OemGetCpuFreq (
>  
>    @return  TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetProcessorInformation (
>    IN UINTN ProcessorIndex,
>    IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
> @@ -106,8 +106,8 @@ OemGetProcessorInformation (
>  
>    @return TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetCacheInformation (
>    IN UINT8   ProcessorIndex,
>    IN UINT8   CacheLevel,
> @@ -120,8 +120,8 @@ OemGetCacheInformation (
>  
>    @return The maximum number of sockets.
>  **/
> -EFIAPI
>  UINT8
> +EFIAPI
>  OemGetProcessorMaxSockets (
>    VOID
>    );
> @@ -132,8 +132,8 @@ OemGetProcessorMaxSockets (
>  
>    @retval EFI_SUCCESS The chassis type was fetched successfully.
>  **/
> -EFIAPI
>  EFI_STATUS
> +EFIAPI
>  OemGetChassisType (
>    OUT UINT8 *ChassisType
>    );
> @@ -144,8 +144,8 @@ OemGetChassisType (
>  
>    @return TRUE is the processor is present, FALSE otherwise.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemIsSocketPresent (
>    IN UINTN ProcessorIndex
>    );
> @@ -156,8 +156,8 @@ OemIsSocketPresent (
>    @param TokenToUpdate The string to update.
>    @param Offset        The field to get information about.
>  **/
> -EFIAPI
>  VOID
> +EFIAPI
>  OemUpdateSmbiosInfo (
>    IN EFI_HII_HANDLE    HiiHandle,
>    IN EFI_STRING_ID     TokenToUpdate,
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index 73cebef2d1b9..6b179941e414 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -23,8 +23,8 @@
>  
>    @return               CPU frequency in Hz
>  **/
> -EFIAPI
>  UINTN
> +EFIAPI
>  OemGetCpuFreq (
>    IN UINT8 ProcessorIndex
>    )
> @@ -43,8 +43,8 @@ OemGetCpuFreq (
>  
>    @return  TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetProcessorInformation (
>    IN UINTN ProcessorIndex,
>    IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
> @@ -66,8 +66,8 @@ OemGetProcessorInformation (
>  
>    @return TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetCacheInformation (
>    IN UINT8   ProcessorIndex,
>    IN UINT8   CacheLevel,
> @@ -84,8 +84,8 @@ OemGetCacheInformation (
>  
>    @return The maximum number of sockets.
>  **/
> -EFIAPI
>  UINT8
> +EFIAPI
>  OemGetProcessorMaxSockets (
>    VOID
>    )
> @@ -117,8 +117,8 @@ OemGetChassisType (
>  
>    @return TRUE is the processor is present, FALSE otherwise.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemIsSocketPresent (
>    IN UINTN ProcessorIndex
>    )
> @@ -133,8 +133,8 @@ OemIsSocketPresent (
>    @param TokenToUpdate The string to update.
>    @param Offset        The field to get information about.
>  **/
> -EFIAPI
>  VOID
> +EFIAPI
>  OemUpdateSmbiosInfo (
>    IN EFI_HII_HANDLE mHiiHandle,
>    IN EFI_STRING_ID TokenToUpdate,
> -- 
> 2.26.2
> 

Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

[Leif Lindholm]

On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:
> o Rename 'Offset' parameter in OemUpdateSmbiosInfo to 'Field'.
> o Rename OemGetProcessorMaxSockets to OemGetMaxProcessors.
> o Rename OemIsSocketPresent to OemIsProcessorPresent.
> o Update Universal/Smbios to follow the changes to OemMiscLib.
> 
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
> ---
>  ArmPkg/Include/Library/OemMiscLib.h                                            | 12 +++---
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c                            | 13 +++----
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c               | 40 ++++++++++----------
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c | 28 +-------------
>  4 files changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index ad0e77685dbe..31dfe7dac2a6 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -116,13 +116,13 @@ OemGetCacheInformation (
>    IN OUT SMBIOS_TABLE_TYPE7 *SmbiosCacheTable
>    );
>  
> -/** Gets the maximum number of sockets supported by the platform.
> +/** Gets the maximum number of processors supported by the platform.
>  
> -  @return The maximum number of sockets.
> +  @return The maximum number of processors.
>  **/
>  UINT8
>  EFIAPI
> -OemGetProcessorMaxSockets (
> +OemGetMaxProcessors (
>    VOID
>    );
>  
> @@ -146,7 +146,7 @@ OemGetChassisType (
>  **/
>  BOOLEAN
>  EFIAPI
> -OemIsSocketPresent (
> +OemIsProcessorPresent (
>    IN UINTN ProcessorIndex
>    );
>  
> @@ -154,14 +154,14 @@ OemIsSocketPresent (
>  
>    @param mHiiHandle    The HII handle.
>    @param TokenToUpdate The string to update.
> -  @param Offset        The field to get information about.
> +  @param Field         The field to get information about.
>  **/
>  VOID
>  EFIAPI
>  OemUpdateSmbiosInfo (
>    IN EFI_HII_HANDLE    HiiHandle,
>    IN EFI_STRING_ID     TokenToUpdate,
> -  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
> +  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
>    );
>  
>  #endif // OEM_MISC_LIB_H_
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index 6b179941e414..b02a568426dd 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -13,7 +13,6 @@
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/HiiLib.h>
> -
>  #include <Library/OemMiscLib.h>
>  
>  
> @@ -80,13 +79,13 @@ OemGetCacheInformation (
>    return TRUE;
>  }
>  
> -/** Gets the maximum number of sockets supported by the platform.
> +/** Gets the maximum number of processors supported by the platform.
>  
> -  @return The maximum number of sockets.
> +  @return The maximum number of processors.
>  **/
>  UINT8
>  EFIAPI
> -OemGetProcessorMaxSockets (
> +OemGetMaxProcessors (
>    VOID
>    )
>  {
> @@ -119,7 +118,7 @@ OemGetChassisType (
>  **/
>  BOOLEAN
>  EFIAPI
> -OemIsSocketPresent (
> +OemIsProcessorPresent (
>    IN UINTN ProcessorIndex
>    )
>  {
> @@ -131,14 +130,14 @@ OemIsSocketPresent (
>  
>    @param mHiiHandle    The HII handle.
>    @param TokenToUpdate The string to update.
> -  @param Offset        The field to get information about.
> +  @param Field         The field to get information about.
>  **/
>  VOID
>  EFIAPI
>  OemUpdateSmbiosInfo (
>    IN EFI_HII_HANDLE mHiiHandle,
>    IN EFI_STRING_ID TokenToUpdate,
> -  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
> +  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
>    )
>  {
>    ASSERT (FALSE);
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> index d03de12a820e..0cb56c53975e 100644
> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -439,8 +439,8 @@ AddSmbiosCacheTypeTable (
>      strings following the data fields.
>  
>    @param[out] Type4Record    The Type 4 structure to allocate and initialize
> -  @param[in]  ProcessorIndex The index of the processor socket
> -  @param[in]  Populated      Whether the specified processor socket is
> +  @param[in]  ProcessorIndex The index of the processor
> +  @param[in]  Populated      Whether the specified processor is
>                               populated.
>  
>    @retval EFI_SUCCESS          The Type 4 structure was successfully
> @@ -460,7 +460,7 @@ AllocateType4AndSetProcessorInformationStrings (
>    EFI_STRING_ID   SerialNumber;
>    EFI_STRING_ID   AssetTag;
>    EFI_STRING_ID   PartNumber;
> -  EFI_STRING      ProcessorSocketStr;
> +  EFI_STRING      ProcessorStr;
>    EFI_STRING      ProcessorManuStr;
>    EFI_STRING      ProcessorVersionStr;
>    EFI_STRING      SerialNumberStr;
> @@ -468,7 +468,7 @@ AllocateType4AndSetProcessorInformationStrings (
>    EFI_STRING      PartNumberStr;
>    CHAR8           *OptionalStrStart;
>    CHAR8           *StrStart;
> -  UINTN           ProcessorSocketStrLen;
> +  UINTN           ProcessorStrLen;
>    UINTN           ProcessorManuStrLen;
>    UINTN           ProcessorVersionStrLen;
>    UINTN           SerialNumberStrLen;
> @@ -497,14 +497,14 @@ AllocateType4AndSetProcessorInformationStrings (
>    SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorAssetTag, AssetTag);
>    SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorPartNumber, PartNumber);
>  
> -  // Processor Socket Designation
> +  // Processor Designation
>    StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
> -  ProcessorSocketStr = AllocateZeroPool (StringBufferSize);
> -  if (ProcessorSocketStr == NULL) {
> +  ProcessorStr = AllocateZeroPool (StringBufferSize);
> +  if (ProcessorStr == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  ProcessorSocketStrLen = UnicodeSPrint (ProcessorSocketStr, StringBufferSize,
> +  ProcessorStrLen = UnicodeSPrint (ProcessorStr, StringBufferSize,
>                                           L"CPU%02d", ProcessorIndex + 1);
>  
>    // Processor Manufacture
> @@ -528,7 +528,7 @@ AllocateType4AndSetProcessorInformationStrings (
>    PartNumberStrLen = StrLen (PartNumberStr);
>  
>    TotalSize = sizeof (SMBIOS_TABLE_TYPE4) +
> -              ProcessorSocketStrLen  + 1 +
> +              ProcessorStrLen        + 1 +
>                ProcessorManuStrLen    + 1 +
>                ProcessorVersionStrLen + 1 +
>                SerialNumberStrLen     + 1 +
> @@ -545,12 +545,12 @@ AllocateType4AndSetProcessorInformationStrings (
>  
>    OptionalStrStart = (CHAR8 *)(*Type4Record + 1);
>    UnicodeStrToAsciiStrS (
> -    ProcessorSocketStr,
> +    ProcessorStr,
>      OptionalStrStart,
> -    ProcessorSocketStrLen + 1
> +    ProcessorStrLen + 1
>      );
>  
> -  StrStart = OptionalStrStart + ProcessorSocketStrLen + 1;
> +  StrStart = OptionalStrStart + ProcessorStrLen + 1;
>    UnicodeStrToAsciiStrS (
>      ProcessorManuStr,
>      StrStart,
> @@ -586,7 +586,7 @@ AllocateType4AndSetProcessorInformationStrings (
>      );
>  
>  Exit:
> -  FreePool (ProcessorSocketStr);
> +  FreePool (ProcessorStr);
>    FreePool (ProcessorManuStr);
>    FreePool (ProcessorVersionStr);
>    FreePool (SerialNumberStr);
> @@ -618,7 +618,7 @@ AddSmbiosProcessorTypeTable (
>    UINT64                      *ProcessorId;
>    PROCESSOR_CHARACTERISTIC_FLAGS ProcessorCharacteristics;
>    OEM_MISC_PROCESSOR_DATA     MiscProcessorData;
> -  BOOLEAN                     SocketPopulated;
> +  BOOLEAN                     ProcessorPopulated;
>  
>    Type4Record         = NULL;
>  
> @@ -632,12 +632,12 @@ AddSmbiosProcessorTypeTable (
>    L2CacheHandle       = 0xFFFF;
>    L3CacheHandle       = 0xFFFF;
>  
> -  SocketPopulated = OemIsSocketPresent(ProcessorIndex);
> +  ProcessorPopulated = OemIsProcessorPresent (ProcessorIndex);
>  
>    Status = AllocateType4AndSetProcessorInformationStrings (
>               &Type4Record,
>               ProcessorIndex,
> -             SocketPopulated
> +             ProcessorPopulated
>               );
>    if (EFI_ERROR (Status)) {
>      return Status;
> @@ -649,7 +649,7 @@ AddSmbiosProcessorTypeTable (
>                                  &Type4Record->ProcessorCharacteristics,
>                                &MiscProcessorData);
>  
> -  if (SocketPopulated) {
> +  if (ProcessorPopulated) {
>      AddSmbiosCacheTypeTable (ProcessorIndex, &L1CacheHandle,
>                               &L2CacheHandle, &L3CacheHandle);
>    }
> @@ -713,7 +713,7 @@ ProcessorSubClassEntryPoint(
>    )
>  {
>    EFI_STATUS    Status;
> -  UINT32        SocketIndex;
> +  UINT32        ProcessorIndex;
>  
>    //
>    // Locate dependent protocols
> @@ -740,8 +740,8 @@ ProcessorSubClassEntryPoint(
>    //
>    // Add SMBIOS tables for populated sockets.
>    //
> -  for (SocketIndex = 0; SocketIndex < OemGetProcessorMaxSockets(); SocketIndex++) {
> -    Status = AddSmbiosProcessorTypeTable (SocketIndex);
> +  for (ProcessorIndex = 0; ProcessorIndex < OemGetMaxProcessors (); ProcessorIndex++) {
> +    Status = AddSmbiosProcessorTypeTable (ProcessorIndex);
>      if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_ERROR, "Add Processor Type Table Failed!  %r.\n", Status));
>        return Status;
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> index e6adbceba2d5..fc4dba319aad 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> @@ -23,27 +23,6 @@
>  
>  #include "SmbiosMisc.h"
>  
> -/**
> - * Returns the chassis type in SMBIOS format.
> - *
> - * @return Chassis type
> -**/
> -UINT8
> -GetChassisType (
> -  VOID
> -  )
> -{
> -  EFI_STATUS                      Status;
> -  UINT8                           ChassisType;
> -
> -  Status = OemGetChassisType (&ChassisType);
> -  if (EFI_ERROR (Status)) {
> -    return 0;
> -  }
> -
> -  return ChassisType;
> -}
> -

This function is outright deleted, not renamed.
Does this belong in another patch?

/
    Leif

>  /**
>    This function makes boot time changes to the contents of the
>    MiscChassisManufacturer (Type 3) record.
> @@ -80,8 +59,6 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
>    CONTAINED_ELEMENT               ContainedElements;
>    UINT8                           ExtendLength;
>  
> -  UINT8                           ChassisType;
> -
>    ExtendLength = 0;
>  
>    //
> @@ -165,10 +142,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
>  
>    SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE3) + ExtendLength + 1;
>  
> -  ChassisType = GetChassisType ();
> -  if (ChassisType != 0) {
> -    SmbiosRecord->Type  = ChassisType;
> -  }
> +  SmbiosRecord->Type = OemGetChassisType ();
>  
>    //ContainedElements
>    ASSERT (ContainedElementCount < 2);
> -- 
> 2.26.2
> 

Re: [PATCH v2 3/3] ArmPkg: Update OemGetChassisType function to return MISC_CHASSIS_TYPE

[Leif Lindholm]

On Sat, Feb 20, 2021 at 20:29:00 -0700, Rebecca Cran wrote:
> Update OemGetChassisType in OemMiscLib to return MISC_CHASSIS_TYPE
> instead of EFI_STATUS, which matches other OemMiscLib functions.
> 
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  ArmPkg/Include/Library/OemMiscLib.h                 |  8 +++-----
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 11 ++++-------
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index 31dfe7dac2a6..5f71b2ad48c7 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -128,14 +128,12 @@ OemGetMaxProcessors (
>  
>  /** Gets the type of chassis for the system.
>  
> -  @param ChassisType The type of the chassis.
> -
> -  @retval EFI_SUCCESS The chassis type was fetched successfully.
> +  @retval The type of the chassis.
>  **/
> -EFI_STATUS
> +MISC_CHASSIS_TYPE
>  EFIAPI
>  OemGetChassisType (
> -  OUT UINT8 *ChassisType
> +  VOID
>    );
>  
>  /** Returns whether the specified processor is present or not.
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index b02a568426dd..6b233742feb0 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -95,19 +95,16 @@ OemGetMaxProcessors (
>  
>  /** Gets the type of chassis for the system.
>  
> -  @param ChassisType The type of the chassis.
> -
> -  @retval EFI_SUCCESS The chassis type was fetched successfully.
> +  @retval The type of the chassis.
>  **/
> -EFI_STATUS
> +MISC_CHASSIS_TYPE
>  EFIAPI
>  OemGetChassisType (
> -  UINT8 *ChassisType
> +  VOID
>    )
>  {
>    ASSERT (FALSE);
> -  *ChassisType = MiscChassisTypeUnknown;
> -  return EFI_SUCCESS;
> +  return MiscChassisTypeUnknown;
>  }
>  
>  /** Returns whether the specified processor is present or not.
> -- 
> 2.26.2
> 

Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

[Rebecca Cran]

On 2/21/21 2:21 PM, Leif Lindholm wrote:
> On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:
>> -UINT8
>> -GetChassisType (
>> -  VOID
>> -  )
>> -{
>> -  EFI_STATUS                      Status;
>> -  UINT8                           ChassisType;
>> -
>> -  Status = OemGetChassisType (&ChassisType);
>> -  if (EFI_ERROR (Status)) {
>> -    return 0;
>> -  }
>> -
>> -  return ChassisType;
>> -}
>> -
> 
> This function is outright deleted, not renamed.
> Does this belong in another patch?

I think it belongs here: it was just a helper function for when 
OemGetChassisType returned EFI_STATUS. Since it now returns the same 
data as this function, we don't need it anymore.

-- 
Rebecca Cran

Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

[Leif Lindholm]

On Sun, Feb 21, 2021 at 16:04:31 -0700, Rebecca Cran wrote:
> On 2/21/21 2:21 PM, Leif Lindholm wrote:
> > On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:

(Adding necessary patch context back in - please don't strip out until
absolutely certain it is irrelevant to conversation.)

diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
index e6adbceba2d5..fc4dba319aad 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
@@ -23,27 +23,6 @@

#include "SmbiosMisc.h"

-/**
- * Returns the chassis type in SMBIOS format.
- *
- * @return Chassis type
-**/

> > > -UINT8
> > > -GetChassisType (
> > > -  VOID
> > > -  )
> > > -{
> > > -  EFI_STATUS                      Status;
> > > -  UINT8                           ChassisType;
> > > -
> > > -  Status = OemGetChassisType (&ChassisType);
> > > -  if (EFI_ERROR (Status)) {
> > > -    return 0;
> > > -  }
> > > -
> > > -  return ChassisType;
> > > -}
> > > -
> > 
> > This function is outright deleted, not renamed.
> > Does this belong in another patch?
> 
> I think it belongs here: it was just a helper function for when
> OemGetChassisType returned EFI_STATUS. Since it now returns the same data as
> this function, we don't need it anymore.

This patch, according to its title and commit message, does renaming.

The refactoring should then be moved over to 3/3, together with this
hunk:

@@ -165,10 +142,7 @@
SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)

   SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE3) + SmbiosRecord->ExtendLength + 1;

-  ChassisType = GetChassisType ();
-  if (ChassisType != 0) {
-    SmbiosRecord->Type  = ChassisType;
-  }
+  SmbiosRecord->Type = OemGetChassisType ();

   //ContainedElements
   ASSERT (ContainedElementCount < 2);

The current layout means this code (in 2/3) relies on the API change
introduced in 3/3.

/
    Leif

> 
> -- 
> Rebecca Cran

Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

[Rebecca Cran]

On 2/22/21 5:35 AM, Leif Lindholm wrote:
> On Sun, Feb 21, 2021 at 16:04:31 -0700, Rebecca Cran wrote:
>> On 2/21/21 2:21 PM, Leif Lindholm wrote:
>>> On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:
> 
> (Adding necessary patch context back in - please don't strip out until
> absolutely certain it is irrelevant to conversation.)
> 
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> index e6adbceba2d5..fc4dba319aad 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> @@ -23,27 +23,6 @@
> 
> #include "SmbiosMisc.h"
> 
> -/**
> - * Returns the chassis type in SMBIOS format.
> - *
> - * @return Chassis type
> -**/
> 
>>>> -UINT8
>>>> -GetChassisType (
>>>> -  VOID
>>>> -  )
>>>> -{
>>>> -  EFI_STATUS                      Status;
>>>> -  UINT8                           ChassisType;
>>>> -
>>>> -  Status = OemGetChassisType (&ChassisType);
>>>> -  if (EFI_ERROR (Status)) {
>>>> -    return 0;
>>>> -  }
>>>> -
>>>> -  return ChassisType;
>>>> -}
>>>> -
>>>
>>> This function is outright deleted, not renamed.
>>> Does this belong in another patch?
>>
>> I think it belongs here: it was just a helper function for when
>> OemGetChassisType returned EFI_STATUS. Since it now returns the same data as
>> this function, we don't need it anymore.
> 
> This patch, according to its title and commit message, does renaming.
> 
> The refactoring should then be moved over to 3/3, together with this
> hunk:
> 
> @@ -165,10 +142,7 @@
> SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
> 
>     SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE3) + SmbiosRecord->ExtendLength + 1;
> 
> -  ChassisType = GetChassisType ();
> -  if (ChassisType != 0) {
> -    SmbiosRecord->Type  = ChassisType;
> -  }
> +  SmbiosRecord->Type = OemGetChassisType ();
> 
>     //ContainedElements
>     ASSERT (ContainedElementCount < 2);
> 
> The current layout means this code (in 2/3) relies on the API change
> introduced in 3/3.

Sorry - I did catch this before sending the series but apparently forgot 
to regenerate the set. I agree, this breaks bisection and needs moved to 
3/3.

-- 
Rebecca Cran