[edk2-devel] [PATCH v3 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation

["Rebecca Cran via groups.io" <rebecca=os.amperecomputing.com@groups.io>]

The calculation of the timer period was broken. Introduce a global
mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz
is only used in one place, remove the global and make it a local
variable. Do the same with mNumTimerTicks.

Signed-off-by: Rebecca Cran <rebecca@os.amperecomputing.com>
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 ++++++++++++++----------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index f8c39458a53a..78cee62a19d6 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -28,13 +28,10 @@
    in a second */
 #define TIME_UNITS_PER_SECOND  10000000
 
-// Tick frequency of the generic timer basis of the generic watchdog.
-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. */
-STATIC UINT64  mNumTimerTicks = 0;
+STATIC UINT64  mTimerPeriod = 0;
 
 #define MAX_UINT48  0xFFFFFFFFFFFFULL
 
@@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent (
   )
 {
   WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mTimerPeriod        = 0;
+  mExitedBootServices = TRUE;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -106,7 +104,6 @@ WatchdogInterruptHandler (
   )
 {
   STATIC CONST CHAR16  ResetString[] = L"The generic watchdog timer ran out.";
-  UINT64               TimerPeriod;
 
   WatchdogDisable ();
 
@@ -119,8 +116,7 @@ WatchdogInterruptHandler (
   // the timer period plus 1.
   //
   if (mWatchdogNotify != NULL) {
-    TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks);
-    mWatchdogNotify (TimerPeriod + 1);
+    mWatchdogNotify (mTimerPeriod + 1);
   }
 
   gRT->ResetSystem (
@@ -200,22 +196,27 @@ WatchdogSetTimerPeriod (
   IN UINT64                            TimerPeriod          // In 100ns units
   )
 {
-  UINTN  SystemCount;
+  UINTN   SystemCount;
+  UINT64  TimerFrequencyHz;
+  UINT64  NumTimerTicks;
 
   // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
-    mNumTimerTicks = 0;
+    mTimerPeriod = 0;
     WatchdogDisable ();
     return EFI_SUCCESS;
   }
 
   // Work out how many timer ticks will equate to TimerPeriod
-  mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
+  TimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
+  ASSERT (TimerFrequencyHz != 0);
+  mTimerPeriod  = TimerPeriod;
+  NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
 
   /* 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_UINT48) {
+  if (NumTimerTicks > MAX_UINT48) {
     /* 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
@@ -223,9 +224,9 @@ WatchdogSetTimerPeriod (
     WatchdogWriteOffsetRegister (MAX_UINT48);
     WatchdogEnable ();
     SystemCount = ArmGenericTimerGetSystemCount ();
-    WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
+    WatchdogWriteCompareRegister (SystemCount + NumTimerTicks);
   } else {
-    WatchdogWriteOffsetRegister (mNumTimerTicks);
+    WatchdogWriteOffsetRegister (NumTimerTicks);
     WatchdogEnable ();
   }
 
@@ -260,7 +261,7 @@ WatchdogGetTimerPeriod (
     return EFI_INVALID_PARAMETER;
   }
 
-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks);
+  *TimerPeriod = mTimerPeriod;
 
   return EFI_SUCCESS;
 }
@@ -327,9 +328,6 @@ GenericWatchdogEntry (
      This will avoid conflicts with the universal watchdog */
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiWatchdogTimerArchProtocolGuid);
 
-  mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
-  ASSERT (mTimerFrequencyHz != 0);
-
   // Install interrupt handler
   Status = mInterruptProtocol->RegisterInterruptSource (
                                  mInterruptProtocol,
@@ -371,7 +369,7 @@ GenericWatchdogEntry (
                   );
   ASSERT_EFI_ERROR (Status);
 
-  mNumTimerTicks = 0;
+  mTimerPeriod = 0;
   WatchdogDisable ();
 
   return EFI_SUCCESS;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113216): https://edk2.groups.io/g/devel/message/113216
Mute This Topic: https://groups.io/mt/103538116/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-