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.web09.4567.1666951522293786550 for ; Fri, 28 Oct 2022 03:05:22 -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 43EDC1FB; Fri, 28 Oct 2022 03:05:28 -0700 (PDT) Received: from [10.57.2.46] (unknown [10.57.2.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 121BD3F71A; Fri, 28 Oct 2022 03:05:20 -0700 (PDT) Message-ID: <666fb7c1-1cc9-3f26-2aa8-22c729af9562@arm.com> Date: Fri, 28 Oct 2022 12:05:19 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH 06/14] DynamicTablesPkg: Fix wrong/missing fields in CmObjParser To: Sami Mujawar , devel@edk2.groups.io Cc: Alexei Fedorov , "nd@arm.com" References: <20221010092058.118714-1-Pierre.Gondois@arm.com> <20221010092058.118714-7-Pierre.Gondois@arm.com> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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.Gondois@arm.com wrote: >> From: Pierre Gondois >> >> Add missing fields to the following CmObjParser objects: >> - EArmObjGicDInfo >> - EArmObjCacheInfo >> and fix wrong formatting of: >> - EArmObjLpiInfo >> >> Signed-off-by: Pierre Gondois >> --- >> .../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.