public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures
@ 2019-09-12  6:08 Kubacki, Michael A
  2019-09-12  8:30 ` Chaganty, Rangasai V
  0 siblings, 1 reply; 3+ messages in thread
From: Kubacki, Michael A @ 2019-09-12  6:08 UTC (permalink / raw)
  To: devel; +Cc: Dandan Bi, Sai Chaganty, Liming Gao

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures
  2019-09-12  6:08 [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures Kubacki, Michael A
@ 2019-09-12  8:30 ` Chaganty, Rangasai V
  2019-09-12 18:46   ` Kubacki, Michael A
  0 siblings, 1 reply; 3+ messages in thread
From: Chaganty, Rangasai V @ 2019-09-12  8:30 UTC (permalink / raw)
  To: Kubacki, Michael A, devel@edk2.groups.io; +Cc: Bi, Dandan, Gao, Liming

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures
  2019-09-12  8:30 ` Chaganty, Rangasai V
@ 2019-09-12 18:46   ` Kubacki, Michael A
  0 siblings, 0 replies; 3+ messages in thread
From: Kubacki, Michael A @ 2019-09-12 18:46 UTC (permalink / raw)
  To: Chaganty, Rangasai V, devel@edk2.groups.io; +Cc: Bi, Dandan, Gao, Liming

While I'm all for improvements throughout the file, the scope of this change is just to fix the GCC failures
and make general style improvements to impacted lines. I'll send a patch V2 with the DEBUG
macro updated to DEBUG_ERROR. Anything further peripheral cleanup is a slippery slope for this patch.

Thanks,
Michael

> -----Original Message-----
> From: Chaganty, Rangasai V
> Sent: Thursday, September 12, 2019 1:31 AM
> To: Kubacki, Michael A <michael.a.kubacki@intel.com>;
> devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix
> GCC Build Failures
> 
> 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-12 18:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-12  6:08 [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures Kubacki, Michael A
2019-09-12  8:30 ` Chaganty, Rangasai V
2019-09-12 18:46   ` Kubacki, Michael A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox