Hi Liming, Mike,
Please share your inputs on this. Thanks, Meenakshi > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindh...@linaro.org] > Sent: Tuesday, December 05, 2017 4:36 PM > To: Udit Kumar <udit.ku...@nxp.com> > Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Meenakshi Aggarwal > <meenakshi.aggar...@nxp.com>; ard.biesheu...@linaro.org; edk2- > de...@lists.01.org; Varun Sethi <v.se...@nxp.com> > Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add > support for Watchdog driver > > On Tue, Dec 05, 2017 at 05:07:00AM +0000, Udit Kumar wrote: > > > I suggest return EFI_UNSUPPORTED for this case. The protocol > implementation > > > could return its status besides spec defined status. > > > > Thanks to help me , how core will treat this error > > 1/ Wdt not available > > 2/ ignoring this error > > 3/ core is not registering handler > > I guess 3 is valid, > > Looking at Core/Dxe/Misc/SetWatchdogTimer.c: > // > // Attempt to set the timeout > // > Status = gWatchdogTimer->SetTimerPeriod (gWatchdogTimer, > MultU64x32 (Timeout, WATCHDOG_TIMER_CALIBRATE_PER_SECOND)); > > // > // Check for errors > // > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > > The SetWatchdogTimer() call would always return EFI_DEVICE_ERROR. > > > On side track, looks wdt is not used by core services then do we > > really need this as part of arch protocol ? > > Yes, that was ultimately what I was implying with my question > regarding whether this protocol is relevant for a watchdog that can > only ever reset the system on timeout. > > The protocol looks to me to be designed to use a dedicated generic > timer as backing for a software watchdog. > > Liming, Mike? > > If that is the case, then I agree this driver should probably not > implement this protocol, but rather set up a timer event (or a > dedicated timer) to stroke the watchdog. > > Regards, > > Leif > > > regards > > Udit > > > > > -----Original Message----- > > > From: Gao, Liming [mailto:liming....@intel.com] > > > Sent: Monday, December 04, 2017 8:53 PM > > > To: Leif Lindholm <leif.lindh...@linaro.org>; Kinney, Michael D > > > <michael.d.kin...@intel.com> > > > Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; > > > ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Udit Kumar > > > <udit.ku...@nxp.com>; Varun Sethi <v.se...@nxp.com> > > > Subject: RE: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add > support > > > for Watchdog driver > > > > > > Leif: > > > I suggest return EFI_UNSUPPORTED for this case. The protocol > implementation > > > could return its status besides spec defined status. > > > > > > Thanks > > > Liming > > > > -----Original Message----- > > > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org] > > > > Sent: Monday, December 4, 2017 10:36 PM > > > > To: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > > > > <liming....@intel.com> > > > > Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; > > > > ard.biesheu...@linaro.org; edk2-devel@lists.01.org; > > > > udit.ku...@nxp.com; v.se...@nxp.com > > > > Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : > Add > > > > support for Watchdog driver > > > > > > > > Mike, Liming, as MdePkg mainteiners - one question below: > > > > > > > > On Mon, Nov 27, 2017 at 04:21:50PM +0530, Meenakshi Aggarwal > wrote: > > > > > diff --git a/Platform/NXP/Drivers/WatchDog/WatchDog.c > > > > > b/Platform/NXP/Drivers/WatchDog/WatchDog.c > > > > > new file mode 100644 > > > > > index 0000000..a9c70ef > > > > > --- /dev/null > > > > > +++ b/Platform/NXP/Drivers/WatchDog/WatchDog.c > > > > > @@ -0,0 +1,421 @@ > > > > > > > > ... > > > > > > > > > +/** > > > > > + 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 > > > > > +WdogRegisterHandler ( > > > > > + 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; > > > > > > > > Michael, Liming - what's your take on this? > > > > > > > > Is EFI_WATCHDOG_TIMER_ARCH_PROTOCOL suitable for use with a > pure-hw > > > > watchdog such as this? > > > > > > > > If so, what would be a suitable return code here? > > > > EFI_INVALID_PARAMETER does not look ideal. > > > > > > > > / > > > > Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
