From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web10.32858.1591630881737190943 for ; Mon, 08 Jun 2020 08:41:22 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=0Ywo17VM; spf=pass (domain: nuviainc.com, ip: 209.85.128.67, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f67.google.com with SMTP id l17so4592070wmj.0 for ; Mon, 08 Jun 2020 08:41:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=LgEkBN1/2FQZqykvXvDql/eMwHZsl1/aNnZnQ2RoK/0=; b=0Ywo17VMJUgzGh9gU2Czyuc1M/Vh+KdtOm9amJcOJXeAw4feOJ+qP4+dFMHs3xiF80 Qx3Ta1v//sTkYwULJR/j42prUuXX7xr4UzRp/elA82AgKxY62a6DZr+q92D4NMkzPr3r sEz28DK3h6LAwXN8ErEVVBu7Jh1Cd6G8tMKmw1JRbffk+gd4o0upf6NfknwEOfmtfgxj kegTV1CSbgCHi1bFEChz/3sa9BsBeKAUQ/JBkHPB94LMQqFpcKaU9QTrfz2YF+XSMdvW WAypCk/FAQ19UmesF3nqR11CJjoPBUpiY3F2MZKtvIR9iZnq7h2inRR/Cry4uWCXx2Cv JW9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=LgEkBN1/2FQZqykvXvDql/eMwHZsl1/aNnZnQ2RoK/0=; b=B2Qz7CiP00moScS+KGvvCR7vJy0ngOquDrfGru65MlG41DZqFfW0Ocwmgwoj+uO4cE nPOy/yplr1jRPvp9eK/lBIwyWELi7D84YCIIzcuI00h2rcrMF3deZAJsLrvim+PuLZUn qZHv2YN2gPVMEywvDwl1nPAxudTv+rfkDNyesoNV+HG3iMgiRSQRu3vvsu7ySYvxk+eK tCM0oXXe3LYbht1nsEgxHDCgoKB1RRT4GIDMrkL7iGrJgKtTqHayBEbd/KxC1GaAWqdk 7vbcMDmDVcmIyEUh+8misjRr4GpiF8+xt3uGL9Q8AlcX5HJ+C6r2UO4rtaLI9FnMQXQ9 tNKg== X-Gm-Message-State: AOAM530nih5bvfNr4Gc+hGrvy4HL4oRnLcbUNN3RnM1Z5lAKf+9MMEEj oyON/pO9+Uk1L/UPFpnRo6JvFA== X-Google-Smtp-Source: ABdhPJxcETb1zoVDMqWEdQXBJQHlhq//LJYdpz1Yf20KvvPOCPopPc3vaeieL/cX5W+jU9ybI+/YoA== X-Received: by 2002:a1c:2485:: with SMTP id k127mr16334647wmk.136.1591630880074; Mon, 08 Jun 2020 08:41:20 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id h5sm85574wrw.85.2020.06.08.08.41.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Jun 2020 08:41:19 -0700 (PDT) Date: Mon, 8 Jun 2020 16:41:16 +0100 From: "Leif Lindholm" To: Ming Huang 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 Message-ID: <20200608154116.GS28566@vanye> References: <1591625397-82711-1-git-send-email-huangming23@huawei.com> <1591625397-82711-2-git-send-email-huangming23@huawei.com> MIME-Version: 1.0 In-Reply-To: <1591625397-82711-2-git-send-email-huangming23@huawei.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > 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 >