[edk2,3/3] ArmPkg: Correct typos in ArmArchTimerLib.c.

Message ID 1457006121-7604-4-git-send-email-evan.lloyd@arm.com
State New
Headers show

Commit Message

Evan Lloyd March 3, 2016, 11:55 a.m.
From: Evan Lloyd <evan.lloyd@arm.com>


SOme minor typographical problems were noticed during previous commits.
This change corrects those, and contains no functional modifications.
The changes are in comments, and one diagnostic message.

Code at: https://github.com/EvanLloyd/tianocore/commit/6de873f7e3fd63b045adf994e1c8289a7da66531

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

---
 ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
2.7.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Ryan Harkin March 3, 2016, 1:12 p.m. | #1
On 3 March 2016 at 11:55,  <evan.lloyd@arm.com> wrote:
> From: Evan Lloyd <evan.lloyd@arm.com>

>

> SOme minor typographical problems were noticed during previous commits.


Minor typographical problem in commit message :-)


> This change corrects those, and contains no functional modifications.

> The changes are in comments, and one diagnostic message.

>

> Code at: https://github.com/EvanLloyd/tianocore/commit/6de873f7e3fd63b045adf994e1c8289a7da66531

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>


Comments below are merely that, nothing to stop me OKing this patch.

Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org>



> ---

>  ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 16 ++++++++--------

>  1 file changed, 8 insertions(+), 8 deletions(-)

>

> diff --git a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> index 2384b40..c97b1e5 100644

> --- a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> +++ b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> @@ -50,7 +50,7 @@ TimerConstructor (

>      if (PcdGet32 (PcdArmArchTimerFreqInHz) != 0) {

>        //

>        // Check if ticks/uS is not 0. The Architectural timer runs at constant

> -      // frequency, irrespective of CPU frequency. According to General Timer

> +      // frequency, irrespective of CPU frequency. According to Generic Timer

>        // Ref manual, lower bound of the frequency is in the range of 1-10MHz.

>        //

>        ASSERT (TICKS_PER_MICRO_SEC);

> @@ -69,14 +69,14 @@ TimerConstructor (

>      }

>

>      //

> -    // Architectural Timer Frequency must be set in the Secure privileged

> +    // Architectural Timer Frequency must be set in Secure privileged

>      // mode (if secure extension is supported).

>      // If the reset value (0) is returned, just ASSERT.

>      //

>      ASSERT (ArmGenericTimerGetTimerFreq () != 0);

>

>    } else {

> -    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library can not be used.\n"));

> +    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library cannot be used.\n"));

>      ASSERT (0);

>    }

>

> @@ -89,7 +89,7 @@ TimerConstructor (

>

>    @param  MicroSeconds  The minimum number of microseconds to delay.

>

> -  @return The value of MicroSeconds inputted.

> +  @return The value of MicroSeconds input.


Not a problem with the patch, but I wonder what the point of that
return value is?!


>

>  **/

>  UINTN

> @@ -107,7 +107,7 @@ MicroSecondDelay (

>      TimerFreq = ArmGenericTimerGetTimerFreq ();

>    }

>

> -  // Calculate counter ticks that can represent requested delay:

> +  // Calculate counter ticks that represent requested delay:

>    //  = MicroSeconds x TICKS_PER_MICRO_SEC

>    //  = MicroSeconds x Frequency.10^-6

>    TimerTicks64 = DivU64x32 (

> @@ -123,7 +123,7 @@ MicroSecondDelay (

>

>    TimerTicks64 += SystemCounterVal;

>

> -  // Wait until delay count is expired.

> +  // Wait until delay count expires.

>    while (SystemCounterVal < TimerTicks64) {

>      SystemCounterVal = ArmGenericTimerGetSystemCount ();

>    }

> @@ -214,12 +214,12 @@ GetPerformanceCounterProperties (

>    )

>  {

>    if (StartValue != NULL) {

> -    // Timer starts with the reload value

> +    // Timer starts at 0


Whilst the comment change is correct, does it really tell us anything
the code doesn't??  I'm happy enough with the change, but I think I'd
have just deleted it!


>      *StartValue = (UINT64)0ULL ;

>    }

>

>    if (EndValue != NULL) {

> -    // Timer counts down to 0x0

> +    // Timer counts up.


This one made me spit my tea out.  I'm sure it's not funny :-)


>      *EndValue = 0xFFFFFFFFFFFFFFFFUL;

>    }

>

> --

> 2.7.0

>

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel April 1, 2016, 3:32 p.m. | #2
On 3 March 2016 at 12:55,  <evan.lloyd@arm.com> wrote:
> From: Evan Lloyd <evan.lloyd@arm.com>

>

> SOme minor typographical problems were noticed during previous commits.

> This change corrects those, and contains no functional modifications.

> The changes are in comments, and one diagnostic message.

>

> Code at: https://github.com/EvanLloyd/tianocore/commit/6de873f7e3fd63b045adf994e1c8289a7da66531

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>


Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Pushed as
f75cda7702e3 ArmPkg/ArmArchTimerLib: correct typos

with the TYpo fixed up :-)

> ---

>  ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 16 ++++++++--------

>  1 file changed, 8 insertions(+), 8 deletions(-)

>

> diff --git a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> index 2384b40..c97b1e5 100644

> --- a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> +++ b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c

> @@ -50,7 +50,7 @@ TimerConstructor (

>      if (PcdGet32 (PcdArmArchTimerFreqInHz) != 0) {

>        //

>        // Check if ticks/uS is not 0. The Architectural timer runs at constant

> -      // frequency, irrespective of CPU frequency. According to General Timer

> +      // frequency, irrespective of CPU frequency. According to Generic Timer

>        // Ref manual, lower bound of the frequency is in the range of 1-10MHz.

>        //

>        ASSERT (TICKS_PER_MICRO_SEC);

> @@ -69,14 +69,14 @@ TimerConstructor (

>      }

>

>      //

> -    // Architectural Timer Frequency must be set in the Secure privileged

> +    // Architectural Timer Frequency must be set in Secure privileged

>      // mode (if secure extension is supported).

>      // If the reset value (0) is returned, just ASSERT.

>      //

>      ASSERT (ArmGenericTimerGetTimerFreq () != 0);

>

>    } else {

> -    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library can not be used.\n"));

> +    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library cannot be used.\n"));

>      ASSERT (0);

>    }

>

> @@ -89,7 +89,7 @@ TimerConstructor (

>

>    @param  MicroSeconds  The minimum number of microseconds to delay.

>

> -  @return The value of MicroSeconds inputted.

> +  @return The value of MicroSeconds input.

>

>  **/

>  UINTN

> @@ -107,7 +107,7 @@ MicroSecondDelay (

>      TimerFreq = ArmGenericTimerGetTimerFreq ();

>    }

>

> -  // Calculate counter ticks that can represent requested delay:

> +  // Calculate counter ticks that represent requested delay:

>    //  = MicroSeconds x TICKS_PER_MICRO_SEC

>    //  = MicroSeconds x Frequency.10^-6

>    TimerTicks64 = DivU64x32 (

> @@ -123,7 +123,7 @@ MicroSecondDelay (

>

>    TimerTicks64 += SystemCounterVal;

>

> -  // Wait until delay count is expired.

> +  // Wait until delay count expires.

>    while (SystemCounterVal < TimerTicks64) {

>      SystemCounterVal = ArmGenericTimerGetSystemCount ();

>    }

> @@ -214,12 +214,12 @@ GetPerformanceCounterProperties (

>    )

>  {

>    if (StartValue != NULL) {

> -    // Timer starts with the reload value

> +    // Timer starts at 0

>      *StartValue = (UINT64)0ULL ;

>    }

>

>    if (EndValue != NULL) {

> -    // Timer counts down to 0x0

> +    // Timer counts up.

>      *EndValue = 0xFFFFFFFFFFFFFFFFUL;

>    }

>

> --

> 2.7.0

>

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
index 2384b40..c97b1e5 100644
--- a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
+++ b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
@@ -50,7 +50,7 @@  TimerConstructor (
     if (PcdGet32 (PcdArmArchTimerFreqInHz) != 0) {
       //
       // Check if ticks/uS is not 0. The Architectural timer runs at constant
-      // frequency, irrespective of CPU frequency. According to General Timer
+      // frequency, irrespective of CPU frequency. According to Generic Timer
       // Ref manual, lower bound of the frequency is in the range of 1-10MHz.
       //
       ASSERT (TICKS_PER_MICRO_SEC);
@@ -69,14 +69,14 @@  TimerConstructor (
     }
 
     //
-    // Architectural Timer Frequency must be set in the Secure privileged
+    // Architectural Timer Frequency must be set in Secure privileged
     // mode (if secure extension is supported).
     // If the reset value (0) is returned, just ASSERT.
     //
     ASSERT (ArmGenericTimerGetTimerFreq () != 0);
 
   } else {
-    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library can not be used.\n"));
+    DEBUG ((EFI_D_ERROR, "ARM Architectural Timer is not available in the CPU, hence this library cannot be used.\n"));
     ASSERT (0);
   }
 
@@ -89,7 +89,7 @@  TimerConstructor (
 
   @param  MicroSeconds  The minimum number of microseconds to delay.
 
-  @return The value of MicroSeconds inputted.
+  @return The value of MicroSeconds input.
 
 **/
 UINTN
@@ -107,7 +107,7 @@  MicroSecondDelay (
     TimerFreq = ArmGenericTimerGetTimerFreq ();
   }
 
-  // Calculate counter ticks that can represent requested delay:
+  // Calculate counter ticks that represent requested delay:
   //  = MicroSeconds x TICKS_PER_MICRO_SEC
   //  = MicroSeconds x Frequency.10^-6
   TimerTicks64 = DivU64x32 (
@@ -123,7 +123,7 @@  MicroSecondDelay (
 
   TimerTicks64 += SystemCounterVal;
 
-  // Wait until delay count is expired.
+  // Wait until delay count expires.
   while (SystemCounterVal < TimerTicks64) {
     SystemCounterVal = ArmGenericTimerGetSystemCount ();
   }
@@ -214,12 +214,12 @@  GetPerformanceCounterProperties (
   )
 {
   if (StartValue != NULL) {
-    // Timer starts with the reload value
+    // Timer starts at 0
     *StartValue = (UINT64)0ULL ;
   }
 
   if (EndValue != NULL) {
-    // Timer counts down to 0x0
+    // Timer counts up.
     *EndValue = 0xFFFFFFFFFFFFFFFFUL;
   }