public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: evan.lloyd@arm.com
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Matteo Carlini <Matteo.Carlini@arm.com>,
	nd@arm.com
Subject: [PATCH 5/5] ArmPkg: Tidy up GenericWatchdogDxe.c
Date: Mon, 11 Sep 2017 16:23:35 +0100	[thread overview]
Message-ID: <20170911152335.72672-6-evan.lloyd@arm.com> (raw)
In-Reply-To: <20170911152335.72672-1-evan.lloyd@arm.com>

From: Evan Lloyd <evan.lloyd@arm.com>

This cosmetic change has no functional content.
It adjusts comment oddities, etc, noticed during previous work.
Specific changes are:
    Re-order #includes
    Use ns consistently (always "100ns" not sometimes "100 nS")
    Reflow overlength comments
    Change multiline comments to C style
    Adjust indent for overlength code lines.
    Replace explicit test and assert with ASSERT_EFI_ERROR.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
Signed-off-by: Alexei Fedorov <alexei.fedorov@arm.com>
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 127 ++++++++++----------
 1 file changed, 61 insertions(+), 66 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 69844db2e11f51907e6c8bff5c67d27ceb498150..7c4c9ecd4e12d433e222d7d08adf20bda1ff9842 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -29,16 +29,16 @@
 
 #include "GenericWatchdog.h"
 
-// The number of 100ns periods (the unit of time passed to these functions)
-// in a second
+/* The number of 100ns periods (the unit of time passed to these functions)
+   in a second */
 #define TIME_UNITS_PER_SECOND 10000000
 
-// Tick frequency of the generic timer that is the basis of the generic watchdog
+// Tick frequency of the generic timer basis of the generic watchdog.
 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.
+/* 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;
 
 EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
@@ -75,8 +75,7 @@ WatchdogDisable (
   return MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_DISABLED);
 }
 
-/**
-    On exiting boot services we must make sure the Watchdog Timer
+/** On exiting boot services we must make sure the Watchdog Timer
     is stopped.
 **/
 VOID
@@ -90,9 +89,8 @@ WatchdogExitBootServicesEvent (
   mNumTimerTicks = 0;
 }
 
-/*
-  This function is called when the watchdog's first signal (WS0) goes high.
-  It uses the ResetSystem Runtime Service to reset the board.
+/* This function is called when the watchdog's first signal (WS0) goes high.
+   It uses the ResetSystem Runtime Service to reset the board.
 */
 VOID
 EFIAPI
@@ -101,7 +99,7 @@ WatchdogInterruptHandler (
   IN  EFI_SYSTEM_CONTEXT          SystemContext
   )
 {
-  STATIC CONST CHAR16      ResetString[] = L"The generic watchdog timer ran out.";
+  STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out.";
 
   WatchdogDisable ();
 
@@ -126,10 +124,10 @@ WatchdogInterruptHandler (
   then the new handler is registered and EFI_SUCCESS is returned.
   If NotifyFunction is NULL, and a handler is already registered,
   then that handler is unregistered.
-  If an attempt is made to register a handler when a handler is already registered,
-  then EFI_ALREADY_STARTED is returned.
-  If an attempt is made to unregister a handler when a handler is not registered,
-  then EFI_INVALID_PARAMETER is returned.
+  If an attempt is made to register a handler when a handler is already
+  registered, then EFI_ALREADY_STARTED is returned.
+  If an attempt is made to unregister a handler when a handler is not
+  registered, then EFI_INVALID_PARAMETER is returned.
 
   @param  This             The EFI_TIMER_ARCH_PROTOCOL instance.
   @param  NotifyFunction   The function to call when a timer interrupt fires.
@@ -139,11 +137,7 @@ WatchdogInterruptHandler (
                            information is used to signal timer based events.
                            NULL will unregister the handler.
 
-  @retval EFI_SUCCESS           The watchdog timer handler was registered.
-  @retval EFI_ALREADY_STARTED   NotifyFunction is not NULL, and a handler is already
-                                registered.
-  @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not
-                                previously registered.
+  @retval EFI_UNSUPPORTED       The code does not support NotifyFunction.
 
 **/
 EFI_STATUS
@@ -160,18 +154,18 @@ WatchdogRegisterHandler (
 
 /**
   This function sets the amount of time to wait before firing the watchdog
-  timer to TimerPeriod 100 nS units.  If TimerPeriod is 0, then the watchdog
+  timer to TimerPeriod 100ns units.  If TimerPeriod is 0, then the watchdog
   timer is disabled.
 
   @param  This             The EFI_WATCHDOG_TIMER_ARCH_PROTOCOL instance.
-  @param  TimerPeriod      The amount of time in 100 nS units to wait before the watchdog
-                           timer is fired. If TimerPeriod is zero, then the watchdog
-                           timer is disabled.
+  @param  TimerPeriod      The amount of time in 100ns units to wait before
+                           the watchdog timer is fired. If TimerPeriod is zero,
+                           then the watchdog timer is disabled.
 
-  @retval EFI_SUCCESS           The watchdog timer has been programmed to fire in Time
-                                100 nS units.
-  @retval EFI_DEVICE_ERROR      A watchdog timer could not be programmed due to a device
-                                error.
+  @retval EFI_SUCCESS           The watchdog timer has been programmed to fire
+                                in Time  100ns units.
+  @retval EFI_DEVICE_ERROR      A watchdog timer could not be programmed due
+                                to a device error.
 
 **/
 EFI_STATUS
@@ -184,7 +178,7 @@ WatchdogSetTimerPeriod (
   UINTN       SystemCount;
   EFI_STATUS  Status;
 
-  // if TimerPerdiod is 0, this is a request to stop the watchdog.
+  // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
     mNumTimerTicks = 0;
     return WatchdogDisable ();
@@ -193,17 +187,16 @@ WatchdogSetTimerPeriod (
   // Work out how many timer ticks will equate to TimerPeriod
   mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
 
-
-  // If the number of required ticks is greater than the max number the
-  // watchdog's offset register (WOR) can hold, we need to manually compute and
-  // set the compare register (WCV)
+  /* 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) {
 
-    // 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.
+    /* 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. */
 
     Status = WatchdogWriteOffsetRegister (MAX_UINT32);
     if (EFI_ERROR (Status)) {
@@ -221,14 +214,14 @@ WatchdogSetTimerPeriod (
 }
 
 /**
-  This function retrieves the period of timer interrupts in 100 ns units,
+  This function retrieves the period of timer interrupts in 100ns units,
   returns that value in TimerPeriod, and returns EFI_SUCCESS.  If TimerPeriod
   is NULL, then EFI_INVALID_PARAMETER is returned.  If a TimerPeriod of 0 is
   returned, then the timer is currently disabled.
 
   @param  This             The EFI_TIMER_ARCH_PROTOCOL instance.
-  @param  TimerPeriod      A pointer to the timer period to retrieve in 100
-                           ns units. If 0 is returned, then the timer is
+  @param  TimerPeriod      A pointer to the timer period to retrieve in
+                           100ns units. If 0 is returned, then the timer is
                            currently disabled.
 
 
@@ -275,19 +268,19 @@ WatchdogGetTimerPeriod (
         this function will not have any chance of executing.
 
   @param SetTimerPeriod
-  Sets the period of the timer interrupt in 100 nS units.
+  Sets the period of the timer interrupt in 100ns units.
   This function is optional, and may return EFI_UNSUPPORTED.
   If this function is supported, then the timer period will
   be rounded up to the nearest supported timer period.
 
   @param GetTimerPeriod
-  Retrieves the period of the timer interrupt in 100 nS units.
+  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
+  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
+  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
+  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
 };
 
 EFI_EVENT                           EfiExitBootServicesEvent = (EFI_EVENT)NULL;
@@ -303,9 +296,9 @@ GenericWatchdogEntry (
   EFI_HANDLE                      Handle;
 
 
-  // Make sure the Watchdog Timer Architectural Protocol has not been installed
-  // in the system yet.
-  // This will avoid conflicts with the universal watchdog
+  /* Make sure the Watchdog Timer Architectural Protocol has not been installed
+     in the system yet.
+     This will avoid conflicts with the universal watchdog */
 
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiWatchdogTimerArchProtocolGuid);
 
@@ -314,8 +307,11 @@ GenericWatchdogEntry (
 
   // Register for an ExitBootServicesEvent
   Status = gBS->CreateEvent (
-                  EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY,
-                  WatchdogExitBootServicesEvent, NULL, &EfiExitBootServicesEvent
+                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
+                  TPL_NOTIFY,
+                  WatchdogExitBootServicesEvent,
+                  NULL,
+                  &EfiExitBootServicesEvent
                   );
   if (!EFI_ERROR (Status)) {
     // Install interrupt handler
@@ -326,32 +322,31 @@ GenericWatchdogEntry (
                     );
     if (!EFI_ERROR (Status)) {
       Status = mInterruptProtocol->RegisterInterruptSource (
-                                    mInterruptProtocol,
-                                    FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
-                                    WatchdogInterruptHandler
-                                    );
+                 mInterruptProtocol,
+                 FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
+                 WatchdogInterruptHandler
+                 );
       if (!EFI_ERROR (Status)) {
         Status = mInterruptProtocol->SetTriggerType (
-                                    mInterruptProtocol,
-                                    FixedPcdGet32 (PcdGenericWatchdogEl2IntrNum),
-                                    EFI_HARDWARE_INTERRUPT2_TRIGGER_EDGE_RISING);
+                   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
-                          );
+                     &Handle,
+                     &gEfiWatchdogTimerArchProtocolGuid,
+                     &gWatchdogTimer,
+                     NULL
+                     );
         }
       }
     }
   }
 
-  if (EFI_ERROR (Status)) {
-    // The watchdog failed to initialize
-    ASSERT (FALSE);
-  }
+  ASSERT_EFI_ERROR (Status);
 
   mNumTimerTicks = 0;
   WatchdogDisable ();
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")



  parent reply	other threads:[~2017-09-11 15:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-11 15:23 [PATCH 0/5] Add HardwareInterrupt2 for ARM evan.lloyd
2017-09-11 15:23 ` [PATCH 1/5] ArmPkg: Tidy GIC code before changes evan.lloyd
2017-09-14 16:41   ` Leif Lindholm
2017-09-21 15:34     ` Evan Lloyd
2017-09-21 17:43       ` Leif Lindholm
2017-09-11 15:23 ` [PATCH 2/5] EmbeddedPkg: Introduce HardwareInterrupt2 protocol evan.lloyd
2017-09-14 16:42   ` Leif Lindholm
2017-09-15  9:21     ` Alexei Fedorov
2017-09-11 15:23 ` [PATCH 3/5] ArmPkg/ArmGicDxe: Expose " evan.lloyd
2017-09-14 17:00   ` Leif Lindholm
2017-09-11 15:23 ` [PATCH 4/5] ArmPkg/GenericWatchdogDxe: Set Watchdog interrupt type evan.lloyd
2017-09-14 17:06   ` Leif Lindholm
2017-09-11 15:23 ` evan.lloyd [this message]
2017-09-14 17:10   ` [PATCH 5/5] ArmPkg: Tidy up GenericWatchdogDxe.c Leif Lindholm
  -- strict thread matches above, loose matches on Subject: below --
2017-02-16 22:14 [PATCH 0/5] HardwareInterrupt2 protocol evan.lloyd
2017-02-16 22:14 ` [PATCH 5/5] ArmPkg: Tidy up GenericWatchdogDxe.c evan.lloyd

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=20170911152335.72672-6-evan.lloyd@arm.com \
    --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