public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: <devel@edk2.groups.io>
Cc: Sami Mujawar <sami.mujawar@arm.com>, <ard.biesheuvel@arm.com>,
	<leif@nuviainc.com>, <thomas.abraham@arm.com>,
	<Matteo.Carlini@arm.com>, <Ben.Adderson@arm.com>, <nd@arm.com>
Subject: [PATCH edk2-platforms v1 1/6] Platform/ARM: FVP: Remove usage of function macros
Date: Fri, 22 Jan 2021 13:29:30 +0000	[thread overview]
Message-ID: <20210122132935.22484-2-sami.mujawar@arm.com> (raw)
In-Reply-To: <20210122132935.22484-1-sami.mujawar@arm.com>

The Configuration Manager code for FVP uses function macros to
prepare and return CM objects.
However, according to section 5.5.2.1 of the edk2 coding standard,
use of functional macros is not allowed.
See https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/
5_source_files/55_preprocessor_directives#5-5-2-1-functional-macros-are-
generally-discouraged

Therefore, replace the function macros with static helper functions.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 347 ++++++++++++++------
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h |  73 +---
 2 files changed, 266 insertions(+), 154 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index a48eb0285c60177078324bdbcd8fa29c308e6a13..3298f9f9f79a2b23f181d3e961298f1c366976ee 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -1,7 +1,7 @@
 /** @file
   Configuration Manager Dxe
 
-  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -219,6 +219,98 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
   }
 };
 
+/** A helper function for returning the Configuration Manager Objects.
+
+  @param [in]       CmObjectId     The Configuration Manager Object ID.
+  @param [in]       Object         Pointer to the Object(s).
+  @param [in]       ObjectSize     Total size of the Object(s).
+  @param [in]       ObjectCount    Number of Objects.
+  @param [in, out]  CmObjectDesc   Pointer to the Configuration Manager Object
+                                   descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObject (
+  IN  CONST CM_OBJECT_ID                CmObjectId,
+  IN        VOID                *       Object,
+  IN  CONST UINTN                       ObjectSize,
+  IN  CONST UINTN                       ObjectCount,
+  IN  OUT   CM_OBJ_DESCRIPTOR   * CONST CmObjectDesc
+  )
+{
+  CmObjectDesc->ObjectId = CmObjectId;
+  CmObjectDesc->Size = ObjectSize;
+  CmObjectDesc->Data = (VOID*)Object;
+  CmObjectDesc->Count = ObjectCount;
+  DEBUG ((
+    DEBUG_INFO,
+    "INFO: CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
+    CmObjectId,
+    CmObjectDesc->Data,
+    CmObjectDesc->Size,
+    CmObjectDesc->Count
+    ));
+  return EFI_SUCCESS;
+}
+
+/** A helper function for returning the Configuration Manager Objects that
+    match the token.
+
+  @param [in]  This               Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId         The Configuration Manager Object ID.
+  @param [in]  Object             Pointer to the Object(s).
+  @param [in]  ObjectSize         Total size of the Object(s).
+  @param [in]  ObjectCount        Number of Objects.
+  @param [in]  Token              A token identifying the object.
+  @param [in]  HandlerProc        A handler function to search the object
+                                  referenced by the token.
+  @param [in, out]  CmObjectDesc  Pointer to the Configuration Manager Object
+                                  descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleCmObjectRefByToken (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  * CONST This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN        VOID                                  *       Object,
+  IN  CONST UINTN                                         ObjectSize,
+  IN  CONST UINTN                                         ObjectCount,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  IN  CONST CM_OBJECT_HANDLER_PROC                        HandlerProc,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     * CONST CmObjectDesc
+  )
+{
+  EFI_STATUS  Status;
+  CmObjectDesc->ObjectId = CmObjectId;
+  if (Token == CM_NULL_TOKEN) {
+    CmObjectDesc->Size = ObjectSize;
+    CmObjectDesc->Data = (VOID*)Object;
+    CmObjectDesc->Count = ObjectCount;
+    Status = EFI_SUCCESS;
+  } else {
+    Status = HandlerProc (This, CmObjectId, Token, CmObjectDesc);
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "INFO: Token = 0x%p, CmObjectId = %x, Ptr = 0x%p, Size = %d, Count = %d\n",
+    (VOID*)Token,
+    CmObjectId,
+    CmObjectDesc->Data,
+    CmObjectDesc->Size,
+    CmObjectDesc->Count
+    ));
+  return Status;
+}
+
 /** Initialize the platform configuration repository.
 
   @param [in]  This        Pointer to the Configuration Manager Protocol.
@@ -304,28 +396,36 @@ GetStandardNameSpaceObject (
   EFI_STATUS                      Status;
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
 
-  Status = EFI_SUCCESS;
   if ((This == NULL) || (CmObject == NULL)) {
     ASSERT (This != NULL);
     ASSERT (CmObject != NULL);
     return EFI_INVALID_PARAMETER;
   }
+
+  Status = EFI_NOT_FOUND;
   PlatformRepo = This->PlatRepoInfo;
 
   switch (GET_CM_OBJECT_ID (CmObjectId)) {
-    HANDLE_CM_OBJECT (
-      EStdObjCfgMgrInfo,
-      CmObjectId,
-      PlatformRepo->CmInfo,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EStdObjAcpiTableList,
-      CmObjectId,
-      PlatformRepo->CmAcpiTableList,
-      (sizeof (PlatformRepo->CmAcpiTableList) /
-        sizeof (PlatformRepo->CmAcpiTableList[0]))
-      );
+    case EStdObjCfgMgrInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->CmInfo,
+                 sizeof (PlatformRepo->CmInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EStdObjAcpiTableList:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->CmAcpiTableList,
+                 sizeof (PlatformRepo->CmAcpiTableList),
+                 ARRAY_SIZE (PlatformRepo->CmAcpiTableList),
+                 CmObject
+                 );
+      break;
+
     default: {
       Status = EFI_NOT_FOUND;
       DEBUG ((
@@ -366,100 +466,149 @@ GetArmNameSpaceObject (
   EFI_STATUS                        Status;
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
 
-  Status = EFI_SUCCESS;
   if ((This == NULL) || (CmObject == NULL)) {
     ASSERT (This != NULL);
     ASSERT (CmObject != NULL);
     return EFI_INVALID_PARAMETER;
   }
+
+  Status = EFI_NOT_FOUND;
   PlatformRepo = This->PlatRepoInfo;
 
   switch (GET_CM_OBJECT_ID (CmObjectId)) {
-    HANDLE_CM_OBJECT (
-      EArmObjBootArchInfo,
-      CmObjectId,
-      PlatformRepo->BootArchInfo,
-      1
-      );
+    case EArmObjBootArchInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->BootArchInfo,
+                 sizeof (PlatformRepo->BootArchInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
 #ifdef HEADLESS_PLATFORM
-    HANDLE_CM_OBJECT (
-      EArmObjFixedFeatureFlags,
-      CmObjectId,
-      PlatformRepo->FixedFeatureFlags,
-      1
-      );
+    case EArmObjFixedFeatureFlags:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->FixedFeatureFlags,
+                 sizeof (PlatformRepo->FixedFeatureFlags),
+                 1,
+                 CmObject
+                 );
+      break;
 #endif
-    HANDLE_CM_OBJECT (
-      EArmObjPowerManagementProfileInfo,
-      CmObjectId,
-      PlatformRepo->PmProfileInfo,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjGenericTimerInfo,
-      CmObjectId,
-      PlatformRepo->GenericTimerInfo,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjPlatformGenericWatchdogInfo,
-      CmObjectId,
-      PlatformRepo->Watchdog,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjPlatformGTBlockInfo,
-      CmObjectId,
-      PlatformRepo->GTBlockInfo,
-      (sizeof (PlatformRepo->GTBlockInfo) /
-         sizeof (PlatformRepo->GTBlockInfo[0]))
-      );
-    HANDLE_CM_OBJECT_REF_BY_TOKEN (
-      EArmObjGTBlockTimerFrameInfo,
-      CmObjectId,
-      PlatformRepo->GTBlock0TimerInfo,
-      (sizeof (PlatformRepo->GTBlock0TimerInfo) /
-         sizeof (PlatformRepo->GTBlock0TimerInfo[0])),
-      Token,
-      GetGTBlockTimerFrameInfo
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjGicCInfo,
-      CmObjectId,
-      PlatformRepo->GicCInfo,
-      (sizeof (PlatformRepo->GicCInfo) /
-         sizeof (PlatformRepo->GicCInfo[0]))
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjGicDInfo,
-      CmObjectId,
-      PlatformRepo->GicDInfo,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjGicRedistributorInfo,
-      CmObjectId,
-      PlatformRepo->GicRedistInfo,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjSerialConsolePortInfo,
-      CmObjectId,
-      PlatformRepo->SpcrSerialPort,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjSerialDebugPortInfo,
-      CmObjectId,
-      PlatformRepo->DbgSerialPort,
-      1
-      );
-    HANDLE_CM_OBJECT (
-      EArmObjGicItsInfo,
-      CmObjectId,
-      PlatformRepo->GicItsInfo,
-      1
-      );
+    case EArmObjPowerManagementProfileInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->PmProfileInfo,
+                 sizeof (PlatformRepo->PmProfileInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGenericTimerInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->GenericTimerInfo,
+                 sizeof (PlatformRepo->GenericTimerInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjPlatformGenericWatchdogInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->Watchdog,
+                 sizeof (PlatformRepo->Watchdog),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjPlatformGTBlockInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 PlatformRepo->GTBlockInfo,
+                 sizeof (PlatformRepo->GTBlockInfo),
+                 ARRAY_SIZE (PlatformRepo->GTBlockInfo),
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGTBlockTimerFrameInfo:
+      Status = HandleCmObjectRefByToken (
+                 This,
+                 CmObjectId,
+                 PlatformRepo->GTBlock0TimerInfo,
+                 sizeof (PlatformRepo->GTBlock0TimerInfo),
+                 ARRAY_SIZE (PlatformRepo->GTBlock0TimerInfo),
+                 Token,
+                 GetGTBlockTimerFrameInfo,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGicCInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 PlatformRepo->GicCInfo,
+                 sizeof (PlatformRepo->GicCInfo),
+                 ARRAY_SIZE (PlatformRepo->GicCInfo),
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGicDInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->GicDInfo,
+                 sizeof (PlatformRepo->GicDInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGicRedistributorInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->GicRedistInfo,
+                 sizeof (PlatformRepo->GicRedistInfo),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjSerialConsolePortInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->SpcrSerialPort,
+                 sizeof (PlatformRepo->SpcrSerialPort),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjSerialDebugPortInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->DbgSerialPort,
+                 sizeof (PlatformRepo->DbgSerialPort),
+                 1,
+                 CmObject
+                 );
+      break;
+
+    case EArmObjGicItsInfo:
+      Status = HandleCmObject (
+                 CmObjectId,
+                 &PlatformRepo->GicItsInfo,
+                 sizeof (PlatformRepo->GicItsInfo),
+                 1,
+                 CmObject
+                 );
+      break;
 
     default: {
       Status = EFI_NOT_FOUND;
diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
index 6fb959ab7ae2c77f750781d05660957488fae8cd..c25ba9fac52fe049236948185dfc052c44a71e4b 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -52,61 +52,24 @@ extern CHAR8  dsdt_aml_code[];
     EnergyEfficiency          /* UINT8   ProcessorPowerEfficiencyClass*/ \
     }
 
-/** A helper macro for returning configuration manager objects
-*/
-#define HANDLE_CM_OBJECT(ObjId, CmObjectId, Object, ObjectCount)      \
-  case ObjId: {                                                       \
-    CmObject->ObjectId = CmObjectId;                                  \
-    CmObject->Size = sizeof (Object);                                 \
-    CmObject->Data = (VOID*)&Object;                                  \
-    CmObject->Count = ObjectCount;                                    \
-    DEBUG ((                                                          \
-      DEBUG_INFO,                                                     \
-      #CmObjectId ": Ptr = 0x%p, Size = %d, Count = %d\n",            \
-      CmObject->Data,                                                 \
-      CmObject->Size,                                                 \
-      CmObject->Count                                                 \
-      ));                                                             \
-    break;                                                            \
-  }
+/** A function that prepares Configuration Manager Objects for returning.
 
-/** A helper macro for returning configuration manager objects
-    referenced by token
-*/
-#define HANDLE_CM_OBJECT_REF_BY_TOKEN(                                      \
-          ObjId,                                                            \
-          CmObjectId,                                                       \
-          Object,                                                           \
-          ObjectCount,                                                      \
-          Token,                                                            \
-          HandlerProc                                                       \
-          )                                                                 \
-  case ObjId: {                                                             \
-    CmObject->ObjectId = CmObjectId;                                        \
-    if (Token == CM_NULL_TOKEN) {                                           \
-      CmObject->Size = sizeof (Object);                                     \
-      CmObject->Data = (VOID*)&Object;                                      \
-      CmObject->Count = ObjectCount;                                        \
-      DEBUG ((                                                              \
-        DEBUG_INFO,                                                         \
-        #CmObjectId ": Ptr = 0x%p, Size = %d, Count = %d\n",                \
-        CmObject->Data,                                                     \
-        CmObject->Size,                                                     \
-        CmObject->Count                                                     \
-        ));                                                                 \
-    } else {                                                                \
-      Status = HandlerProc (This, CmObjectId, Token, CmObject);             \
-      DEBUG ((                                                              \
-        DEBUG_INFO,                                                         \
-        #CmObjectId ": Token = 0x%p, Ptr = 0x%p, Size = %d, Count = %d\n",  \
-        (VOID*)Token,                                                       \
-        CmObject->Data,                                                     \
-        CmObject->Size,                                                     \
-        CmObject->Count                                                     \
-        ));                                                                 \
-    }                                                                       \
-    break;                                                                  \
-  }
+  @param [in]  This        Pointer to the Configuration Manager Protocol.
+  @param [in]  CmObjectId  The Configuration Manager Object ID.
+  @param [in]  Token       A token for identifying the object.
+  @param [out] CmObject    Pointer to the Configuration Manager Object
+                           descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS           Success.
+  @retval EFI_INVALID_PARAMETER A parameter is invalid.
+  @retval EFI_NOT_FOUND         The required object information is not found.
+**/
+typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  * CONST This,
+  IN  CONST CM_OBJECT_ID                                  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN                               Token,
+  IN  OUT   CM_OBJ_DESCRIPTOR                     * CONST CmObject
+  );
 
 /** The number of CPUs
 */
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


  reply	other threads:[~2021-01-22 13:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22 13:29 [PATCH edk2-platforms v1 0/6] Fix platform support for dynamic tables framework Sami Mujawar
2021-01-22 13:29 ` Sami Mujawar [this message]
2021-01-22 13:29 ` [PATCH edk2-platforms v1 2/6] Platform/ARM: Juno: Remove usage of function macros Sami Mujawar
2021-01-22 13:29 ` [PATCH edk2-platforms v1 3/6] Platform/ARM: FVP: Fix serial port usage Sami Mujawar
2021-01-22 13:29 ` [PATCH edk2-platforms v1 4/6] Platform/ARM: Juno: Remove PL050 KMI description from DSDT Sami Mujawar
2021-01-22 13:29 ` [PATCH edk2-platforms v1 5/6] Platform/ARM: Juno: Remove Serial port SSDT Sami Mujawar
2021-01-22 13:29 ` [PATCH edk2-platforms v1 6/6] Platform/ARM: FVP: Remove Serial port description from DSDT Sami Mujawar
2021-01-25 12:56 ` [PATCH edk2-platforms v1 0/6] Fix platform support for dynamic tables framework 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=20210122132935.22484-2-sami.mujawar@arm.com \
    --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