From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.26862.1660729909469148068 for ; Wed, 17 Aug 2022 02:51:49 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A4C01113E; Wed, 17 Aug 2022 02:51:49 -0700 (PDT) Received: from [10.34.100.114] (pierre123.nice.arm.com [10.34.100.114]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 182703F67D; Wed, 17 Aug 2022 02:51:47 -0700 (PDT) Message-ID: Date: Wed, 17 Aug 2022 11:51:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 From: "PierreGondois" Subject: Re: [PATCH v2] DynamicTablesPkg: Add support to build _DSD To: Jeff Brasen , devel@edk2.groups.io Cc: sami.mujawar@arm.com, Alexei.Fedorov@arm.com, nd@arm.com References: <271411ea70c897459a6e257e75cbed40018cde00.1660674939.git.jbrasen@nvidia.com> In-Reply-To: <271411ea70c897459a6e257e75cbed40018cde00.1660674939.git.jbrasen@nvidia.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Jeff, Thanks for the v2. I only have small NITs. With the above: Reviewed-by: Pierre Gondois On 8/16/22 20:37, Jeff Brasen wrote: > Add APIs needed to build _DSD with different UUIDs. > This is per ACPI specification 6.4 s6.2.5. > > Adds support for building data packages with format > Package {"Name", Integer} > > Signed-off-by: Jeff Brasen > --- > .../Include/Library/AmlLib/AmlLib.h | 55 ++++ > .../Common/AmlLib/CodeGen/AmlCodeGen.c | 249 ++++++++++++++++++ > 2 files changed, 304 insertions(+) > > diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > index 6f214c0dfa..86a0e802f4 100644 > --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > @@ -1280,6 +1280,61 @@ AmlAddLpiState ( > IN AML_OBJECT_NODE_HANDLE LpiNode > ); > > +/** AML code generation for a _DSD device data object. > + > + AmlAddDeviceDataDescriptorPackage (Uuid, DsdNode, PackageNode) is > + equivalent of the following ASL code: > + ToUUID(Uuid), > + Package () {} > + > + Cf ACPI 6.4 specification, s6.2.5 "_DSD (Device Specific Data)". Would it be possible to add a reference to: _DSD (Device Specific Data) Implementation Guide > + > + @ingroup CodeGenApis > + > + @param [in] Uuid The Uuid of the descriptor to be created > + @param [in] DsdNode Node of the DSD Package. > + @param [out] PackageNode If success, contains the created package node. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. > +**/ > +EFI_STATUS > +EFIAPI > +AmlAddDeviceDataDescriptorPackage ( > + IN CONST EFI_GUID *Uuid, > + IN AML_OBJECT_NODE_HANDLE DsdNode, > + OUT AML_OBJECT_NODE_HANDLE *PackageNode > + ); > + > +/** AML code generation to add a package with a name and value, > + to a parent package. > + This is useful for build the _DSD package but can be used in other cases. > + > + AmlAddNameValuePackage ("Name", Value, ParentNode) is Just a typo: s/ParentNode/PackageNode (same for the function definition) Also, would it be possible to add a reference to: _DSD (Device Specific Data) Implementation Guide s3. 'Well-Known _DSD UUIDs and Data Structure Formats' and explicitly state that, if uesed to generate a _DSD object, this function should be used with the following UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 to avoid people using a wrong uuid when advertising integer values ? > + equivalent of the following ASL code: > + Package (2) {"Name", Value} > + > + Cf ACPI 6.4 specification, s6.2.5 "_DSD (Device Specific Data)". > + > + @ingroup CodeGenApis > + > + @param [in] Name String to place in first entry of package > + @param [in] Value Integer to place in second entry of package > + @param [in] PackageNode Package to add new sub package to. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. > +**/ > +EFI_STATUS > +EFIAPI > +AmlAddNameIntegerPackage ( > + IN CHAR8 *Name, > + IN UINT64 Value, > + IN AML_OBJECT_NODE_HANDLE PackageNode > + ); > + > // DEPRECATED APIS > #ifndef DISABLE_NEW_DEPRECATED_INTERFACES >