Re: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

["Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>]

Two comments on SolStatus.c:
1. "SolEnabled" seems more appropriate variable name than just "Enabled. Should we consider renaming the variable name?
2. Please replace EFI_D_ERROR with DEBUG_ERROR

-----Original Message-----
From: Kubacki, Michael A 
Sent: Wednesday, September 11, 2019 11:09 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2059

These build failures can be reproduced simply by building the AdvancedFeaturePkg.dsc file in GCC5. To build the whole package DSC (not pull individual features into other packages), set the WORKSPACE variable to the edk2 directory in the workspace as is done by executing edksetup.sh then create the PACKAGES_PATH variable and add the Platform/Intel and Silicon/Intel directories to the variable value. Then start the build of AdvancedFeaturePkg.dsc:
  'build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc'

This change corrects the following issues reported by GCC:
  * BmcAcpi.c - Cast the pointer actual parameter types passed to
                functions to the formal parameter type.
  * OsWdt.c - Return the Status variable as the function return value
              in DriverInit ().
  * PeiIpmiInit.c - Return the Status variable as the function return
                    value in PeimIpmiInterfaceInit ()
  * SolStatus.c - Remove the unused variable SOLEnabled. The variable
                  was not removed after a code refactoring.

All future contributions to AdvancedFeaturePkg must successfully build in GCC after this change.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c      | 16 ++++++++++++----
 Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c | 15 ++++++++-------
 Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c          |  9 +++++----
 Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c  | 14 +++++---------
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
index 3b330da160..990b4b9e83 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
@@ -1,7 +1,7 @@
 /** @file
   BMC ACPI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -154,7 +154,7 @@ UpdateDeviceSsdtTable (
       IoRsc->BaseAddressMax = PcdGet16(PcdIpmiIoBaseAddress);
     }
   }
-  
+
   return EFI_SUCCESS;
 }
 
@@ -202,7 +202,7 @@ BmcAcpiEntryPoint (
   //
   // Locate the firmware volume protocol
   //
-  Status = LocateSupportProtocol (&gEfiFirmwareVolume2ProtocolGuid, &FwVol, 1);
+  Status = LocateSupportProtocol (&gEfiFirmwareVolume2ProtocolGuid, 
+ (VOID **) &FwVol, 1);
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -216,7 +216,15 @@ BmcAcpiEntryPoint (
   while (!EFI_ERROR (Status)) {
     CurrentTable = NULL;
 
-    Status = FwVol->ReadSection (FwVol, &gEfiCallerIdGuid, EFI_SECTION_RAW, Instance, &CurrentTable, (UINTN *) &Size, &FvStatus);
+    Status = FwVol->ReadSection (
+                      FwVol,
+                      &gEfiCallerIdGuid,
+                      EFI_SECTION_RAW,
+                      Instance,
+                      (VOID **) &CurrentTable,
+                      (UINTN *) &Size,
+                      &FvStatus
+                      );
     if (!EFI_ERROR (Status)) {
       //
       // Perform any table specific updates.
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
index 8245aac8e9..062d20c44e 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
@@ -1,7 +1,7 @@
 /** @file
-    IPMI stack initialization in PEI.
+  IPMI stack initialization in PEI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -27,7 +27,7 @@ Routine Description:
 
 Arguments:
 
-Returns: 
+Returns:
   Status
 
 --*/
@@ -35,7 +35,7 @@ Returns:
   EFI_STATUS                   Status;
   IPMI_GET_DEVICE_ID_RESPONSE  BmcInfo;
   UINT32                       Retries;
-  
+
   //
   // Set up a loop to retry for up to 30 seconds. Calculate retries not timeout
   // so that in case KCS is not enabled and EfiIpmiSendCommand() returns @@ -71,9 +71,10 @@ Returns:
   The entry point of the Ipmi PEIM.
 
   @param  FileHandle  Handle of the file being invoked.
-  @param  PeiServices Describes the list of possible PEI Services. 
+  @param  PeiServices Describes the list of possible PEI Services.
 
   @retval EFI_SUCCESS   Indicates that Ipmi initialization completed successfully.
+  @retval Others        Indicates that Ipmi initialization could not complete successfully.
 **/
 EFI_STATUS
 EFIAPI
@@ -85,11 +86,11 @@ PeimIpmiInterfaceInit (
   BOOLEAN      UpdateMode;
   EFI_STATUS   Status;
 
-  DEBUG((EFI_D_ERROR,"IPMI Peim:Get BMC Device Id\n"));
+  DEBUG ((DEBUG_INFO, "IPMI Peim:Get BMC Device Id\n"));
 
   //
   // Get the Device ID and check if the system is in Force Update mode.
   //
   Status = GetDeviceId (&UpdateMode);
-  return EFI_SUCCESS;
+  return Status;
 }
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c b/Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c
index c5612d4b6d..25139eadba 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c
@@ -1,7 +1,7 @@
 /** @file
   IPMI Os watchdog timer Driver.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -93,11 +93,12 @@ Arguments:
   SystemTable - Pointer to the System Table
 
 Returns:
-  EFI_SUCCESS     - Protocol successfully started and installed.
+  @retval EFI_SUCCESS           Protocol successfully started and installed.
+  @retval EFI_OUT_OF_RESOURCES  The event could not be allocated.
 
 --*/
 {
-  EFI_STATUS                     Status;
+  EFI_STATUS  Status;
 
   Status = gBS->CreateEvent (
                   EVT_SIGNAL_EXIT_BOOT_SERVICES, @@ -107,5 +108,5 @@ Returns:
                   &mExitBootServicesEvent
                   );
 
-  return EFI_SUCCESS;
+  return Status;
 }
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c b/Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c
index 69479bdbf5..36de0604ee 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c
@@ -1,7 +1,7 @@
 /** @file
   IPMI Serial Over Lan Driver.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -70,7 +70,7 @@ GetSOLStatus (
   if (Status == EFI_SUCCESS) {
     *Data = GetConfigurationParametersResponse.ParameterData[0];
   }
-  
+
   return Status;
 }
 
@@ -132,11 +132,11 @@ SolStatusEntryPoint (
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 /*++
-  
+
   Routine Description:
     This is the standard EFI driver point. This function intitializes
     the private data required for creating SOL Status Driver.
-    
+
   Arguments:
     ImageHandle     - Handle for the image of this driver
     SystemTable     - Pointer to the EFI System Table
@@ -150,14 +150,10 @@ SolStatusEntryPoint (
   EFI_STATUS  Status = EFI_SUCCESS;
   UINT8       Channel;
   BOOLEAN     Enabled = FALSE;
-  BOOLEAN     SOLEnabled = FALSE;
-  
+
   for (Channel = 1; Channel <= PcdGet8(PcdMaxSOLChannels); Channel++) {
     Status = GetSOLStatus (Channel, IPMI_SOL_CONFIGURATION_PARAMETER_SOL_ENABLE, &Enabled);
     if (Status == EFI_SUCCESS) {
-      if (Enabled == TRUE) {
-        SOLEnabled = TRUE;
-	    }  
       DEBUG ((EFI_D_ERROR, "SOL enabling status for channel %x is %x\n", Channel, Enabled));
     } else {
       DEBUG ((EFI_D_ERROR, "Failed to get channel %x SOL status from BMC!, status is %x\n", Channel, Status));
--
2.16.2.windows.1