From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: edk2-devel@lists.01.org
Subject: [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code
Date: Tue, 18 Dec 2018 14:10:13 +0100 [thread overview]
Message-ID: <20181218131015.20062-4-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <20181218131015.20062-1-ard.biesheuvel@linaro.org>
Clean up the code, by adding missing STATIC modifiers, drop
redundant casts, and get rid of the 'success handling' anti
pattern in the entry point code.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 111 +++++++++++---------
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf | 11 +-
2 files changed, 64 insertions(+), 58 deletions(-)
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 8ccf15366dfa..717a180a64ec 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -34,15 +34,16 @@
#define TIME_UNITS_PER_SECOND 10000000
// Tick frequency of the generic timer basis of the generic watchdog.
-UINTN mTimerFrequencyHz = 0;
+STATIC UINTN mTimerFrequencyHz = 0;
/* In cases where the compare register was set manually, information about
how long the watchdog was asked to wait cannot be retrieved from hardware.
It is therefore stored here. 0 means the timer is not running. */
-UINT64 mNumTimerTicks = 0;
+STATIC UINT64 mNumTimerTicks = 0;
-EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
+STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
+STATIC
VOID
WatchdogWriteOffsetRegister (
UINT32 Value
@@ -51,6 +52,7 @@ WatchdogWriteOffsetRegister (
MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
}
+STATIC
VOID
WatchdogWriteCompareRegister (
UINT64 Value
@@ -60,6 +62,7 @@ WatchdogWriteCompareRegister (
MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & MAX_UINT32);
}
+STATIC
VOID
WatchdogEnable (
VOID
@@ -68,6 +71,7 @@ WatchdogEnable (
MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);
}
+STATIC
VOID
WatchdogDisable (
VOID
@@ -79,6 +83,7 @@ WatchdogDisable (
/** On exiting boot services we must make sure the Watchdog Timer
is stopped.
**/
+STATIC
VOID
EFIAPI
WatchdogExitBootServicesEvent (
@@ -93,6 +98,7 @@ WatchdogExitBootServicesEvent (
/* This function is called when the watchdog's first signal (WS0) goes high.
It uses the ResetSystem Runtime Service to reset the board.
*/
+STATIC
VOID
EFIAPI
WatchdogInterruptHandler (
@@ -141,10 +147,11 @@ WatchdogInterruptHandler (
@retval EFI_UNSUPPORTED The code does not support NotifyFunction.
**/
+STATIC
EFI_STATUS
EFIAPI
WatchdogRegisterHandler (
- IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
+ IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction
)
{
@@ -167,10 +174,11 @@ WatchdogRegisterHandler (
in TimerPeriod 100ns units.
**/
+STATIC
EFI_STATUS
EFIAPI
WatchdogSetTimerPeriod (
- IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
+ IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
IN UINT64 TimerPeriod // In 100ns units
)
{
@@ -178,8 +186,8 @@ WatchdogSetTimerPeriod (
// if TimerPeriod is 0, this is a request to stop the watchdog.
if (TimerPeriod == 0) {
- mNumTimerTicks = 0;
- WatchdogDisable ();
+ //mNumTimerTicks = 0;
+ //WatchdogDisable ();
return EFI_SUCCESS;
}
@@ -222,10 +230,11 @@ WatchdogSetTimerPeriod (
@retval EFI_INVALID_PARAMETER TimerPeriod is NULL.
**/
+STATIC
EFI_STATUS
EFIAPI
WatchdogGetTimerPeriod (
- IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
+ IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
OUT UINT64 *TimerPeriod
)
{
@@ -270,13 +279,13 @@ WatchdogGetTimerPeriod (
Retrieves the period of the timer interrupt in 100ns units.
**/
-EFI_WATCHDOG_TIMER_ARCH_PROTOCOL gWatchdogTimer = {
- (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
- (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
- (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
+STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {
+ WatchdogRegisterHandler,
+ WatchdogSetTimerPeriod,
+ WatchdogGetTimerPeriod
};
-EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
EFI_STATUS
EFIAPI
@@ -288,6 +297,10 @@ GenericWatchdogEntry (
EFI_STATUS Status;
EFI_HANDLE Handle;
+ Status = gBS->LocateProtocol (&gHardwareInterrupt2ProtocolGuid, NULL,
+ (VOID **)&mInterruptProtocol);
+ ASSERT_EFI_ERROR (Status);
+
/* Make sure the Watchdog Timer Architectural Protocol has not been installed
in the system yet.
This will avoid conflicts with the universal watchdog */
@@ -296,51 +309,45 @@ GenericWatchdogEntry (
mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
ASSERT (mTimerFrequencyHz != 0);
- // Register for an ExitBootServicesEvent
- Status = gBS->CreateEvent (
- EVT_SIGNAL_EXIT_BOOT_SERVICES,
- TPL_NOTIFY,
- WatchdogExitBootServicesEvent,
- NULL,
- &EfiExitBootServicesEvent
- );
- if (!EFI_ERROR (Status)) {
- // Install interrupt handler
- Status = gBS->LocateProtocol (
- &gHardwareInterrupt2ProtocolGuid,
- NULL,
- (VOID **)&mInterruptProtocol
- );
- if (!EFI_ERROR (Status)) {
- Status = mInterruptProtocol->RegisterInterruptSource (
- mInterruptProtocol,
- FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
- WatchdogInterruptHandler
- );
- if (!EFI_ERROR (Status)) {
- Status = mInterruptProtocol->SetTriggerType (
- mInterruptProtocol,
- FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
- EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING
- );
- if (!EFI_ERROR (Status)) {
- // Install the Timer Architectural Protocol onto a new handle
- Handle = NULL;
- Status = gBS->InstallMultipleProtocolInterfaces (
- &Handle,
- &gEfiWatchdogTimerArchProtocolGuid,
- &gWatchdogTimer,
- NULL
- );
- }
- }
- }
+ // Install interrupt handler
+ Status = mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol,
+ FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+ WatchdogInterruptHandler);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = mInterruptProtocol->SetTriggerType (mInterruptProtocol,
+ FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+ EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING);
+ if (EFI_ERROR (Status)) {
+ goto UnregisterHandler;
}
+ // Install the Timer Architectural Protocol onto a new handle
+ Handle = NULL;
+ Status = gBS->InstallMultipleProtocolInterfaces (&Handle,
+ &gEfiWatchdogTimerArchProtocolGuid, &mWatchdogTimer,
+ NULL);
+ if (EFI_ERROR (Status)) {
+ goto UnregisterHandler;
+ }
+
+ // Register for an ExitBootServicesEvent
+ Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY,
+ WatchdogExitBootServicesEvent, NULL,
+ &mEfiExitBootServicesEvent);
ASSERT_EFI_ERROR (Status);
mNumTimerTicks = 0;
WatchdogDisable ();
+ return EFI_SUCCESS;
+
+UnregisterHandler:
+ // Unregister the handler
+ mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol,
+ FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+ NULL);
return Status;
}
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
index ba0403d7fdc3..7992f3ac78bb 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
@@ -16,17 +16,16 @@
FILE_GUID = 0619f5c2-4858-4caa-a86a-73a21a18df6b
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
-
ENTRY_POINT = GenericWatchdogEntry
[Sources.common]
GenericWatchdogDxe.c
[Packages]
- MdePkg/MdePkg.dec
- EmbeddedPkg/EmbeddedPkg.dec
- ArmPkg/ArmPkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
+ ArmPkg/ArmPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
[LibraryClasses]
ArmGenericTimerCounterLib
@@ -46,8 +45,8 @@
gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum
[Protocols]
- gEfiWatchdogTimerArchProtocolGuid
- gHardwareInterrupt2ProtocolGuid
+ gEfiWatchdogTimerArchProtocolGuid ## ALWAYS_PRODUCES
+ gHardwareInterrupt2ProtocolGuid ## ALWAYS_CONSUMES
[Depex]
gHardwareInterrupt2ProtocolGuid
--
2.17.1
next prev parent reply other threads:[~2018-12-18 13:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-18 13:10 [PATCH 0/4] ArmPkg, ArmPlatformPkg: watchdog driver cleanup Ard Biesheuvel
2018-12-18 13:10 ` [PATCH 1/4] ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup Ard Biesheuvel
2018-12-18 13:35 ` Leif Lindholm
2018-12-18 13:10 ` [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode Ard Biesheuvel
2018-12-18 13:39 ` Leif Lindholm
2018-12-18 16:29 ` Ard Biesheuvel
2018-12-19 9:03 ` Thomas Abraham
2018-12-18 15:58 ` Udit Kumar
2018-12-18 16:23 ` Ard Biesheuvel
2018-12-18 13:10 ` Ard Biesheuvel [this message]
2018-12-18 13:43 ` [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code Leif Lindholm
2018-12-18 15:19 ` Ard Biesheuvel
2018-12-18 15:24 ` Leif Lindholm
2018-12-18 13:10 ` [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement RegisterHandler() method Ard Biesheuvel
2018-12-18 13:44 ` Leif Lindholm
2018-12-18 16:00 ` Udit Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181218131015.20062-4-ard.biesheuvel@linaro.org \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox