Re: [edk2-devel] [PATCH v1 2/3] DynamicTablesPkg: Adds ACPI HPET Table generator

["Abdul Lateef Attar via groups.io" <AbdulLateef.Attar=amd.com@groups.io>]

Hi Pierre Gondois,

     Thanks for review the comment, i will make the changes accordingly.

Please find my response in line.

Thanks

AbduL

On 27-02-2024 21:32, Pierre Gondois wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
>
>
> Hello Abdul,
>
> From the HPET spec:
> """
> For the case where there may be additional Event Timer Blocks 
> implemented in the system, their base
> addresses would be described in ACPI Name space.
> """
> So it seems it might be good (but not necessary) to to add a 
> description in
> a SSDT/DSDT table of the object (with _HID=PNP0103).
[Abdul] I will submit another Generator library for creating the 
platform devices in ACPI namespace.
>
> ---
>
> Same comment about than for the other patches about adding objects to
> the common Arch namespace.`
>
[Abdul] This Arch namespace changes are non-disruptive and wont cause 
any failure to existing ARM namespace.

I'll submit a separate patch for acpiview to parse the HPET and WSMT table.

>
> On 2/20/24 07:48, Abdul Lateef Attar wrote:
>> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>>
>> Adds generic ACPI HPET table generator library.
>> Register/Deregister HPET table.
>> Update the HPET table during boot as per specification.
>>
>> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>> ---
>>   DynamicTablesPkg/DynamicTables.dsc.inc        |   2 +
>>   DynamicTablesPkg/DynamicTablesPkg.ci.yaml     |   3 +-
>>   DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
>>   .../Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf  |  38 ++++
>>   .../Library/Acpi/AcpiHpetLib/HpetGenerator.c  | 208 ++++++++++++++++++
>>   5 files changed, 251 insertions(+), 1 deletion(-)
>>   create mode 100644 
>> DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
>>   create mode 100644 
>> DynamicTablesPkg/Library/Acpi/AcpiHpetLib/HpetGenerator.c
>>
>> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
>> b/DynamicTablesPkg/DynamicTables.dsc.inc
>> index 5ec9ffac06..af70785520 100644
>> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
>> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
>> @@ -35,6 +35,7 @@
>>     # Generators
>>     #
>>     DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
>> +  DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
>>
>>     #
>>     # Dynamic Table Factory Dxe
>> @@ -42,6 +43,7 @@
>> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf 
>> {
>>       <LibraryClasses>
>> NULL|DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
>> + NULL|DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
>>     }
>>
>>   [Components.ARM, Components.AARCH64]
>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
>> b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
>> index 1ad5540e24..cacdaa1df6 100644
>> --- a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
>> +++ b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
>> @@ -53,7 +53,8 @@
>>               "EmbeddedPkg/EmbeddedPkg.dec",
>>               "DynamicTablesPkg/DynamicTablesPkg.dec",
>>               "MdeModulePkg/MdeModulePkg.dec",
>> -            "MdePkg/MdePkg.dec"
>> +            "MdePkg/MdePkg.dec",
>> +            "PcAtChipsetPkg/PcAtChipsetPkg.dec"
>>           ],
>>           # For host based unit tests
>>           "AcceptableDependencies-HOST_APPLICATION":[
>> diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h 
>> b/DynamicTablesPkg/Include/AcpiTableGenerator.h
>> index d0eda011c3..18b5f99f47 100644and
>> --- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
>> +++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
>> @@ -99,6 +99,7 @@ typedef enum StdAcpiTableId {
>>     EStdAcpiTableIdSsdtCpuTopology,               ///< SSDT Cpu Topology
>>     EStdAcpiTableIdSsdtPciExpress,                ///< SSDT Pci 
>> Express Generator
>>     EStdAcpiTableIdPcct,                          ///< PCCT Generator
>> +  EStdAcpiTableIdHpet,                          ///< HPET Generator
>>     EStdAcpiTableIdMax
>>   } ESTD_ACPI_TABLE_ID;
>>
>> diff --git 
>> a/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf 
>> b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
>> new file mode 100644
>> index 0000000000..74a1358ffe
>> --- /dev/null
>> +++ b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
>> @@ -0,0 +1,38 @@
>> +## @file
>> +#  HPET Table Generator
>> +#
>> +#  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION    = 1.27
>> +  BASE_NAME      = AcpiHpetLib
>> +  FILE_GUID      = 4E75F653-C356-48B3-B32C-D1B901ECF90A
>> +  VERSION_STRING = 1.0
>> +  MODULE_TYPE    = DXE_DRIVER
>> +  LIBRARY_CLASS  = NULL|DXE_DRIVER
>> +  CONSTRUCTOR    = AcpiHpetLibConstructor
>> +  DESTRUCTOR     = AcpiHpetLibDestructor
>> +
>> +[Sources]
>> +  HpetGenerator.c
>> +
>> +[Packages]
>> +  DynamicTablesPkg/DynamicTablesPkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>
> I think the dependency could be deleted, along with:
> HpetGenerator.c:19:#include <Library/AcpiLib.h>
>
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  PcAtChipsetPkg/PcAtChipsetPkg.dec
>
> (for Sami)
> A dependency over the PcAtChipsetPkg is introduced here.
>
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  IoLib
>> +  PcdLib
>> +
>> +[Pcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
>> +  gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress
>
>
> [snip]


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116116): https://edk2.groups.io/g/devel/message/116116
Mute This Topic: https://groups.io/mt/104463460/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-