Re: [PATCH 3/3] Hisilicon/D05: Support SBSA watchdog

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 7 March 2018 at 06:55, Heyi Guo <heyi.guo@linaro.org> wrote:
> From: Chenhui Sun <sunchenhui@huawei.com>
>
> Add description of SBSA watchdogs to ACPI GTDT on D05.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chenhui Sun <sunchenhui@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Graeme Gregory <graeme.gregory@linaro.org>
> ---
>  Platform/Hisilicon/D05/D05.dsc                              |  4 ++++
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf |  2 ++
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc            | 19 +++++++------------
>  3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b0814ea1..22eaf356224d 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -418,6 +418,10 @@ [PcdsFixedAtBuild.common]
>
>    gHisiTokenSpaceGuid.Pcdsoctype|0x1610
>
> +  # SBSA watchdog on Hi1616
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x40500000
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x40600000
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> index bb279c8e428e..6955e6145c30 100644
> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> @@ -55,5 +55,7 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
>    gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
>    gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>
> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> index 2a9d209c00f0..6bc1bde2a490 100644
> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> @@ -29,6 +29,7 @@
>  #define GTDT_TIMER_ALWAYS_ON_CAPABILITY EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
>
>  #define GTDT_GTIMER_FLAGS           (GTDT_TIMER_ALWAYS_ON_CAPABILITY | GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
> +#define WATCHDOG_SPAN  0x20000000
>

Please don't use

gArmTokenSpaceGuid.PcdGenericWatchdogXXXBase

to describe two different instances of the IP that are %!@ MB apart.

Instead, you could introduce your own PCDs in the HiSilicon token
space, but I am also fine with creating local #defines in this file if
the watchdog is not used anywhere else.


>  #pragma pack (1)
>
> @@ -57,22 +58,16 @@ EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>      FixedPcdGet32 (PcdArmArchTimerHypIntrNum),    // UINT32  NonSecurePL2TimerGSIV
>      GTDT_GTIMER_FLAGS,                            // UINT32  NonSecurePL2TimerFlags
>      0xFFFFFFFFFFFFFFFF,                           // UINT64  CntReadBasePhysicalAddress
> -#ifdef notyet
> -    PV660_WATCHDOG_COUNT,                          // UINT32  PlatformTimerCount
> +    HI1616_WATCHDOG_COUNT,                        // UINT32  PlatformTimerCount
>      sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 PlatfromTimerOffset
>    },
>    {
> -    EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> -        //FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 93, 0),
> -        0, 0, 0, 0),
> -    EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> -        //FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 94, EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
> -        0, 0, 0, 0)
> +    EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> +      FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 400, 0),
> +    EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> +      FixedPcdGet32 (PcdGenericWatchdogRefreshBase) + WATCHDOG_SPAN, FixedPcdGet32 (PcdGenericWatchdogControlBase) + WATCHDOG_SPAN, 496, 0)
> +
>    }
> -#else /* !notyet */
> -  0, 0
> -  }
> -#endif
>    };
>
>  //
> --
> 2.7.4
>