Re: [edk2] [PATCH 1/4] ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup
On Tue, Dec 18, 2018 at 02:10:11PM +0100, Ard Biesheuvel wrote: > Before fixing the SP805 driver, let's clean it up a bit. No > functional changes. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c | 97 > ++-- > ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf | 13 +-- > 2 files changed, 53 insertions(+), 57 deletions(-) > > diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > index 0a9f64095bf8..12c2f0a1fe49 100644 > --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > @@ -1,6 +1,7 @@ > /** @file > * > * Copyright (c) 2011-2013, ARM Limited. All rights reserved. > +* Copyright (c) 2018, Linaro Limited. All rights reserved. > * > * This program and the accompanying materials > * are licensed and made available under the terms and conditions of the BSD > License > @@ -19,16 +20,13 @@ > #include > #include > #include > -#include > #include > -#include > -#include > > #include > > #include "SP805Watchdog.h" > > -EFI_EVENT EfiExitBootServicesEvent = > (EFI_EVENT)NULL; > +STATIC EFI_EVENT mEfiExitBootServicesEvent; > > /** >Make sure the SP805 registers are unlocked for writing. > @@ -43,8 +41,8 @@ SP805Unlock ( >VOID >) > { > - if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED ) { > -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE); > + if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED) { > +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE); >} > } > > @@ -61,9 +59,9 @@ SP805Lock ( >VOID >) > { > - if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED ) { > + if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED) { > // To lock it, just write in any number (except the special unlock code). > -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED); > +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED); >} > } > > @@ -77,8 +75,8 @@ SP805Stop ( >) > { >// Disable interrupts > - if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0 ) { > -MmioAnd32(SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN); > + if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0) { > +MmioAnd32 (SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN); >} > } > > @@ -94,8 +92,8 @@ SP805Start ( >) > { >// Enable interrupts > - if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0 ) { > -MmioOr32(SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN); > + if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0) { > +MmioOr32 (SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN); >} > } > > @@ -103,6 +101,7 @@ SP805Start ( > On exiting boot services we must make sure the SP805 Watchdog Timer > is stopped. > **/ > +STATIC > VOID > EFIAPI > ExitBootServicesEvent ( > @@ -110,9 +109,9 @@ ExitBootServicesEvent ( >IN VOID *Context >) > { > - SP805Unlock(); > - SP805Stop(); > - SP805Lock(); > + SP805Unlock (); > + SP805Stop (); > + SP805Lock (); > } > > /** > @@ -142,10 +141,11 @@ ExitBootServicesEvent ( > previously registered. > > **/ > +STATIC > EFI_STATUS > EFIAPI > SP805RegisterHandler ( > - IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, > + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, >IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction >) > { > @@ -182,22 +182,24 @@ SP805RegisterHandler ( >@retval EFI_DEVICE_ERROR The timer period could not be changed due to > a device error. > > **/ > +STATIC > EFI_STATUS > EFIAPI > SP805SetTimerPeriod ( > - IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, > + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, >IN UINT64 TimerPeriod // In 100ns units >) > { > - EFI_STATUS Status = EFI_SUCCESS; > + EFI_STATUS Status; >UINT64 Ticks64bit; > > - SP805Unlock(); > + SP805Unlock (); > > - if( TimerPeriod == 0 ) { > + Status = EFI_SUCCESS; > + > + if (TimerPeriod == 0) { > // This is a watchdog stop request > -SP805Stop(); > -goto EXIT; > +SP805Stop (); >} else { > // Calculate the Watchdog ticks required for a delay of (TimerTicks * > 100) nanoseconds > // The SP805 will count down to ZERO once, generate an interrupt and > @@ -211,10 +213,11 @@ SP805SetTimerPeriod ( > // > // WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ; > > -Ticks64bit = DivU64x32(MultU64x32(TimerPeriod, > (UINTN)PcdGet32(PcdSP805WatchdogClockFrequencyInHz)), 2000); >
[edk2] [PATCH 1/4] ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup
Before fixing the SP805 driver, let's clean it up a bit. No functional changes. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c | 97 ++-- ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf | 13 +-- 2 files changed, 53 insertions(+), 57 deletions(-) diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c index 0a9f64095bf8..12c2f0a1fe49 100644 --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c @@ -1,6 +1,7 @@ /** @file * * Copyright (c) 2011-2013, ARM Limited. All rights reserved. +* Copyright (c) 2018, Linaro Limited. All rights reserved. * * This program and the accompanying materials * are licensed and made available under the terms and conditions of the BSD License @@ -19,16 +20,13 @@ #include #include #include -#include #include -#include -#include #include #include "SP805Watchdog.h" -EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL; +STATIC EFI_EVENT mEfiExitBootServicesEvent; /** Make sure the SP805 registers are unlocked for writing. @@ -43,8 +41,8 @@ SP805Unlock ( VOID ) { - if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED ) { -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE); + if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED) { +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE); } } @@ -61,9 +59,9 @@ SP805Lock ( VOID ) { - if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED ) { + if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED) { // To lock it, just write in any number (except the special unlock code). -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED); +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED); } } @@ -77,8 +75,8 @@ SP805Stop ( ) { // Disable interrupts - if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0 ) { -MmioAnd32(SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN); + if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0) { +MmioAnd32 (SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN); } } @@ -94,8 +92,8 @@ SP805Start ( ) { // Enable interrupts - if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0 ) { -MmioOr32(SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN); + if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0) { +MmioOr32 (SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN); } } @@ -103,6 +101,7 @@ SP805Start ( On exiting boot services we must make sure the SP805 Watchdog Timer is stopped. **/ +STATIC VOID EFIAPI ExitBootServicesEvent ( @@ -110,9 +109,9 @@ ExitBootServicesEvent ( IN VOID *Context ) { - SP805Unlock(); - SP805Stop(); - SP805Lock(); + SP805Unlock (); + SP805Stop (); + SP805Lock (); } /** @@ -142,10 +141,11 @@ ExitBootServicesEvent ( previously registered. **/ +STATIC EFI_STATUS EFIAPI SP805RegisterHandler ( - IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction ) { @@ -182,22 +182,24 @@ SP805RegisterHandler ( @retval EFI_DEVICE_ERROR The timer period could not be changed due to a device error. **/ +STATIC EFI_STATUS EFIAPI SP805SetTimerPeriod ( - IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, IN UINT64 TimerPeriod // In 100ns units ) { - EFI_STATUS Status = EFI_SUCCESS; + EFI_STATUS Status; UINT64 Ticks64bit; - SP805Unlock(); + SP805Unlock (); - if( TimerPeriod == 0 ) { + Status = EFI_SUCCESS; + + if (TimerPeriod == 0) { // This is a watchdog stop request -SP805Stop(); -goto EXIT; +SP805Stop (); } else { // Calculate the Watchdog ticks required for a delay of (TimerTicks * 100) nanoseconds // The SP805 will count down to ZERO once, generate an interrupt and @@ -211,10 +213,11 @@ SP805SetTimerPeriod ( // // WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ; -Ticks64bit = DivU64x32(MultU64x32(TimerPeriod, (UINTN)PcdGet32(PcdSP805WatchdogClockFrequencyInHz)), 2000); +Ticks64bit = MultU64x32 (TimerPeriod, PcdGet32 (PcdSP805WatchdogClockFrequencyInHz)); +Ticks64bit = DivU64x32 (Ticks64bit, 2000); // The registers in the SP805 are only 32 bits -if(Ticks64bit > (UINT64)0x) { +if (Ticks64bit > MAX_UINT32) { // We could load the watchdog with the maximum supported value but // if a smaller value was request