On Mon, Sep 11, 2017 at 04:23:35PM +0100, evan.ll...@arm.com wrote:
> From: Evan Lloyd <evan.ll...@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")
Yes, in general I prefer fewer Siemens in my code.
> 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.ll...@arm.com>
> Signed-off-by: Alexei Fedorov <alexei.fedo...@arm.com>
No objections to any of the changes, but I would be really happy if
this was reordered such that this comes before any patches with
functional changes.
That way git blame/praise output is more helpful.
/
Leif
> ---
> 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")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
