From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751922AbbKCFFI (ORCPT ); Tue, 3 Nov 2015 00:05:08 -0500 Received: from mail-bl2on0108.outbound.protection.outlook.com ([65.55.169.108]:13077 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750742AbbKCFFF (ORCPT ); Tue, 3 Nov 2015 00:05:05 -0500 X-Greylist: delayed 487 seconds by postgrey-1.27 at vger.kernel.org; Tue, 03 Nov 2015 00:05:04 EST Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=freescale.com; lists.infradead.org; dkim=none (message not signed) header.d=none;lists.infradead.org; dmarc=none action=none header.from=freescale.com; Date: Tue, 3 Nov 2015 12:47:44 +0800 From: Robin Gong To: Guenter Roeck CC: , , , Subject: Re: [PATCH v1 1/2] watchdog: add WDIOC_SETPRETIMEOUT and WDIOC_GETPRETIMEOUT Message-ID: <20151103044741.GA26305@shlinux2> References: <1446521367-25748-1-git-send-email-b38343@freescale.com> <56383244.3030801@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <56383244.3030801@roeck-us.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11FD032;1:dac/PR44xyb8fhE080XZZWdk7eZYMy0xo5QWZdIXq2dx40lsbaS1v1BeuqtcLdd1Lfiix0jttHcr9Z3RxyjIahryeKCwinu4SDFw5j2S13FMXXJjTlI5Z/SazCHH1Ijmn9ZU3FjyPT0XTRCw3OXw2T1ZgAoKAx2bj0eo4UDaa4tw0tgPuOzRWPbz5o/x4lq2GdJJJqPeq2+kV/kvhywhBB8TbklwtSmY/gxQR91H2rhitI3Sex1SydJK3/P/6DdWv2QBSXRr8e3cKcQ9b855hAEW1WCsTsts+ibjnlZgSCRr56CgDzETN/R2gWXLXPX3AMZLJ+FJNLq6hIiwZyxooQlaXUgfVoAdSoXoZ9gNH5a7oItzDCNJpqTm31G+GB03 X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1109001)(1110001)(339900001)(24454002)(199003)(479174004)(189002)(377454003)(23726002)(19580405001)(2950100001)(87936001)(46406003)(92566002)(97736004)(83506001)(81156007)(4001350100001)(33656002)(85426001)(106466001)(105606002)(5008740100001)(47776003)(5007970100001)(19580395003)(189998001)(50466002)(6806005)(104016004)(11100500001)(33716001)(97756001)(575784001)(5001960100002)(54356999)(110136002)(76176999)(77096005)(50986999)(42262002);DIR:OUT;SFP:1102;SCL:1;SRVR:SN2PR03MB096;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB096;2:kUA3EAU0iKCB7IgkXl5MhQTYrRMM0UOM7p5wM7rhjViFNOA+57b2q5W1VSycnQjwrDVdOjTbJHtk70I368qHfYE6WSirgy9ZtzZzwKzdUEaOpd03/6BRbY1zdI4/hKX0TcchY/1psqgKMbC5OpcQ5bQEkK3eqqIrm3XCWCh7vIY=;3:6YpYJ/x6q2Vs9+/hKV7mfTJ5PGKEv/Nc91wxFo3zJH72DivPuOJO6UvwIMOWJ2f8KvfJF+NLePIxb2pK05PzttBS4cD9OyI5m7rnpHd9RMPyTTTsXIbTZa1DE13GIc6W7S5iF3clUelgfOyfZ++2TbuMqdyAnhZEujxgzdwdrxKztcQdxUy+4N+xi5oGdqVT1aqec60Mag20qnxks2s4hcJxzDkjAKKYDNOm1p7YPds=;25:lT9ENf59/ylm3jpwtvTkAJZR9reBYYi/8nEHAbRU7asGnAJtolyzqGTEesl1F42ZwvTSbY5c8oLDiTXwCUrP3VttXgog7qSCx4AUo2/OyjZfXAezd320srdbyWDZQ3EbGlmyLn5xd2w1jt9YAWk5y8770Gi7krNFFMZzhd1qXqdUKyIp+BdyZOyvMtfRKEL9UVB32/kMBy2/+oKAgLzIpWPs3tuNZCjZFuc6GnhvslHLXsE5a2G2NSCb+6+txU8BFhqbdcvkZH9ReF2fU1uR4g== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN2PR03MB096; X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB096;20:7OAr4DjzuCSejFUwkLGp1XAh7o0/W9fs3INewQQ39K/N4Dqml/dGAOCi55y+vz3em9jEJai9GLEnNMce4AlK/Z2MK1KeBqnGT3Pj3GLNKy3jd/8tx/f1EEL4heLMnQyc035csanPbqE17ObESPMbRflrgz3PjDMFqvpZuJkCcuWAwfEtPcwnhAiQHG7GS62m2j0AJyaMPUVXJusiqR/KKC93qE8hatKf1BUiFQDEmA7MuRK45uTrWK0X/jlkA7gN9VqibHF4QjbjbvXwenHYBdjUxVjtkWSAujCcnMnmaeUSn5gdz2u96oS/NZ+K0wWhKPsf3pJv9lz2KPiXzgmhgFIe9B1HSet1v2MFMlE443g=;4:tBOS9rbL5F9aZb0l4kkyI3R303JqcmHNibniUHf2rf5l6+kULv/g6uvFKfD1LF64ZBexJhDuooz2uezJsVvzY78B1lK1lYCokv2aHJVjG5aBKV8cmXsEAcQt5uJgPJd+MkcDCzWhqqi53lCTm9HXBQ3pwUkaNfoQ8alKe+xqdwCYOwiMb5HY96eE4YHpt84jxxi06dBEIZW8MMEe3gvpFmClNlO72ygqypbrW2Mv0/CuFJuDK6ZUwjvKgKgFX3ZKeZCzLotK0KLdTcQo2tsr/ZE+iGJ5daazNFyR18t6i/krUOUHLMX+/bX42NJrf/LVvVz/uwiqzrKrqhGO5X3YsGMtsN0eoHnCaVnU1rUNAdR3tj61+feIsXqgtiM5bDTU X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(101931422205132); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001)(10201501046);SRVR:SN2PR03MB096;BCL:0;PCL:0;RULEID:;SRVR:SN2PR03MB096; X-Forefront-PRVS: 0749DC2CE6 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;SN2PR03MB096;23:8psQZii4xcwTzyPmUrniKdB+8dQuCik2TgSnXE/HGZ?= =?us-ascii?Q?Qkouz4VlTiHjic+hFnWtF+bURU7vH+3NRdi2xL5uLk8QEppPAxL/UhNvfuJt?= =?us-ascii?Q?JIwI5SF79zz3EJOALBYokRXv7XPfu8T1fKkZyzUcg0+AwfQ0YBm7hy+fPoHB?= =?us-ascii?Q?99ABa8m/+hfMvTIzs2H8WcwrmKVe6i2YkYv8Jt5Q6S5p7XEo//wt5MoD3fJW?= =?us-ascii?Q?9HGPaSeNGSE3RErGhV44DDUbEyyGwSwF1wv4xQVStay1Kzh+kFFVmQyChHj1?= =?us-ascii?Q?nrmTp3P4qPzck9NCYkiDPQUNI5j4lo8N4+/oapcVpRG+xTmkqQFuTKXR58va?= =?us-ascii?Q?dOagdz+r4KC5qrj35i0OC6bsKt7g9yjweubxRB0yGAZAf0p+2ukgbePKJs+i?= =?us-ascii?Q?enQqmV4CPUW1U0Liv/rpqBGPT0bgc2so/qkBIvtOPEoYNknu+2kQqPAjnTS/?= =?us-ascii?Q?RhDBnbc605EQO/d/MsJGdnfHEFfHHtlBY5GmGf9rHf2TyKpd8AQ4PmAye/kj?= =?us-ascii?Q?E9KzrrOW3DDTUGBFp/wbCu1U2EQy05DPceDCpeltf1H6pn3tb9c4FEG6bPYJ?= =?us-ascii?Q?kPj44UbyCa/lQx2BFipcunW7g/j9QqtZO7OHVRzZsDcFf0YVmvocWXLeLiCm?= =?us-ascii?Q?2A4eA7JcQA1R32xTXiGnZ6tYaMWX/rJ7hgCF+JItjn939sdzf8ldDA6rdf3y?= =?us-ascii?Q?CaaggXuZwiy/kl0QGVGX4TofmklF/VWQoj9qWy9XubynaprOm7pv4Q8frwrw?= =?us-ascii?Q?+f2GSIDzl/Vnke1Iu6uYOj89e5bIsjCy2AhceUV/QNM3ZApDFaqoRj+Iuycn?= =?us-ascii?Q?Ra8+DTgTYtDYO2SsgrIPdVivXNBhGjXH2Q2/tmtrKMgDAcCthWqpSfDr5dJt?= =?us-ascii?Q?tjaFcdKObQP8iPps04XqUTkVGG8HZiA+rEN97wBIMvZ9/oiKaZdC9CqUQVJj?= =?us-ascii?Q?vggzcRreoIkqTQH0gYLIPpOEvnwJ/0ywVCebHAh7gYhrKtuQozTjwL+cqwlG?= =?us-ascii?Q?C5AVeYhNGokXJkkcqElVDvQN6gZoVu0c8S6Uw7TYH6MOKI26B9NaBg1rRB1A?= =?us-ascii?Q?tezENRTUne+pE9e8OSxms5I/jFD9uY4qotcoh8C8PrW37ZYLeMZMl9GkpIxu?= =?us-ascii?Q?diyD2NH2128uI0oR/2wCLvUyHp+voRWR2JA1KXCE2XDELa7I7ppA=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB096;5:dhNXKlwVmhjkvm1qLK2XVtDhi0J6KxrPIQW6ZGEss9qBIz6bOhrP7iT5Jd+X/MrgEDVS4d53/lx7ux/lVIk2+eZUdGiN7bWZ1J0ph1Y0OVtsZCsFK9QAHqwc2HJ6Bn+/cn2ISmwoq8amE2/mBCYmXg==;24:SqzLJ2pQcqY1PBxuts67pbWrr9VP17FRbf/pHQ9urSpM4hcX/5Umq9rRGibmVbivoKUlkEAWLC1eqo6kxkggomZofhlS6k0jj0GXofIZ2RY=;20:pTPios/1MXPm6IeWFuWG7Ig8O7/7UEwEYRYsqlYHUU0YM31GLIso1KE8n/Q1DxLRoSe97FAlFRi4C2UZje11zA== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2015 04:49:38.4102 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN2PR03MB096 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 02, 2015 at 08:04:20PM -0800, Guenter Roeck wrote: > On 11/02/2015 07:29 PM, Robin Gong wrote: > >Since the watchdog common framework centrialize the IOCTL interfaces of > >device driver now, the SETPRETIMEOUT and GETPRETIMEOUT need to be added > >in the common code. > > > >Signed-off-by: Robin Gong > >--- > > drivers/watchdog/watchdog_dev.c | 38 ++++++++++++++++++++++++++++++++++++++ > > include/linux/watchdog.h | 9 +++++++++ > > 2 files changed, 47 insertions(+) > > > >diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c > >index 6aaefba..02632fe 100644 > >--- a/drivers/watchdog/watchdog_dev.c > >+++ b/drivers/watchdog/watchdog_dev.c > >@@ -218,6 +218,37 @@ out_timeout: > > } > > > > /* > >+ * watchdog_set_pretimeout: set the watchdog timer pretimeout > >+ * @wddev: the watchdog device to set the timeout for > >+ * @timeout: pretimeout to set in seconds > >+ */ > >+ > >+static int watchdog_set_pretimeout(struct watchdog_device *wddev, > >+ unsigned int timeout) > > Please align with "(". Will fix in v2. > > >+{ > >+ int err; > >+ > >+ if ((wddev->ops->set_pretimeout == NULL) || > > Unnecessary ( ), and "== NULL" is unnecessary as well. > why? It's useful if other device driver didn't implement set_pretimeout. > >+ !(wddev->info->options & WDIOF_PRETIMEOUT)) > >+ return -EOPNOTSUPP; > >+ if (watchdog_pretimeout_invalid(wddev, timeout)) > >+ return -EINVAL; > >+ > >+ mutex_lock(&wddev->lock); > >+ > >+ if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { > >+ err = -ENODEV; > >+ goto out_timeout; > >+ } > >+ > >+ err = wddev->ops->set_pretimeout(wddev, timeout); > >+ > >+out_timeout: > >+ mutex_unlock(&wddev->lock); > >+ return err; > >+} > >+ > >+/* > > * watchdog_get_timeleft: wrapper to get the time left before a reboot > > * @wddev: the watchdog device to get the remaining time from > > * @timeleft: the time that's left > >@@ -393,6 +424,13 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd, > > if (err) > > return err; > > return put_user(val, p); > >+ case WDIOC_SETPRETIMEOUT: > >+ if (get_user(val, p)) > >+ return -EFAULT; > >+ err = watchdog_set_pretimeout(wdd, val); > >+ return err; > > return watchdog_set_pretimeout(wdd, val); > > >+ case WDIOC_GETPRETIMEOUT: > >+ return put_user(wdd->pretimeout, p); > > default: > > return -ENOTTY; > > } > >diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h > >index d74a0e9..6ddb2d6 100644 > >--- a/include/linux/watchdog.h > >+++ b/include/linux/watchdog.h > >@@ -25,6 +25,7 @@ struct watchdog_device; > > * @ping: The routine that sends a keepalive ping to the watchdog device. > > * @status: The routine that shows the status of the watchdog device. > > * @set_timeout:The routine for setting the watchdog devices timeout value. > >+ * @set_pretimeout:The routine for setting the watchdog devices pretimeout. > > * @get_timeleft:The routine that get's the time that's left before a reset. > > * @ref: The ref operation for dyn. allocated watchdog_device structs > > * @unref: The unref operation for dyn. allocated watchdog_device structs > >@@ -44,6 +45,7 @@ struct watchdog_ops { > > int (*ping)(struct watchdog_device *); > > unsigned int (*status)(struct watchdog_device *); > > int (*set_timeout)(struct watchdog_device *, unsigned int); > >+ int (*set_pretimeout)(struct watchdog_device *, unsigned int); > > unsigned int (*get_timeleft)(struct watchdog_device *); > > void (*ref)(struct watchdog_device *); > > void (*unref)(struct watchdog_device *); > >@@ -86,6 +88,7 @@ struct watchdog_device { > > const struct watchdog_ops *ops; > > unsigned int bootstatus; > > unsigned int timeout; > >+ unsigned int pretimeout; > > Description (further below) missing. > I describe it in the ahead of this structure as the above. > > unsigned int min_timeout; > > unsigned int max_timeout; > > void *driver_data; > >@@ -123,6 +126,12 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne > > (t < wdd->min_timeout || t > wdd->max_timeout)); > > } > > > >+/* Use the following function to check if a pretimeout value is invalid */ > >+static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd, unsigned int t) > > Please fix checkpatch warnings. > You mean over 80 characters? Will fix in v2. > >+{ > >+ return ((wdd->timeout != 0) && (t >= wdd->timeout)); > > Unnecessary ( ), and "!= 0" is unnecessary. > I think t >= wdd->timeout is need, since it's a pre-timeout. > >+} > >+ > > /* Use the following functions to manipulate watchdog driver specific data */ > > static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data) > > { > > >