* [PATCH v1 0/1] Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver
@ 2020-10-08 14:56 rajesh.ravi
2020-10-08 14:56 ` [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add " rajesh.ravi
0 siblings, 1 reply; 3+ messages in thread
From: rajesh.ravi @ 2020-10-08 14:56 UTC (permalink / raw)
To: devel; +Cc: Rajesh Ravi, Leif Lindholm, Ard Biesheuvel
From: Rajesh Ravi <rajesh.ravi@broadcom.com>
Signed-off-by: Rajesh Ravi <rajesh.ravi@broadcom.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver.
https://github.com/RajeshRavi-brcm/edk2/pull/new/sbsa_wd
Rajesh Ravi (1):
ArmPkg/GenericWatchdogDxe: Add rev 1 support to ARM Generic Watchdog
driver
.../GenericWatchdogDxe/GenericWatchdog.h | 8 ++-
.../GenericWatchdogDxe/GenericWatchdogDxe.c | 53 +++++++++++++++++--
2 files changed, 56 insertions(+), 5 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add rev 1 support to ARM Generic Watchdog driver 2020-10-08 14:56 [PATCH v1 0/1] Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver rajesh.ravi @ 2020-10-08 14:56 ` rajesh.ravi 2020-10-08 17:24 ` [edk2-devel] " Sami Mujawar 0 siblings, 1 reply; 3+ messages in thread From: rajesh.ravi @ 2020-10-08 14:56 UTC (permalink / raw) To: devel; +Cc: Rajesh Ravi, Leif Lindholm, Ard Biesheuvel From: Rajesh Ravi <rajesh.ravi@broadcom.com> Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver. Signed-off-by: Rajesh Ravi <rajesh.ravi@broadcom.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h | 8 ++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 53 ++++++++++++++++++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h index c64bc5c4627d..ed74bcf95021 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h @@ -13,12 +13,18 @@ // Control Frame: #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) -#define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x00C) #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) +#define GENERIC_WDOG_IIDR_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0xFCC) // Values of bit 0 of the Control/Status Register #define GENERIC_WDOG_ENABLED 1 #define GENERIC_WDOG_DISABLED 0 +#define GENERIC_WDOG_ARCH_REV_OFFSET 16 +#define GENERIC_WDOG_ARCH_REV_MASK 0xF +#define SBSA_WDOG_WOR_WIDTH 48 +#define MAX_OFFSET_REG_VAL (((UINT64)1 << 48) - 1) #endif // __GENERIC_WATCHDOG_H__ diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index f79cc9170f8a..7ef9b1ff08ab 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -35,16 +35,53 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +STATIC UINT32 mWatchDogRev = 0; + +STATIC UINT64 mWatchdogMaxOffsetVal = MAX_UINT32; + STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC +inline +UINT32 +WatchdogReadRevisionRegister ( + VOID + ) +{ + return MmioRead32 (GENERIC_WDOG_IIDR_REG); +} + +STATIC +inline +UINT32 +WatchdogGetRevision ( + VOID + ) +{ + UINT32 IidrRegVal; + + IidrRegVal = WatchdogReadRevisionRegister(); + + return ((IidrRegVal >> GENERIC_WDOG_ARCH_REV_OFFSET) & GENERIC_WDOG_ARCH_REV_MASK); +} + STATIC VOID WatchdogWriteOffsetRegister ( - UINT32 Value + UINT64 Value ) { - MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); + if(Value >> SBSA_WDOG_WOR_WIDTH) { + return; + } + + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, ((UINT32)Value & MAX_UINT32)); + if (mWatchDogRev) { + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT32); + } else { + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, 0); + } } STATIC @@ -207,12 +244,12 @@ WatchdogSetTimerPeriod ( /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT32) { + if (mNumTimerTicks > mWatchdogMaxOffsetVal) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't trigger an interrupt, set the offset to max. */ - WatchdogWriteOffsetRegister (MAX_UINT32); + WatchdogWriteOffsetRegister (MAX_OFFSET_REG_VAL); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); @@ -319,6 +356,14 @@ GenericWatchdogEntry ( mTimerFrequencyHz = ArmGenericTimerGetTimerFreq (); ASSERT (mTimerFrequencyHz != 0); + mWatchDogRev = WatchdogGetRevision(); + + if (mWatchDogRev) { + mWatchdogMaxOffsetVal = MAX_OFFSET_REG_VAL; + } else { + mWatchdogMaxOffsetVal = MAX_UINT32; + } + // Install interrupt handler Status = mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol, FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum), -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add rev 1 support to ARM Generic Watchdog driver 2020-10-08 14:56 ` [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add " rajesh.ravi @ 2020-10-08 17:24 ` Sami Mujawar 0 siblings, 0 replies; 3+ messages in thread From: Sami Mujawar @ 2020-10-08 17:24 UTC (permalink / raw) To: devel@edk2.groups.io, rajesh.ravi@broadcom.com Cc: Leif Lindholm, Ard Biesheuvel Hi Rajesh, Thank you for this patch. Please see my response inline marked as [SAMI]. Regards, Sami Mujawar -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of rajesh.ravi@broadcom.com via groups.io Sent: 08 October 2020 03:56 PM To: devel@edk2.groups.io Cc: Rajesh Ravi <rajesh.ravi@broadcom.com>; Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com> Subject: [edk2-devel] [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add rev 1 support to ARM Generic Watchdog driver From: Rajesh Ravi <rajesh.ravi@broadcom.com> Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver. Signed-off-by: Rajesh Ravi <rajesh.ravi@broadcom.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h | 8 ++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 53 ++++++++++++++++++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h index c64bc5c4627d..ed74bcf95021 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h @@ -13,12 +13,18 @@ // Control Frame: #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) -#define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x00C) #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) +#define GENERIC_WDOG_IIDR_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0xFCC) // Values of bit 0 of the Control/Status Register #define GENERIC_WDOG_ENABLED 1 #define GENERIC_WDOG_DISABLED 0 +#define GENERIC_WDOG_ARCH_REV_OFFSET 16 +#define GENERIC_WDOG_ARCH_REV_MASK 0xF +#define SBSA_WDOG_WOR_WIDTH 48 +#define MAX_OFFSET_REG_VAL (((UINT64)1 << 48) - 1) #endif // __GENERIC_WATCHDOG_H__ diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index f79cc9170f8a..7ef9b1ff08ab 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -35,16 +35,53 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +STATIC UINT32 mWatchDogRev = 0; + +STATIC UINT64 mWatchdogMaxOffsetVal = MAX_UINT32; + STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC +inline +UINT32 +WatchdogReadRevisionRegister ( + VOID + ) [SAMI] Please add doxygen style function headers. Same at other places.[/SAMI] +{ + return MmioRead32 (GENERIC_WDOG_IIDR_REG); +} + +STATIC +inline +UINT32 +WatchdogGetRevision ( + VOID + ) +{ + UINT32 IidrRegVal; + + IidrRegVal = WatchdogReadRevisionRegister(); + + return ((IidrRegVal >> GENERIC_WDOG_ARCH_REV_OFFSET) & GENERIC_WDOG_ARCH_REV_MASK); +} + STATIC VOID WatchdogWriteOffsetRegister ( - UINT32 Value + UINT64 Value ) { - MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); + if(Value >> SBSA_WDOG_WOR_WIDTH) { [SAMI] The condition in an if statement must be a Boolean expression. Same at other places. [/SAMI] + return; + } + + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, ((UINT32)Value & MAX_UINT32)); + if (mWatchDogRev) { + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT32); + } else { + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, 0); [SAMI] According to SBSA spec v3.1, the offsets 0x00C - 0x00F are reserved for Generic Watchdog version 0. Writing zeros may have undesirable results on some platforms. [/SAMI] + } } STATIC @@ -207,12 +244,12 @@ WatchdogSetTimerPeriod ( /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT32) { + if (mNumTimerTicks > mWatchdogMaxOffsetVal) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't trigger an interrupt, set the offset to max. */ - WatchdogWriteOffsetRegister (MAX_UINT32); + WatchdogWriteOffsetRegister (MAX_OFFSET_REG_VAL); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); @@ -319,6 +356,14 @@ GenericWatchdogEntry ( mTimerFrequencyHz = ArmGenericTimerGetTimerFreq (); ASSERT (mTimerFrequencyHz != 0); + mWatchDogRev = WatchdogGetRevision(); [SAMI] Space needed between function name and opening bracket. [/SAMI] + + if (mWatchDogRev) { + mWatchdogMaxOffsetVal = MAX_OFFSET_REG_VAL; + } else { + mWatchdogMaxOffsetVal = MAX_UINT32; + } + // Install interrupt handler Status = mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol, FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum), -- 2.17.1 IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-08 17:24 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-08 14:56 [PATCH v1 0/1] Add SBSA watchdog rev 1 support to ARM Generic Watchdog driver rajesh.ravi 2020-10-08 14:56 ` [PATCH v1 1/1] ArmPkg/GenericWatchdogDxe: Add " rajesh.ravi 2020-10-08 17:24 ` [edk2-devel] " Sami Mujawar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox