From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754708AbbFKQiv (ORCPT ); Thu, 11 Jun 2015 12:38:51 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:52908 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751108AbbFKQis (ORCPT ); Thu, 11 Jun 2015 12:38:48 -0400 Date: Thu, 11 Jun 2015 09:38:43 -0700 From: Guenter Roeck To: Fu Wei Cc: Suravee Suthikulpanit , Linaro ACPI Mailman List , linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, Wei Fu , G Gregory , Al Stone , Hanjun Guo , Timur Tabi , Ashwin Chaugule , Arnd Bergmann , Vipul Gandhi , Wim Van Sebroeck , Jon Masters , Leo Duran , Jon Corbet , Mark Rutland , Catalin Marinas , Will Deacon , rjw@rjwysocki.net Subject: Re: [PATCH v5 4/8] Watchdog: introdouce "pretimeout" into framework Message-ID: <20150611163843.GA23656@roeck-us.net> References: <=fu.wei@linaro.org> <1433943713-32466-1-git-send-email-fu.wei@linaro.org> <1433943713-32466-5-git-send-email-fu.wei@linaro.org> <5578641E.1060209@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=0.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 11, 2015 at 07:22:44PM +0800, Fu Wei wrote: > Hi Guenter, > [ ... ] > > > value we are trying to set. Effectively the above accepts every pretimeout > > if wdd->pretimeout is 0. It also accepts every pretimeout if > > max_pretimeout == 0, even if wdd->timeout is set and t >= wdd->timeout. > > > > Try > > > > return (wdd->max_pretimeout && (t < wdd->min_pretimeout || > > t > wdd->max_pretimeout)) || > > (wdd->timeout && t >= wdd->timeout); > > > >> } > > /* > * Use the following function to check if a pretimeout value is invalid. > * It can be "0", that means we don't use pretimeout. > * This function returns 0, when pretimeout is 0. returns false if pretimeout is 0. > */ > static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd, > unsigned int t) > { > return t && (wdd->max_pretimeout && > (t < wdd->min_pretimeout || t > wdd->max_pretimeout)) || > (wdd->timeout && t >= wdd->timeout); > } > Makes sense. Be careful with (), though. This evaluates to return t && (...) || (wdd->timeout && t >= wdd->timeout); but it should probably be return t && ((...) || (wdd->timeout && t >= wdd->timeout)); That doesn't make a difference in practice (if t == 0, it is never >= timeout if timeout is > 0), but it would be a bit cleaner. > > >> > >> /* Use the following functions to manipulate watchdog driver specific > >> data */ > >> @@ -132,11 +153,20 @@ static inline void *watchdog_get_drvdata(struct > >> watchdog_device *wdd) > >> } > >> > >> /* drivers/watchdog/watchdog_core.c */ > >> -extern int watchdog_init_timeout(struct watchdog_device *wdd, > >> - unsigned int timeout_parm, struct device > >> *dev); > >> +int watchdog_init_timeouts(struct watchdog_device *wdd, > >> + unsigned int pretimeout_parm, > >> + unsigned int timeout_parm, > >> + struct device *dev); > > > > > > Please align continuation lines with '('. > > Fixed , thanks > > > > > > >> extern int watchdog_register_device(struct watchdog_device *); > >> extern void watchdog_unregister_device(struct watchdog_device *); > >> > >> +static inline int watchdog_init_timeout(struct watchdog_device *wdd, > >> + unsigned int timeout_parm, > >> + struct device *dev) > >> +{ > >> + return watchdog_init_timeouts(wdd, 0, timeout_parm, dev); > >> +} > >> + > >> #ifdef CONFIG_HARDLOCKUP_DETECTOR > >> void watchdog_nmi_disable_all(void); > >> void watchdog_nmi_enable_all(void); > >> > > > > > > -- > Best regards, > > Fu Wei > Software Engineer > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch > Ph: +86 21 61221326(direct) > Ph: +86 186 2020 4684 (mobile) > Room 1512, Regus One Corporate Avenue,Level 15, > One Corporate Avenue,222 Hubin Road,Huangpu District, > Shanghai,China 200021 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Subject: Re: [PATCH v5 4/8] Watchdog: introdouce "pretimeout" into framework Date: Thu, 11 Jun 2015 09:38:43 -0700 Message-ID: <20150611163843.GA23656@roeck-us.net> References: <=fu.wei@linaro.org> <1433943713-32466-1-git-send-email-fu.wei@linaro.org> <1433943713-32466-5-git-send-email-fu.wei@linaro.org> <5578641E.1060209@roeck-us.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-watchdog-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Fu Wei Cc: Suravee Suthikulpanit , Linaro ACPI Mailman List , linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Wei Fu , G Gregory , Al Stone , Hanjun Guo , Timur Tabi , Ashwin Chaugule , Arnd Bergmann , Vipul Gandhi , Wim Van Sebroeck , Jon Masters , Leo Duran , Jon Corbet , Mark Rutland , Catalin Marinas , Will Deacon , rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org List-Id: devicetree@vger.kernel.org On Thu, Jun 11, 2015 at 07:22:44PM +0800, Fu Wei wrote: > Hi Guenter, > [ ... ] > > > value we are trying to set. Effectively the above accepts every pretimeout > > if wdd->pretimeout is 0. It also accepts every pretimeout if > > max_pretimeout == 0, even if wdd->timeout is set and t >= wdd->timeout. > > > > Try > > > > return (wdd->max_pretimeout && (t < wdd->min_pretimeout || > > t > wdd->max_pretimeout)) || > > (wdd->timeout && t >= wdd->timeout); > > > >> } > > /* > * Use the following function to check if a pretimeout value is invalid. > * It can be "0", that means we don't use pretimeout. > * This function returns 0, when pretimeout is 0. returns false if pretimeout is 0. > */ > static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd, > unsigned int t) > { > return t && (wdd->max_pretimeout && > (t < wdd->min_pretimeout || t > wdd->max_pretimeout)) || > (wdd->timeout && t >= wdd->timeout); > } > Makes sense. Be careful with (), though. This evaluates to return t && (...) || (wdd->timeout && t >= wdd->timeout); but it should probably be return t && ((...) || (wdd->timeout && t >= wdd->timeout)); That doesn't make a difference in practice (if t == 0, it is never >= timeout if timeout is > 0), but it would be a bit cleaner. > > >> > >> /* Use the following functions to manipulate watchdog driver specific > >> data */ > >> @@ -132,11 +153,20 @@ static inline void *watchdog_get_drvdata(struct > >> watchdog_device *wdd) > >> } > >> > >> /* drivers/watchdog/watchdog_core.c */ > >> -extern int watchdog_init_timeout(struct watchdog_device *wdd, > >> - unsigned int timeout_parm, struct device > >> *dev); > >> +int watchdog_init_timeouts(struct watchdog_device *wdd, > >> + unsigned int pretimeout_parm, > >> + unsigned int timeout_parm, > >> + struct device *dev); > > > > > > Please align continuation lines with '('. > > Fixed , thanks > > > > > > >> extern int watchdog_register_device(struct watchdog_device *); > >> extern void watchdog_unregister_device(struct watchdog_device *); > >> > >> +static inline int watchdog_init_timeout(struct watchdog_device *wdd, > >> + unsigned int timeout_parm, > >> + struct device *dev) > >> +{ > >> + return watchdog_init_timeouts(wdd, 0, timeout_parm, dev); > >> +} > >> + > >> #ifdef CONFIG_HARDLOCKUP_DETECTOR > >> void watchdog_nmi_disable_all(void); > >> void watchdog_nmi_enable_all(void); > >> > > > > > > -- > Best regards, > > Fu Wei > Software Engineer > Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch > Ph: +86 21 61221326(direct) > Ph: +86 186 2020 4684 (mobile) > Room 1512, Regus One Corporate Avenue,Level 15, > One Corporate Avenue,222 Hubin Road,Huangpu District, > Shanghai,China 200021 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html