ArmPkg: Fix bug in Generic Watchdog driver

In ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c, the following
functions:

  WatchdogWriteOffsetRegister()
  WatchdogWriteCompareRegister()
  WatchdogEnable()
  WatchdogDisable()

provide write access to ARM Generic Watchdog registers and use the values
returned by MmioWrite32() and MmioWrite64() as EFI_STATUS return codes.

Because MmioWriteXY() return the value passed as its write parameter,
Generic Watchdog access functions can spuriously return error codes which
are different from EFI_SUCCESS, e.g. the following call

    Status = WatchdogWriteOffsetRegister (MAX_UINT32);
    if (EFI_ERROR (Status)) {
      return Status;
    }

will return MAX_UINT32 defined in MdePkg/Include/Base.h as

 #define MAX_UINT32  ((UINT32)0xFFFFFFFF)

This commit declares all the functions listed above as VOID
and removes the code for checking their return values.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Alexei Fedorov <alexei.fedorov@arm.com>
Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This commit is contained in:
AlexeiFedorov 2018-04-27 14:58:43 +01:00 committed by Ard Biesheuvel
parent f9dff90289
commit 8fe18cba76
1 changed files with 16 additions and 21 deletions

View File

@ -1,6 +1,6 @@
/** @file /** @file
* *
* Copyright (c) 2013-2017, ARM Limited. All rights reserved. * Copyright (c) 2013-2018, ARM Limited. All rights reserved.
* *
* This program and the accompanying materials * This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD * are licensed and made available under the terms and conditions of the BSD
@ -43,36 +43,36 @@ UINT64 mNumTimerTicks = 0;
EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
EFI_STATUS VOID
WatchdogWriteOffsetRegister ( WatchdogWriteOffsetRegister (
UINT32 Value UINT32 Value
) )
{ {
return MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
} }
EFI_STATUS VOID
WatchdogWriteCompareRegister ( WatchdogWriteCompareRegister (
UINT64 Value UINT64 Value
) )
{ {
return MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value); MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
} }
EFI_STATUS VOID
WatchdogEnable ( WatchdogEnable (
VOID VOID
) )
{ {
return MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED); MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);
} }
EFI_STATUS VOID
WatchdogDisable ( WatchdogDisable (
VOID VOID
) )
{ {
return MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_DISABLED); 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
@ -163,9 +163,7 @@ WatchdogRegisterHandler (
then the watchdog timer is disabled. then the watchdog timer is disabled.
@retval EFI_SUCCESS The watchdog timer has been programmed to fire @retval EFI_SUCCESS The watchdog timer has been programmed to fire
in Time 100ns units. in TimerPeriod 100ns units.
@retval EFI_DEVICE_ERROR A watchdog timer could not be programmed due
to a device error.
**/ **/
EFI_STATUS EFI_STATUS
@ -176,12 +174,12 @@ WatchdogSetTimerPeriod (
) )
{ {
UINTN SystemCount; UINTN SystemCount;
EFI_STATUS Status;
// if TimerPeriod 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) { if (TimerPeriod == 0) {
mNumTimerTicks = 0; mNumTimerTicks = 0;
return WatchdogDisable (); WatchdogDisable ();
return EFI_SUCCESS;
} }
// Work out how many timer ticks will equate to TimerPeriod // Work out how many timer ticks will equate to TimerPeriod
@ -195,19 +193,16 @@ WatchdogSetTimerPeriod (
because enabling the watchdog causes an "explicit refresh", which because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't clobbers the compare register (WCV). In order to make sure this doesn't
trigger an interrupt, set the offset to max. */ trigger an interrupt, set the offset to max. */
Status = WatchdogWriteOffsetRegister (MAX_UINT32); WatchdogWriteOffsetRegister (MAX_UINT32);
if (EFI_ERROR (Status)) {
return Status;
}
WatchdogEnable (); WatchdogEnable ();
SystemCount = ArmGenericTimerGetSystemCount (); SystemCount = ArmGenericTimerGetSystemCount ();
Status = WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
} else { } else {
Status = WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks); WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks);
WatchdogEnable (); WatchdogEnable ();
} }
return Status; return EFI_SUCCESS;
} }
/** /**