Re: [PATCH 05/14] DynamicTablesPkg: Fix GTBlock and GTBlockTimerFrame CmObjParsers

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Pierre,

Thank you for this patch.

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

Regards,

Sami Mujawar

On 10/10/2022 10:20 am, Pierre.Gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> The CmObjParsers of the following objects was inverted, probably
> due to a wrong ordering placement in the file defining the structures:
> -EArmObjGTBlockTimerFrameInfo
> -EArmObjPlatformGTBlockInfo
>
> Assign the correct parser for each object, and re-order the
> structures in the file defining them.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>   .../Include/ArmNameSpaceObjects.h             | 32 +++++++++----------
>   .../ConfigurationManagerObjectParser.c        | 22 ++++++-------
>   2 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> index c66b441d532d..cd39e609d6c4 100644
> --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> @@ -353,6 +353,22 @@ typedef struct CmArmGenericTimerInfo {
>     UINT32    VirtualPL2TimerFlags;
>   } CM_ARM_GENERIC_TIMER_INFO;
>   
> +/** A structure that describes the
> +    Platform Generic Block Timer information for the Platform.
> +
> +    ID: EArmObjPlatformGTBlockInfo
> +*/
> +typedef struct CmArmGTBlockInfo {
> +  /// The physical base address for the GT Block Timer structure
> +  UINT64             GTBlockPhysicalAddress;
> +
> +  /// The number of timer frames implemented in the GT Block
> +  UINT32             GTBlockTimerFrameCount;
> +
> +  /// Reference token for the GT Block timer frame list
> +  CM_OBJECT_TOKEN    GTBlockTimerFrameToken;
> +} CM_ARM_GTBLOCK_INFO;
> +
>   /** A structure that describes the
>       Platform Generic Block Timer Frame information for the Platform.
>   
> @@ -390,22 +406,6 @@ typedef struct CmArmGTBlockTimerFrameInfo {
>     UINT32    CommonFlags;
>   } CM_ARM_GTBLOCK_TIMER_FRAME_INFO;
>   
> -/** A structure that describes the
> -    Platform Generic Block Timer information for the Platform.
> -
> -    ID: EArmObjPlatformGTBlockInfo
> -*/
> -typedef struct CmArmGTBlockInfo {
> -  /// The physical base address for the GT Block Timer structure
> -  UINT64             GTBlockPhysicalAddress;
> -
> -  /// The number of timer frames implemented in the GT Block
> -  UINT32             GTBlockTimerFrameCount;
> -
> -  /// Reference token for the GT Block timer frame list
> -  CM_OBJECT_TOKEN    GTBlockTimerFrameToken;
> -} CM_ARM_GTBLOCK_INFO;
> -
>   /** A structure that describes the
>       Arm Generic Watchdog information for the Platform.
>   
> diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> index f82541240ae0..040aaa4cbb17 100644
> --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
> @@ -127,6 +127,14 @@ STATIC CONST CM_OBJ_PARSER  CmArmGenericTimerInfoParser[] = {
>   
>   /** A parser for EArmObjPlatformGTBlockInfo.
>   */
> +STATIC CONST CM_OBJ_PARSER  CmArmGTBlockInfoParser[] = {
> +  { "GTBlockPhysicalAddress", 8,                        "0x%llx", NULL },
> +  { "GTBlockTimerFrameCount", 4,                        "0x%x",   NULL },
> +  { "GTBlockTimerFrameToken", sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
> +};
> +
> +/** A parser for EArmObjGTBlockTimerFrameInfo.
> +*/
>   STATIC CONST CM_OBJ_PARSER  CmArmGTBlockTimerFrameInfoParser[] = {
>     { "FrameNumber",               1, "0x%x",   NULL },
>     { "PhysicalAddressCntBase",    8, "0x%llx", NULL },
> @@ -138,14 +146,6 @@ STATIC CONST CM_OBJ_PARSER  CmArmGTBlockTimerFrameInfoParser[] = {
>     { "CommonFlags",               4, "0x%x",   NULL }
>   };
>   
> -/** A parser for EArmObjGTBlockTimerFrameInfo.
> -*/
> -STATIC CONST CM_OBJ_PARSER  CmArmGTBlockInfoParser[] = {
> -  { "GTBlockPhysicalAddress", 8,                        "0x%llx", NULL },
> -  { "GTBlockTimerFrameCount", 4,                        "0x%x",   NULL },
> -  { "GTBlockTimerFrameToken", sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
> -};
> -
>   /** A parser for EArmObjPlatformGenericWatchdogInfo.
>   */
>   STATIC CONST CM_OBJ_PARSER  CmArmGenericWatchdogInfoParser[] = {
> @@ -560,10 +560,10 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
>       ARRAY_SIZE (CmArmSerialPortInfoParser) },
>     { "EArmObjGenericTimerInfo",             CmArmGenericTimerInfoParser,
>       ARRAY_SIZE (CmArmGenericTimerInfoParser) },
> -  { "EArmObjPlatformGTBlockInfo",          CmArmGTBlockTimerFrameInfoParser,
> -    ARRAY_SIZE (CmArmGTBlockTimerFrameInfoParser) },
> -  { "EArmObjGTBlockTimerFrameInfo",        CmArmGTBlockInfoParser,
> +  { "EArmObjPlatformGTBlockInfo",          CmArmGTBlockInfoParser,
>       ARRAY_SIZE (CmArmGTBlockInfoParser) },
> +  { "EArmObjGTBlockTimerFrameInfo",        CmArmGTBlockTimerFrameInfoParser,
> +    ARRAY_SIZE (CmArmGTBlockTimerFrameInfoParser) },
>     { "EArmObjPlatformGenericWatchdogInfo",  CmArmGenericWatchdogInfoParser,
>       ARRAY_SIZE (CmArmGenericWatchdogInfoParser) },
>     { "EArmObjPciConfigSpaceInfo",           CmArmPciConfigSpaceInfoParser,