From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: michael.a.kubacki@intel.com) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by groups.io with SMTP; Thu, 12 Sep 2019 11:46:58 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Sep 2019 11:46:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="269175603" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by orsmga001.jf.intel.com with ESMTP; 12 Sep 2019 11:46:57 -0700 Received: from orsmsx155.amr.corp.intel.com (10.22.240.21) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 12 Sep 2019 11:46:57 -0700 Received: from orsmsx121.amr.corp.intel.com ([169.254.10.57]) by ORSMSX155.amr.corp.intel.com ([169.254.7.185]) with mapi id 14.03.0439.000; Thu, 12 Sep 2019 11:46:57 -0700 From: "Kubacki, Michael A" To: "Chaganty, Rangasai V" , "devel@edk2.groups.io" CC: "Bi, Dandan" , "Gao, Liming" Subject: Re: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures Thread-Topic: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures Thread-Index: AQHVaTCK0KrgyVInhEC0dCib7IIyC6cnseRggACvMJA= Date: Thu, 12 Sep 2019 18:46:56 +0000 Message-ID: <49AB4ACB9627B8468F29D589A27B745588AC129C@ORSMSX121.amr.corp.intel.com> References: <20190912060834.31356-1-michael.a.kubacki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTRlZDYxNjQtYWU1ZS00OGEyLWJjMzYtYjUwYWMwYmFhYmY5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYXdJQ1JqdjdMaUFHQkFyUk03b0dSN29WejNQUVNoSU8yUlJrdmYyWEdJb05wVlpjZE93K1FPVnRVaXA4T2R0UyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Return-Path: michael.a.kubacki@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable While I'm all for improvements throughout the file, the scope of this chang= e 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 slip= pery slope for this patch. Thanks, Michael > -----Original Message----- > From: Chaganty, Rangasai V > Sent: Thursday, September 12, 2019 1:31 AM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Bi, Dandan ; Gao, Liming > Subject: RE: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix > GCC Build Failures >=20 > 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 >=20 > -----Original Message----- > From: Kubacki, Michael A > Sent: Wednesday, September 11, 2019 11:09 PM > To: devel@edk2.groups.io > Cc: Bi, Dandan ; Chaganty, Rangasai V > ; Gao, Liming > Subject: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC > Build Failures >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2059 >=20 > 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' >=20 > 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. >=20 > All future contributions to AdvancedFeaturePkg must successfully build in > GCC after this change. >=20 > Cc: Dandan Bi > Cc: Sai Chaganty > Cc: Liming Gao > Signed-off-by: Michael Kubacki > --- > 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(-) >=20 > 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. >=20 > -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -154,7 +154,7 @@ UpdateDeviceSsdtTable ( > IoRsc->BaseAddressMax =3D PcdGet16(PcdIpmiIoBaseAddress); > } > } > - > + > return EFI_SUCCESS; > } >=20 > @@ -202,7 +202,7 @@ BmcAcpiEntryPoint ( > // > // Locate the firmware volume protocol > // > - Status =3D LocateSupportProtocol (&gEfiFirmwareVolume2ProtocolGuid, > &FwVol, 1); > + Status =3D LocateSupportProtocol (&gEfiFirmwareVolume2ProtocolGuid, > + (VOID **) &FwVol, 1); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -216,7 +216,15 @@ BmcAcpiEntryPoint ( > while (!EFI_ERROR (Status)) { > CurrentTable =3D NULL; >=20 > - Status =3D FwVol->ReadSection (FwVol, &gEfiCallerIdGuid, > EFI_SECTION_RAW, Instance, &CurrentTable, (UINTN *) &Size, &FvStatus); > + Status =3D 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. >=20 > -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -27,7 +27,7 @@ Routine Description: >=20 > Arguments: >=20 > -Returns: > +Returns: > Status >=20 > --*/ > @@ -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. >=20 > @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. >=20 > @retval EFI_SUCCESS Indicates that Ipmi initialization completed > successfully. > + @retval Others Indicates that Ipmi initialization could not com= plete > successfully. > **/ > EFI_STATUS > EFIAPI > @@ -85,11 +86,11 @@ PeimIpmiInterfaceInit ( > BOOLEAN UpdateMode; > EFI_STATUS Status; >=20 > - DEBUG((EFI_D_ERROR,"IPMI Peim:Get BMC Device Id\n")); > + DEBUG ((DEBUG_INFO, "IPMI Peim:Get BMC Device Id\n")); >=20 > // > // Get the Device ID and check if the system is in Force Update mode. > // > Status =3D 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. >=20 > -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -93,11 +93,12 @@ Arguments: > SystemTable - Pointer to the System Table >=20 > Returns: > - EFI_SUCCESS - Protocol successfully started and installed. > + @retval EFI_SUCCESS Protocol successfully started and instal= led. > + @retval EFI_OUT_OF_RESOURCES The event could not be allocated. >=20 > --*/ > { > - EFI_STATUS Status; > + EFI_STATUS Status; >=20 > Status =3D gBS->CreateEvent ( > EVT_SIGNAL_EXIT_BOOT_SERVICES, @@ -107,5 +108,5 @@ > Returns: > &mExitBootServicesEvent > ); >=20 > - 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. >=20 > -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -70,7 +70,7 @@ GetSOLStatus ( > if (Status =3D=3D EFI_SUCCESS) { > *Data =3D GetConfigurationParametersResponse.ParameterData[0]; > } > - > + > return Status; > } >=20 > @@ -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 =3D EFI_SUCCESS; > UINT8 Channel; > BOOLEAN Enabled =3D FALSE; > - BOOLEAN SOLEnabled =3D FALSE; > - > + > for (Channel =3D 1; Channel <=3D PcdGet8(PcdMaxSOLChannels); Channel++= ) { > Status =3D GetSOLStatus (Channel, > IPMI_SOL_CONFIGURATION_PARAMETER_SOL_ENABLE, &Enabled); > if (Status =3D=3D EFI_SUCCESS) { > - if (Enabled =3D=3D TRUE) { > - SOLEnabled =3D 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