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

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

On 7 March 2018 at 16:10, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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.
>

I don't know what happened there :-) That should be 512 MB, obviously.

> 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
>>