| Message ID | 1457006121-7604-4-git-send-email-evan.lloyd@arm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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; }