Re: [edk2-devel] [PATCH] DynamicTablesPkg: Adds integer to the AML package node

["PierreGondois" <pierre.gondois@arm.com>]

Hello Abdul,
With the changes below:

Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>


On 5/6/24 05:08, Abdul Lateef Attar wrote:
> Adds an AmlAddIntegerPackageEntry() API to generate AML code,
> which adds an integer value to the package node.
> 
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> ---
>   .../Include/Library/AmlLib/AmlLib.h           | 41 +++++++++-
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 75 +++++++++++++++++++
>   2 files changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index 82d5464084..7ae11e3624 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -2,7 +2,7 @@
>     AML Lib.
>   
>     Copyright (c) 2019 - 2023, Arm Limited. All rights reserved.<BR>
> -  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights reserved.<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   **/
> @@ -1838,4 +1838,43 @@ AmlCreatePsdNode (
>     OUT AML_OBJECT_NODE_HANDLE  *NewPsdNode   OPTIONAL
>     );
>   
> +/** Add an integer value to Package node.
> +
> +  AmlCodeGenNamePackage ("_CID", NULL, &PackageNode);
> +  AmlGetEisaIdFromString ("PNP0A03", &EisaId);
> +  AmlAddIntegerPackageEntry (EisaId, PackageNameNode);
> +  AmlGetEisaIdFromString ("PNP0A08", &EisaId);
> +  AmlAddIntegerPackageEntry (EisaId, PackageNameNode);
> +
> +  equivalent of the following ASL code:
> +  Name (_CID, Package (0x02)  // _CID: Compatible ID
> +  {
> +      EisaId ("PNP0A03"),
> +      EisaId ("PNP0A08")
> +  })
> +
> +  The package is added at the tail of the list of the input package node
> +  name:
> +    Name ("NamePackageNode", Package () {
> +      [Pre-existing package entries],
> +      [Newly created integer entry]
> +    })
> +
> +
> +  @ingroup CodeGenApis
> +
> +  @param [in]       Integer          Integer value that need to be added to package node.
> +  @param [in, out]  PackageNameNode  Package named node to add the object to ....
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval Others                  Error occurred during the operation.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlAddIntegerPackageEntry (

Is it possible to rename the function "AmlAddIntegerToNamedPackage())",
so it is similar to AmlAddNameStringToNamedPackage(),
and place it just after AmlAddNameStringToNamedPackage() ?

> +  IN        UINT32                  Integer,
> +  IN  OUT   AML_OBJECT_NODE_HANDLE  PackageNameNode

Is it possible to rename PackageNameNode -> NamedNode ?

> +  );
> +
>   #endif // AML_LIB_H_
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index 22c2d598d0..735b359bd5 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -4296,3 +4296,78 @@ error_handler:
>     AmlDeleteTree ((AML_NODE_HANDLE)PsdNode);
>     return Status;
>   }
> +
> +/** Add an integer value to Package node.
> +
> +  AmlCodeGenNamePackage ("_CID", NULL, &PackageNode);
> +  AmlGetEisaIdFromString ("PNP0A03", &EisaId);
> +  AmlAddIntegerPackageEntry (EisaId, PackageNameNode);
> +  AmlGetEisaIdFromString ("PNP0A08", &EisaId);
> +  AmlAddIntegerPackageEntry (EisaId, PackageNameNode);
> +
> +  equivalent of the following ASL code:
> +  Name (_CID, Package (0x02)  // _CID: Compatible ID
> +  {
> +      EisaId ("PNP0A03"),
> +      EisaId ("PNP0A08")
> +  })
> +
> +  The package is added at the tail of the list of the input package node
> +  name:
> +    Name ("NamePackageNode", Package () {
> +      [Pre-existing package entries],
> +      [Newly created integer entry]
> +    })
> +
> +
> +  @ingroup CodeGenApis
> +
> +  @param [in]       Integer          Integer value that need to be added to package node.
> +  @param [in, out]  PackageNameNode  Package named node to add the object to ....


"object to ...." -> "object to."

> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval Others                  Error occurred during the operation.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlAddIntegerPackageEntry (
> +  IN        UINT32                  Integer,
> +  IN  OUT   AML_OBJECT_NODE_HANDLE  PackageNameNode
> +  )
> +{
> +  EFI_STATUS       Status;
> +  AML_OBJECT_NODE  *PackageEntryList;

Is it possible to rename PackageEntryList; -> PackageNode ?

> +
> +  if (PackageNameNode == NULL) {
> +    ASSERT_EFI_ERROR (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }

I think this check is not necessary, there is a similar check just below.

> +
> +  if ((PackageNameNode == NULL)     ||
> +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNameNode) != EAmlNodeObject) ||
> +      (!AmlNodeHasOpCode (PackageNameNode, AML_NAME_OP, 0)))
> +  {
> +    ASSERT_EFI_ERROR (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  PackageEntryList = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
> +                                               PackageNameNode,
> +                                               EAmlParseIndexTerm1
> +                                               );
> +  if ((PackageEntryList == NULL)                                              ||
> +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageEntryList) != EAmlNodeObject)  ||
> +      (!AmlNodeHasOpCode (PackageEntryList, AML_PACKAGE_OP, 0)))
> +  {
> +    ASSERT_EFI_ERROR (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = AmlAddRegisterOrIntegerToPackage (NULL, Integer, PackageEntryList);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  return Status;
> +}


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