From: "Leif Lindholm" <leif@nuviainc.com>
To: Ming Huang <huangming23@huawei.com>
Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org,
lidongzhan@huawei.com, songdongkuang@huawei.com,
wanghuiqiang@huawei.com, qiuliangen@huawei.com,
shenlimei@huawei.com, xiewenyi2@huawei.com
Subject: Re: [PATCH edk2-platforms v1 1/2] Silicon/Hisilicon/Smbios: correct coding style issue in type 9
Date: Mon, 8 Jun 2020 16:41:16 +0100 [thread overview]
Message-ID: <20200608154116.GS28566@vanye> (raw)
In-Reply-To: <1591625397-82711-2-git-send-email-huangming23@huawei.com>
On Mon, Jun 08, 2020 at 22:09:56 +0800, Ming Huang wrote:
> This patch is prepare for optimizing Smbios type 9.
How?
The commit message should describe what the patch does.
(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.)
Also, This patch changes from EFI_D_ERROR to DEBUG_ERROR. This is
absolutely an improvement, but needs to be mentioned in commit
message.
Regards,
Leif
> Signed-off-by: Ming Huang <huangming23@huawei.com>
> ---
> Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.c | 286 ++++++++++----------
> 1 file changed, 146 insertions(+), 140 deletions(-)
>
> diff --git a/Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.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 ();
>
> 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_REPORT_MAX];
> -
> - GetPciDidVid ((VOID *) ReportPcieDidVid);
> -
> - Status = 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_REPORT_MAX];
> +
> + GetPciDidVid ((VOID *)ReportPcieDidVid);
> +
> + Status = 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 = 0; HandleIndex < HandleCount; HandleIndex++) {
> + Status = 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 = 0; HandleIndex < HandleCount; HandleIndex++) {
> - Status = gBS->HandleProtocol (
> - HandleBuffer[HandleIndex],
> - &gEfiPciIoProtocolGuid,
> - (VOID **)&PciIo
> - );
> - if (EFI_ERROR (Status)) {
> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Status : %r\n", __FUNCTION__, __LINE__, Status));
> - continue;
> - }
> - (VOID)PciIo->GetLocation(PciIo, &SegmentNumber, &BusNumber, &DeviceNumber, &FunctionNumber);
> - for(Index = 0; Index < sizeof(ReportPcieDidVid) / sizeof(REPORT_PCIEDIDVID2BMC); Index++){
> - if (Type9Record->SlotID == ReportPcieDidVid[Index].Slot + 1) {
> - if((BusNumber == ReportPcieDidVid[Index].Bus) && (DeviceNumber == 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 = SegmentNumber;
> - Type9Record->BusNum = BusNumber;
> - Type9Record->DevFuncNum = (DeviceNumber << 3) | FunctionNumber;
> - Type9Record->CurrentUsage = SlotUsageInUse;
> - break;
> - }
> - }
> + (VOID)PciIo->GetLocation (PciIo, &SegmentNumber, &BusNumber, &DeviceNumber, &FunctionNumber);
> + for (Index = 0; Index < sizeof(ReportPcieDidVid) / sizeof(REPORT_PCIEDIDVID2BMC); Index++) {
> + if (Type9Record->SlotID == ReportPcieDidVid[Index].Slot + 1) {
> + if ((BusNumber == ReportPcieDidVid[Index].Bus) && (DeviceNumber == 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 = SegmentNumber;
> + Type9Record->BusNum = BusNumber;
> + Type9Record->DevFuncNum = (DeviceNumber << 3) | FunctionNumber;
> + Type9Record->CurrentUsage = 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 = NULL;
> - CHAR8 *OptionalStrStart;
> -
> - UINT8 SmbiosAddType9Number;
> - UINT8 Index;
> -
> - CHAR16 *SlotDesignation = NULL;
> - UINTN SlotDesignationStrLen;
> -
> - Status = 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 = NULL;
> + CHAR8 *OptionalStrStart;
> +
> + UINT8 SmbiosAddType9Number;
> + UINT8 Index;
> +
> + CHAR16 *SlotDesignation = NULL;
> + UINTN SlotDesignationStrLen;
> +
> + Status = 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 = SMBIOS_HANDLE_PI_RESERVED;
> + SmbiosType = EFI_SMBIOS_TYPE_SYSTEM_SLOTS;
> + Status = Smbios->GetNext (Smbios, &SmbiosHandle, &SmbiosType, &Record, NULL);
> + if (!EFI_ERROR (Status)) {
> + Status = Smbios->Remove (Smbios, SmbiosHandle);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Remove System Slot Failed. Status : %r\n",
> + __FUNCTION__, __LINE__, Status));
> + break;
> + }
> }
> + } while (SmbiosHandle != SMBIOS_HANDLE_PI_RESERVED);
>
> - do {
> - SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> - SmbiosType = EFI_SMBIOS_TYPE_SYSTEM_SLOTS;
> - Status = Smbios->GetNext (Smbios, &SmbiosHandle, &SmbiosType, &Record, NULL);
> - if (!EFI_ERROR(Status)) {
> - Status = Smbios->Remove (Smbios, SmbiosHandle);
> - if (EFI_ERROR(Status)) {
> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Remove System Slot Failed. Status : %r\n", __FUNCTION__, __LINE__, Status));
> - break;
> - }
> - }
> - } while (SmbiosHandle != SMBIOS_HANDLE_PI_RESERVED);
> -
> - SmbiosAddType9Number = OemGetPcieSlotNumber();
> + SmbiosAddType9Number = OemGetPcieSlotNumber ();
>
> - for (Index = 0; Index < SmbiosAddType9Number; Index++)
> - {
> - if (gPcieSlotInfo[Index].Hdr.Type != EFI_SMBIOS_TYPE_SYSTEM_SLOTS)
> - {
> - continue;
> - }
> -
> - Type9Record = &gPcieSlotInfo[Index];
> -
> - UpdateSmbiosType9Info (Type9Record);
> - SlotDesignation = AllocateZeroPool ((sizeof (CHAR16)) * SMBIOS_STRING_MAX_LENGTH);
> - if (NULL == SlotDesignation)
> - {
> - Status = EFI_OUT_OF_RESOURCES;
> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Status : %r\n", __FUNCTION__, __LINE__, Status));
> -
> - goto Exit;
> - }
> + for (Index = 0; Index < SmbiosAddType9Number; Index++) {
> + if (gPcieSlotInfo[Index].Hdr.Type != EFI_SMBIOS_TYPE_SYSTEM_SLOTS) {
> + continue;
> + }
>
> - SlotDesignationStrLen = UnicodeSPrint (SlotDesignation, SMBIOS_STRING_MAX_LENGTH - 1, L"PCIE Slot%d", Type9Record->SlotID);
> + Type9Record = &gPcieSlotInfo[Index];
>
> - //
> - // Two zeros following the last string.
> - //
> - SmbiosRecord = AllocateZeroPool(sizeof (SMBIOS_TABLE_TYPE9) + SlotDesignationStrLen + 1 + 1);
> - if(NULL == SmbiosRecord)
> - {
> - Status = EFI_OUT_OF_RESOURCES;
> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Status : %r\n", __FUNCTION__, __LINE__, Status));
> + UpdateSmbiosType9Info (Type9Record);
> + SlotDesignation = AllocateZeroPool ((sizeof (CHAR16)) * SMBIOS_STRING_MAX_LENGTH);
> + if (SlotDesignation == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Status : %r\n",
> + __FUNCTION__, __LINE__, Status));
>
> - goto Exit;
> - }
> + goto Exit;
> + }
>
> - (VOID)CopyMem(SmbiosRecord, Type9Record, sizeof (SMBIOS_TABLE_TYPE9));
> + SlotDesignationStrLen = UnicodeSPrint (
> + SlotDesignation,
> + SMBIOS_STRING_MAX_LENGTH - 1,
> + L"PCIE Slot%d",
> + Type9Record->SlotID);
> +
> + //
> + // Two zeros following the last string.
> + //
> + SmbiosRecord = AllocateZeroPool (sizeof (SMBIOS_TABLE_TYPE9) + SlotDesignationStrLen + 1 + 1);
> + if (SmbiosRecord == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] AllocateZeroPool Failed. Status : %r\n",
> + __FUNCTION__, __LINE__, Status));
> +
> + goto Exit;
> + }
>
> - SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE9);
> + (VOID)CopyMem (SmbiosRecord, Type9Record, sizeof (SMBIOS_TABLE_TYPE9));
>
> - OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
> - UnicodeStrToAsciiStr(SlotDesignation, OptionalStrStart);
> + SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE9);
>
> - //
> - // Now we have got the full smbios record, call smbios protocol to add this record.
> - //
> - SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> - Status = Smbios->Add (Smbios, NULL, &SmbiosHandle, (EFI_SMBIOS_TABLE_HEADER *)SmbiosRecord);
> - if(EFI_ERROR(Status))
> - {
> - DEBUG((EFI_D_ERROR, "[%a]:[%dL] Smbios Type09 Table Log Failed! %r \n", __FUNCTION__, __LINE__, Status));
> - goto Exit;
> - }
> + OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
> + UnicodeStrToAsciiStr (SlotDesignation, OptionalStrStart);
>
> - FreePool(SmbiosRecord);
> - FreePool(SlotDesignation);
> + //
> + // Now we have got the full smbios record, call smbios protocol to add this record.
> + //
> + SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> + Status = 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;
> }
>
> - return EFI_SUCCESS;
> + FreePool (SmbiosRecord);
> + FreePool (SlotDesignation);
> + }
> +
> + return EFI_SUCCESS;
>
> Exit:
> - if(SmbiosRecord != NULL)
> - {
> - FreePool(SmbiosRecord);
> - }
> + if(SmbiosRecord != NULL) {
> + FreePool (SmbiosRecord);
> + }
>
> - if(SlotDesignation != NULL)
> - {
> - FreePool(SlotDesignation);
> - }
> + if(SlotDesignation != NULL) {
> + FreePool (SlotDesignation);
> + }
>
> - return Status;
> + return Status;
> }
>
> --
> 2.8.1
>
next prev parent reply other threads:[~2020-06-08 15:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-08 14:09 [PATCH edk2-platforms v1 0/2] Optimize type 9 huangming23
2020-06-08 14:09 ` [PATCH edk2-platforms v1 1/2] Silicon/Hisilicon/Smbios: correct coding style issue in " Ming Huang
2020-06-08 15:41 ` Leif Lindholm [this message]
2020-06-09 13:39 ` Ming Huang
2020-06-08 14:09 ` [PATCH edk2-platforms v1 2/2] Silicon/Hisilicon/Smbios: Optimize " Ming Huang
2020-06-08 19:09 ` 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=20200608154116.GS28566@vanye \
--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