From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from huawei.com (huawei.com [45.249.212.35]) by mx.groups.io with SMTP id smtpd.web10.7467.1591710009760979636 for ; Tue, 09 Jun 2020 06:40:12 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.35, mailfrom: huangming23@huawei.com) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 2BA104625FED3819F454; Tue, 9 Jun 2020 21:40:02 +0800 (CST) Received: from [127.0.0.1] (10.78.51.60) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.487.0; Tue, 9 Jun 2020 21:39:52 +0800 Subject: Re: [PATCH edk2-platforms v1 1/2] Silicon/Hisilicon/Smbios: correct coding style issue in type 9 To: Leif Lindholm CC: , , , , , , , References: <1591625397-82711-1-git-send-email-huangming23@huawei.com> <1591625397-82711-2-git-send-email-huangming23@huawei.com> <20200608154116.GS28566@vanye> From: "Ming Huang" Message-ID: Date: Tue, 9 Jun 2020 21:39:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20200608154116.GS28566@vanye> X-Originating-IP: [10.78.51.60] X-CFilter-Loop: Reflected Content-Type: text/plain; charset="gbk" Content-Transfer-Encoding: quoted-printable =D4=DA 2020/6/8 23:41, Leif Lindholm =D0=B4=B5=C0: > On Mon, Jun 08, 2020 at 22:09:56 +0800, Ming Huang wrote: >> This patch is prepare for optimizing Smbios type 9. >=20 > How? > The commit message should describe what the patch does. >=20 > (Yes, I can see with "git diff -w" that this is mainly > whitespace/indentation fixes and wrapping of long lines, but the > message should state this.) >=20 > Also, This patch changes from EFI_D_ERROR to DEBUG_ERROR. This is > absolutely an improvement, but needs to be mentioned in commit > message. Ok, I will add it in v2. Thanks, Ming >=20 > Regards, >=20 > Leif >=20 >> Signed-off-by: Ming Huang >> --- >> Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.c | 28= 6 ++++++++++---------- >> 1 file changed, 146 insertions(+), 140 deletions(-) >> >> diff --git a/Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbios= Type9.c b/Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.= c >> index 87a06a2..2398c6b 100644 >> --- a/Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.c >> +++ b/Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.c >> @@ -1,6 +1,6 @@ >> /** @file >> * >> -* Copyright (c) 2015, Hisilicon Limited. All rights reserved. >> +* Copyright (c) 2015 - 2020, Hisilicon Limited. All rights reserved. >> * Copyright (c) 2015, Linaro Limited. All rights reserved. >> * >> * SPDX-License-Identifier: BSD-2-Clause-Patent >> @@ -14,178 +14,184 @@ extern UINT8 OemGetPcieSlotNumber (); >> =20 >> VOID >> EFIAPI >> -UpdateSmbiosType9Info( >> +UpdateSmbiosType9Info ( >> IN OUT SMBIOS_TABLE_TYPE9 *Type9Record >> ) >> { >> - EFI_STATUS Status; >> - UINTN HandleIndex; >> - EFI_HANDLE *HandleBuffer; >> - UINTN HandleCount; >> - EFI_PCI_IO_PROTOCOL *PciIo; >> - UINTN SegmentNumber; >> - UINTN BusNumber; >> - UINTN DeviceNumber; >> - UINTN FunctionNumber; >> - UINTN Index; >> - REPORT_PCIEDIDVID2BMC ReportPcieDidVid[PCIEDEVICE_RE= PORT_MAX]; >> - >> - GetPciDidVid ((VOID *) ReportPcieDidVid); >> - >> - Status =3D gBS->LocateHandleBuffer ( >> - ByProtocol, >> - &gEfiPciIoProtocolGuid, >> - NULL, >> - &HandleCount, >> - &HandleBuffer >> - ); >> - if(EFI_ERROR(Status)) { >> - DEBUG((EFI_D_ERROR, " Locate gEfiPciIoProtocol Failed.\n")); >> - gBS->FreePool ((VOID *)HandleBuffer); >> - return; >> + EFI_STATUS Status; >> + UINTN HandleIndex; >> + EFI_HANDLE *HandleBuffer; >> + UINTN HandleCount; >> + EFI_PCI_IO_PROTOCOL *PciIo; >> + UINTN SegmentNumber; >> + UINTN BusNumber; >> + UINTN DeviceNumber; >> + UINTN FunctionNumber; >> + UINTN Index; >> + REPORT_PCIEDIDVID2BMC ReportPcieDidVid[PCIEDEVICE_REPO= RT_MAX]; >> + >> + GetPciDidVid ((VOID *)ReportPcieDidVid); >> + >> + Status =3D gBS->LocateHandleBuffer ( >> + ByProtocol, >> + &gEfiPciIoProtocolGuid, >> + NULL, >> + &HandleCount, >> + &HandleBuffer >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, " Locate gEfiPciIoProtocol Failed.\n")); >> + gBS->FreePool ((VOID *)HandleBuffer); >> + return; >> + } >> + >> + for (HandleIndex =3D 0; HandleIndex < HandleCount; HandleIndex++) { >> + Status =3D gBS->HandleProtocol ( >> + HandleBuffer[HandleIndex], >> + &gEfiPciIoProtocolGuid, >> + (VOID **)&PciIo >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Status : %r\n", __FUNCTION__, = __LINE__, Status)); >> + continue; >> } >> - for (HandleIndex =3D 0; HandleIndex < HandleCount; HandleIndex++)= { >> - Status =3D gBS->HandleProtocol ( >> - HandleBuffer[HandleIndex], >> - &gEfiPciIoProtocolGuid, >> - (VOID **)&PciIo >> - ); >> - if (EFI_ERROR (Status)) { >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Status : %r\n", __FUNCTIO= N__, __LINE__, Status)); >> - continue; >> - } >> - (VOID)PciIo->GetLocation(PciIo, &SegmentNumber, &BusNumber, &= DeviceNumber, &FunctionNumber); >> - for(Index =3D 0; Index < sizeof(ReportPcieDidVid) / sizeof(RE= PORT_PCIEDIDVID2BMC); Index++){ >> - if (Type9Record->SlotID =3D=3D ReportPcieDidVid[Index].Sl= ot + 1) { >> - if((BusNumber =3D=3D ReportPcieDidVid[Index].Bus) && = (DeviceNumber =3D=3D ReportPcieDidVid[Index].Device)) { >> - DEBUG((EFI_D_ERROR,"PCIe device plot in slot Seg = %d bdf %d %d %d\r\n",SegmentNumber,BusNumber,DeviceNumber,FunctionNumber= )); >> - Type9Record->SegmentGroupNum =3D SegmentNumber; >> - Type9Record->BusNum =3D BusNumber; >> - Type9Record->DevFuncNum =3D (DeviceNumber = << 3) | FunctionNumber; >> - Type9Record->CurrentUsage =3D SlotUsageInUse= ; >> - break; >> - } >> - } >> + (VOID)PciIo->GetLocation (PciIo, &SegmentNumber, &BusNumber, &Dev= iceNumber, &FunctionNumber); >> + for (Index =3D 0; Index < sizeof(ReportPcieDidVid) / sizeof(REPOR= T_PCIEDIDVID2BMC); Index++) { >> + if (Type9Record->SlotID =3D=3D ReportPcieDidVid[Index].Slot + 1= ) { >> + if ((BusNumber =3D=3D ReportPcieDidVid[Index].Bus) && (Device= Number =3D=3D ReportPcieDidVid[Index].Device)) { >> + DEBUG ((DEBUG_ERROR, "PCIe device plot in slot Seg %d bdf = %d %d %d\r\n", >> + SegmentNumber, BusNumber, DeviceNumber, FunctionNumber)); >> + Type9Record->SegmentGroupNum =3D SegmentNumber; >> + Type9Record->BusNum =3D BusNumber; >> + Type9Record->DevFuncNum =3D (DeviceNumber << 3) | Fu= nctionNumber; >> + Type9Record->CurrentUsage =3D SlotUsageInUse; >> + break; >> } >> + } >> } >> - gBS->FreePool ((VOID *)HandleBuffer); >> - return; >> + } >> + >> + gBS->FreePool ((VOID *)HandleBuffer); >> + return; >> } >> + >> EFI_STATUS >> EFIAPI >> AddSmbiosType9Entry ( >> IN EFI_HANDLE ImageHandle, >> - IN EFI_SYSTEM_TABLE *SystemTable >> + IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> - EFI_STATUS Status; >> - EFI_SMBIOS_TYPE SmbiosType; >> - EFI_SMBIOS_HANDLE SmbiosHandle; >> - EFI_SMBIOS_PROTOCOL *Smbios; >> - EFI_SMBIOS_TABLE_HEADER *Record; >> - SMBIOS_TABLE_TYPE9 *Type9Record; >> - SMBIOS_TABLE_TYPE9 *SmbiosRecord =3D NULL; >> - CHAR8 *OptionalStrStart; >> - >> - UINT8 SmbiosAddType9Number; >> - UINT8 Index; >> - >> - CHAR16 *SlotDesignation =3D NULL; >> - UINTN SlotDesignationStrLen; >> - >> - Status =3D gBS->LocateProtocol ( >> + EFI_STATUS Status; >> + EFI_SMBIOS_TYPE SmbiosType; >> + EFI_SMBIOS_HANDLE SmbiosHandle; >> + EFI_SMBIOS_PROTOCOL *Smbios; >> + EFI_SMBIOS_TABLE_HEADER *Record; >> + SMBIOS_TABLE_TYPE9 *Type9Record; >> + SMBIOS_TABLE_TYPE9 *SmbiosRecord =3D NULL; >> + CHAR8 *OptionalStrStart; >> + >> + UINT8 SmbiosAddType9Number; >> + UINT8 Index; >> + >> + CHAR16 *SlotDesignation =3D NULL; >> + UINTN SlotDesignationStrLen; >> + >> + Status =3D gBS->LocateProtocol ( >> &gEfiSmbiosProtocolGuid, >> NULL, >> (VOID **) &Smbios >> ); >> - if (EFI_ERROR (Status)) { >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] LocateProtocol Failed. Status= : %r\n", __FUNCTION__, __LINE__, Status)); >> - return Status; >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] LocateProtocol Failed. Status = : %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + return Status; >> + } >> + >> + do { >> + SmbiosHandle =3D SMBIOS_HANDLE_PI_RESERVED; >> + SmbiosType =3D EFI_SMBIOS_TYPE_SYSTEM_SLOTS; >> + Status =3D Smbios->GetNext (Smbios, &SmbiosHandle, &SmbiosType, &= Record, NULL); >> + if (!EFI_ERROR (Status)) { >> + Status =3D Smbios->Remove (Smbios, SmbiosHandle); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Remove System Slot Failed. S= tatus : %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + break; >> + } >> } >> + } while (SmbiosHandle !=3D SMBIOS_HANDLE_PI_RESERVED); >> =20 >> - do { >> - SmbiosHandle =3D SMBIOS_HANDLE_PI_RESERVED; >> - SmbiosType =3D EFI_SMBIOS_TYPE_SYSTEM_SLOTS; >> - Status =3D Smbios->GetNext (Smbios, &SmbiosHandle, &SmbiosTyp= e, &Record, NULL); >> - if (!EFI_ERROR(Status)) { >> - Status =3D Smbios->Remove (Smbios, SmbiosHandle); >> - if (EFI_ERROR(Status)) { >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Remove System Slot Fa= iled. Status : %r\n", __FUNCTION__, __LINE__, Status)); >> - break; >> - } >> - } >> - } while (SmbiosHandle !=3D SMBIOS_HANDLE_PI_RESERVED); >> - >> - SmbiosAddType9Number =3D OemGetPcieSlotNumber(); >> + SmbiosAddType9Number =3D OemGetPcieSlotNumber (); >> =20 >> - for (Index =3D 0; Index < SmbiosAddType9Number; Index++) >> - { >> - if (gPcieSlotInfo[Index].Hdr.Type !=3D EFI_SMBIOS_TYPE_SYSTEM= _SLOTS) >> - { >> - continue; >> - } >> - >> - Type9Record =3D &gPcieSlotInfo[Index]; >> - >> - UpdateSmbiosType9Info (Type9Record); >> - SlotDesignation =3D AllocateZeroPool ((sizeof (CHAR16)) * SMB= IOS_STRING_MAX_LENGTH); >> - if (NULL =3D=3D SlotDesignation) >> - { >> - Status =3D EFI_OUT_OF_RESOURCES; >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. = Status : %r\n", __FUNCTION__, __LINE__, Status)); >> - >> - goto Exit; >> - } >> + for (Index =3D 0; Index < SmbiosAddType9Number; Index++) { >> + if (gPcieSlotInfo[Index].Hdr.Type !=3D EFI_SMBIOS_TYPE_SYSTEM_SLO= TS) { >> + continue; >> + } >> =20 >> - SlotDesignationStrLen =3D UnicodeSPrint (SlotDesignation, SMB= IOS_STRING_MAX_LENGTH - 1, L"PCIE Slot%d", Type9Record->SlotID); >> + Type9Record =3D &gPcieSlotInfo[Index]; >> =20 >> - // >> - // Two zeros following the last string. >> - // >> - SmbiosRecord =3D AllocateZeroPool(sizeof (SMBIOS_TABLE_TYPE9)= + SlotDesignationStrLen + 1 + 1); >> - if(NULL =3D=3D SmbiosRecord) >> - { >> - Status =3D EFI_OUT_OF_RESOURCES; >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. = Status : %r\n", __FUNCTION__, __LINE__, Status)); >> + UpdateSmbiosType9Info (Type9Record); >> + SlotDesignation =3D AllocateZeroPool ((sizeof (CHAR16)) * SMBIOS_= STRING_MAX_LENGTH); >> + if (SlotDesignation =3D=3D NULL) { >> + Status =3D EFI_OUT_OF_RESOURCES; >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Statu= s : %r\n", >> + __FUNCTION__, __LINE__, Status)); >> =20 >> - goto Exit; >> - } >> + goto Exit; >> + } >> =20 >> - (VOID)CopyMem(SmbiosRecord, Type9Record, sizeof (SMBIOS_TABLE= _TYPE9)); >> + SlotDesignationStrLen =3D UnicodeSPrint ( >> + SlotDesignation, >> + SMBIOS_STRING_MAX_LENGTH - 1, >> + L"PCIE Slot%d", >> + Type9Record->SlotID); >> + >> + // >> + // Two zeros following the last string. >> + // >> + SmbiosRecord =3D AllocateZeroPool (sizeof (SMBIOS_TABLE_TYPE9) + = SlotDesignationStrLen + 1 + 1); >> + if (SmbiosRecord =3D=3D NULL) { >> + Status =3D EFI_OUT_OF_RESOURCES; >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Statu= s : %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + >> + goto Exit; >> + } >> =20 >> - SmbiosRecord->Hdr.Length =3D sizeof (SMBIOS_TABLE_TYPE9); >> + (VOID)CopyMem (SmbiosRecord, Type9Record, sizeof (SMBIOS_TABLE_TY= PE9)); >> =20 >> - OptionalStrStart =3D (CHAR8 *)(SmbiosRecord + 1); >> - UnicodeStrToAsciiStr(SlotDesignation, OptionalStrStart); >> + SmbiosRecord->Hdr.Length =3D sizeof (SMBIOS_TABLE_TYPE9); >> =20 >> - // >> - // Now we have got the full smbios record, call smbios protoc= ol to add this record. >> - // >> - SmbiosHandle =3D SMBIOS_HANDLE_PI_RESERVED; >> - Status =3D Smbios->Add (Smbios, NULL, &SmbiosHandle, (EFI_SMB= IOS_TABLE_HEADER *)SmbiosRecord); >> - if(EFI_ERROR(Status)) >> - { >> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Smbios Type09 Table Log F= ailed! %r \n", __FUNCTION__, __LINE__, Status)); >> - goto Exit; >> - } >> + OptionalStrStart =3D (CHAR8 *)(SmbiosRecord + 1); >> + UnicodeStrToAsciiStr (SlotDesignation, OptionalStrStart); >> =20 >> - FreePool(SmbiosRecord); >> - FreePool(SlotDesignation); >> + // >> + // Now we have got the full smbios record, call smbios protocol t= o add this record. >> + // >> + SmbiosHandle =3D SMBIOS_HANDLE_PI_RESERVED; >> + Status =3D Smbios->Add (Smbios, NULL, &SmbiosHandle, (EFI_SMBIOS_= TABLE_HEADER *)SmbiosRecord); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type09 Table Log Failed= ! %r \n", >> + __FUNCTION__, __LINE__, Status)); >> + goto Exit; >> } >> =20 >> - return EFI_SUCCESS; >> + FreePool (SmbiosRecord); >> + FreePool (SlotDesignation); >> + } >> + >> + return EFI_SUCCESS; >> =20 >> Exit: >> - if(SmbiosRecord !=3D NULL) >> - { >> - FreePool(SmbiosRecord); >> - } >> + if(SmbiosRecord !=3D NULL) { >> + FreePool (SmbiosRecord); >> + } >> =20 >> - if(SlotDesignation !=3D NULL) >> - { >> - FreePool(SlotDesignation); >> - } >> + if(SlotDesignation !=3D NULL) { >> + FreePool (SlotDesignation); >> + } >> =20 >> - return Status; >> + return Status; >> } >> =20 >> --=20 >> 2.8.1 >> >=20 > . >=20