Re: [PATCH edk2-platforms v1 01/12] Silicon/Hisilicon/D06: Add watchdog to GTDT

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Oct 29, 2018 at 11:32:38AM +0800, Ming Huang wrote:
> Add watchdog to GTDT for SBSA test 41,42.
> 
> Change-Id: I175ba59f160e4965c26df62e195e423e4e421e8b
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h |  2 +-
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc        | 31 ++++++++++----------
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h
> index faaeb83781..eaf3ff10e3 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h
> @@ -22,6 +22,6 @@
>  
>  #include <PlatformArch.h>
>  
> -#define HI1620_WATCHDOG_COUNT  2
> +#define HI1620_WATCHDOG_COUNT  1
>  
>  #endif
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc
> index 45f5d20704..d07070a912 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc
> @@ -30,6 +30,16 @@
>  #define SYSTEM_TIMER_BASE_ADDRESS     0xFFFFFFFFFFFFFFFF
>  
>  #define GTDT_GTIMER_FLAGS           (GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
> +#define GENERIC_WATCHDOG_CONTROL_BASE_CPU1_TOTEM_A 0x9C200000
> +#define GENERIC_WATCHDOG_REFRESH_BASE_CPU1_TOTEM_A 0X9C210000
> +
> +#define EFI_ACPI_6_2_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(RefreshFramePhysicalAddress,                  \
> +    ControlFramePhysicalAddress, WatchdogTimerGSIV, WatchdogTimerFlags)                                 \
> +  {                                                                                                     \
> +    EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG, sizeof(EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), \
> +    EFI_ACPI_RESERVED_BYTE, RefreshFramePhysicalAddress, ControlFramePhysicalAddress,                   \
> +    WatchdogTimerGSIV, WatchdogTimerFlags                                                               \
> +  }
>  
>  #pragma pack (1)
>  
> @@ -44,7 +54,7 @@ EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>    {
>      ARM_ACPI_HEADER(
>        EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> -      EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE,
> +      EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLES,
>        EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
>      ),
>      SYSTEM_TIMER_BASE_ADDRESS,                    // UINT64  PhysicalAddress
> @@ -58,25 +68,14 @@ EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>      FixedPcdGet32 (PcdArmArchTimerHypIntrNum),    // UINT32  NonSecurePL2TimerGSIV
>      GTDT_GTIMER_FLAGS,                            // UINT32  NonSecurePL2TimerFlags
>      0xFFFFFFFFFFFFFFFF,                           // UINT64  CntReadBasePhysicalAddress
> -#ifdef notyet

Clearly this code wasn't sufficiently reviewed on the way in - that
ifdef should never have got through. At least we're getting rid of it
now.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif

> -    PV660_WATCHDOG_COUNT,                          // UINT32  PlatformTimerCount
> +    HI1620_WATCHDOG_COUNT,                        // UINT32  PlatformTimerCount
>      sizeof (EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 PlatfromTimerOffset
>    },
>    {
> -    {
> -      EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG, sizeof(EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE),
> -      EFI_ACPI_RESERVED_BYTE, 0, 0, 0, 0
> -    },
> -    {
> -      EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG, sizeof(EFI_ACPI_6_2_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE),
> -      EFI_ACPI_RESERVED_BYTE, 0, 0, 0, 0
> -    }
> -  }
> -#else /* !notyet */
> -  0, 0
> +    EFI_ACPI_6_2_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> +      GENERIC_WATCHDOG_REFRESH_BASE_CPU1_TOTEM_A, GENERIC_WATCHDOG_CONTROL_BASE_CPU1_TOTEM_A, 88, 1)
>    }
> -#endif
> -  };
> +};
>  
>  //
>  // Reference the table being generated to prevent the optimizer from removing the
> -- 
> 2.18.0
>