* [PATCH v5 01/10] watchdog: add pretimeout support to the core
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 02/10] fs: compat_ioctl: add pretimeout functions for watchdogs Vladimir Zapolskiy
` (10 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang
Cc: linux-watchdog, Wolfram Sang, Robin Gong
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Since the watchdog framework centrializes the IOCTL interfaces of device
drivers now, SETPRETIMEOUT and GETPRETIMEOUT need to be added in the
common code.
Signed-off-by: Robin Gong <b38343@freescale.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[vzapolskiy: added conditional pretimeout sysfs attribute visibility]
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Changes from v4 to v5:
* fixed a minor whitespace issue reproted by checkpatch for v4
Changes from v3 to v4:
* included back to the main series as it was done in v2,
* added a fix suggested by Guenter, if a watchdog device does not have
its own set_timeout() function, disable pretimeout if a new timeout
value is less or equal to the set pretimeout period
* added Reviewed-by: Guenter tag
Changes from v2 to v3:
* none (not published)
Changes from v1 to v2:
* removed my wrong generalization of wdd->pretimeout update
* added seconds unit of time to describe internal kernel API
Documentation/watchdog/watchdog-kernel-api.txt | 20 +++++++++
drivers/watchdog/watchdog_dev.c | 56 +++++++++++++++++++++++++-
include/linux/watchdog.h | 11 +++++
3 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 7f31125c123e..3402dcad5b03 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -50,6 +50,7 @@ struct watchdog_device {
const struct watchdog_ops *ops;
unsigned int bootstatus;
unsigned int timeout;
+ unsigned int pretimeout;
unsigned int min_timeout;
unsigned int max_timeout;
unsigned int min_hw_heartbeat_ms;
@@ -77,6 +78,7 @@ It contains following fields:
* timeout: the watchdog timer's timeout value (in seconds).
This is the time after which the system will reboot if user space does
not send a heartbeat request if WDOG_ACTIVE is set.
+* pretimeout: the watchdog timer's pretimeout value (in seconds).
* min_timeout: the watchdog timer's minimum timeout value (in seconds).
If set, the minimum configurable value for 'timeout'.
* max_timeout: the watchdog timer's maximum timeout value (in seconds),
@@ -121,6 +123,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 *);
int (*restart)(struct watchdog_device *);
void (*ref)(struct watchdog_device *) __deprecated;
@@ -188,6 +191,23 @@ they are supported. These optional routines/operations are:
If set_timeout is not provided but, WDIOF_SETTIMEOUT is set, the watchdog
infrastructure updates the timeout value of the watchdog_device internally
to the requested value.
+ If the pretimeout feature is used (WDIOF_PRETIMEOUT), then set_timeout must
+ also take care of checking if pretimeout is still valid and set up the timer
+ accordingly. This can't be done in the core without races, so it is the
+ duty of the driver.
+* set_pretimeout: this routine checks and changes the pretimeout value of
+ the watchdog. It is optional because not all watchdogs support pretimeout
+ notification. The timeout value is not an absolute time, but the number of
+ seconds before the actual timeout would happen. It returns 0 on success,
+ -EINVAL for "parameter out of range" and -EIO for "could not write value to
+ the watchdog". A value of 0 disables pretimeout notification.
+ (Note: the WDIOF_PRETIMEOUT needs to be set in the options field of the
+ watchdog's info structure).
+ If the watchdog driver does not have to perform any action but setting the
+ watchdog_device.pretimeout, this callback can be omitted. That means if
+ set_pretimeout is not provided but WDIOF_PRETIMEOUT is set, the watchdog
+ infrastructure updates the pretimeout value of the watchdog_device internally
+ to the requested value.
* get_timeleft: this routines returns the time that's left before a reset.
* restart: this routine restarts the machine. It returns 0 on success or a
negative errno code for failure.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 040bf8382f46..4b381a6be2ff 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -335,10 +335,14 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
if (watchdog_timeout_invalid(wdd, timeout))
return -EINVAL;
- if (wdd->ops->set_timeout)
+ if (wdd->ops->set_timeout) {
err = wdd->ops->set_timeout(wdd, timeout);
- else
+ } else {
wdd->timeout = timeout;
+ /* Disable pretimeout if it doesn't fit the new timeout */
+ if (wdd->pretimeout >= wdd->timeout)
+ wdd->pretimeout = 0;
+ }
watchdog_update_worker(wdd);
@@ -346,6 +350,31 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
}
/*
+ * watchdog_set_pretimeout: set the watchdog timer pretimeout
+ * @wdd: the watchdog device to set the timeout for
+ * @timeout: pretimeout to set in seconds
+ */
+
+static int watchdog_set_pretimeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ int err = 0;
+
+ if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+ return -EOPNOTSUPP;
+
+ if (watchdog_pretimeout_invalid(wdd, timeout))
+ return -EINVAL;
+
+ if (wdd->ops->set_pretimeout)
+ err = wdd->ops->set_pretimeout(wdd, timeout);
+ else
+ wdd->pretimeout = timeout;
+
+ return err;
+}
+
+/*
* watchdog_get_timeleft: wrapper to get the time left before a reboot
* @wdd: the watchdog device to get the remaining time from
* @timeleft: the time that's left
@@ -429,6 +458,15 @@ static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(timeout);
+static ssize_t pretimeout_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", wdd->pretimeout);
+}
+static DEVICE_ATTR_RO(pretimeout);
+
static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -459,6 +497,9 @@ static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
if (attr == &dev_attr_timeleft.attr && !wdd->ops->get_timeleft)
mode = 0;
+ else if (attr == &dev_attr_pretimeout.attr &&
+ !(wdd->info->options & WDIOF_PRETIMEOUT))
+ mode = 0;
return mode;
}
@@ -466,6 +507,7 @@ static struct attribute *wdt_attrs[] = {
&dev_attr_state.attr,
&dev_attr_identity.attr,
&dev_attr_timeout.attr,
+ &dev_attr_pretimeout.attr,
&dev_attr_timeleft.attr,
&dev_attr_bootstatus.attr,
&dev_attr_status.attr,
@@ -646,6 +688,16 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
break;
err = put_user(val, p);
break;
+ case WDIOC_SETPRETIMEOUT:
+ if (get_user(val, p)) {
+ err = -EFAULT;
+ break;
+ }
+ err = watchdog_set_pretimeout(wdd, val);
+ break;
+ case WDIOC_GETPRETIMEOUT:
+ err = put_user(wdd->pretimeout, p);
+ break;
default:
err = -ENOTTY;
break;
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 7047bc7f8106..4035df7ec023 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -28,6 +28,7 @@ struct watchdog_core_data;
* @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 (in seconds).
+ * @set_pretimeout:The routine for setting the watchdog devices pretimeout.
* @get_timeleft:The routine that gets the time left before a reset (in seconds).
* @restart: The routine for restarting the machine.
* @ioctl: The routines that handles extra ioctl calls.
@@ -46,6 +47,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 *);
int (*restart)(struct watchdog_device *, unsigned long, void *);
long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
@@ -61,6 +63,7 @@ struct watchdog_ops {
* @ops: Pointer to the list of watchdog operations.
* @bootstatus: Status of the watchdog device at boot.
* @timeout: The watchdog devices timeout value (in seconds).
+ * @pretimeout: The watchdog devices pre_timeout value.
* @min_timeout:The watchdog devices minimum timeout value (in seconds).
* @max_timeout:The watchdog devices maximum timeout value (in seconds)
* as configurable from user space. Only relevant if
@@ -96,6 +99,7 @@ struct watchdog_device {
const struct watchdog_ops *ops;
unsigned int bootstatus;
unsigned int timeout;
+ unsigned int pretimeout;
unsigned int min_timeout;
unsigned int max_timeout;
unsigned int min_hw_heartbeat_ms;
@@ -163,6 +167,13 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne
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)
+{
+ return t && wdd->timeout && t >= wdd->timeout;
+}
+
/* Use the following functions to manipulate watchdog driver specific data */
static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
{
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 02/10] fs: compat_ioctl: add pretimeout functions for watchdogs
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 01/10] watchdog: add pretimeout support to the core Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 03/10] watchdog: add watchdog pretimeout governor framework Vladimir Zapolskiy
` (9 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang
Cc: linux-watchdog, Wolfram Sang, Alexander Viro, linux-fsdevel
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Watchdog core now handles those ioctls centrally, so we want 64 bit
support, too.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
Changes from v4 to v5:
* none
Changes from v3 to v4:
* included back to the main series as it was done in v2,
* added Acked-by: Guenter tag
Changes from v2 to v3:
* none (not published)
Changes from v1 to v2:
* none
fs/compat_ioctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index c1e9f29c924c..f2d7402abe02 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1209,6 +1209,8 @@ COMPATIBLE_IOCTL(WDIOC_SETOPTIONS)
COMPATIBLE_IOCTL(WDIOC_KEEPALIVE)
COMPATIBLE_IOCTL(WDIOC_SETTIMEOUT)
COMPATIBLE_IOCTL(WDIOC_GETTIMEOUT)
+COMPATIBLE_IOCTL(WDIOC_SETPRETIMEOUT)
+COMPATIBLE_IOCTL(WDIOC_GETPRETIMEOUT)
/* Big R */
COMPATIBLE_IOCTL(RNDGETENTCNT)
COMPATIBLE_IOCTL(RNDADDTOENTCNT)
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 03/10] watchdog: add watchdog pretimeout governor framework
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 01/10] watchdog: add pretimeout support to the core Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 02/10] fs: compat_ioctl: add pretimeout functions for watchdogs Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 04/10] watchdog: pretimeout: add panic pretimeout governor Vladimir Zapolskiy
` (8 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
The change adds a simple watchdog pretimeout framework infrastructure,
its purpose is to allow users to select a desired handling of watchdog
pretimeout events, which may be generated by some watchdog devices.
A user selects a default watchdog pretimeout governor during
compilation stage.
Watchdogs with WDIOF_PRETIMEOUT capability now have one more device
attribute in sysfs, pretimeout_governor attribute is intended to display
the selected watchdog pretimeout governor.
The framework has no impact at runtime on watchdog devices with no
WDIOF_PRETIMEOUT capability set.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
Changes from v4 to v5:
* none
Changes from v3 to v4:
* documented new watchdog_notify_pretimeout() function for developers
of watchdog device drivers (Guenter)
* reordered logical operations in wdt_is_visible() for better clarity
(Guenter)
* if watchdog pretimeout governor is not registered return empty
string on reading "pretimeout_governor" device attribute, however with
the default governor built-in this should never happen on practice
(Guenter)
* removed a number of sanity checks from functions which are assumed
to be exported, callers are expected to write correct code, also this
fixes one bug in watchdog_register_governor() noticed by Guenter
(Guenter)
* reworded commit message (Guenter)
* moved registration of watchdog pretimeout event by
watchdog_register_pretimeout() from watchdog_core.c to watchdog_dev.c
(Wolfram)
* if CONFIG_WATCHDOG_PRETIMEOUT_GOV is not selected during compilation
stage on watchdog pretimeout event panic occurs
* split the main change into 3 to hopefully enlighten review process:
1) default governor support only, 2) selectable by a user governor in
runtime, 3) added pretimeout_available_governors attribute
* fixed a list element deregistration bug
Changes from v2 to v3:
* essentially simplified the implementation due to removal of runtime
dynamic selection of watchdog pretimeout governors by a user, this
feature is supposed to be added later on
* removed support of sleepable watchdog pretimeout governors
* moved sysfs device attributes to watchdog_dev.c, this required to
add exported watchdog_pretimeout_governor_name() interface
* if pretimeout framework is not selected, then pretimeout event
ends up in kernel panic -- this behaviour as a default one was asked
by Guenter
Changes from v1 to v2:
* removed framework private bits from struct watchdog_governor,
* centralized compile-time selection of a default governor in
watchdog_pretimeout.h,
* added can_sleep option, now only sleeping governors (e.g. userspace)
will be executed in a special workqueue,
* changed fallback logic, if a governor in use is removed, now this
situation is not possible, because in use governors have non-zero
module refcount
Documentation/watchdog/watchdog-kernel-api.txt | 12 +++
drivers/watchdog/Kconfig | 7 ++
drivers/watchdog/Makefile | 5 +-
drivers/watchdog/watchdog_dev.c | 23 +++++
drivers/watchdog/watchdog_pretimeout.c | 131 +++++++++++++++++++++++++
drivers/watchdog/watchdog_pretimeout.h | 40 ++++++++
include/linux/watchdog.h | 13 +++
7 files changed, 230 insertions(+), 1 deletion(-)
create mode 100644 drivers/watchdog/watchdog_pretimeout.c
create mode 100644 drivers/watchdog/watchdog_pretimeout.h
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 3402dcad5b03..b1d09b3a00be 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -48,6 +48,7 @@ struct watchdog_device {
const struct attribute_group **groups;
const struct watchdog_info *info;
const struct watchdog_ops *ops;
+ const struct watchdog_governor *gov;
unsigned int bootstatus;
unsigned int timeout;
unsigned int pretimeout;
@@ -75,6 +76,7 @@ It contains following fields:
* info: a pointer to a watchdog_info structure. This structure gives some
additional information about the watchdog timer itself. (Like it's unique name)
* ops: a pointer to the list of watchdog operations that the watchdog supports.
+* gov: a pointer to the assigned watchdog device pretimeout governor or NULL.
* timeout: the watchdog timer's timeout value (in seconds).
This is the time after which the system will reboot if user space does
not send a heartbeat request if WDOG_ACTIVE is set.
@@ -288,3 +290,13 @@ User should follow the following guidelines for setting the priority:
* 128: default restart handler, use if no other handler is expected to be
available, and/or if restart is sufficient to restart the entire system
* 255: highest priority, will preempt all other restart handlers
+
+To raise a pretimeout notification, the following function should be used:
+
+void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+
+The function can be called in the interrupt context. If watchdog pretimeout
+governor framework (kbuild CONFIG_WATCHDOG_PRETIMEOUT_GOV symbol) is enabled,
+an action is taken by a preconfigured pretimeout governor preassigned to
+the watchdog device. If watchdog pretimeout governor framework is not
+enabled, watchdog_notify_pretimeout() equals to the immediate panic() call.
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1bffe006ca9a..04d535ae497d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1831,4 +1831,11 @@ config USBPCWATCHDOG
Most people will say N.
+comment "Watchdog Pretimeout Governors"
+
+config WATCHDOG_PRETIMEOUT_GOV
+ bool "Enable watchdog pretimeout governors"
+ help
+ The option allows to select watchdog pretimeout governors.
+
endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index c22ad3ea3539..990c36ed4716 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -3,9 +3,12 @@
#
# The WatchDog Timer Driver Core.
-watchdog-objs += watchdog_core.o watchdog_dev.o
obj-$(CONFIG_WATCHDOG_CORE) += watchdog.o
+watchdog-objs += watchdog_core.o watchdog_dev.o
+
+watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
+
# Only one watchdog can succeed. We probe the ISA/PCI/USB based
# watchdog-cards first, then the architecture specific watchdog
# drivers and then the architecture independent "softdog" driver.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 4b381a6be2ff..d2d0b5e37a35 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -49,6 +49,7 @@
#include <linux/uaccess.h> /* For copy_to_user/put_user/... */
#include "watchdog_core.h"
+#include "watchdog_pretimeout.h"
/*
* struct watchdog_core_data - watchdog core internal data
@@ -488,6 +489,16 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(state);
+static ssize_t pretimeout_governor_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+ return watchdog_pretimeout_governor_get(wdd, buf);
+}
+static DEVICE_ATTR_RO(pretimeout_governor);
+
static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -500,6 +511,10 @@ static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
else if (attr == &dev_attr_pretimeout.attr &&
!(wdd->info->options & WDIOF_PRETIMEOUT))
mode = 0;
+ else if (attr == &dev_attr_pretimeout_governor.attr &&
+ (!(wdd->info->options & WDIOF_PRETIMEOUT) ||
+ !IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)))
+ mode = 0;
return mode;
}
@@ -512,6 +527,7 @@ static struct attribute *wdt_attrs[] = {
&dev_attr_bootstatus.attr,
&dev_attr_status.attr,
&dev_attr_nowayout.attr,
+ &dev_attr_pretimeout_governor.attr,
NULL,
};
@@ -989,6 +1005,12 @@ int watchdog_dev_register(struct watchdog_device *wdd)
return PTR_ERR(dev);
}
+ ret = watchdog_register_pretimeout(wdd);
+ if (ret) {
+ device_destroy(&watchdog_class, devno);
+ watchdog_cdev_unregister(wdd);
+ }
+
return ret;
}
@@ -1002,6 +1024,7 @@ int watchdog_dev_register(struct watchdog_device *wdd)
void watchdog_dev_unregister(struct watchdog_device *wdd)
{
+ watchdog_unregister_pretimeout(wdd);
device_destroy(&watchdog_class, wdd->wd_data->cdev.dev);
watchdog_cdev_unregister(wdd);
}
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
new file mode 100644
index 000000000000..72612255fb55
--- /dev/null
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2015-2016 Mentor Graphics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/* Default watchdog pretimeout governor */
+static struct watchdog_governor *default_gov;
+
+/* The spinlock protects default_gov, wdd->gov and pretimeout_list */
+static DEFINE_SPINLOCK(pretimeout_lock);
+
+/* List of watchdog devices, which can generate a pretimeout event */
+static LIST_HEAD(pretimeout_list);
+
+struct watchdog_pretimeout {
+ struct watchdog_device *wdd;
+ struct list_head entry;
+};
+
+int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf)
+{
+ int count = 0;
+
+ spin_lock_irq(&pretimeout_lock);
+ if (wdd->gov)
+ count = sprintf(buf, "%s\n", wdd->gov->name);
+ spin_unlock_irq(&pretimeout_lock);
+
+ return count;
+}
+
+void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&pretimeout_lock, flags);
+ if (!wdd->gov) {
+ spin_unlock_irqrestore(&pretimeout_lock, flags);
+ return;
+ }
+
+ wdd->gov->pretimeout(wdd);
+ spin_unlock_irqrestore(&pretimeout_lock, flags);
+}
+EXPORT_SYMBOL_GPL(watchdog_notify_pretimeout);
+
+int watchdog_register_governor(struct watchdog_governor *gov)
+{
+ struct watchdog_pretimeout *p;
+
+ if (!default_gov) {
+ spin_lock_irq(&pretimeout_lock);
+ default_gov = gov;
+
+ list_for_each_entry(p, &pretimeout_list, entry)
+ if (!p->wdd->gov)
+ p->wdd->gov = default_gov;
+ spin_unlock_irq(&pretimeout_lock);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(watchdog_register_governor);
+
+void watchdog_unregister_governor(struct watchdog_governor *gov)
+{
+ struct watchdog_pretimeout *p;
+
+ spin_lock_irq(&pretimeout_lock);
+ if (gov == default_gov)
+ default_gov = NULL;
+
+ list_for_each_entry(p, &pretimeout_list, entry)
+ if (p->wdd->gov == gov)
+ p->wdd->gov = default_gov;
+ spin_unlock_irq(&pretimeout_lock);
+}
+EXPORT_SYMBOL(watchdog_unregister_governor);
+
+int watchdog_register_pretimeout(struct watchdog_device *wdd)
+{
+ struct watchdog_pretimeout *p;
+
+ if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+ return 0;
+
+ p = kzalloc(sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ spin_lock_irq(&pretimeout_lock);
+ list_add(&p->entry, &pretimeout_list);
+ p->wdd = wdd;
+ wdd->gov = default_gov;
+ spin_unlock_irq(&pretimeout_lock);
+
+ return 0;
+}
+
+void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
+{
+ struct watchdog_pretimeout *p, *t;
+
+ if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+ return;
+
+ spin_lock_irq(&pretimeout_lock);
+ wdd->gov = NULL;
+
+ list_for_each_entry_safe(p, t, &pretimeout_list, entry) {
+ if (p->wdd == wdd) {
+ list_del(&p->entry);
+ break;
+ }
+ }
+ spin_unlock_irq(&pretimeout_lock);
+
+ kfree(p);
+}
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
new file mode 100644
index 000000000000..c6cd9f80adb2
--- /dev/null
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -0,0 +1,40 @@
+#ifndef __WATCHDOG_PRETIMEOUT_H
+#define __WATCHDOG_PRETIMEOUT_H
+
+#define WATCHDOG_GOV_NAME_MAXLEN 20
+
+struct watchdog_device;
+
+struct watchdog_governor {
+ const char name[WATCHDOG_GOV_NAME_MAXLEN];
+ void (*pretimeout)(struct watchdog_device *wdd);
+};
+
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
+/* Interfaces to watchdog pretimeout governors */
+int watchdog_register_governor(struct watchdog_governor *gov);
+void watchdog_unregister_governor(struct watchdog_governor *gov);
+
+/* Interfaces to watchdog_dev.c */
+int watchdog_register_pretimeout(struct watchdog_device *wdd);
+void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
+int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf);
+
+#else
+static inline int watchdog_register_pretimeout(struct watchdog_device *wdd)
+{
+ return 0;
+}
+
+static inline void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
+{
+}
+
+static inline int watchdog_pretimeout_governor_get(struct watchdog_device *wdd,
+ char *buf)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 4035df7ec023..c776debde228 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -19,6 +19,7 @@
struct watchdog_ops;
struct watchdog_device;
struct watchdog_core_data;
+struct watchdog_governor;
/** struct watchdog_ops - The watchdog-devices operations
*
@@ -61,6 +62,7 @@ struct watchdog_ops {
* watchdog device.
* @info: Pointer to a watchdog_info structure.
* @ops: Pointer to the list of watchdog operations.
+ * @gov: Pointer to watchdog pretimeout governor.
* @bootstatus: Status of the watchdog device at boot.
* @timeout: The watchdog devices timeout value (in seconds).
* @pretimeout: The watchdog devices pre_timeout value.
@@ -97,6 +99,7 @@ struct watchdog_device {
const struct attribute_group **groups;
const struct watchdog_info *info;
const struct watchdog_ops *ops;
+ const struct watchdog_governor *gov;
unsigned int bootstatus;
unsigned int timeout;
unsigned int pretimeout;
@@ -185,6 +188,16 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
return wdd->driver_data;
}
+/* Use the following functions to report watchdog pretimeout event */
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
+void watchdog_notify_pretimeout(struct watchdog_device *wdd);
+#else
+static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+{
+ panic("watchdog pretimeout event\n");
+}
+#endif
+
/* drivers/watchdog/watchdog_core.c */
void watchdog_set_restart_priority(struct watchdog_device *wdd, int priority);
extern int watchdog_init_timeout(struct watchdog_device *wdd,
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 04/10] watchdog: pretimeout: add panic pretimeout governor
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (2 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 03/10] watchdog: add watchdog pretimeout governor framework Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 05/10] watchdog: pretimeout: add noop " Vladimir Zapolskiy
` (7 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
The change adds panic watchdog pretimeout governor, on watchdog
pretimeout event the kernel shall panic.
While introducing the first pretimeout governor the selected design
assumes that the default pretimeout governor is selected by its name
and it is always built-in, thus the default pretimeout governor can
not be unregistered and the correspondent check can be removed from
watchdog_unregister_governor() function.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
Changes from v4 to v5:
* fixed in source tree compilation issue, thanks to Wolfram
Changes from v3 to v4:
* returned tristate option back, it is safe to have it, because
the single governor is the default one, thus it is built-in, but with
the introduction of another governor it should be tristate
* removed "panic panic" duplication in a user's message (Wolfram)
* added minor changes to watchdog_pretimeout.c to support built time
selectable default governor
Changes from v2 to v3:
* temporarily removed tristate option, the governor can be built-in only
Changes from v1 to v2:
* removed #ifdef CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC,
now this selection is done in a centralized manner,
* added module owner reference to count users of the governor,
* slightly improved description of the governors in Kconfig.
drivers/watchdog/Kconfig | 28 ++++++++++++++++++++
drivers/watchdog/Makefile | 2 ++
drivers/watchdog/pretimeout_panic.c | 47 ++++++++++++++++++++++++++++++++++
drivers/watchdog/watchdog_pretimeout.c | 6 ++---
drivers/watchdog/watchdog_pretimeout.h | 4 +++
5 files changed, 83 insertions(+), 4 deletions(-)
create mode 100644 drivers/watchdog/pretimeout_panic.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 04d535ae497d..5f82a3fd2186 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1838,4 +1838,32 @@ config WATCHDOG_PRETIMEOUT_GOV
help
The option allows to select watchdog pretimeout governors.
+if WATCHDOG_PRETIMEOUT_GOV
+
+choice
+ prompt "Default Watchdog Pretimeout Governor"
+ default WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
+ help
+ This option selects a default watchdog pretimeout governor.
+ The governor takes its action, if a watchdog is capable
+ to report a pretimeout event.
+
+config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
+ bool "panic"
+ select WATCHDOG_PRETIMEOUT_GOV_PANIC
+ help
+ Use panic watchdog pretimeout governor by default, if
+ a watchdog pretimeout event happens, consider that
+ a watchdog feeder is dead and reboot is unavoidable.
+
+endchoice
+
+config WATCHDOG_PRETIMEOUT_GOV_PANIC
+ tristate "Panic watchdog pretimeout governor"
+ help
+ Panic watchdog pretimeout governor, on watchdog pretimeout
+ event put the kernel into panic.
+
+endif # WATCHDOG_PRETIMEOUT_GOV
+
endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 990c36ed4716..179df1d63c32 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -9,6 +9,8 @@ watchdog-objs += watchdog_core.o watchdog_dev.o
watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
+obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
+
# Only one watchdog can succeed. We probe the ISA/PCI/USB based
# watchdog-cards first, then the architecture specific watchdog
# drivers and then the architecture independent "softdog" driver.
diff --git a/drivers/watchdog/pretimeout_panic.c b/drivers/watchdog/pretimeout_panic.c
new file mode 100644
index 000000000000..0c197a1c97f4
--- /dev/null
+++ b/drivers/watchdog/pretimeout_panic.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015-2016 Mentor Graphics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/**
+ * pretimeout_panic - Panic on watchdog pretimeout event
+ * @wdd - watchdog_device
+ *
+ * Panic, watchdog has not been fed till pretimeout event.
+ */
+static void pretimeout_panic(struct watchdog_device *wdd)
+{
+ panic("watchdog pretimeout event\n");
+}
+
+static struct watchdog_governor watchdog_gov_panic = {
+ .name = "panic",
+ .pretimeout = pretimeout_panic,
+};
+
+static int __init watchdog_gov_panic_register(void)
+{
+ return watchdog_register_governor(&watchdog_gov_panic);
+}
+
+static void __exit watchdog_gov_panic_unregister(void)
+{
+ watchdog_unregister_governor(&watchdog_gov_panic);
+}
+module_init(watchdog_gov_panic_register);
+module_exit(watchdog_gov_panic_unregister);
+
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>");
+MODULE_DESCRIPTION("Panic watchdog pretimeout governor");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index 72612255fb55..098c965f6c78 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -60,7 +60,8 @@ int watchdog_register_governor(struct watchdog_governor *gov)
{
struct watchdog_pretimeout *p;
- if (!default_gov) {
+ if (!strncmp(gov->name, WATCHDOG_PRETIMEOUT_DEFAULT_GOV,
+ WATCHDOG_GOV_NAME_MAXLEN)) {
spin_lock_irq(&pretimeout_lock);
default_gov = gov;
@@ -79,9 +80,6 @@ void watchdog_unregister_governor(struct watchdog_governor *gov)
struct watchdog_pretimeout *p;
spin_lock_irq(&pretimeout_lock);
- if (gov == default_gov)
- default_gov = NULL;
-
list_for_each_entry(p, &pretimeout_list, entry)
if (p->wdd->gov == gov)
p->wdd->gov = default_gov;
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index c6cd9f80adb2..fea56bd42379 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -20,6 +20,10 @@ int watchdog_register_pretimeout(struct watchdog_device *wdd);
void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf);
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "panic"
+#endif
+
#else
static inline int watchdog_register_pretimeout(struct watchdog_device *wdd)
{
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 05/10] watchdog: pretimeout: add noop pretimeout governor
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (3 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 04/10] watchdog: pretimeout: add panic pretimeout governor Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 06/10] watchdog: pretimeout: add option to select a pretimeout governor in runtime Vladimir Zapolskiy
` (6 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
Noop watchdog pretimeout governor, only an informational message is
added to the kernel log buffer.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
Changes from v4 to v5:
* fixed in source tree compilation issue, thanks to Wolfram
Changes from v3 to v4:
* returned tristate option back
* report watchdog name in a message given by noop governor (Wolfram)
Changes from v2 to v3:
* temporarily removed tristate option, the governor can be built-in only
Changes from v1 to v2:
* removed #ifdef CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOOP,
now this selection is done in a centralized manner,
* added module owner reference to count users of the governor,
* slightly improved description of the governors in Kconfig.
drivers/watchdog/Kconfig | 14 ++++++++++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/pretimeout_noop.c | 47 ++++++++++++++++++++++++++++++++++
drivers/watchdog/watchdog_pretimeout.h | 2 ++
4 files changed, 64 insertions(+)
create mode 100644 drivers/watchdog/pretimeout_noop.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 5f82a3fd2186..7d791fc02c42 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1856,6 +1856,14 @@ config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
a watchdog pretimeout event happens, consider that
a watchdog feeder is dead and reboot is unavoidable.
+config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOOP
+ bool "noop"
+ select WATCHDOG_PRETIMEOUT_GOV_NOOP
+ help
+ Use noop watchdog pretimeout governor by default. If noop
+ governor is selected by a user, write a short message to
+ the kernel log buffer and don't do any system changes.
+
endchoice
config WATCHDOG_PRETIMEOUT_GOV_PANIC
@@ -1864,6 +1872,12 @@ config WATCHDOG_PRETIMEOUT_GOV_PANIC
Panic watchdog pretimeout governor, on watchdog pretimeout
event put the kernel into panic.
+config WATCHDOG_PRETIMEOUT_GOV_NOOP
+ tristate "Noop watchdog pretimeout governor"
+ help
+ Noop watchdog pretimeout governor, only an informational
+ message is added to kernel log buffer.
+
endif # WATCHDOG_PRETIMEOUT_GOV
endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 179df1d63c32..c7925ee2d971 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -10,6 +10,7 @@ watchdog-objs += watchdog_core.o watchdog_dev.o
watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
+obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOOP) += pretimeout_noop.o
# Only one watchdog can succeed. We probe the ISA/PCI/USB based
# watchdog-cards first, then the architecture specific watchdog
diff --git a/drivers/watchdog/pretimeout_noop.c b/drivers/watchdog/pretimeout_noop.c
new file mode 100644
index 000000000000..85f5299d197c
--- /dev/null
+++ b/drivers/watchdog/pretimeout_noop.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015-2016 Mentor Graphics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/**
+ * pretimeout_noop - No operation on watchdog pretimeout event
+ * @wdd - watchdog_device
+ *
+ * This function prints a message about pretimeout to kernel log.
+ */
+static void pretimeout_noop(struct watchdog_device *wdd)
+{
+ pr_alert("watchdog%d: pretimeout event\n", wdd->id);
+}
+
+static struct watchdog_governor watchdog_gov_noop = {
+ .name = "noop",
+ .pretimeout = pretimeout_noop,
+};
+
+static int __init watchdog_gov_noop_register(void)
+{
+ return watchdog_register_governor(&watchdog_gov_noop);
+}
+
+static void __exit watchdog_gov_noop_unregister(void)
+{
+ watchdog_unregister_governor(&watchdog_gov_noop);
+}
+module_init(watchdog_gov_noop_register);
+module_exit(watchdog_gov_noop_unregister);
+
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>");
+MODULE_DESCRIPTION("Panic watchdog pretimeout governor");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index fea56bd42379..8f9804c2dd88 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -22,6 +22,8 @@ int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf);
#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "panic"
+#elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOOP)
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "noop"
#endif
#else
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 06/10] watchdog: pretimeout: add option to select a pretimeout governor in runtime
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (4 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 05/10] watchdog: pretimeout: add noop " Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 07/10] watchdog: pretimeout: add pretimeout_available_governors attribute Vladimir Zapolskiy
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
The change converts watchdog device attribute "pretimeout_governor" from
read-only to read-write type to allow users to select a desirable
watchdog pretimeout governor in runtime, e.g.
% echo -n panic > /sys/..../watchdog/watchdog0/pretimeout
To get this working a list of registered pretimeout governors is created
and a new helper function watchdog_pretimeout_governor_set() is exported
to watchdog_dev.c.
If a selected governor is gone, a watchdog device pretimeout notification
is delegated to a default built-in pretimeout governor.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
Changes from v4 to v5:
* replaced strncmp() with more suitable sysfs_streq() to
compare strings passed over sysfs (Wolfram)
Changes from v3 to v4:
* new change, this functional change was removed in v3
Changes from v2 1/4 to v4:
* introduced watchdog_pretimeout_governor_set() and moved device
attribute registration to watchdog_dev.c
* fixed a list element deregistration bug
drivers/watchdog/watchdog_dev.c | 15 ++++++-
drivers/watchdog/watchdog_pretimeout.c | 76 ++++++++++++++++++++++++++++++++++
drivers/watchdog/watchdog_pretimeout.h | 8 ++++
3 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index d2d0b5e37a35..3fdbe0ab1365 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -497,7 +497,20 @@ static ssize_t pretimeout_governor_show(struct device *dev,
return watchdog_pretimeout_governor_get(wdd, buf);
}
-static DEVICE_ATTR_RO(pretimeout_governor);
+
+static ssize_t pretimeout_governor_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct watchdog_device *wdd = dev_get_drvdata(dev);
+ int ret = watchdog_pretimeout_governor_set(wdd, buf);
+
+ if (!ret)
+ ret = count;
+
+ return ret;
+}
+static DEVICE_ATTR_RW(pretimeout_governor);
static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index 098c965f6c78..c9e4a0326938 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/string.h>
#include <linux/watchdog.h>
#include "watchdog_pretimeout.h"
@@ -29,6 +30,28 @@ struct watchdog_pretimeout {
struct list_head entry;
};
+/* The mutex protects governor list and serializes external interfaces */
+static DEFINE_MUTEX(governor_lock);
+
+/* List of the registered watchdog pretimeout governors */
+static LIST_HEAD(governor_list);
+
+struct governor_priv {
+ struct watchdog_governor *gov;
+ struct list_head entry;
+};
+
+static struct governor_priv *find_governor_by_name(const char *gov_name)
+{
+ struct governor_priv *priv;
+
+ list_for_each_entry(priv, &governor_list, entry)
+ if (sysfs_streq(gov_name, priv->gov->name))
+ return priv;
+
+ return NULL;
+}
+
int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf)
{
int count = 0;
@@ -41,6 +64,28 @@ int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf)
return count;
}
+int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
+ const char *buf)
+{
+ struct governor_priv *priv;
+
+ mutex_lock(&governor_lock);
+
+ priv = find_governor_by_name(buf);
+ if (!priv) {
+ mutex_unlock(&governor_lock);
+ return -EINVAL;
+ }
+
+ spin_lock_irq(&pretimeout_lock);
+ wdd->gov = priv->gov;
+ spin_unlock_irq(&pretimeout_lock);
+
+ mutex_unlock(&governor_lock);
+
+ return 0;
+}
+
void watchdog_notify_pretimeout(struct watchdog_device *wdd)
{
unsigned long flags;
@@ -59,6 +104,22 @@ EXPORT_SYMBOL_GPL(watchdog_notify_pretimeout);
int watchdog_register_governor(struct watchdog_governor *gov)
{
struct watchdog_pretimeout *p;
+ struct governor_priv *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_lock(&governor_lock);
+
+ if (find_governor_by_name(gov->name)) {
+ mutex_unlock(&governor_lock);
+ kfree(priv);
+ return -EBUSY;
+ }
+
+ priv->gov = gov;
+ list_add(&priv->entry, &governor_list);
if (!strncmp(gov->name, WATCHDOG_PRETIMEOUT_DEFAULT_GOV,
WATCHDOG_GOV_NAME_MAXLEN)) {
@@ -71,6 +132,8 @@ int watchdog_register_governor(struct watchdog_governor *gov)
spin_unlock_irq(&pretimeout_lock);
}
+ mutex_unlock(&governor_lock);
+
return 0;
}
EXPORT_SYMBOL(watchdog_register_governor);
@@ -78,12 +141,25 @@ EXPORT_SYMBOL(watchdog_register_governor);
void watchdog_unregister_governor(struct watchdog_governor *gov)
{
struct watchdog_pretimeout *p;
+ struct governor_priv *priv, *t;
+
+ mutex_lock(&governor_lock);
+
+ list_for_each_entry_safe(priv, t, &governor_list, entry) {
+ if (priv->gov == gov) {
+ list_del(&priv->entry);
+ kfree(priv);
+ break;
+ }
+ }
spin_lock_irq(&pretimeout_lock);
list_for_each_entry(p, &pretimeout_list, entry)
if (p->wdd->gov == gov)
p->wdd->gov = default_gov;
spin_unlock_irq(&pretimeout_lock);
+
+ mutex_unlock(&governor_lock);
}
EXPORT_SYMBOL(watchdog_unregister_governor);
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index 8f9804c2dd88..afb868eedc65 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -19,6 +19,8 @@ void watchdog_unregister_governor(struct watchdog_governor *gov);
int watchdog_register_pretimeout(struct watchdog_device *wdd);
void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf);
+int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
+ const char *buf);
#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV "panic"
@@ -41,6 +43,12 @@ static inline int watchdog_pretimeout_governor_get(struct watchdog_device *wdd,
{
return -EINVAL;
}
+
+static inline int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
+ const char *buf)
+{
+ return -EINVAL;
+}
#endif
#endif
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 07/10] watchdog: pretimeout: add pretimeout_available_governors attribute
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (5 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 06/10] watchdog: pretimeout: add option to select a pretimeout governor in runtime Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 08/10] watchdog: softdog: implement pretimeout support Vladimir Zapolskiy
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
The change adds an option to a user with CONFIG_WATCHDOG_SYSFS and
CONFIG_WATCHDOG_PRETIMEOUT_GOV enabled to get information about all
registered watchdog pretimeout governors by reading watchdog device
attribute named "pretimeout_available_governors".
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
Changes from v4 to v5:
* none
Changes from v3 to v4:
* new change, this functional change was removed in v3
Changes from v2 1/4 to v4:
* introduced watchdog_pretimeout_available_governors_get() and moved
device attribute registration to watchdog_dev.c
* made attribute visibility more precise
drivers/watchdog/watchdog_dev.c | 11 ++++++++++-
drivers/watchdog/watchdog_pretimeout.c | 15 +++++++++++++++
drivers/watchdog/watchdog_pretimeout.h | 6 ++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 3fdbe0ab1365..32930a073a12 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -489,6 +489,13 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(state);
+static ssize_t pretimeout_available_governors_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return watchdog_pretimeout_available_governors_get(buf);
+}
+static DEVICE_ATTR_RO(pretimeout_available_governors);
+
static ssize_t pretimeout_governor_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -524,7 +531,8 @@ static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
else if (attr == &dev_attr_pretimeout.attr &&
!(wdd->info->options & WDIOF_PRETIMEOUT))
mode = 0;
- else if (attr == &dev_attr_pretimeout_governor.attr &&
+ else if ((attr == &dev_attr_pretimeout_governor.attr ||
+ attr == &dev_attr_pretimeout_available_governors.attr) &&
(!(wdd->info->options & WDIOF_PRETIMEOUT) ||
!IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)))
mode = 0;
@@ -541,6 +549,7 @@ static struct attribute *wdt_attrs[] = {
&dev_attr_status.attr,
&dev_attr_nowayout.attr,
&dev_attr_pretimeout_governor.attr,
+ &dev_attr_pretimeout_available_governors.attr,
NULL,
};
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index c9e4a0326938..9db07bfb4334 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -52,6 +52,21 @@ static struct governor_priv *find_governor_by_name(const char *gov_name)
return NULL;
}
+int watchdog_pretimeout_available_governors_get(char *buf)
+{
+ struct governor_priv *priv;
+ int count = 0;
+
+ mutex_lock(&governor_lock);
+
+ list_for_each_entry(priv, &governor_list, entry)
+ count += sprintf(buf + count, "%s\n", priv->gov->name);
+
+ mutex_unlock(&governor_lock);
+
+ return count;
+}
+
int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf)
{
int count = 0;
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index afb868eedc65..0166dea234d6 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -18,6 +18,7 @@ void watchdog_unregister_governor(struct watchdog_governor *gov);
/* Interfaces to watchdog_dev.c */
int watchdog_register_pretimeout(struct watchdog_device *wdd);
void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
+int watchdog_pretimeout_available_governors_get(char *buf);
int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf);
int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
const char *buf);
@@ -38,6 +39,11 @@ static inline void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
{
}
+static inline int watchdog_pretimeout_available_governors_get(char *buf)
+{
+ return -EINVAL;
+}
+
static inline int watchdog_pretimeout_governor_get(struct watchdog_device *wdd,
char *buf)
{
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 08/10] watchdog: softdog: implement pretimeout support
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (6 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 07/10] watchdog: pretimeout: add pretimeout_available_governors attribute Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 09/10] watchdog: imx2_wdt: use preferred BIT macro instead of open coded values Vladimir Zapolskiy
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang
Cc: linux-watchdog, Wolfram Sang
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Give devices which do not have hardware support for pretimeout at least a
software version of it.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
New change in the series.
Changes from Wolfram's v2 to v5:
* fixed a minor whitespace issue reported by checkpatch
Changes in Wolfram's series from v1 to v2
* added 'else' block in softdog_ping(), to actually
disable the timer when pretimeout value got cleared.
drivers/watchdog/softdog.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index b067edf246df..1b914a0d94c3 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -72,10 +72,27 @@ static void softdog_fire(unsigned long data)
static struct timer_list softdog_ticktock =
TIMER_INITIALIZER(softdog_fire, 0, 0);
+static struct watchdog_device softdog_dev;
+
+static void softdog_pretimeout(unsigned long data)
+{
+ watchdog_notify_pretimeout(&softdog_dev);
+}
+
+static struct timer_list softdog_preticktock =
+ TIMER_INITIALIZER(softdog_pretimeout, 0, 0);
+
static int softdog_ping(struct watchdog_device *w)
{
if (!mod_timer(&softdog_ticktock, jiffies + (w->timeout * HZ)))
__module_get(THIS_MODULE);
+
+ if (w->pretimeout)
+ mod_timer(&softdog_preticktock, jiffies +
+ (w->timeout - w->pretimeout) * HZ);
+ else
+ del_timer(&softdog_preticktock);
+
return 0;
}
@@ -84,12 +101,15 @@ static int softdog_stop(struct watchdog_device *w)
if (del_timer(&softdog_ticktock))
module_put(THIS_MODULE);
+ del_timer(&softdog_preticktock);
+
return 0;
}
static struct watchdog_info softdog_info = {
.identity = "Software Watchdog",
- .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
+ WDIOF_PRETIMEOUT,
};
static struct watchdog_ops softdog_ops = {
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 09/10] watchdog: imx2_wdt: use preferred BIT macro instead of open coded values
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (7 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 08/10] watchdog: softdog: implement pretimeout support Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-08-31 11:52 ` [PATCH v5 10/10] watchdog: imx2_wdt: add pretimeout function support Vladimir Zapolskiy
` (2 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
This is a nonfunctional change, declare register bit values with BIT()
helper macro.
The issues are reported by checkpatch:
CHECK: Prefer using the BIT macro
#40: FILE: drivers/watchdog/imx2_wdt.c:40:
+#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External Reset WDOG_B */
etc.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
New change in the series, practically this trivial change is
independent on the previous ones, but it's worth to have it
for a cleaner version of v5 10/10 change.
drivers/watchdog/imx2_wdt.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 62f346bb4348..d17643eb7683 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -37,18 +37,18 @@
#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
-#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External Reset WDOG_B */
-#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
-#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
-#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
-#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
+#define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset WDOG_B */
+#define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset Signal */
+#define IMX2_WDT_WCR_WRE BIT(3) /* -> WDOG Reset Enable */
+#define IMX2_WDT_WCR_WDE BIT(2) /* -> Watchdog Enable */
+#define IMX2_WDT_WCR_WDZST BIT(0) /* -> Watchdog timer Suspend */
#define IMX2_WDT_WSR 0x02 /* Service Register */
#define IMX2_WDT_SEQ1 0x5555 /* -> service sequence 1 */
#define IMX2_WDT_SEQ2 0xAAAA /* -> service sequence 2 */
#define IMX2_WDT_WRSR 0x04 /* Reset Status Register */
-#define IMX2_WDT_WRSR_TOUT (1 << 1) /* -> Reset due to Timeout */
+#define IMX2_WDT_WRSR_TOUT BIT(1) /* -> Reset due to Timeout */
#define IMX2_WDT_WMCR 0x08 /* Misc Register */
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 10/10] watchdog: imx2_wdt: add pretimeout function support
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (8 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 09/10] watchdog: imx2_wdt: use preferred BIT macro instead of open coded values Vladimir Zapolskiy
@ 2016-08-31 11:52 ` Vladimir Zapolskiy
2016-09-05 16:49 ` [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Wolfram Sang
2016-09-28 8:49 ` Wim Van Sebroeck
11 siblings, 0 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-31 11:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Wolfram Sang; +Cc: linux-watchdog
The change adds watchdog pretimeout notification handling to imx2_wdt
driver, if device data contains information about a valid interrupt.
It is unlikely but still possible (e.g. through a software limitation)
that only a subset of watchdogs on SoC has interrupt lines, hence
functionally the devices from these two groups have different
capabilities, and this is reflected in different watchdog_info
structs assigned to the devices.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
New change in the series.
drivers/watchdog/imx2_wdt.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index d17643eb7683..4874b0f18650 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -24,6 +24,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -50,6 +51,11 @@
#define IMX2_WDT_WRSR 0x04 /* Reset Status Register */
#define IMX2_WDT_WRSR_TOUT BIT(1) /* -> Reset due to Timeout */
+#define IMX2_WDT_WICR 0x06 /* Interrupt Control Register */
+#define IMX2_WDT_WICR_WIE BIT(15) /* -> Interrupt Enable */
+#define IMX2_WDT_WICR_WTIS BIT(14) /* -> Interrupt Status */
+#define IMX2_WDT_WICR_WICT 0xFF /* -> Interrupt Count Timeout */
+
#define IMX2_WDT_WMCR 0x08 /* Misc Register */
#define IMX2_WDT_MAX_TIME 128
@@ -80,6 +86,12 @@ static const struct watchdog_info imx2_wdt_info = {
.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
};
+static const struct watchdog_info imx2_wdt_pretimeout_info = {
+ .identity = "imx2+ watchdog",
+ .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
+ WDIOF_PRETIMEOUT,
+};
+
static int imx2_wdt_restart(struct watchdog_device *wdog, unsigned long action,
void *data)
{
@@ -169,6 +181,35 @@ static int imx2_wdt_set_timeout(struct watchdog_device *wdog,
return 0;
}
+static int imx2_wdt_set_pretimeout(struct watchdog_device *wdog,
+ unsigned int new_pretimeout)
+{
+ struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
+
+ if (new_pretimeout >= IMX2_WDT_MAX_TIME)
+ return -EINVAL;
+
+ wdog->pretimeout = new_pretimeout;
+
+ regmap_update_bits(wdev->regmap, IMX2_WDT_WICR,
+ IMX2_WDT_WICR_WIE | IMX2_WDT_WICR_WICT,
+ IMX2_WDT_WICR_WIE | (new_pretimeout << 1));
+ return 0;
+}
+
+static irqreturn_t imx2_wdt_isr(int irq, void *wdog_arg)
+{
+ struct watchdog_device *wdog = wdog_arg;
+ struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
+
+ regmap_write_bits(wdev->regmap, IMX2_WDT_WICR,
+ IMX2_WDT_WICR_WTIS, IMX2_WDT_WICR_WTIS);
+
+ watchdog_notify_pretimeout(wdog);
+
+ return IRQ_HANDLED;
+}
+
static int imx2_wdt_start(struct watchdog_device *wdog)
{
struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
@@ -188,6 +229,7 @@ static const struct watchdog_ops imx2_wdt_ops = {
.start = imx2_wdt_start,
.ping = imx2_wdt_ping,
.set_timeout = imx2_wdt_set_timeout,
+ .set_pretimeout = imx2_wdt_set_pretimeout,
.restart = imx2_wdt_restart,
};
@@ -236,6 +278,12 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
wdog->max_hw_heartbeat_ms = IMX2_WDT_MAX_TIME * 1000;
wdog->parent = &pdev->dev;
+ ret = platform_get_irq(pdev, 0);
+ if (ret > 0)
+ if (!devm_request_irq(&pdev->dev, ret, imx2_wdt_isr, 0,
+ dev_name(&pdev->dev), wdog))
+ wdog->info = &imx2_wdt_pretimeout_info;
+
ret = clk_prepare_enable(wdev->clk);
if (ret)
return ret;
--
2.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (9 preceding siblings ...)
2016-08-31 11:52 ` [PATCH v5 10/10] watchdog: imx2_wdt: add pretimeout function support Vladimir Zapolskiy
@ 2016-09-05 16:49 ` Wolfram Sang
2016-09-06 10:33 ` Vladimir Zapolskiy
2016-09-28 8:49 ` Wim Van Sebroeck
11 siblings, 1 reply; 21+ messages in thread
From: Wolfram Sang @ 2016-09-05 16:49 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]
On Wed, Aug 31, 2016 at 02:52:40PM +0300, Vladimir Zapolskiy wrote:
> The change adds a simple watchdog pretimeout framework infrastructure,
> its purpose is to allow users to select a desired handling of watchdog
> pretimeout events, which may be generated by a watchdog driver.
>
> The idea of adding this kind of a framework appeared after reviewing
> several attempts to add hardcoded pretimeout event handling to some
> watchdog driver and after a discussion with Guenter, see
> https://lkml.org/lkml/2015/11/4/346
>
> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
> attributes in sysfs: read only pretimeout value attribute, read/write
> pretimeout_governor attribute, read only pretimeout_available_governors
> attribute.
>
> To throw a pretimeout event for further processing a watchdog driver
> should call exported watchdog_notify_pretimeout(wdd) interface.
>
> In addition to the framework two simple watchdog pretimeout governors
> are added for review: panic and noop.
>
> Changes from v4 to v5:
> * fixed in source tree compilation issue, thanks to Wolfram
> * replaced strncmp() with more suitable sysfs_streq() to
> compare strings passed over sysfs (Wolfram)
> * added Wolfram's implementation of pretimeout to softdog driver
> * added watchdog pretimeout support to iMX2+ driver
> * minor whitespace issue fixes reported by checkpatch
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
So, I checked it on my Renesas M3 Salvator board and it worked as
expected. I also looked over the code and it looks good to me. Note that
I looked quite often over the code, so a "fresh" view might not hurt.
Despite that, I think the code is generally good to go.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-05 16:49 ` [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Wolfram Sang
@ 2016-09-06 10:33 ` Vladimir Zapolskiy
2016-09-06 14:05 ` Wolfram Sang
2016-09-09 4:47 ` Guenter Roeck
0 siblings, 2 replies; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-09-06 10:33 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog
Hi Wolfram,
On 09/05/2016 07:49 PM, Wolfram Sang wrote:
> On Wed, Aug 31, 2016 at 02:52:40PM +0300, Vladimir Zapolskiy wrote:
>> The change adds a simple watchdog pretimeout framework infrastructure,
>> its purpose is to allow users to select a desired handling of watchdog
>> pretimeout events, which may be generated by a watchdog driver.
>>
>> The idea of adding this kind of a framework appeared after reviewing
>> several attempts to add hardcoded pretimeout event handling to some
>> watchdog driver and after a discussion with Guenter, see
>> https://lkml.org/lkml/2015/11/4/346
>>
>> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
>> attributes in sysfs: read only pretimeout value attribute, read/write
>> pretimeout_governor attribute, read only pretimeout_available_governors
>> attribute.
>>
>> To throw a pretimeout event for further processing a watchdog driver
>> should call exported watchdog_notify_pretimeout(wdd) interface.
>>
>> In addition to the framework two simple watchdog pretimeout governors
>> are added for review: panic and noop.
>>
>> Changes from v4 to v5:
>> * fixed in source tree compilation issue, thanks to Wolfram
>> * replaced strncmp() with more suitable sysfs_streq() to
>> compare strings passed over sysfs (Wolfram)
>> * added Wolfram's implementation of pretimeout to softdog driver
>> * added watchdog pretimeout support to iMX2+ driver
>> * minor whitespace issue fixes reported by checkpatch
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
thank you for testing and poking me to complete the work.
> So, I checked it on my Renesas M3 Salvator board and it worked as
> expected. I also looked over the code and it looks good to me. Note that
> I looked quite often over the code, so a "fresh" view might not hurt.
> Despite that, I think the code is generally good to go.
>
I suppose watchdog maintainers would like to provide more review comments,
so probably this is not the last version of the change, anyway do you
consider to send SH-Mobile watchdog driver changes for v4.9?
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-06 10:33 ` Vladimir Zapolskiy
@ 2016-09-06 14:05 ` Wolfram Sang
2016-09-09 4:47 ` Guenter Roeck
1 sibling, 0 replies; 21+ messages in thread
From: Wolfram Sang @ 2016-09-06 14:05 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 303 bytes --]
> I suppose watchdog maintainers would like to provide more review comments,
> so probably this is not the last version of the change, anyway do you
> consider to send SH-Mobile watchdog driver changes for v4.9?
SH Mobile doesn't have HW support for that, this is why the softdog
updates are needed.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-06 10:33 ` Vladimir Zapolskiy
2016-09-06 14:05 ` Wolfram Sang
@ 2016-09-09 4:47 ` Guenter Roeck
2016-09-21 18:34 ` Wolfram Sang
1 sibling, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2016-09-09 4:47 UTC (permalink / raw)
To: Vladimir Zapolskiy, Wolfram Sang; +Cc: Wim Van Sebroeck, linux-watchdog
On 09/06/2016 03:33 AM, Vladimir Zapolskiy wrote:
> Hi Wolfram,
>
> On 09/05/2016 07:49 PM, Wolfram Sang wrote:
>> On Wed, Aug 31, 2016 at 02:52:40PM +0300, Vladimir Zapolskiy wrote:
>>> The change adds a simple watchdog pretimeout framework infrastructure,
>>> its purpose is to allow users to select a desired handling of watchdog
>>> pretimeout events, which may be generated by a watchdog driver.
>>>
>>> The idea of adding this kind of a framework appeared after reviewing
>>> several attempts to add hardcoded pretimeout event handling to some
>>> watchdog driver and after a discussion with Guenter, see
>>> https://lkml.org/lkml/2015/11/4/346
>>>
>>> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
>>> attributes in sysfs: read only pretimeout value attribute, read/write
>>> pretimeout_governor attribute, read only pretimeout_available_governors
>>> attribute.
>>>
>>> To throw a pretimeout event for further processing a watchdog driver
>>> should call exported watchdog_notify_pretimeout(wdd) interface.
>>>
>>> In addition to the framework two simple watchdog pretimeout governors
>>> are added for review: panic and noop.
>>>
>>> Changes from v4 to v5:
>>> * fixed in source tree compilation issue, thanks to Wolfram
>>> * replaced strncmp() with more suitable sysfs_streq() to
>>> compare strings passed over sysfs (Wolfram)
>>> * added Wolfram's implementation of pretimeout to softdog driver
>>> * added watchdog pretimeout support to iMX2+ driver
>>> * minor whitespace issue fixes reported by checkpatch
>>
>> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>
> thank you for testing and poking me to complete the work.
>
>> So, I checked it on my Renesas M3 Salvator board and it worked as
>> expected. I also looked over the code and it looks good to me. Note that
>> I looked quite often over the code, so a "fresh" view might not hurt.
>> Despite that, I think the code is generally good to go.
>>
>
> I suppose watchdog maintainers would like to provide more review comments,
> so probably this is not the last version of the change, anyway do you
> consider to send SH-Mobile watchdog driver changes for v4.9?
>
I am sure there is something I missed, but all I could find was nitpicks
not worth a resend, so at least for my part I think we should get this in.
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-09 4:47 ` Guenter Roeck
@ 2016-09-21 18:34 ` Wolfram Sang
2016-09-21 21:03 ` Guenter Roeck
0 siblings, 1 reply; 21+ messages in thread
From: Wolfram Sang @ 2016-09-21 18:34 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Vladimir Zapolskiy, Wim Van Sebroeck, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 282 bytes --]
> I am sure there is something I missed, but all I could find was nitpicks
> not worth a resend, so at least for my part I think we should get this in.
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Thanks! Your watchdog-next is not included in linux-next, though?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-21 18:34 ` Wolfram Sang
@ 2016-09-21 21:03 ` Guenter Roeck
0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2016-09-21 21:03 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Vladimir Zapolskiy, Wim Van Sebroeck, linux-watchdog
On Wed, Sep 21, 2016 at 08:34:24PM +0200, Wolfram Sang wrote:
>
> > I am sure there is something I missed, but all I could find was nitpicks
> > not worth a resend, so at least for my part I think we should get this in.
> >
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> Thanks! Your watchdog-next is not included in linux-next, though?
>
No.
Guenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-08-31 11:52 [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Vladimir Zapolskiy
` (10 preceding siblings ...)
2016-09-05 16:49 ` [PATCH v5 00/10] watchdog: add watchdog pretimeout framework Wolfram Sang
@ 2016-09-28 8:49 ` Wim Van Sebroeck
2016-09-28 11:34 ` Vladimir Zapolskiy
11 siblings, 1 reply; 21+ messages in thread
From: Wim Van Sebroeck @ 2016-09-28 8:49 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Guenter Roeck, Wolfram Sang, linux-watchdog
Hi All,
> The change adds a simple watchdog pretimeout framework infrastructure,
> its purpose is to allow users to select a desired handling of watchdog
> pretimeout events, which may be generated by a watchdog driver.
>
> The idea of adding this kind of a framework appeared after reviewing
> several attempts to add hardcoded pretimeout event handling to some
> watchdog driver and after a discussion with Guenter, see
> https://lkml.org/lkml/2015/11/4/346
>
> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
> attributes in sysfs: read only pretimeout value attribute, read/write
> pretimeout_governor attribute, read only pretimeout_available_governors
> attribute.
>
> To throw a pretimeout event for further processing a watchdog driver
> should call exported watchdog_notify_pretimeout(wdd) interface.
>
> In addition to the framework two simple watchdog pretimeout governors
> are added for review: panic and noop.
I still have 2 questions about this series:
1) Patch 3:
+/* Use the following functions to report watchdog pretimeout event */
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
+void watchdog_notify_pretimeout(struct watchdog_device *wdd);
+#else
+static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+{
+ panic("watchdog pretimeout event\n");
+}
+#endif
+
Why a panic here? Why not just a printk like in the noop pretimeout governor?
We now also don't do a panic, which means that changing this to a panic means that we change the default behaviour...
2) Why did we choose the panic pretimeout governer as the default and not the noop governor?
This is basically the same question/remark as my previous question. noop is more the old behaviour, panic is a change in regards to the old/current behaviour...
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-28 8:49 ` Wim Van Sebroeck
@ 2016-09-28 11:34 ` Vladimir Zapolskiy
2016-10-04 13:29 ` Vladimir Zapolskiy
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-09-28 11:34 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: Guenter Roeck, Wolfram Sang, linux-watchdog
Hello Wim,
On 09/28/2016 11:49 AM, Wim Van Sebroeck wrote:
> Hi All,
>
>> The change adds a simple watchdog pretimeout framework infrastructure,
>> its purpose is to allow users to select a desired handling of watchdog
>> pretimeout events, which may be generated by a watchdog driver.
>>
>> The idea of adding this kind of a framework appeared after reviewing
>> several attempts to add hardcoded pretimeout event handling to some
>> watchdog driver and after a discussion with Guenter, see
>> https://lkml.org/lkml/2015/11/4/346
>>
>> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
>> attributes in sysfs: read only pretimeout value attribute, read/write
>> pretimeout_governor attribute, read only pretimeout_available_governors
>> attribute.
>>
>> To throw a pretimeout event for further processing a watchdog driver
>> should call exported watchdog_notify_pretimeout(wdd) interface.
>>
>> In addition to the framework two simple watchdog pretimeout governors
>> are added for review: panic and noop.
>
> I still have 2 questions about this series:
> 1) Patch 3:
> +/* Use the following functions to report watchdog pretimeout event */
> +#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
> +void watchdog_notify_pretimeout(struct watchdog_device *wdd);
> +#else
> +static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd)
> +{
> + panic("watchdog pretimeout event\n");
> +}
> +#endif
> +
>
> Why a panic here? Why not just a printk like in the noop pretimeout governor?
> We now also don't do a panic, which means that changing this to a panic means that we change the default behaviour...
>
> 2) Why did we choose the panic pretimeout governer as the default and not the noop governor?
> This is basically the same question/remark as my previous question. noop is more the old behaviour, panic is a change in regards to the old/current behaviour...
>
regarding a selection of panic as a default action I had a discussion with
Guenter about it right in the beginnig of the development (v1 of the series
introduced printk by default, v2 and all later series select panic), please
follow the link to it (ctrl-f panic):
http://www.spinics.net/lists/linux-watchdog/msg07955.html
I generally don't have a strong preference, because it is configurable both
during build time and runtime, and I'm flexible to accept any selection.
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-09-28 11:34 ` Vladimir Zapolskiy
@ 2016-10-04 13:29 ` Vladimir Zapolskiy
2016-10-07 7:24 ` Wim Van Sebroeck
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Zapolskiy @ 2016-10-04 13:29 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: Guenter Roeck, Wolfram Sang, linux-watchdog
Hello Wim,
On 09/28/2016 02:34 PM, Vladimir Zapolskiy wrote:
> Hello Wim,
>
> On 09/28/2016 11:49 AM, Wim Van Sebroeck wrote:
>> Hi All,
>>
>>> The change adds a simple watchdog pretimeout framework infrastructure,
>>> its purpose is to allow users to select a desired handling of watchdog
>>> pretimeout events, which may be generated by a watchdog driver.
>>>
>>> The idea of adding this kind of a framework appeared after reviewing
>>> several attempts to add hardcoded pretimeout event handling to some
>>> watchdog driver and after a discussion with Guenter, see
>>> https://lkml.org/lkml/2015/11/4/346
>>>
>>> Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
>>> attributes in sysfs: read only pretimeout value attribute, read/write
>>> pretimeout_governor attribute, read only pretimeout_available_governors
>>> attribute.
>>>
>>> To throw a pretimeout event for further processing a watchdog driver
>>> should call exported watchdog_notify_pretimeout(wdd) interface.
>>>
>>> In addition to the framework two simple watchdog pretimeout governors
>>> are added for review: panic and noop.
>>
>> I still have 2 questions about this series:
>> 1) Patch 3:
>> +/* Use the following functions to report watchdog pretimeout event */
>> +#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
>> +void watchdog_notify_pretimeout(struct watchdog_device *wdd);
>> +#else
>> +static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd)
>> +{
>> + panic("watchdog pretimeout event\n");
>> +}
>> +#endif
>> +
>>
>> Why a panic here? Why not just a printk like in the noop pretimeout governor?
>> We now also don't do a panic, which means that changing this to a panic means that we change the default behaviour...
>>
>> 2) Why did we choose the panic pretimeout governer as the default and not the noop governor?
>> This is basically the same question/remark as my previous question. noop is more the old behaviour, panic is a change in regards to the old/current behaviour...
>>
>
> regarding a selection of panic as a default action I had a discussion with
> Guenter about it right in the beginnig of the development (v1 of the series
> introduced printk by default, v2 and all later series select panic), please
> follow the link to it (ctrl-f panic):
>
> http://www.spinics.net/lists/linux-watchdog/msg07955.html
>
> I generally don't have a strong preference, because it is configurable both
> during build time and runtime, and I'm flexible to accept any selection.
>
I noticed that the complete series is not on your watchdog/master branch,
for clarity do you request to do any changes to merge it into v4.9?
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework
2016-10-04 13:29 ` Vladimir Zapolskiy
@ 2016-10-07 7:24 ` Wim Van Sebroeck
0 siblings, 0 replies; 21+ messages in thread
From: Wim Van Sebroeck @ 2016-10-07 7:24 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Guenter Roeck, Wolfram Sang, linux-watchdog
Hi Vladimir,
> On 09/28/2016 02:34 PM, Vladimir Zapolskiy wrote:
> >Hello Wim,
> >
> >On 09/28/2016 11:49 AM, Wim Van Sebroeck wrote:
> >>Hi All,
> >>
> >>>The change adds a simple watchdog pretimeout framework infrastructure,
> >>>its purpose is to allow users to select a desired handling of watchdog
> >>>pretimeout events, which may be generated by a watchdog driver.
> >>>
> >>>The idea of adding this kind of a framework appeared after reviewing
> >>>several attempts to add hardcoded pretimeout event handling to some
> >>>watchdog driver and after a discussion with Guenter, see
> >>>https://lkml.org/lkml/2015/11/4/346
> >>>
> >>>Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
> >>>attributes in sysfs: read only pretimeout value attribute, read/write
> >>>pretimeout_governor attribute, read only pretimeout_available_governors
> >>>attribute.
> >>>
> >>>To throw a pretimeout event for further processing a watchdog driver
> >>>should call exported watchdog_notify_pretimeout(wdd) interface.
> >>>
> >>>In addition to the framework two simple watchdog pretimeout governors
> >>>are added for review: panic and noop.
> >>
> >>I still have 2 questions about this series:
> >>1) Patch 3:
> >>+/* Use the following functions to report watchdog pretimeout event */
> >>+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
> >>+void watchdog_notify_pretimeout(struct watchdog_device *wdd);
> >>+#else
> >>+static inline void watchdog_notify_pretimeout(struct watchdog_device
> >>*wdd)
> >>+{
> >>+ panic("watchdog pretimeout event\n");
> >>+}
> >>+#endif
> >>+
> >>
> >>Why a panic here? Why not just a printk like in the noop pretimeout
> >>governor?
> >>We now also don't do a panic, which means that changing this to a panic
> >>means that we change the default behaviour...
> >>
> >>2) Why did we choose the panic pretimeout governer as the default and not
> >>the noop governor?
> >>This is basically the same question/remark as my previous question. noop
> >>is more the old behaviour, panic is a change in regards to the
> >>old/current behaviour...
> >>
> >
> >regarding a selection of panic as a default action I had a discussion with
> >Guenter about it right in the beginnig of the development (v1 of the series
> >introduced printk by default, v2 and all later series select panic), please
> >follow the link to it (ctrl-f panic):
> >
> > http://www.spinics.net/lists/linux-watchdog/msg07955.html
> >
> >I generally don't have a strong preference, because it is configurable both
> >during build time and runtime, and I'm flexible to accept any selection.
> >
>
> I noticed that the complete series is not on your watchdog/master branch,
> for clarity do you request to do any changes to merge it into v4.9?
I'll make the changes this weekend. I want to have the noop as default.
So rest should go in this weekend so that we can still get it in for 4.9.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 21+ messages in thread