[PATCH v1 0/2] Define AML_NAME_SEG_SIZE

[PATCH v1 0/2] Define AML_NAME_SEG_SIZE

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

There is currently multiple AML_NAME_SEG_SIZE define in the edk2
repository. One in the MdeModulePkg and one in the DynamicTablesPkg
package. Since the value can be inferred from the ACPI specification,
it could be moved to MdePkg/Include/IndustryStandard/ and avoid
re-definitions 

The two patches should be merged at once to avoid having
multiple definitions of AML_NAME_SEG_SIZE.

The changes can be seen at:
https://github.com/PierreARM/edk2/tree/1750_Add_AML_NAMESEG_SIZE_v1
The results of the CI can be seen at:
https://github.com/tianocore/edk2/pull/1681

Pierre Gondois (2):
  MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition
  DynamicTablesPkg: Use AML_NAME_SEG_SIZE define

 .../Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  | 4 ++--
 .../AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c   | 4 ++--
 DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h      | 9 +--------
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h       | 1 -
 MdePkg/Include/IndustryStandard/AcpiAml.h                | 7 ++++++-
 5 files changed, 11 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH v1 1/2] MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

A NameSeg is made 4 chars.
Cf. ACPI 6.4 s20.2.2 "Name Objects Encoding":
NameSeg := <leadnamechar namechar namechar namechar>
Notice that NameSegs shorter than 4 characters are filled
with trailing underscores (‘_’s).

AML_NAME_SEG_SIZE is currently defined in:
- DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h
- MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
Since the value can be inferred from the ACPI specification
and to avoid multiple definitions, move it to
MdePkg/Include/IndustryStandard/

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 1 -
 MdePkg/Include/IndustryStandard/AcpiAml.h          | 7 ++++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
index 50d4c96edb63..1b26729e71c3 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
@@ -56,7 +56,6 @@ typedef struct _EFI_AML_NODE_LIST EFI_AML_NODE_LIST;
 //  Size is the total size of this ACPI node buffer.
 //  Children is the children linked list of this node.
 //
-#define AML_NAME_SEG_SIZE  4
 
 struct _EFI_AML_NODE_LIST {
   UINT32                  Signature;
diff --git a/MdePkg/Include/IndustryStandard/AcpiAml.h b/MdePkg/Include/IndustryStandard/AcpiAml.h
index 74622e912ea4..4255ca3d7087 100644
--- a/MdePkg/Include/IndustryStandard/AcpiAml.h
+++ b/MdePkg/Include/IndustryStandard/AcpiAml.h
@@ -2,7 +2,7 @@
   This file contains AML code definition in the latest ACPI spec.
 
   Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>
-  Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -176,4 +176,9 @@
 #define AML_FIELD_CONNECTION_OP      0x02
 #define AML_FIELD_EXT_ACCESS_OP      0x03
 
+//
+// AML Name segment definitions
+//
+#define AML_NAME_SEG_SIZE            4
+
 #endif
-- 
2.17.1

回复: [PATCH v1 1/2] MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
> 发送时间: 2021年6月2日 6:05
> 收件人: devel@edk2.groups.io; dandan.bi@intel.com;
> gaoliming@byosoft.com.cn; michael.d.kinney@intel.com;
> Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com
> 主题: [PATCH v1 1/2] MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE
> definition
> 
> From: Pierre Gondois <Pierre.Gondois@arm.com>
> 
> A NameSeg is made 4 chars.
> Cf. ACPI 6.4 s20.2.2 "Name Objects Encoding":
> NameSeg := <leadnamechar namechar namechar namechar>
> Notice that NameSegs shorter than 4 characters are filled
> with trailing underscores (‘_’s).
> 
> AML_NAME_SEG_SIZE is currently defined in:
> - DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h
> - MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
> Since the value can be inferred from the ACPI specification
> and to avoid multiple definitions, move it to
> MdePkg/Include/IndustryStandard/
> 
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 1 -
>  MdePkg/Include/IndustryStandard/AcpiAml.h          | 7 ++++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
> index 50d4c96edb63..1b26729e71c3 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
> @@ -56,7 +56,6 @@ typedef struct _EFI_AML_NODE_LIST
> EFI_AML_NODE_LIST;
>  //  Size is the total size of this ACPI node buffer.
>  //  Children is the children linked list of this node.
>  //
> -#define AML_NAME_SEG_SIZE  4
> 
>  struct _EFI_AML_NODE_LIST {
>    UINT32                  Signature;
> diff --git a/MdePkg/Include/IndustryStandard/AcpiAml.h
> b/MdePkg/Include/IndustryStandard/AcpiAml.h
> index 74622e912ea4..4255ca3d7087 100644
> --- a/MdePkg/Include/IndustryStandard/AcpiAml.h
> +++ b/MdePkg/Include/IndustryStandard/AcpiAml.h
> @@ -2,7 +2,7 @@
>    This file contains AML code definition in the latest ACPI spec.
> 
>    Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>
> -  Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -176,4 +176,9 @@
>  #define AML_FIELD_CONNECTION_OP      0x02
>  #define AML_FIELD_EXT_ACCESS_OP      0x03
> 
> +//
> +// AML Name segment definitions
> +//
> +#define AML_NAME_SEG_SIZE            4
> +
>  #endif
> --
> 2.17.1

[PATCH v1 2/2] DynamicTablesPkg: Use AML_NAME_SEG_SIZE define

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

Use the newly introduced defined value in:
MdePkg/Include/IndustryStandard/AcpiAml.h

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  | 4 ++--
 .../AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c   | 4 ++--
 DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h      | 9 +--------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c
index 97a5c55fa3f6..1e8c2bfca5de 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c
@@ -1,7 +1,7 @@
 /** @file
   SSDT CMN-600 AML Table Generator.
 
-  Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -511,7 +511,7 @@ BuildSsdtCmn600TableEx (
   UINT64                            Index;
   CM_ARM_CMN_600_INFO             * Cmn600Info;
   UINT32                            Cmn600Count;
-  CHAR8                             NewName[5];
+  CHAR8                             NewName[AML_NAME_SEG_SIZE + 1];
   EFI_ACPI_DESCRIPTION_HEADER    ** TableList;
 
   ASSERT (This != NULL);
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c
index 6a1e7487dfaf..570f53aacf16 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c
@@ -1,7 +1,7 @@
 /** @file
   SSDT Serial Port Table Generator.
 
-  Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -171,7 +171,7 @@ BuildSsdtSerialPortTableEx (
   CM_ARM_SERIAL_PORT_INFO       * SerialPortInfo;
   UINT32                          SerialPortCount;
   UINTN                           Index;
-  CHAR8                           NewName[5];
+  CHAR8                           NewName[AML_NAME_SEG_SIZE + 1];
   UINT64                          Uid;
   EFI_ACPI_DESCRIPTION_HEADER  ** TableList;
 
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h b/DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h
index cbae14d78802..fff99e644b8d 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h
@@ -1,7 +1,7 @@
 /** @file
   AML Defines.
 
-  Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -138,13 +138,6 @@ typedef enum EAmlParseIndex {
 */
 #define AML_FIELD_NAMED_OP            0x04
 
-/** Size of a NameSeg.
-    Cf. ACPI 6.3 specification, s20.2.
-
-  @ingroup TreeStructures
-*/
- #define AML_NAME_SEG_SIZE            4U
-
 /** AML object types.
 
   The ACPI specification defines several object types. They are listed
-- 
2.17.1

Re: [edk2-devel] [PATCH v1 0/2] Define AML_NAME_SEG_SIZE

[Zeng, Star]

Just like to give my RB: Reviewed-by: Star Zeng <star.zeng@intel.com>
Please also get the RB from maintainer. 😊


Thanks,
Star
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois
Sent: 2021年6月2日 6:05
To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>; gaoliming@byosoft.com.cn; Kinney, Michael D <michael.d.kinney@intel.com>; Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com
Subject: [edk2-devel] [PATCH v1 0/2] Define AML_NAME_SEG_SIZE

From: Pierre Gondois <Pierre.Gondois@arm.com>

There is currently multiple AML_NAME_SEG_SIZE define in the edk2 repository. One in the MdeModulePkg and one in the DynamicTablesPkg package. Since the value can be inferred from the ACPI specification, it could be moved to MdePkg/Include/IndustryStandard/ and avoid re-definitions 

The two patches should be merged at once to avoid having multiple definitions of AML_NAME_SEG_SIZE.

The changes can be seen at:
https://github.com/PierreARM/edk2/tree/1750_Add_AML_NAMESEG_SIZE_v1
The results of the CI can be seen at:
https://github.com/tianocore/edk2/pull/1681

Pierre Gondois (2):
  MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition
  DynamicTablesPkg: Use AML_NAME_SEG_SIZE define

 .../Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  | 4 ++--
 .../AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c   | 4 ++--
 DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h      | 9 +--------
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h       | 1 -
 MdePkg/Include/IndustryStandard/AcpiAml.h                | 7 ++++++-
 5 files changed, 11 insertions(+), 14 deletions(-)

--
2.17.1

Re: [PATCH v1 0/2] Define AML_NAME_SEG_SIZE

[Sami Mujawar]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

Hi Pierre,

Thank you for this patch series.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

From: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
Date: Tuesday, 1 June 2021 at 23:05
To: devel@edk2.groups.io <devel@edk2.groups.io>, dandan.bi@intel.com <dandan.bi@intel.com>, gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>, michael.d.kinney@intel.com <michael.d.kinney@intel.com>, Sami Mujawar <Sami.Mujawar@arm.com>, Alexei Fedorov <Alexei.Fedorov@arm.com>
Subject: [PATCH v1 0/2] Define AML_NAME_SEG_SIZE
From: Pierre Gondois <Pierre.Gondois@arm.com>

There is currently multiple AML_NAME_SEG_SIZE define in the edk2
repository. One in the MdeModulePkg and one in the DynamicTablesPkg
package. Since the value can be inferred from the ACPI specification,
it could be moved to MdePkg/Include/IndustryStandard/ and avoid
re-definitions

The two patches should be merged at once to avoid having
multiple definitions of AML_NAME_SEG_SIZE.

The changes can be seen at:
https://github.com/PierreARM/edk2/tree/1750_Add_AML_NAMESEG_SIZE_v1
The results of the CI can be seen at:
https://github.com/tianocore/edk2/pull/1681

Pierre Gondois (2):
  MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition
  DynamicTablesPkg: Use AML_NAME_SEG_SIZE define

 .../Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  | 4 ++--
 .../AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c   | 4 ++--
 DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h      | 9 +--------
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h       | 1 -
 MdePkg/Include/IndustryStandard/AcpiAml.h                | 7 ++++++-
 5 files changed, 11 insertions(+), 14 deletions(-)

--
2.17.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 4911 bytes --]

Re: [PATCH v1 0/2] Define AML_NAME_SEG_SIZE

[Sami Mujawar]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

Pushed as b5379899b38e..1f515342d8d8

Thanks.

Regards,

Sami Mujawar

From: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
Date: Tuesday, 1 June 2021 at 23:05
To: devel@edk2.groups.io <devel@edk2.groups.io>, dandan.bi@intel.com <dandan.bi@intel.com>, gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>, michael.d.kinney@intel.com <michael.d.kinney@intel.com>, Sami Mujawar <Sami.Mujawar@arm.com>, Alexei Fedorov <Alexei.Fedorov@arm.com>
Subject: [PATCH v1 0/2] Define AML_NAME_SEG_SIZE
From: Pierre Gondois <Pierre.Gondois@arm.com>

There is currently multiple AML_NAME_SEG_SIZE define in the edk2
repository. One in the MdeModulePkg and one in the DynamicTablesPkg
package. Since the value can be inferred from the ACPI specification,
it could be moved to MdePkg/Include/IndustryStandard/ and avoid
re-definitions

The two patches should be merged at once to avoid having
multiple definitions of AML_NAME_SEG_SIZE.

The changes can be seen at:
https://github.com/PierreARM/edk2/tree/1750_Add_AML_NAMESEG_SIZE_v1
The results of the CI can be seen at:
https://github.com/tianocore/edk2/pull/1681

Pierre Gondois (2):
  MdePkg/MdeModulePkg: Move AML_NAME_SEG_SIZE definition
  DynamicTablesPkg: Use AML_NAME_SEG_SIZE define

 .../Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600Generator.c  | 4 ++--
 .../AcpiSsdtSerialPortLibArm/SsdtSerialPortGenerator.c   | 4 ++--
 DynamicTablesPkg/Library/Common/AmlLib/AmlDefines.h      | 9 +--------
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h       | 1 -
 MdePkg/Include/IndustryStandard/AcpiAml.h                | 7 ++++++-
 5 files changed, 11 insertions(+), 14 deletions(-)

--
2.17.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 4790 bytes --]