public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Rebecca Cran <rebecca@nuviainc.com>
Cc: devel@edk2.groups.io, Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib
Date: Sun, 21 Feb 2021 21:21:53 +0000	[thread overview]
Message-ID: <20210221212153.GO1664@vanye> (raw)
In-Reply-To: <20210221032900.6485-3-rebecca@nuviainc.com>

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
> 

  reply	other threads:[~2021-02-21 21:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-21  3:28 [PATCH v2 0/3] ArmPkg: Fix several issues in OemMiscLib Rebecca Cran
2021-02-21  3:28 ` [PATCH v2 1/3] ArmPkg: Fix ordering of return type and EFIAPI specifier " Rebecca Cran
2021-02-21 20:48   ` Leif Lindholm
2021-02-21  3:28 ` [PATCH v2 2/3] ArmPkg: Rename some functions and parameters " Rebecca Cran
2021-02-21 21:21   ` Leif Lindholm [this message]
2021-02-21 23:04     ` Rebecca Cran
2021-02-22 12:35       ` Leif Lindholm
2021-02-22 14:33         ` Rebecca Cran
2021-02-21  3:29 ` [PATCH v2 3/3] ArmPkg: Update OemGetChassisType function to return MISC_CHASSIS_TYPE Rebecca Cran
2021-02-21 21:36   ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210221212153.GO1664@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox