Re: [PATCH 3/3] CBUS: Make retu watchdog behave like a standard Linux watchdog
* Tony Lindgren <[EMAIL PROTECTED]> [080512 18:17]: > * Igor Stoppa <[EMAIL PROTECTED]> [080512 17:56]: > > Hi Tony, > > On Mon, 2008-05-12 at 17:35 -0700, ext Tony Lindgren wrote: > > > Make retu watchdog behave like a standard Linux watchdog. > > > > > > Let the kernel do the kicking until the watchdog device is opened. > > > > This is not always the desidered behavior: the powerdown wd is used to > > ensure that the whole sw stack is healty: doing the kicking in > > kernelspace for free introduces the case where userspace can get stuck > > and the device does not powerdown. > > That's why there's CONFIG_WATCHDOG_NOWAYOUT where the ping timer is > not enabled at all, and the watchdog is just set to max until userspace > watchdog software kicking starts. > > > Also the unconditional loading of the maximum value during probe is not > > aligned with the original reset logic, which was to have the powerdown > > wd to allow for 2 boot attempts: > > > > -cold boot -> load max value in retu wd (63s) > >-> load 30s in omap wd > > -try to kick both wds > > Well you can set those values via /dev/watchdog too, right? > And then use CONFIG_WATCHDOG_NOWAYOUT. > > And ff CONFIG_WATCHDOG_NOWAYOUT is not set, the kernel ping timer only > happens when /dev/watchdog is not open. > > > if fail, then omap reboots, but retu keeps counting down > > > > -warm boot -> let the retu wd untouched > >-> load 30s in omap wd > > -try to kick both wds > > > > if fail, retu powers down > > > > That was the original idea in 770 times and i still like it. > > > > To conclude, i'd see inkernel kicking more as a debugging feature while > > one is hacking at the kernel than a desirable quality of a stable > > kernel. > > Considering that the /dev/watchdog interface is the standard, I see this > patch as the only way we can get this code ever merged upstream. And it's > easy to patch back the non-standard if you want to. I'll push the first two patches today, third one still needs to be checked. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] CBUS: Make retu watchdog behave like a standard Linux watchdog
* Igor Stoppa <[EMAIL PROTECTED]> [080512 17:56]: > Hi Tony, > On Mon, 2008-05-12 at 17:35 -0700, ext Tony Lindgren wrote: > > Make retu watchdog behave like a standard Linux watchdog. > > > > Let the kernel do the kicking until the watchdog device is opened. > > This is not always the desidered behavior: the powerdown wd is used to > ensure that the whole sw stack is healty: doing the kicking in > kernelspace for free introduces the case where userspace can get stuck > and the device does not powerdown. That's why there's CONFIG_WATCHDOG_NOWAYOUT where the ping timer is not enabled at all, and the watchdog is just set to max until userspace watchdog software kicking starts. > Also the unconditional loading of the maximum value during probe is not > aligned with the original reset logic, which was to have the powerdown > wd to allow for 2 boot attempts: > > -cold boot -> load max value in retu wd (63s) >-> load 30s in omap wd > -try to kick both wds Well you can set those values via /dev/watchdog too, right? And then use CONFIG_WATCHDOG_NOWAYOUT. And ff CONFIG_WATCHDOG_NOWAYOUT is not set, the kernel ping timer only happens when /dev/watchdog is not open. > if fail, then omap reboots, but retu keeps counting down > > -warm boot -> let the retu wd untouched >-> load 30s in omap wd > -try to kick both wds > > if fail, retu powers down > > That was the original idea in 770 times and i still like it. > > To conclude, i'd see inkernel kicking more as a debugging feature while > one is hacking at the kernel than a desirable quality of a stable > kernel. Considering that the /dev/watchdog interface is the standard, I see this patch as the only way we can get this code ever merged upstream. And it's easy to patch back the non-standard if you want to. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] CBUS: Make retu watchdog behave like a standard Linux watchdog
Hi Tony, On Mon, 2008-05-12 at 17:35 -0700, ext Tony Lindgren wrote: > Make retu watchdog behave like a standard Linux watchdog. > > Let the kernel do the kicking until the watchdog device is opened. This is not always the desidered behavior: the powerdown wd is used to ensure that the whole sw stack is healty: doing the kicking in kernelspace for free introduces the case where userspace can get stuck and the device does not powerdown. Also the unconditional loading of the maximum value during probe is not aligned with the original reset logic, which was to have the powerdown wd to allow for 2 boot attempts: -cold boot -> load max value in retu wd (63s) -> load 30s in omap wd -try to kick both wds if fail, then omap reboots, but retu keeps counting down -warm boot -> let the retu wd untouched -> load 30s in omap wd -try to kick both wds if fail, retu powers down That was the original idea in 770 times and i still like it. To conclude, i'd see inkernel kicking more as a debugging feature while one is hacking at the kernel than a desirable quality of a stable kernel. -- Cheers, Igor --- Igor Stoppa Nokia Devices R&D - Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] CBUS: Make retu watchdog behave like a standard Linux watchdog
Make retu watchdog behave like a standard Linux watchdog.
Let the kernel do the kicking until the watchdog device is opened.
Note: We should remove the old non-standard interface, please
change to use standard /dev/watchdog instead.
Signed-off-by: Tony Lindgren <[EMAIL PROTECTED]>
---
drivers/cbus/Kconfig|2 +-
drivers/cbus/retu-wdt.c | 184 ++-
2 files changed, 183 insertions(+), 3 deletions(-)
diff --git a/drivers/cbus/Kconfig b/drivers/cbus/Kconfig
index 25f8039..c344a99 100644
--- a/drivers/cbus/Kconfig
+++ b/drivers/cbus/Kconfig
@@ -72,7 +72,7 @@ config CBUS_RETU_RTC
RTC in Retu. This will expose a sysfs interface for it.
config CBUS_RETU_WDT
- depends on CBUS_RETU && SYSFS
+ depends on CBUS_RETU && SYSFS && WATCHDOG
tristate "Support for Retu watchdog timer"
---help---
Say Y here if you want support for the watchdog in Retu. This will
diff --git a/drivers/cbus/retu-wdt.c b/drivers/cbus/retu-wdt.c
index b7b20b7..8ca5c72 100644
--- a/drivers/cbus/retu-wdt.c
+++ b/drivers/cbus/retu-wdt.c
@@ -25,11 +25,19 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
#include
+#include
+#include
+
+#include
+
+#include
#include "cbus.h"
#include "retu.h"
@@ -46,6 +54,17 @@ static DEFINE_MUTEX(retu_wdt_mutex);
static unsigned int period_val = RETU_WDT_DEFAULT_TIMER;
static int counter_param = RETU_WDT_MAX_TIMER;
+struct retu_wdt_dev {
+ struct device *dev;
+ int users;
+ struct miscdevice retu_wdt_miscdev;
+ struct timer_list ping_timer;
+};
+
+static struct retu_wdt_dev *retu_wdt;
+
+static void retu_wdt_set_ping_timer(unsigned long enable);
+
static int retu_modify_counter(unsigned int new)
{
int ret = 0;
@@ -69,6 +88,10 @@ static ssize_t retu_wdt_period_show(struct device *dev,
return sprintf(buf, "%u\n", (u16)period_val);
}
+/*
+ * Note: This inteface is non-standard and likely to disappear!
+ * Use /dev/watchdog instead, that's the standard.
+ */
static ssize_t retu_wdt_period_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
@@ -76,6 +99,10 @@ static ssize_t retu_wdt_period_store(struct device *dev,
unsigned int new_period;
int ret;
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+ retu_wdt_set_ping_timer(0);
+#endif
+
if (sscanf(buf, "%u", &new_period) != 1) {
printk(KERN_ALERT "retu_wdt_period_store: Invalid input\n");
return -EINVAL;
@@ -104,31 +131,184 @@ static DEVICE_ATTR(period, S_IRUGO | S_IWUSR,
retu_wdt_period_show, \
retu_wdt_period_store);
static DEVICE_ATTR(counter, S_IRUGO, retu_wdt_counter_show, NULL);
+/**/
+
+/*
+ * Since retu watchdog cannot be disabled in hardware, we must kick it
+ * with a timer until userspace watchdog software takes over. Do this
+ * unless /dev/watchdog is open or CONFIG_WATCHDOG_NOWAYOUT is set.
+ */
+static void retu_wdt_set_ping_timer(unsigned long enable)
+{
+ retu_modify_counter(RETU_WDT_MAX_TIMER);
+ if (enable)
+ mod_timer(&retu_wdt->ping_timer,
+ jiffies + RETU_WDT_DEFAULT_TIMER * HZ);
+ else
+ del_timer_sync(&retu_wdt->ping_timer);
+}
+
+static int retu_wdt_open(struct inode *inode, struct file *file)
+{
+ if (test_and_set_bit(1, (unsigned long *)&(retu_wdt->users)))
+ return -EBUSY;
+
+ file->private_data = (void *)retu_wdt;
+ retu_wdt_set_ping_timer(0);
+
+ return nonseekable_open(inode, file);
+}
+
+static int retu_wdt_release(struct inode *inode, struct file *file)
+{
+ struct retu_wdt_dev *wdev = file->private_data;
+
+#ifndef CONFIG_WATCHDOG_NOWAYOUT
+ retu_wdt_set_ping_timer(1);
+#endif
+ wdev->users = 0;
+
+ return 0;
+}
+
+static ssize_t retu_wdt_write(struct file *file, const char __user *data,
+ size_t len, loff_t *ppos)
+{
+ if (len)
+ retu_modify_counter(RETU_WDT_MAX_TIMER);
+
+ return len;
+}
+
+static int retu_wdt_ioctl(struct inode *inode, struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ int new_margin;
+
+ static struct watchdog_info ident = {
+ .identity = "Retu Watchdog",
+ .options = WDIOF_SETTIMEOUT,
+ .firmware_version = 0,
+ };
+
+ switch (cmd) {
+ default:
+ return -ENOTTY;
+ case WDIOC_GETSUPPORT:
+ return copy_to_user((struct watchdog_info __user *)arg, &ident,
+ sizeof(ident));
+ case WDIOC_GETSTATUS:
+
