From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web10.4302.1570704000643750085 for ; Thu, 10 Oct 2019 03:40:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=UfRpLUZg; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: leif.lindholm@linaro.org) Received: by mail-wr1-f65.google.com with SMTP id o18so7191690wrv.13 for ; Thu, 10 Oct 2019 03:40:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=XvbhfUKpI2B4k7Ucs2vGAm/C2bqD93lpuiMXQJInc/M=; b=UfRpLUZgryZj3aF6ko9Z3XnbC56QFjT6ZO658U4poaQ29S4dDcujLlqEK7a1oZqaFV 3le7FItBMDKgDVQFpmmq3eut/Psjk+k/MltCprwqrgp/qyAt0iTdc1e4paE5ImE+qLA/ mSlBOATqY/mEnFndr5yyTCen0aiapQYoNeJymluJujFx925NhjSI8hU4AgjyJVMPmMKD 8Bf+mSWZhBhmVzWaiCVcc0f6WbEJL147UAP9Pa1W0ScmeZ6q8sRL51PcKXOpou43Emgo sQBrxMaDbmHNbX2TXMX26mt0Z3/U6egk6eUVf9RbVUrAlZHeuGZ/T/J7pUMbO6B8SP3X cu2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=XvbhfUKpI2B4k7Ucs2vGAm/C2bqD93lpuiMXQJInc/M=; b=Ksy+HUCIzvMZxzAcSZFr+s/Qi7qBA98cfDdkGmIqs1QJL5VUMNqQuqaM8bHbw0ByN2 6jPiLn3RP+vC8HxJHTjersT2tS/TLE8NTZBrklfxbf6Kz+0wH5HYEk8cHyVyQhAKdDr/ sfxL1Mz9zUaygdvHn78C6wb0ubqB9axKzkkbRjdc2CPy9Hg5Bgc5xZJRRtNLJJRuXqc/ wZ55ByMNK4hTovpptPgIaI4rKup4q71AlfZB94psK4yzxPO7ry/ucbSobncyQDTGbr/C HshMp5TnQj27mLyiUpwU0Z7nNW+rCkFmqhxmnvKU/nNEueWD5vIf/4BJPTBIVH+oSSW3 bVng== X-Gm-Message-State: APjAAAVpKmDpjiVPzRbzSe/OE4/R1l1lUowJv7mV80qufZDaaXTgMRg8 qTiqkDowRl1seY4+yIneOk2lFA== X-Google-Smtp-Source: APXvYqwtu7+wdt7mZ0X73C0Z2wgDmylWqvDLoelUNU+MePJFfISrkt4CCJpMEUbMRJhMvPj3gE2JWg== X-Received: by 2002:adf:f40d:: with SMTP id g13mr7405082wro.368.1570703998956; Thu, 10 Oct 2019 03:39:58 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id l18sm4594543wrc.18.2019.10.10.03.39.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Oct 2019 03:39:58 -0700 (PDT) Date: Thu, 10 Oct 2019 11:39:56 +0100 From: "Leif Lindholm" To: Meenakshi Aggarwal Cc: ard.biesheuvel@linaro.org, michael.d.kinney@intel.com, devel@edk2.groups.io, v.sethi@nxp.com Subject: Re: [PATCH edk2-platforms 03/12] Silicon/NXP : Add support for Watchdog driver Message-ID: <20191010103956.GC25504@bivouac.eciton.net> References: <1543417315-5763-1-git-send-email-meenakshi.aggarwal@nxp.com> <1570639758-30355-1-git-send-email-meenakshi.aggarwal@nxp.com> <1570639758-30355-4-git-send-email-meenakshi.aggarwal@nxp.com> MIME-Version: 1.0 In-Reply-To: <1570639758-30355-4-git-send-email-meenakshi.aggarwal@nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Meenakshi, As you will have no doubt noticed, edk2-devel@lists.01.org has been retired - we are now using devel@edk2.groups.io, please subscribe (and post patches) to that one instead. I have bounced my replies to your first two patches there, and will try to remember to manually change the list address in my replies to the remaining patches. On Wed, Oct 09, 2019 at 10:19:09PM +0530, Meenakshi Aggarwal wrote: > Installs watchdog timer arch protocol > I am a little bit surprised by this patch being submitted again - in our last communication on this set (29 January), you said "Decided to use watchdog driver from MdeModulePkg." Did you change your mind? This driver still registers a EFI_WATCHDOG_TIMER_ARCH_PROTOCOL, which is a poor match for what this hardware actually does. Best Regards, Leif > Signed-off-by: Meenakshi Aggarwal > --- > Silicon/NXP/Drivers/WatchDog/WatchDog.c | 396 +++++++++++++++++++++++++++ > Silicon/NXP/Drivers/WatchDog/WatchDog.h | 32 +++ > Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf | 41 +++ > 3 files changed, 469 insertions(+) > create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDog.c > create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDog.h > create mode 100644 Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf > > diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDog.c b/Silicon/NXP/Drivers/WatchDog/WatchDog.c > new file mode 100644 > index 0000000..c2d104a > --- /dev/null > +++ b/Silicon/NXP/Drivers/WatchDog/WatchDog.c > @@ -0,0 +1,396 @@ > +/** WatchDog.c > +* > +* Based on Watchdog driver implemenation available in > +* ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > +* > +* Copyright 2017-2019 NXP > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "WatchDog.h" > + > +STATIC EFI_EVENT mEfiExitBootServicesEvent; > +STATIC EFI_EVENT mWatchdogFeedEvent; > +STATIC MMIO_OPERATIONS_16 *mMmioOps; > + > + > +STATIC > +VOID > +WatchdogPing ( > + VOID > + ) > +{ > + // > + // To reload a timeout value to the counter the proper service sequence begins by > + // writing 0x_5555 followed by 0x_AAAA to the Watchdog Service Register (WATCHDOG_WSR). > + // This service sequence will reload the counter with the timeout value WATCHDOG[7:0] of > + // Watchdog Control Register (WATCHDOG_WCR). > + // > + > + mMmioOps->Write16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WSR_OFFSET, > + WATCHDOG_SERVICE_SEQ1); > + mMmioOps->Write16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WSR_OFFSET, > + WATCHDOG_SERVICE_SEQ2); > +} > + > +/** > + Stop the Watchdog watchdog timer from counting down. > +**/ > +STATIC > +VOID > +WatchdogStop ( > + VOID > + ) > +{ > + // Watchdog cannot be disabled by software once started. > + // At best, we can keep reload counter with maximum value > + > + mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET, > + (UINT16)(~WATCHDOG_WCR_TIMEOUT), > + (WATCHDOG_COUNT (WATCHDOG_MAX_TIME) & WATCHDOG_COUNT_MASK)); > + WatchdogPing (); > +} > + > +/** > + Starts the Watchdog counting down by feeding Service register with > + desired pattern. > + The count down will start from the value stored in the Load register, > + not from the value where it was previously stopped. > +**/ > +STATIC > +VOID > +WatchdogStart ( > + VOID > + ) > +{ > + //Reload the timeout value > + WatchdogPing (); > +} > + > +/** > + On exiting boot services we must make sure the Watchdog Watchdog Timer > + is stopped. > +**/ > +STATIC > +VOID > +EFIAPI > +ExitBootServicesEvent ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + WatchdogStop (); > +} > + > +/** > + This function registers the handler NotifyFunction so it is called every time > + the watchdog timer expires. It also passes the amount of time since the last > + handler call to the NotifyFunction. > + If NotifyFunction is not NULL and a handler is not already registered, > + 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. > + > + @param This The EFI_TIMER_ARCH_PROTOCOL instance. > + @param NotifyFunction The function to call when a timer interrupt fires. This > + function executes at TPL_HIGH_LEVEL. The DXE Core will > + register a handler for the timer interrupt, so it can know > + how much time has passed. This 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. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +WatchdogRegisterHandler ( > + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, > + IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction > + ) > +{ > + // ERROR: This function is not supported. > + // The hardware watchdog will reset the board > + return EFI_INVALID_PARAMETER; > +} > + > +/** > + > + This function adjusts the period of timer interrupts to the value specified > + by TimerPeriod. If the timer period is updated, then the selected timer > + period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned. If > + the timer hardware is not programmable, then EFI_UNSUPPORTED is returned. > + If an error occurs while attempting to update the timer period, then the > + timer hardware will be put back in its state prior to this call, and > + EFI_DEVICE_ERROR is returned. If TimerPeriod is 0, then the timer interrupt > + is disabled. This is not the same as disabling the CPU's interrupts. > + Instead, it must either turn off the timer hardware, or it must adjust the > + interrupt controller so that a CPU interrupt is not generated when the timer > + interrupt fires. > + > + @param This The EFI_TIMER_ARCH_PROTOCOL instance. > + @param TimerPeriod The rate to program the timer interrupt in 100 nS units. If > + the timer hardware is not programmable, then EFI_UNSUPPORTED is > + returned. If the timer is programmable, then the timer period > + will be rounded up to the nearest timer period that is supported > + by the timer hardware. If TimerPeriod is set to 0, then the > + timer interrupts will be disabled. > + > + > + @retval EFI_SUCCESS The timer period was changed. > + @retval EFI_UNSUPPORTED The platform cannot change the period of the timer interrupt. > + @retval EFI_DEVICE_ERROR The timer period could not be changed due to a device error. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +WatchdogSetTimerPeriod ( > + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, > + IN UINT64 TimerPeriod // In 100ns units > + ) > +{ > + EFI_STATUS Status; > + UINT64 TimerPeriodInSec; > + UINT16 Val; > + > + Status = EFI_SUCCESS; > + > + if (TimerPeriod == 0) { > + // This is a watchdog stop request > + WatchdogStop (); > + return Status; > + } else { > + // Convert the TimerPeriod (in 100 ns unit) to an equivalent second value > + > + TimerPeriodInSec = DivU64x32 (TimerPeriod, NANO_SECOND_BASE); > + > + // The registers in the Watchdog are only 32 bits > + if (TimerPeriodInSec > WATCHDOG_MAX_TIME) { > + // We could load the watchdog with the maximum supported value but > + // if a smaller value was requested, this could have the watchdog > + // triggering before it was intended. > + // Better generate an error to let the caller know. > + Status = EFI_DEVICE_ERROR; > + return Status; > + } > + > + // set the new timeout value in the WCR > + // Convert the timeout value from Seconds to timer count > + Val = ((WATCHDOG_COUNT(TimerPeriodInSec) & WATCHDOG_COUNT_MASK) << 8); > + > + mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET, > + (UINT16)(~WATCHDOG_WCR_TIMEOUT), > + Val); > + // Start the watchdog > + WatchdogStart (); > + } > + > + return Status; > +} > + > +/** > + This function retrieves the period of timer interrupts in 100 ns 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 currently disabled. > + > + > + @retval EFI_SUCCESS The timer period was returned in TimerPeriod. > + @retval EFI_INVALID_PARAMETER TimerPeriod is NULL. > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +WatchdogGetTimerPeriod ( > + IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This, > + OUT UINT64 *TimerPeriod > + ) > +{ > + EFI_STATUS Status; > + UINT64 ReturnValue; > + UINT16 Val; > + > + Status = EFI_SUCCESS; > + > + if (TimerPeriod == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // Check if the watchdog is stopped > + if ((mMmioOps->Read16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET) > + & WATCHDOG_WCR_ENABLE) == 0 ) { > + // It is stopped, so return zero. > + ReturnValue = 0; > + } else { > + // Convert the Watchdog ticks into equivalent TimerPeriod second value. > + Val = (mMmioOps->Read16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET) > + & WATCHDOG_WCR_TIMEOUT ) >> 8; > + ReturnValue = WATCHDOG_SEC(Val); > + } > + > + *TimerPeriod = ReturnValue; > + return Status; > +} > + > +/** > + Interface structure for the Watchdog Architectural Protocol. > + > + @par Protocol Description: > + This protocol provides a service to set the amount of time to wait > + before firing the watchdog timer, and it also provides a service to > + register a handler that is invoked when the watchdog timer fires. > + > + @par When the watchdog timer fires, control will be passed to a handler > + if one has been registered. If no handler has been registered, > + or the registered handler returns, then the system will be > + reset by calling the Runtime Service ResetSystem(). > + > + @param RegisterHandler > + Registers a handler that will be called each time the > + watchdogtimer interrupt fires. TimerPeriod defines the minimum > + time between timer interrupts, so TimerPeriod will also > + be the minimum time between calls to the registered > + handler. > + NOTE: If the watchdog resets the system in hardware, then > + this function will not have any chance of executing. > + > + @param SetTimerPeriod > + Sets the period of the timer interrupt in 100 nS 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. > + > +**/ > +STATIC > +EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { > + WatchdogRegisterHandler, > + WatchdogSetTimerPeriod, > + WatchdogGetTimerPeriod > +}; > + > +/** > + Call back function when the timer event is signaled. > + This function will feed the watchdog with maximum value > + so that system wont reset in idle case e.g. stopped on UEFI shell. > + > + @param[in] Event The Event this notify function registered to. > + @param[in] Context Pointer to the context data registered to the > + Event. > + > +**/ > +VOID > +EFIAPI > +WatchdogFeed ( > + IN EFI_EVENT Event, > + IN VOID* Context > + ) > +{ > + WatchdogPing(); > +} > +/** > + Initialize state information for the Watchdog Timer Architectural Protocol. > + > + @param ImageHandle of the loaded driver > + @param SystemTable Pointer to the System Table > + > + @retval EFI_SUCCESS Protocol registered > + @retval EFI_OUT_OF_RESOURCES Cannot allocate protocol data structure > + @retval EFI_DEVICE_ERROR Hardware problems > + > +**/ > +EFI_STATUS > +EFIAPI > +WatchdogInitialize ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + > + mMmioOps = GetMmioOperations16 (FixedPcdGetBool (PcdWatchdogBigEndian)); > + > + mMmioOps->AndThenOr16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET, > + (UINT16)(~WATCHDOG_WCR_TIMEOUT), > + (WATCHDOG_COUNT (WATCHDOG_MAX_TIME) & WATCHDOG_COUNT_MASK)); > + > + mMmioOps->Or16 (PcdGet64 (PcdWatchdog1BaseAddr) + WATCHDOG_WCR_OFFSET, WATCHDOG_WCR_ENABLE); > + > + // > + // 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); > + > + // Register for an ExitBootServicesEvent > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY, > + ExitBootServicesEvent, NULL, &mEfiExitBootServicesEvent); > + if (EFI_ERROR (Status)) { > + Status = EFI_OUT_OF_RESOURCES; > + return Status; > + } > + > + // > + // Start the timer to feed Watchdog with maximum timeout value. > + // > + Status = gBS->CreateEvent ( > + EVT_TIMER | EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + WatchdogFeed, > + NULL, > + &mWatchdogFeedEvent > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status = gBS->SetTimer (mWatchdogFeedEvent, TimerPeriodic, WATCHDOG_FEED_INTERVAL); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + // Install the Timer Architectural Protocol onto a new handle > + Handle = NULL; > + Status = gBS->InstallMultipleProtocolInterfaces ( > + &Handle, > + &gEfiWatchdogTimerArchProtocolGuid, &mWatchdogTimer, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + gBS->CloseEvent (mEfiExitBootServicesEvent); > + Status = EFI_OUT_OF_RESOURCES; > + return Status; > + } > + > + WatchdogPing (); > + > + return Status; > +} > diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDog.h b/Silicon/NXP/Drivers/WatchDog/WatchDog.h > new file mode 100644 > index 0000000..8262a69 > --- /dev/null > +++ b/Silicon/NXP/Drivers/WatchDog/WatchDog.h > @@ -0,0 +1,32 @@ > +/** WatchDog.h > +* > +* Copyright 2017-2019 NXP > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#ifndef WATCHDOG_H_ > +#define WATCHDOG_H_ > + > +#define WATCHDOG_SIZE 0x1000 > +#define WATCHDOG_WCR_OFFSET 0 > +#define WATCHDOG_WSR_OFFSET 2 > +#define WATCHDOG_WRSR_OFFSET 4 > +#define WATCHDOG_WICR_OFFSET 6 > +#define WATCHDOG_WCR_TIMEOUT (0xFF << 8) > +#define WATCHDOG_WCR_ENABLE (1 << 2) > +#define WATCHDOG_SERVICE_SEQ1 0x5555 > +#define WATCHDOG_SERVICE_SEQ2 0xAAAA > +#define WATCHDOG_WCR_WRE (1 << 3) /* -> WATCHDOG Reset Enable */ > + > +#define WATCHDOG_MAX_TIME 128 > +#define WATCHDOG_COUNT(Sec) (((Sec) * 2 - 1) << 8) > +#define WATCHDOG_COUNT_MASK 0xff00 > +#define WATCHDOG_SEC(Cnt) (((Cnt) + 1) / 2) > + > +#define NANO_SECOND_BASE 10000000 > + > +#define WATCHDOG_FEED_INTERVAL (WATCHDOG_MAX_TIME * NANO_SECOND_BASE) > + > +#endif //WATCHDOG_H_ > diff --git a/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf b/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf > new file mode 100644 > index 0000000..2d410a9 > --- /dev/null > +++ b/Silicon/NXP/Drivers/WatchDog/WatchDogDxe.inf > @@ -0,0 +1,41 @@ > +# WatchDog.inf > +# > +# Component description file for WatchDog module > +# > +# Copyright 2017-2019 NXP > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = WatchDogDxe > + FILE_GUID = 0358b544-ec65-4339-89cd-cad60a3dd787 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = WatchdogInitialize > + > +[Sources.common] > + WatchDog.c > + > +[Packages] > + MdePkg/MdePkg.dec > + Silicon/NXP/NxpQoriqLs.dec > + > +[LibraryClasses] > + BaseLib > + IoAccessLib > + PcdLib > + UefiBootServicesTableLib > + UefiDriverEntryPoint > + > +[Pcd] > + gNxpQoriqLsTokenSpaceGuid.PcdWatchdog1BaseAddr > + gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian > + > +[Protocols] > + gEfiWatchdogTimerArchProtocolGuid > + > +[Depex] > + TRUE > -- > 1.9.1 >