From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C714021130707 for ; Thu, 13 Sep 2018 21:42:04 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2018 21:42:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,372,1531810800"; d="scan'208";a="91627593" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.8]) ([10.239.9.8]) by orsmga002.jf.intel.com with ESMTP; 13 Sep 2018 21:41:57 -0700 From: "Ni, Ruiyu" To: Star Zeng , edk2-devel@lists.01.org Cc: Michael D Kinney , Younas khan , Jiewen Yao , Liming Gao References: <1536834420-16620-1-git-send-email-star.zeng@intel.com> <1536834420-16620-5-git-send-email-star.zeng@intel.com> <00f404cc-bc58-91d4-8546-d72b73f90912@Intel.com> Message-ID: Date: Fri, 14 Sep 2018 12:42:49 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <00f404cc-bc58-91d4-8546-d72b73f90912@Intel.com> Subject: Re: [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Sep 2018 04:42:05 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit On 9/14/2018 12:41 PM, Ni, Ruiyu wrote: > On 9/13/2018 6:26 PM, Star Zeng wrote: >> https://bugzilla.tianocore.org/show_bug.cgi?id=967 >> Request to add a library function for GetAcpiTable() in order >> to get ACPI table using signature as input. >> >> After evaluation, we found there are many duplicated code to >> find ACPI table by signature in different modules. >> >> This patch updates PcatRealTimeClockRuntimeDxe to use new >> EfiLocateFirstAcpiTable() and remove the duplicated code. >> >> Cc: Younas khan >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Jiewen Yao >> Cc: Ruiyu Ni >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Star Zeng >> --- >>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 >> +--------------------- >>   1 file changed, 3 insertions(+), 77 deletions(-) >> >> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c >> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c >> index 2105acf35f7b..7965eb8aa55b 100644 >> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c >> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c >> @@ -1203,49 +1203,6 @@ IsWithinOneDay ( >>   } >>   /** >> -  This function find ACPI table with the specified signature in RSDT >> or XSDT. >> - >> -  @param Sdt              ACPI RSDT or XSDT. >> -  @param Signature        ACPI table signature. >> -  @param TablePointerSize Size of table pointer: 4 or 8. >> - >> -  @return ACPI table or NULL if not found. >> -**/ >> -VOID * >> -ScanTableInSDT ( >> -  IN EFI_ACPI_DESCRIPTION_HEADER    *Sdt, >> -  IN UINT32                         Signature, >> -  IN UINTN                          TablePointerSize >> -  ) >> -{ >> -  UINTN                          Index; >> -  UINTN                          EntryCount; >> -  UINTN                          EntryBase; >> -  EFI_ACPI_DESCRIPTION_HEADER    *Table; >> - >> -  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / >> TablePointerSize; >> - >> -  EntryBase = (UINTN) (Sdt + 1); >> -  for (Index = 0; Index < EntryCount; Index++) { >> -    // >> -    // When TablePointerSize is 4 while sizeof (VOID *) is 8, make >> sure the upper 4 bytes are zero. >> -    // >> -    Table = 0; >> -    CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), >> TablePointerSize); >> - >> -    if (Table == NULL) { >> -      continue; >> -    } >> - >> -    if (Table->Signature == Signature) { >> -      return Table; >> -    } >> -  } >> - >> -  return NULL; >> -} >> - >> -/** >>     Get the century RTC address from the ACPI FADT table. >>     @return  The century RTC address or 0 if not found. >> @@ -1255,42 +1212,11 @@ GetCenturyRtcAddress ( >>     VOID >>     ) >>   { >> -  EFI_STATUS                                    Status; >> -  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp; >>     EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt; >> -  Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID >> **) &Rsdp); >> -  if (EFI_ERROR (Status)) { >> -    Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, >> (VOID **) &Rsdp); >> -  } >> - >> -  if (EFI_ERROR (Status) || (Rsdp == NULL)) { >> -    return 0; >> -  } >> - >> -  Fadt = NULL; >> - >> -  // >> -  // Find FADT in XSDT >> -  // >> -  if (Rsdp->Revision >= >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && >> Rsdp->XsdtAddress != 0) { >> -    Fadt = ScanTableInSDT ( >> -             (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress, >> -             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, >> -             sizeof (UINTN) >> -             ); >> -  } >> - >> -  // >> -  // Find FADT in RSDT >> -  // >> -  if (Fadt == NULL && Rsdp->RsdtAddress != 0) { >> -    Fadt = ScanTableInSDT ( >> -             (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress, >> -             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, >> -             sizeof (UINT32) >> -             ); >> -  } >> +  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) >> EfiLocateFirstAcpiTable ( >> + >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE >> +                                                         ); >>     if ((Fadt != NULL) && >>         (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < >> 0x80) >> > > Reviewed-by: Ruiyu Ni > By the way, can we remove the Acpi10/Acpi20 GUID references from INF file? -- Thanks, Ray