ArmPkg/GenericWatchdogDxe: clean up the code
authorArd Biesheuvel <ard.biesheuvel@linaro.org>
Tue, 18 Dec 2018 13:10:13 +0000 (14:10 +0100)
committerArd Biesheuvel <ard.biesheuvel@linaro.org>
Thu, 20 Dec 2018 11:41:21 +0000 (12:41 +0100)
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>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf

index 8ccf153..285727f 100644 (file)
 #define TIME_UNITS_PER_SECOND 10000000\r
 \r
 // Tick frequency of the generic timer basis of the generic watchdog.\r
-UINTN mTimerFrequencyHz = 0;\r
+STATIC UINTN mTimerFrequencyHz = 0;\r
 \r
 /* In cases where the compare register was set manually, information about\r
    how long the watchdog was asked to wait cannot be retrieved from hardware.\r
    It is therefore stored here. 0 means the timer is not running. */\r
-UINT64 mNumTimerTicks = 0;\r
+STATIC UINT64 mNumTimerTicks = 0;\r
 \r
-EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;\r
+STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;\r
 \r
+STATIC\r
 VOID\r
 WatchdogWriteOffsetRegister (\r
   UINT32  Value\r
@@ -51,6 +52,7 @@ WatchdogWriteOffsetRegister (
   MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);\r
 }\r
 \r
+STATIC\r
 VOID\r
 WatchdogWriteCompareRegister (\r
   UINT64  Value\r
@@ -60,6 +62,7 @@ WatchdogWriteCompareRegister (
   MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & MAX_UINT32);\r
 }\r
 \r
+STATIC\r
 VOID\r
 WatchdogEnable (\r
   VOID\r
@@ -68,6 +71,7 @@ WatchdogEnable (
   MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);\r
 }\r
 \r
+STATIC\r
 VOID\r
 WatchdogDisable (\r
   VOID\r
@@ -79,6 +83,7 @@ WatchdogDisable (
 /** On exiting boot services we must make sure the Watchdog Timer\r
     is stopped.\r
 **/\r
+STATIC\r
 VOID\r
 EFIAPI\r
 WatchdogExitBootServicesEvent (\r
@@ -93,6 +98,7 @@ WatchdogExitBootServicesEvent (
 /* This function is called when the watchdog's first signal (WS0) goes high.\r
    It uses the ResetSystem Runtime Service to reset the board.\r
 */\r
+STATIC\r
 VOID\r
 EFIAPI\r
 WatchdogInterruptHandler (\r
@@ -141,10 +147,11 @@ WatchdogInterruptHandler (
   @retval EFI_UNSUPPORTED       The code does not support NotifyFunction.\r
 \r
 **/\r
+STATIC\r
 EFI_STATUS\r
 EFIAPI\r
 WatchdogRegisterHandler (\r
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,\r
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL         *This,\r
   IN EFI_WATCHDOG_TIMER_NOTIFY                NotifyFunction\r
   )\r
 {\r
@@ -167,10 +174,11 @@ WatchdogRegisterHandler (
                                 in TimerPeriod 100ns units.\r
 \r
 **/\r
+STATIC\r
 EFI_STATUS\r
 EFIAPI\r
 WatchdogSetTimerPeriod (\r
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,\r
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL         *This,\r
   IN UINT64                                   TimerPeriod   // In 100ns units\r
   )\r
 {\r
@@ -222,10 +230,11 @@ WatchdogSetTimerPeriod (
   @retval EFI_INVALID_PARAMETER TimerPeriod is NULL.\r
 \r
 **/\r
+STATIC\r
 EFI_STATUS\r
 EFIAPI\r
 WatchdogGetTimerPeriod (\r
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,\r
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL         *This,\r
   OUT UINT64                                  *TimerPeriod\r
   )\r
 {\r
@@ -270,13 +279,13 @@ WatchdogGetTimerPeriod (
   Retrieves the period of the timer interrupt in 100ns units.\r
 \r
 **/\r
-EFI_WATCHDOG_TIMER_ARCH_PROTOCOL    gWatchdogTimer = {\r
-  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,\r
-  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,\r
-  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod\r
+STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {\r
+  WatchdogRegisterHandler,\r
+  WatchdogSetTimerPeriod,\r
+  WatchdogGetTimerPeriod\r
 };\r
 \r
-EFI_EVENT                           EfiExitBootServicesEvent = (EFI_EVENT)NULL;\r
+STATIC EFI_EVENT mEfiExitBootServicesEvent;\r
 \r
 EFI_STATUS\r
 EFIAPI\r
@@ -288,6 +297,10 @@ GenericWatchdogEntry (
   EFI_STATUS                      Status;\r
   EFI_HANDLE                      Handle;\r
 \r
+  Status = gBS->LocateProtocol (&gHardwareInterrupt2ProtocolGuid, NULL,\r
+                  (VOID **)&mInterruptProtocol);\r
+  ASSERT_EFI_ERROR (Status);\r
+\r
   /* Make sure the Watchdog Timer Architectural Protocol has not been installed\r
      in the system yet.\r
      This will avoid conflicts with the universal watchdog */\r
@@ -296,51 +309,45 @@ GenericWatchdogEntry (
   mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();\r
   ASSERT (mTimerFrequencyHz != 0);\r
 \r
-  // Register for an ExitBootServicesEvent\r
-  Status = gBS->CreateEvent (\r
-                  EVT_SIGNAL_EXIT_BOOT_SERVICES,\r
-                  TPL_NOTIFY,\r
-                  WatchdogExitBootServicesEvent,\r
-                  NULL,\r
-                  &EfiExitBootServicesEvent\r
-                  );\r
-  if (!EFI_ERROR (Status)) {\r
-    // Install interrupt handler\r
-    Status = gBS->LocateProtocol (\r
-                    &gHardwareInterrupt2ProtocolGuid,\r
-                    NULL,\r
-                    (VOID **)&mInterruptProtocol\r
-                    );\r
-    if (!EFI_ERROR (Status)) {\r
-      Status = mInterruptProtocol->RegisterInterruptSource (\r
-                 mInterruptProtocol,\r
-                 FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),\r
-                 WatchdogInterruptHandler\r
-                 );\r
-      if (!EFI_ERROR (Status)) {\r
-        Status = mInterruptProtocol->SetTriggerType (\r
-                   mInterruptProtocol,\r
-                   FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),\r
-                   EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING\r
-                   );\r
-        if (!EFI_ERROR (Status)) {\r
-          // Install the Timer Architectural Protocol onto a new handle\r
-          Handle = NULL;\r
-          Status = gBS->InstallMultipleProtocolInterfaces (\r
-                          &Handle,\r
-                          &gEfiWatchdogTimerArchProtocolGuid,\r
-                          &gWatchdogTimer,\r
-                          NULL\r
-                          );\r
-        }\r
-      }\r
-    }\r
+  // Install interrupt handler\r
+  Status = mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol,\r
+                                 FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),\r
+                                 WatchdogInterruptHandler);\r
+  if (EFI_ERROR (Status)) {\r
+    return Status;\r
+  }\r
+\r
+  Status = mInterruptProtocol->SetTriggerType (mInterruptProtocol,\r
+                                 FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),\r
+                                 EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING);\r
+  if (EFI_ERROR (Status)) {\r
+    goto UnregisterHandler;\r
   }\r
 \r
+  // Install the Timer Architectural Protocol onto a new handle\r
+  Handle = NULL;\r
+  Status = gBS->InstallMultipleProtocolInterfaces (&Handle,\r
+                  &gEfiWatchdogTimerArchProtocolGuid, &mWatchdogTimer,\r
+                  NULL);\r
+  if (EFI_ERROR (Status)) {\r
+    goto UnregisterHandler;\r
+  }\r
+\r
+  // Register for an ExitBootServicesEvent\r
+  Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY,\r
+                  WatchdogExitBootServicesEvent, NULL,\r
+                  &mEfiExitBootServicesEvent);\r
   ASSERT_EFI_ERROR (Status);\r
 \r
   mNumTimerTicks = 0;\r
   WatchdogDisable ();\r
 \r
+  return EFI_SUCCESS;\r
+\r
+UnregisterHandler:\r
+  // Unregister the handler\r
+  mInterruptProtocol->RegisterInterruptSource (mInterruptProtocol,\r
+                        FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),\r
+                        NULL);\r
   return Status;\r
 }\r
index ba0403d..171bf5b 100644 (file)
   FILE_GUID                      = 0619f5c2-4858-4caa-a86a-73a21a18df6b\r
   MODULE_TYPE                    = DXE_DRIVER\r
   VERSION_STRING                 = 1.0\r
-\r
   ENTRY_POINT                    = GenericWatchdogEntry\r
 \r
 [Sources.common]\r
   GenericWatchdogDxe.c\r
 \r
 [Packages]\r
-  MdePkg/MdePkg.dec\r
-  EmbeddedPkg/EmbeddedPkg.dec\r
   ArmPkg/ArmPkg.dec\r
   ArmPlatformPkg/ArmPlatformPkg.dec\r
+  EmbeddedPkg/EmbeddedPkg.dec\r
+  MdePkg/MdePkg.dec\r
 \r
 [LibraryClasses]\r
   ArmGenericTimerCounterLib\r
@@ -46,8 +45,8 @@
   gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum\r
 \r
 [Protocols]\r
-  gEfiWatchdogTimerArchProtocolGuid\r
-  gHardwareInterrupt2ProtocolGuid\r
+  gEfiWatchdogTimerArchProtocolGuid       ## ALWAYS_PRODUCES\r
+  gHardwareInterrupt2ProtocolGuid         ## ALWAYS_CONSUMES\r
 \r
 [Depex]\r
   gHardwareInterrupt2ProtocolGuid\r