Re: [edk2-devel] [PATCH 06/14] DynamicTablesPkg: Fix wrong/missing fields in CmObjParser

[PierreGondois]

Hi Sami,

On 10/26/22 14:34, Sami Mujawar wrote:

Hi Pierre,

I have one comment marked inline as [SAMI].

I believe other than that change this patch should be good.

Yes indeed, thanks for spotting it and for making the modification,

Regards,
Pierre

Regards,

Sami Mujawar

On 10/10/2022 10:20 am, pierre.gond...@arm.com wrote:

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

Add missing fields to the following CmObjParser objects:
- EArmObjGicDInfo
- EArmObjCacheInfo
and fix wrong formatting of:
- EArmObjLpiInfo

Signed-off-by: Pierre Gondois <pierre.gond...@arm.com>
---
   .../ConfigurationManagerObjectParser.c        | 24 ++++++++++---------
   1 file changed, 13 insertions(+), 11 deletions(-)

diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index 040aaa4cbb17..2126beba8b9f 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -303,7 +303,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmProcHierarchyInfoParser[] 
= {
     { "ParentToken",                sizeof (CM_OBJECT_TOKEN), "0x%p", NULL },
     { "GicCToken",                  sizeof (CM_OBJECT_TOKEN), "0x%p", NULL },
     { "NoOfPrivateResources",       4,                        "0x%x", NULL },
-  { "PrivateResourcesArrayToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }
+  { "PrivateResourcesArrayToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL },
+  { "LpiToken",                   sizeof (CM_OBJECT_TOKEN), "0x%p", NULL },
   };

/** A parser for EArmObjCacheInfo.

@@ -315,7 +316,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmCacheInfoParser[] = {
     { "NumberOfSets",          4,                        "0x%x", NULL },
     { "Associativity",         4,                        "0x%x", NULL },
     { "Attributes",            1,                        "0x%x", NULL },
-  { "LineSize",              2,                        "0x%x", NULL }
+  { "LineSize",              2,                        "0x%x", NULL },
+  { "CacheId",               4,                        "0x%x", NULL },
   };

/** A parser for EArmObjProcNodeIdInfo.

@@ -400,14 +402,14 @@ STATIC CONST CM_OBJ_PARSER  AcpiGenericAddressParser[] = {
   /** A parser for EArmObjLpiInfo.
   */
   STATIC CONST CM_OBJ_PARSER  CmArmLpiInfoParser[] = {
-  { "MinResidency",             4,                                               
"0x%x",   NULL },
-  { "WorstCaseWakeLatency",     4,                                               
"0x%x",   NULL },
-  { "Flags",                    4,                                               
"0x%x",   NULL },
-  { "ArchFlags",                4,                                               
"0x%x",   NULL },
-  { "ResCntFreq",               4,                                               
"0x%x",   NULL },
-  { "EnableParentState",        4,                                               
"0x%x",   NULL },
-  { "IsInteger",                1,                                               
"%d",     NULL },
-  { "IntegerEntryMethod",       8,                                               
"0x%llx", NULL },
+  { "MinResidency",             4,                                               
"0x%x",   NULL        },
+  { "WorstCaseWakeLatency",     4,                                               
"0x%x",   NULL        },
+  { "Flags",                    4,                                               
"0x%x",   NULL        },
+  { "ArchFlags",                4,                                               
"0x%x",   NULL        },
+  { "ResCntFreq",               4,                                               
"0x%x",   NULL        },
+  { "EnableParentState",        4,                                               
"0x%x",   NULL        },
+  { "IsInteger",                1,                                               
"%d",     NULL        },
+  { "IntegerEntryMethod",       8,                                               
"0x%llx", NULL        },
     { "RegisterEntryMethod",      sizeof 
(EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
       NULL, NULL, AcpiGenericAddressParser,
       ARRAY_SIZE (AcpiGenericAddressParser) },
@@ -417,7 +419,7 @@ STATIC CONST CM_OBJ_PARSER  CmArmLpiInfoParser[] = {
     { "UsageCounterRegister",     sizeof 
(EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE),
       NULL, NULL, AcpiGenericAddressParser,
       ARRAY_SIZE (AcpiGenericAddressParser) },
-  { "StateName",                16,                                              
"0x%a",   NULL },
+  { "StateName",                16,                                              
"NULL",   PrintString },

[SAMI] I think the format specifier should be NULL and not enclosed in
quotes. If you agree I will fix this before merging.

   };

/** A parser for EArmObjPciAddressMapInfo.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95679): https://edk2.groups.io/g/devel/message/95679
Mute This Topic: https://groups.io/mt/94232525/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-