From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::242; helo=mail-wr0-x242.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 68BB420356257 for ; Tue, 5 Dec 2017 03:01:40 -0800 (PST) Received: by mail-wr0-x242.google.com with SMTP id z34so20585432wrz.10 for ; Tue, 05 Dec 2017 03:06:10 -0800 (PST) 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=eucckSXF7gNLMbgCzPmLz9kZB+t9wpGr4E7HEcsGoRA=; b=My2zXTjLRPJtCtVCY5KmtWqGU1emLhJeliP4ghaqRJXGGD5m1r9GyTwIWaMLoJSvSr +kLoOFNgbxpXky5Eim7limb6R8hWTq7orfauNhhl8mF546S87oo9sSOEq4NGsG6zk3MU oamO/99kZf73yEo8f7TcbZ3ZRw17lampQOJgU= 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=eucckSXF7gNLMbgCzPmLz9kZB+t9wpGr4E7HEcsGoRA=; b=aMP4VUzDOMghZZhszcV1/IkiUh78DDDQoh1vpYh4LEvYdRhbGtZM6Wv9bqkhIEF/sn Q83Y4xkMl59baNhtLuTy5of473My8Pn33Osp6jCxjQjiw4X+J13mCVsrzEj4eIzLJVSt FspihCIZRQfd3xVCWoGpwBueQn9rPajtcgtGI+xKKHCQF9O4edzww0h3jU/SpaWk7Nix wEdynbLy62o/+fLx7ORjOdhalolCE7q1sDVH3/oHEVxVmL/lIjXUrkMI0xBZ+4fPSJN3 VVSeJmDCkGYN6N1RkVPdo1u/kHwDUKYuRgxAhzMFEXgMlawCoiLhHqVBcMmhwx7VzMVG 1HZg== X-Gm-Message-State: AJaThX54qb9sIY4M8wyNUGcrOV/OTClH3203WPr+nbxhy4Ah/lAFtA6t /JurJ9r2ZaMRfGaF+g1s7yXB7ryC1cA= X-Google-Smtp-Source: AGs4zMYo7PRb5RM04Nu4EzlqNKGHRX1bKl3wvZpJA+m1/nVbNbBz/v5WjN6n8KeOrfKdSD9pWc/jog== X-Received: by 10.223.182.153 with SMTP id j25mr15017589wre.242.1512471969559; Tue, 05 Dec 2017 03:06:09 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id w133sm228657wmg.9.2017.12.05.03.06.08 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 05 Dec 2017 03:06:08 -0800 (PST) Date: Tue, 5 Dec 2017 11:06:06 +0000 From: Leif Lindholm To: Udit Kumar Cc: "Gao, Liming" , "Kinney, Michael D" , Meenakshi Aggarwal , "ard.biesheuvel@linaro.org" , "edk2-devel@lists.01.org" , Varun Sethi Message-ID: <20171205110606.374td5x3rhdnhh5g@bivouac.eciton.net> References: <1511779917-9255-1-git-send-email-meenakshi.aggarwal@nxp.com> <1511779917-9255-3-git-send-email-meenakshi.aggarwal@nxp.com> <20171204143537.abxvjztldex2bjde@bivouac.eciton.net> <4A89E2EF3DFEDB4C8BFDE51014F606A14E189CAA@SHSMSX104.ccr.corp.intel.com> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add support for Watchdog driver X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Dec 2017 11:01:40 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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.gao@intel.com] > > Sent: Monday, December 04, 2017 8:53 PM > > To: Leif Lindholm ; Kinney, Michael D > > > > Cc: Meenakshi Aggarwal ; > > ard.biesheuvel@linaro.org; edk2-devel@lists.01.org; Udit Kumar > > ; Varun Sethi > > 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.lindholm@linaro.org] > > > Sent: Monday, December 4, 2017 10:36 PM > > > To: Kinney, Michael D ; Gao, Liming > > > > > > Cc: Meenakshi Aggarwal ; > > > ard.biesheuvel@linaro.org; edk2-devel@lists.01.org; > > > udit.kumar@nxp.com; v.sethi@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