* add bcm47xx watchdog driver
@ 2009-06-04 20:24 matthieu castet
2009-06-05 13:58 ` Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: matthieu castet @ 2009-06-04 20:24 UTC (permalink / raw)
To: wim, Linux Kernel list, linux-mips; +Cc: Aleksandar Radovanovic
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: bcm47xx_watchdog.diff --]
[-- Type: text/x-diff, Size: 8392 bytes --]
This add watchdog driver for broadcom 47xx device.
It uses the ssb subsytem to access embeded watchdog device.
Because the watchdog timeout is very short (about 2s), a soft timer is used
to increase the watchdog period.
Note : A patch for exporting the ssb_watchdog_timer_set will
be submitted on next linux-mips merge. Without this patch it can't
be build as a module.
Signed-off-by: Aleksandar Radovanovic <biblbroks@sezampro.rs>
Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
Index: linux-2.6/drivers/watchdog/Kconfig
===================================================================
--- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200
+++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200
@@ -764,6 +764,12 @@
help
Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs.
+config BCM47XX_WDT
+ tristate "Broadcom BCM47xx Watchdog Timer"
+ depends on BCM47XX
+ help
+ Hardware driver for the Broadcom BCM47xx Watchog Timer.
+
# PARISC Architecture
# POWERPC Architecture
Index: linux-2.6/drivers/watchdog/Makefile
===================================================================
--- linux-2.6.orig/drivers/watchdog/Makefile 2009-05-25 22:22:02.000000000 +0200
+++ linux-2.6/drivers/watchdog/Makefile 2009-05-25 23:02:27.000000000 +0200
@@ -105,6 +105,7 @@
obj-$(CONFIG_SIBYTE_WDOG) += sb_wdog.o
obj-$(CONFIG_AR7_WDT) += ar7_wdt.o
obj-$(CONFIG_TXX9_WDT) += txx9wdt.o
+obj-$(CONFIG_BCM47XX_WDT) += bcm47xx_wdt.o
# PARISC Architecture
Index: linux-2.6/drivers/watchdog/bcm47xx_wdt.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/drivers/watchdog/bcm47xx_wdt.c 2009-05-25 23:29:25.000000000 +0200
@@ -0,0 +1,289 @@
+/*
+ * Watchdog driver for Broadcom BCM47XX
+ *
+ * Copyright (C) 2008 Aleksandar Radovanovic <biblbroks@sezampro.rs>
+ *
+ * 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/bitops.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/notifier.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/watchdog.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/ssb/ssb_embedded.h>
+#include <asm/mach-bcm47xx/bcm47xx.h>
+
+#define DRV_NAME "bcm47xx_wdt"
+
+#define WDT_DEFAULT_TIME 30 /* seconds */
+#define WDT_MAX_TIME 256 /* seconds */
+
+static int wdt_time = WDT_DEFAULT_TIME;
+static int nowayout = WATCHDOG_NOWAYOUT;
+
+module_param(wdt_time, int, 0);
+MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default="
+ __MODULE_STRING(WDT_DEFAULT_TIME) ")");
+
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout,
+ "Watchdog cannot be stopped once started (default="
+ __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+#endif
+
+static struct platform_device *bcm47xx_wdt_platform_device;
+
+static unsigned long bcm47xx_wdt_busy;
+static char expect_release;
+static struct timer_list wdt_timer;
+static atomic_t ticks;
+
+static inline void bcm47xx_wdt_hw_start(void)
+{
+ /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */
+ ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff);
+}
+
+static inline int bcm47xx_wdt_hw_stop(void)
+{
+ return ssb_watchdog_timer_set(&ssb_bcm47xx, 0);
+}
+
+static void bcm47xx_timer_tick(unsigned long unused)
+{
+ if(!atomic_dec_and_test(&ticks)) {
+ bcm47xx_wdt_hw_start();
+ mod_timer(&wdt_timer, jiffies + HZ);
+ }
+ else {
+ printk(KERN_CRIT PFX "Watchdog will fire soon!!!.\n");
+ }
+}
+
+static inline void bcm47xx_wdt_pet(void)
+{
+ atomic_set(&ticks, wdt_time);
+}
+
+static void bcm47xx_wdt_start(void)
+{
+ bcm47xx_wdt_pet();
+ bcm47xx_timer_tick(0);
+}
+
+static void bcm47xx_wdt_pause(void)
+{
+ del_timer(&wdt_timer);
+ bcm47xx_wdt_hw_stop();
+}
+
+static void bcm47xx_wdt_stop(void)
+{
+ bcm47xx_wdt_pause();
+}
+
+static int bcm47xx_wdt_settimeout(int new_time)
+{
+ if ((new_time <= 0) || (new_time > WDT_MAX_TIME))
+ return -EINVAL;
+
+ wdt_time = new_time;
+ return 0;
+}
+
+static int bcm47xx_wdt_open(struct inode *inode, struct file *file)
+{
+ if (test_and_set_bit(0, &bcm47xx_wdt_busy))
+ return -EBUSY;
+
+ bcm47xx_wdt_start();
+ return nonseekable_open(inode, file);
+}
+
+static int bcm47xx_wdt_release(struct inode *inode, struct file *file)
+{
+ if (expect_release == 42) {
+ bcm47xx_wdt_stop();
+ } else {
+ printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping watchdog!\n");
+ bcm47xx_wdt_start();
+ }
+
+ clear_bit(0, &bcm47xx_wdt_busy);
+ expect_release = 0;
+ return 0;
+}
+
+static ssize_t bcm47xx_wdt_write (struct file *file, const char __user *data,
+ size_t len, loff_t * ppos)
+{
+ if (len) {
+ if (!nowayout) {
+ size_t i;
+
+ expect_release = 0;
+
+ for (i = 0; i != len; i++) {
+ char c;
+ if (get_user(c, data + i))
+ return -EFAULT;
+ if (c == 'V')
+ expect_release = 42;
+ }
+ }
+ bcm47xx_wdt_pet();
+ }
+ return len;
+}
+
+static struct watchdog_info bcm47xx_wdt_info = {
+ .identity = DRV_NAME,
+ .options = WDIOF_SETTIMEOUT |
+ WDIOF_KEEPALIVEPING |
+ WDIOF_MAGICCLOSE,
+};
+
+static long bcm47xx_wdt_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ int new_value, retval = -EINVAL;;
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &bcm47xx_wdt_info,
+ sizeof(bcm47xx_wdt_info)) ? -EFAULT : 0;
+
+ case WDIOC_GETSTATUS:
+ case WDIOC_GETBOOTSTATUS:
+ return put_user(0, p);
+
+ case WDIOC_SETOPTIONS:
+ if (get_user(new_value, p))
+ return -EFAULT;
+
+ if (new_value & WDIOS_DISABLECARD) {
+ bcm47xx_wdt_stop();
+ retval = 0;
+ }
+
+ if (new_value & WDIOS_ENABLECARD) {
+ bcm47xx_wdt_start();
+ retval = 0;
+ }
+
+ return retval;
+
+ case WDIOC_KEEPALIVE:
+ bcm47xx_wdt_pet();
+ return 0;
+
+ case WDIOC_SETTIMEOUT:
+ if (get_user(new_value, p))
+ return -EFAULT;
+
+ if (bcm47xx_wdt_settimeout(new_value))
+ return -EINVAL;
+
+ bcm47xx_wdt_pet();
+
+ case WDIOC_GETTIMEOUT:
+ return put_user(wdt_time, p);
+
+ default:
+ return -ENOTTY;
+ }
+}
+
+static int bcm47xx_wdt_notify_sys(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ bcm47xx_wdt_stop();
+ return NOTIFY_DONE;
+}
+
+static const struct file_operations bcm47xx_wdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .unlocked_ioctl = bcm47xx_wdt_ioctl,
+ .open = bcm47xx_wdt_open,
+ .release = bcm47xx_wdt_release,
+ .write = bcm47xx_wdt_write,
+};
+
+static struct miscdevice bcm47xx_wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &bcm47xx_wdt_fops,
+};
+
+static struct notifier_block bcm47xx_wdt_notifier = {
+ .notifier_call = bcm47xx_wdt_notify_sys,
+};
+
+static int __init bcm47xx_wdt_init(void)
+{
+ int ret;
+
+ if (bcm47xx_wdt_hw_stop() < 0)
+ return -ENODEV;
+
+ setup_timer(&wdt_timer, bcm47xx_timer_tick, 0L);
+
+ if (bcm47xx_wdt_settimeout(wdt_time)) {
+ bcm47xx_wdt_settimeout(WDT_DEFAULT_TIME);
+ printk(KERN_INFO DRV_NAME ": wdt_time value must be 1 <= wdt_time <= 256, using %d\n",
+ wdt_time);
+ }
+
+ ret = register_reboot_notifier(&bcm47xx_wdt_notifier);
+ if (ret)
+ ret;
+
+ ret = misc_register(&bcm47xx_wdt_miscdev);
+ if (ret) {
+ unregister_reboot_notifier(&bcm47xx_wdt_notifier);
+ return ret;
+ }
+
+ printk(KERN_INFO "BCM47xx Watchdog Timer enabled (%d seconds%s)\n",
+ wdt_time, nowayout ? ", nowayout" : "");
+ return 0;
+}
+
+static void __exit bcm47xx_wdt_exit(void)
+{
+ if (!nowayout)
+ bcm47xx_wdt_stop();
+
+ misc_deregister(&bcm47xx_wdt_miscdev);
+
+ unregister_reboot_notifier(&bcm47xx_wdt_notifier);
+
+ return 0;
+}
+
+module_init(bcm47xx_wdt_init);
+module_exit(bcm47xx_wdt_exit);
+
+MODULE_AUTHOR("Aleksandar Radovanovic");
+MODULE_DESCRIPTION("Watchdog driver for Broadcom BCM47xx");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_ALIAS("platform:bcm47xx_wdt");
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: add bcm47xx watchdog driver 2009-06-04 20:24 add bcm47xx watchdog driver matthieu castet @ 2009-06-05 13:58 ` Florian Fainelli 2009-06-05 14:58 ` castet.matthieu 2009-06-05 16:57 ` add bcm47xx watchdog driver v2 matthieu castet 2009-06-05 19:48 ` add bcm47xx watchdog driver Andrew Morton 2009-06-05 19:50 ` Andrew Morton 2 siblings, 2 replies; 13+ messages in thread From: Florian Fainelli @ 2009-06-05 13:58 UTC (permalink / raw) To: matthieu castet Cc: wim, Linux Kernel list, linux-mips, Aleksandar Radovanovic Salut Matthieu, Le Thursday 04 June 2009 22:24:56 matthieu castet, vous avez écrit : > This add watchdog driver for broadcom 47xx device. > It uses the ssb subsytem to access embeded watchdog device. > > Because the watchdog timeout is very short (about 2s), a soft timer is used > to increase the watchdog period. > > Note : A patch for exporting the ssb_watchdog_timer_set will > be submitted on next linux-mips merge. Without this patch it can't > be build as a module. Your driver looks good, could you turn this into a platform device/driver instead ? You declare bcm47xx_wdt_platform_device which is unused and you also declare a MODULE_ALIAS which suggets it is one. You are also missing your name in both the header the the MODULE_AUTHOR macro. -- Best regards, Florian Fainelli Email : florian@openwrt.org http://openwrt.org ------------------------------- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-05 13:58 ` Florian Fainelli @ 2009-06-05 14:58 ` castet.matthieu 2009-06-06 10:58 ` Florian Fainelli 2009-06-05 16:57 ` add bcm47xx watchdog driver v2 matthieu castet 1 sibling, 1 reply; 13+ messages in thread From: castet.matthieu @ 2009-06-05 14:58 UTC (permalink / raw) To: Florian Fainelli Cc: matthieu castet, wim, Linux Kernel list, linux-mips, Aleksandar Radovanovic Hi Florian, Quoting Florian Fainelli <florian@openwrt.org>: > > Your driver looks good, could you turn this into a platform device/driver > instead ? You declare bcm47xx_wdt_platform_device which is unused and you > also declare a MODULE_ALIAS which suggets it is one. What's the advantage of using platform device/driver ? Not all watchdog driver use it (for example softdog). This seems useless in this case because the driver don't have any resource, don't care about suspend/resume and add complexity in the code (2 registers in module probe, ...). Matthieu ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-05 14:58 ` castet.matthieu @ 2009-06-06 10:58 ` Florian Fainelli 0 siblings, 0 replies; 13+ messages in thread From: Florian Fainelli @ 2009-06-06 10:58 UTC (permalink / raw) To: castet.matthieu Cc: wim, Linux Kernel list, linux-mips, Aleksandar Radovanovic Le Friday 05 June 2009 16:58:42 castet.matthieu@free.fr, vous avez écrit : > Hi Florian, > > Quoting Florian Fainelli <florian@openwrt.org>: > > Your driver looks good, could you turn this into a platform device/driver > > instead ? You declare bcm47xx_wdt_platform_device which is unused and you > > also declare a MODULE_ALIAS which suggets it is one. > > What's the advantage of using platform device/driver ? > Not all watchdog driver use it (for example softdog). > This seems useless in this case because the driver don't have any resource, > don't care about suspend/resume and add complexity in the code (2 registers > in module probe, ...). Indeed, I was suggesting that either you turn it into a full platform driver or your remove references to it (bcm47xx_wdt_platform_device and MODULE_ALIAS). -- Best regards, Florian Fainelli Email : florian@openwrt.org http://openwrt.org ------------------------------- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver v2 2009-06-05 13:58 ` Florian Fainelli 2009-06-05 14:58 ` castet.matthieu @ 2009-06-05 16:57 ` matthieu castet 1 sibling, 0 replies; 13+ messages in thread From: matthieu castet @ 2009-06-05 16:57 UTC (permalink / raw) To: Florian Fainelli Cc: wim, Linux Kernel list, linux-mips, Aleksandar Radovanovic [-- Attachment #1: Type: text/plain, Size: 812 bytes --] Florian Fainelli wrote: > Salut Matthieu, > > Le Thursday 04 June 2009 22:24:56 matthieu castet, vous avez écrit : >> This add watchdog driver for broadcom 47xx device. >> It uses the ssb subsytem to access embeded watchdog device. >> >> Because the watchdog timeout is very short (about 2s), a soft timer is used >> to increase the watchdog period. >> >> Note : A patch for exporting the ssb_watchdog_timer_set will >> be submitted on next linux-mips merge. Without this patch it can't >> be build as a module. > > Your driver looks good, could you turn this into a platform device/driver > instead ? You declare bcm47xx_wdt_platform_device which is unused and you > also declare a MODULE_ALIAS which suggets it is one. oops I attached the wrong patch. This one should fix some build failure ;) Matthieu [-- Attachment #2: bcm47xx_watchdog.diff --] [-- Type: text/x-diff, Size: 8285 bytes --] This add watchdog driver for broadcom 47xx device. It uses the ssb subsytem to access embeded watchdog device. Because the watchdog timeout is very short (about 2s), a soft timer is used to increase the watchdog period. Note : A patch for exporting the ssb_watchdog_timer_set will be submitted on next linux-mips merge. Without this patch it can't be build as a module. Signed-off-by: Aleksandar Radovanovic <biblbroks@sezampro.rs> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr> Index: linux-2.6/drivers/watchdog/Kconfig =================================================================== --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 @@ -764,6 +764,12 @@ help Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. +config BCM47XX_WDT + tristate "Broadcom BCM47xx Watchdog Timer" + depends on BCM47XX + help + Hardware driver for the Broadcom BCM47xx Watchog Timer. + # PARISC Architecture # POWERPC Architecture Index: linux-2.6/drivers/watchdog/Makefile =================================================================== --- linux-2.6.orig/drivers/watchdog/Makefile 2009-05-25 22:22:02.000000000 +0200 +++ linux-2.6/drivers/watchdog/Makefile 2009-05-25 23:02:27.000000000 +0200 @@ -105,6 +105,7 @@ obj-$(CONFIG_SIBYTE_WDOG) += sb_wdog.o obj-$(CONFIG_AR7_WDT) += ar7_wdt.o obj-$(CONFIG_TXX9_WDT) += txx9wdt.o +obj-$(CONFIG_BCM47XX_WDT) += bcm47xx_wdt.o # PARISC Architecture Index: linux-2.6/drivers/watchdog/bcm47xx_wdt.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ linux-2.6/drivers/watchdog/bcm47xx_wdt.c 2009-06-05 18:52:51.000000000 +0200 @@ -0,0 +1,284 @@ +/* + * Watchdog driver for Broadcom BCM47XX + * + * Copyright (C) 2008 Aleksandar Radovanovic <biblbroks@sezampro.rs> + * + * 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/bitops.h> +#include <linux/errno.h> +#include <linux/fs.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/reboot.h> +#include <linux/types.h> +#include <linux/uaccess.h> +#include <linux/watchdog.h> +#include <linux/timer.h> +#include <linux/jiffies.h> +#include <linux/ssb/ssb_embedded.h> +#include <asm/mach-bcm47xx/bcm47xx.h> + +#define DRV_NAME "bcm47xx_wdt" + +#define WDT_DEFAULT_TIME 30 /* seconds */ +#define WDT_MAX_TIME 256 /* seconds */ + +static int wdt_time = WDT_DEFAULT_TIME; +static int nowayout = WATCHDOG_NOWAYOUT; + +module_param(wdt_time, int, 0); +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); + +#ifdef CONFIG_WATCHDOG_NOWAYOUT +module_param(nowayout, int, 0); +MODULE_PARM_DESC(nowayout, + "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +#endif + +static unsigned long bcm47xx_wdt_busy; +static char expect_release; +static struct timer_list wdt_timer; +static atomic_t ticks; + +static inline void bcm47xx_wdt_hw_start(void) +{ + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); +} + +static inline int bcm47xx_wdt_hw_stop(void) +{ + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); +} + +static void bcm47xx_timer_tick(unsigned long unused) +{ + if(!atomic_dec_and_test(&ticks)) { + bcm47xx_wdt_hw_start(); + mod_timer(&wdt_timer, jiffies + HZ); + } + else { + printk(KERN_CRIT DRV_NAME "Watchdog will fire soon!!!\n"); + } +} + +static inline void bcm47xx_wdt_pet(void) +{ + atomic_set(&ticks, wdt_time); +} + +static void bcm47xx_wdt_start(void) +{ + bcm47xx_wdt_pet(); + bcm47xx_timer_tick(0); +} + +static void bcm47xx_wdt_pause(void) +{ + del_timer(&wdt_timer); + bcm47xx_wdt_hw_stop(); +} + +static void bcm47xx_wdt_stop(void) +{ + bcm47xx_wdt_pause(); +} + +static int bcm47xx_wdt_settimeout(int new_time) +{ + if ((new_time <= 0) || (new_time > WDT_MAX_TIME)) + return -EINVAL; + + wdt_time = new_time; + return 0; +} + +static int bcm47xx_wdt_open(struct inode *inode, struct file *file) +{ + if (test_and_set_bit(0, &bcm47xx_wdt_busy)) + return -EBUSY; + + bcm47xx_wdt_start(); + return nonseekable_open(inode, file); +} + +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) +{ + if (expect_release == 42) { + bcm47xx_wdt_stop(); + } else { + printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping watchdog!\n"); + bcm47xx_wdt_start(); + } + + clear_bit(0, &bcm47xx_wdt_busy); + expect_release = 0; + return 0; +} + +static ssize_t bcm47xx_wdt_write (struct file *file, const char __user *data, + size_t len, loff_t * ppos) +{ + if (len) { + if (!nowayout) { + size_t i; + + expect_release = 0; + + for (i = 0; i != len; i++) { + char c; + if (get_user(c, data + i)) + return -EFAULT; + if (c == 'V') + expect_release = 42; + } + } + bcm47xx_wdt_pet(); + } + return len; +} + +static struct watchdog_info bcm47xx_wdt_info = { + .identity = DRV_NAME, + .options = WDIOF_SETTIMEOUT | + WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE, +}; + +static long bcm47xx_wdt_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + void __user *argp = (void __user *)arg; + int __user *p = argp; + int new_value, retval = -EINVAL;; + + switch (cmd) { + case WDIOC_GETSUPPORT: + return copy_to_user(argp, &bcm47xx_wdt_info, + sizeof(bcm47xx_wdt_info)) ? -EFAULT : 0; + + case WDIOC_GETSTATUS: + case WDIOC_GETBOOTSTATUS: + return put_user(0, p); + + case WDIOC_SETOPTIONS: + if (get_user(new_value, p)) + return -EFAULT; + + if (new_value & WDIOS_DISABLECARD) { + bcm47xx_wdt_stop(); + retval = 0; + } + + if (new_value & WDIOS_ENABLECARD) { + bcm47xx_wdt_start(); + retval = 0; + } + + return retval; + + case WDIOC_KEEPALIVE: + bcm47xx_wdt_pet(); + return 0; + + case WDIOC_SETTIMEOUT: + if (get_user(new_value, p)) + return -EFAULT; + + if (bcm47xx_wdt_settimeout(new_value)) + return -EINVAL; + + bcm47xx_wdt_pet(); + + case WDIOC_GETTIMEOUT: + return put_user(wdt_time, p); + + default: + return -ENOTTY; + } +} + +static int bcm47xx_wdt_notify_sys(struct notifier_block *this, unsigned long code, + void *unused) +{ + if (code == SYS_DOWN || code == SYS_HALT) + bcm47xx_wdt_stop(); + return NOTIFY_DONE; +} + +static const struct file_operations bcm47xx_wdt_fops = { + .owner = THIS_MODULE, + .llseek = no_llseek, + .unlocked_ioctl = bcm47xx_wdt_ioctl, + .open = bcm47xx_wdt_open, + .release = bcm47xx_wdt_release, + .write = bcm47xx_wdt_write, +}; + +static struct miscdevice bcm47xx_wdt_miscdev = { + .minor = WATCHDOG_MINOR, + .name = "watchdog", + .fops = &bcm47xx_wdt_fops, +}; + +static struct notifier_block bcm47xx_wdt_notifier = { + .notifier_call = bcm47xx_wdt_notify_sys, +}; + +static int __init bcm47xx_wdt_init(void) +{ + int ret; + + if (bcm47xx_wdt_hw_stop() < 0) + return -ENODEV; + + setup_timer(&wdt_timer, bcm47xx_timer_tick, 0L); + + if (bcm47xx_wdt_settimeout(wdt_time)) { + bcm47xx_wdt_settimeout(WDT_DEFAULT_TIME); + printk(KERN_INFO DRV_NAME ": wdt_time value must be 1 <= wdt_time <= 256, using %d\n", + wdt_time); + } + + ret = register_reboot_notifier(&bcm47xx_wdt_notifier); + if (ret) + return ret; + + ret = misc_register(&bcm47xx_wdt_miscdev); + if (ret) { + unregister_reboot_notifier(&bcm47xx_wdt_notifier); + return ret; + } + + printk(KERN_INFO "BCM47xx Watchdog Timer enabled (%d seconds%s)\n", + wdt_time, nowayout ? ", nowayout" : ""); + return 0; +} + +static void __exit bcm47xx_wdt_exit(void) +{ + if (!nowayout) + bcm47xx_wdt_stop(); + + misc_deregister(&bcm47xx_wdt_miscdev); + + unregister_reboot_notifier(&bcm47xx_wdt_notifier); +} + +module_init(bcm47xx_wdt_init); +module_exit(bcm47xx_wdt_exit); + +MODULE_AUTHOR("Aleksandar Radovanovic"); +MODULE_DESCRIPTION("Watchdog driver for Broadcom BCM47xx"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-04 20:24 add bcm47xx watchdog driver matthieu castet 2009-06-05 13:58 ` Florian Fainelli @ 2009-06-05 19:48 ` Andrew Morton 2009-06-05 20:30 ` matthieu castet 2009-06-05 19:50 ` Andrew Morton 2 siblings, 1 reply; 13+ messages in thread From: Andrew Morton @ 2009-06-05 19:48 UTC (permalink / raw) To: matthieu castet; +Cc: wim, linux-kernel, linux-mips, biblbroks On Thu, 04 Jun 2009 22:24:56 +0200 matthieu castet <castet.matthieu@free.fr> wrote: > > > This add watchdog driver for broadcom 47xx device. > It uses the ssb subsytem to access embeded watchdog device. > > Because the watchdog timeout is very short (about 2s), a soft timer is used > to increase the watchdog period. > > Note : A patch for exporting the ssb_watchdog_timer_set will > be submitted on next linux-mips merge. Without this patch it can't > be build as a module. > > > ... > > --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 > +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 > @@ -764,6 +764,12 @@ > help > Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. > > +config BCM47XX_WDT > + tristate "Broadcom BCM47xx Watchdog Timer" > + depends on BCM47XX > + help > + Hardware driver for the Broadcom BCM47xx Watchog Timer. > + Please indent the kconfig body with a single tab character. > > ... > > +#define DRV_NAME "bcm47xx_wdt" > + > +#define WDT_DEFAULT_TIME 30 /* seconds */ > +#define WDT_MAX_TIME 256 /* seconds */ > + > +static int wdt_time = WDT_DEFAULT_TIME; > +static int nowayout = WATCHDOG_NOWAYOUT; > + > +module_param(wdt_time, int, 0); > +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" > + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); > + > +#ifdef CONFIG_WATCHDOG_NOWAYOUT > +module_param(nowayout, int, 0); > +MODULE_PARM_DESC(nowayout, > + "Watchdog cannot be stopped once started (default=" > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > +#endif hm, now what's happening with the third arg to module_param()? > +static struct platform_device *bcm47xx_wdt_platform_device; > + > +static unsigned long bcm47xx_wdt_busy; > +static char expect_release; > +static struct timer_list wdt_timer; > +static atomic_t ticks; > + > +static inline void bcm47xx_wdt_hw_start(void) > +{ > + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ > + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); > +} > + > +static inline int bcm47xx_wdt_hw_stop(void) > +{ > + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); > +} > + > +static void bcm47xx_timer_tick(unsigned long unused) > +{ > + if(!atomic_dec_and_test(&ticks)) { Please pass this patch (and all others) through scripts/checkpatch.pl and review the resulting output. > + bcm47xx_wdt_hw_start(); > + mod_timer(&wdt_timer, jiffies + HZ); > + } > + else { > + printk(KERN_CRIT PFX "Watchdog will fire soon!!!.\n"); > + } > +} > + > +static inline void bcm47xx_wdt_pet(void) > +{ > + atomic_set(&ticks, wdt_time); > +} What does "pet" stand for? > +static void bcm47xx_wdt_start(void) > +{ > + bcm47xx_wdt_pet(); > + bcm47xx_timer_tick(0); > +} > + > +static void bcm47xx_wdt_pause(void) > +{ > + del_timer(&wdt_timer); Should this be del_timer_sync()? The timer callback can still be executing after del_timer() returns. > + bcm47xx_wdt_hw_stop(); > +} > + > +static void bcm47xx_wdt_stop(void) > +{ > + bcm47xx_wdt_pause(); > +} > + > +static int bcm47xx_wdt_settimeout(int new_time) > +{ > + if ((new_time <= 0) || (new_time > WDT_MAX_TIME)) > + return -EINVAL; > + > + wdt_time = new_time; > + return 0; > +} > + > +static int bcm47xx_wdt_open(struct inode *inode, struct file *file) > +{ > + if (test_and_set_bit(0, &bcm47xx_wdt_busy)) > + return -EBUSY; > + > + bcm47xx_wdt_start(); > + return nonseekable_open(inode, file); > +} > + > +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) > +{ > + if (expect_release == 42) { > + bcm47xx_wdt_stop(); > + } else { > + printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping watchdog!\n"); Can this happen? > + bcm47xx_wdt_start(); > + } > + > + clear_bit(0, &bcm47xx_wdt_busy); > + expect_release = 0; > + return 0; > +} > + > > ... > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-05 19:48 ` add bcm47xx watchdog driver Andrew Morton @ 2009-06-05 20:30 ` matthieu castet 2009-06-08 14:15 ` Florian Fainelli 0 siblings, 1 reply; 13+ messages in thread From: matthieu castet @ 2009-06-05 20:30 UTC (permalink / raw) To: Andrew Morton; +Cc: wim, linux-kernel, linux-mips, biblbroks [-- Attachment #1: Type: text/plain, Size: 4513 bytes --] Andrew Morton wrote: > On Thu, 04 Jun 2009 22:24:56 +0200 > matthieu castet <castet.matthieu@free.fr> wrote: > >> >> This add watchdog driver for broadcom 47xx device. >> It uses the ssb subsytem to access embeded watchdog device. >> >> Because the watchdog timeout is very short (about 2s), a soft timer is used >> to increase the watchdog period. >> >> Note : A patch for exporting the ssb_watchdog_timer_set will >> be submitted on next linux-mips merge. Without this patch it can't >> be build as a module. >> >> >> ... >> >> --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 >> +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 >> @@ -764,6 +764,12 @@ >> help >> Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. >> >> +config BCM47XX_WDT >> + tristate "Broadcom BCM47xx Watchdog Timer" >> + depends on BCM47XX >> + help >> + Hardware driver for the Broadcom BCM47xx Watchog Timer. >> + > > Please indent the kconfig body with a single tab character. > Done >> ... >> >> +#define DRV_NAME "bcm47xx_wdt" >> + >> +#define WDT_DEFAULT_TIME 30 /* seconds */ >> +#define WDT_MAX_TIME 256 /* seconds */ >> + >> +static int wdt_time = WDT_DEFAULT_TIME; >> +static int nowayout = WATCHDOG_NOWAYOUT; >> + >> +module_param(wdt_time, int, 0); >> +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" >> + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); >> + >> +#ifdef CONFIG_WATCHDOG_NOWAYOUT >> +module_param(nowayout, int, 0); >> +MODULE_PARM_DESC(nowayout, >> + "Watchdog cannot be stopped once started (default=" >> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); >> +#endif > > hm, now what's happening with the third arg to module_param()? I don't understand what you mean. This thing is common in watchdog drivers. For example drivers/watchdog/at91rm9200_wdt.c does the same thing. > >> +static struct platform_device *bcm47xx_wdt_platform_device; >> + >> +static unsigned long bcm47xx_wdt_busy; >> +static char expect_release; >> +static struct timer_list wdt_timer; >> +static atomic_t ticks; >> + >> +static inline void bcm47xx_wdt_hw_start(void) >> +{ >> + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ >> + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); >> +} >> + >> +static inline int bcm47xx_wdt_hw_stop(void) >> +{ >> + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); >> +} >> + >> +static void bcm47xx_timer_tick(unsigned long unused) >> +{ >> + if(!atomic_dec_and_test(&ticks)) { > > Please pass this patch (and all others) through scripts/checkpatch.pl > and review the resulting output. Done, everything is ok, expect a printk line over 80 characters. > >> + bcm47xx_wdt_hw_start(); >> + mod_timer(&wdt_timer, jiffies + HZ); >> + } >> + else { >> + printk(KERN_CRIT PFX "Watchdog will fire soon!!!.\n"); >> + } >> +} >> + >> +static inline void bcm47xx_wdt_pet(void) >> +{ >> + atomic_set(&ticks, wdt_time); >> +} > > What does "pet" stand for? > A watchdog timer is a computer hardware timing device that triggers a system reset if the main program, due to some fault condition, such as a hang, neglects to regularly service the watchdog (writing a “service pulse” to it, also referred to as “petting the dog” [1] [1] http://en.wikipedia.org/wiki/Watchdog_timer But I can change the name if you want. Note that pet appear in drivers/watchdog/sb_wdog.c and drivers/watchdog/sbc_epx_c3.c >> +static void bcm47xx_wdt_start(void) >> +{ >> + bcm47xx_wdt_pet(); >> + bcm47xx_timer_tick(0); >> +} >> + >> +static void bcm47xx_wdt_pause(void) >> +{ >> + del_timer(&wdt_timer); > > Should this be del_timer_sync()? The timer callback can still be > executing after del_timer() returns. Yes, changed to del_timer_sync() >> +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) >> +{ >> + if (expect_release == 42) { >> + bcm47xx_wdt_stop(); >> + } else { >> + printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping watchdog!\n"); > > Can this happen? yes : this is a common pattern in watchdog driver (check for example softdog) : - expect_release is in bss (set to 0) - we set expect_release to this magic value, only if we get a write with a special character and we are not in nowayout. - So for example doing a "cat /dev/watchdog" should go in this path. > >> + bcm47xx_wdt_start(); >> + } >> + >> + clear_bit(0, &bcm47xx_wdt_busy); >> + expect_release = 0; >> + return 0; >> +} >> + Thanks for the review. I attach a new version. [-- Attachment #2: bcm47xx_watchdog_v3.diff --] [-- Type: text/x-diff, Size: 8352 bytes --] This add watchdog driver for broadcom 47xx device. It uses the ssb subsytem to access embeded watchdog device. Because the watchdog timeout is very short (about 2s), a soft timer is used to increase the watchdog period. Note : A patch for exporting the ssb_watchdog_timer_set will be submitted on next linux-mips merge. Without this patch it can't be build as a module. Signed-off-by: Aleksandar Radovanovic <biblbroks@sezampro.rs> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr> Index: linux-2.6/drivers/watchdog/Kconfig =================================================================== --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 +++ linux-2.6/drivers/watchdog/Kconfig 2009-06-05 22:05:19.000000000 +0200 @@ -764,6 +764,12 @@ help Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. +config BCM47XX_WDT + tristate "Broadcom BCM47xx Watchdog Timer" + depends on BCM47XX + help + Hardware driver for the Broadcom BCM47xx Watchog Timer. + # PARISC Architecture # POWERPC Architecture Index: linux-2.6/drivers/watchdog/Makefile =================================================================== --- linux-2.6.orig/drivers/watchdog/Makefile 2009-05-25 22:22:02.000000000 +0200 +++ linux-2.6/drivers/watchdog/Makefile 2009-05-25 23:02:27.000000000 +0200 @@ -105,6 +105,7 @@ obj-$(CONFIG_SIBYTE_WDOG) += sb_wdog.o obj-$(CONFIG_AR7_WDT) += ar7_wdt.o obj-$(CONFIG_TXX9_WDT) += txx9wdt.o +obj-$(CONFIG_BCM47XX_WDT) += bcm47xx_wdt.o # PARISC Architecture Index: linux-2.6/drivers/watchdog/bcm47xx_wdt.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ linux-2.6/drivers/watchdog/bcm47xx_wdt.c 2009-06-05 22:25:37.000000000 +0200 @@ -0,0 +1,286 @@ +/* + * Watchdog driver for Broadcom BCM47XX + * + * Copyright (C) 2008 Aleksandar Radovanovic <biblbroks@sezampro.rs> + * Copyright (C) 2009 Matthieu CASTET <castet.matthieu@free.fr> + * + * 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/bitops.h> +#include <linux/errno.h> +#include <linux/fs.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/reboot.h> +#include <linux/types.h> +#include <linux/uaccess.h> +#include <linux/watchdog.h> +#include <linux/timer.h> +#include <linux/jiffies.h> +#include <linux/ssb/ssb_embedded.h> +#include <asm/mach-bcm47xx/bcm47xx.h> + +#define DRV_NAME "bcm47xx_wdt" + +#define WDT_DEFAULT_TIME 30 /* seconds */ +#define WDT_MAX_TIME 256 /* seconds */ + +static int wdt_time = WDT_DEFAULT_TIME; +static int nowayout = WATCHDOG_NOWAYOUT; + +module_param(wdt_time, int, 0); +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); + +#ifdef CONFIG_WATCHDOG_NOWAYOUT +module_param(nowayout, int, 0); +MODULE_PARM_DESC(nowayout, + "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +#endif + +static unsigned long bcm47xx_wdt_busy; +static char expect_release; +static struct timer_list wdt_timer; +static atomic_t ticks; + +static inline void bcm47xx_wdt_hw_start(void) +{ + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); +} + +static inline int bcm47xx_wdt_hw_stop(void) +{ + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); +} + +static void bcm47xx_timer_tick(unsigned long unused) +{ + if (!atomic_dec_and_test(&ticks)) { + bcm47xx_wdt_hw_start(); + mod_timer(&wdt_timer, jiffies + HZ); + } else { + printk(KERN_CRIT DRV_NAME "Watchdog will fire soon!!!\n"); + } +} + +static inline void bcm47xx_wdt_pet(void) +{ + atomic_set(&ticks, wdt_time); +} + +static void bcm47xx_wdt_start(void) +{ + bcm47xx_wdt_pet(); + bcm47xx_timer_tick(0); +} + +static void bcm47xx_wdt_pause(void) +{ + del_timer_sync(&wdt_timer); + bcm47xx_wdt_hw_stop(); +} + +static void bcm47xx_wdt_stop(void) +{ + bcm47xx_wdt_pause(); +} + +static int bcm47xx_wdt_settimeout(int new_time) +{ + if ((new_time <= 0) || (new_time > WDT_MAX_TIME)) + return -EINVAL; + + wdt_time = new_time; + return 0; +} + +static int bcm47xx_wdt_open(struct inode *inode, struct file *file) +{ + if (test_and_set_bit(0, &bcm47xx_wdt_busy)) + return -EBUSY; + + bcm47xx_wdt_start(); + return nonseekable_open(inode, file); +} + +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) +{ + if (expect_release == 42) { + bcm47xx_wdt_stop(); + } else { + printk(KERN_CRIT DRV_NAME + ": Unexpected close, not stopping watchdog!\n"); + bcm47xx_wdt_start(); + } + + clear_bit(0, &bcm47xx_wdt_busy); + expect_release = 0; + return 0; +} + +static ssize_t bcm47xx_wdt_write(struct file *file, const char __user *data, + size_t len, loff_t *ppos) +{ + if (len) { + if (!nowayout) { + size_t i; + + expect_release = 0; + + for (i = 0; i != len; i++) { + char c; + if (get_user(c, data + i)) + return -EFAULT; + if (c == 'V') + expect_release = 42; + } + } + bcm47xx_wdt_pet(); + } + return len; +} + +static struct watchdog_info bcm47xx_wdt_info = { + .identity = DRV_NAME, + .options = WDIOF_SETTIMEOUT | + WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE, +}; + +static long bcm47xx_wdt_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + void __user *argp = (void __user *)arg; + int __user *p = argp; + int new_value, retval = -EINVAL;; + + switch (cmd) { + case WDIOC_GETSUPPORT: + return copy_to_user(argp, &bcm47xx_wdt_info, + sizeof(bcm47xx_wdt_info)) ? -EFAULT : 0; + + case WDIOC_GETSTATUS: + case WDIOC_GETBOOTSTATUS: + return put_user(0, p); + + case WDIOC_SETOPTIONS: + if (get_user(new_value, p)) + return -EFAULT; + + if (new_value & WDIOS_DISABLECARD) { + bcm47xx_wdt_stop(); + retval = 0; + } + + if (new_value & WDIOS_ENABLECARD) { + bcm47xx_wdt_start(); + retval = 0; + } + + return retval; + + case WDIOC_KEEPALIVE: + bcm47xx_wdt_pet(); + return 0; + + case WDIOC_SETTIMEOUT: + if (get_user(new_value, p)) + return -EFAULT; + + if (bcm47xx_wdt_settimeout(new_value)) + return -EINVAL; + + bcm47xx_wdt_pet(); + + case WDIOC_GETTIMEOUT: + return put_user(wdt_time, p); + + default: + return -ENOTTY; + } +} + +static int bcm47xx_wdt_notify_sys(struct notifier_block *this, + unsigned long code, void *unused) +{ + if (code == SYS_DOWN || code == SYS_HALT) + bcm47xx_wdt_stop(); + return NOTIFY_DONE; +} + +static const struct file_operations bcm47xx_wdt_fops = { + .owner = THIS_MODULE, + .llseek = no_llseek, + .unlocked_ioctl = bcm47xx_wdt_ioctl, + .open = bcm47xx_wdt_open, + .release = bcm47xx_wdt_release, + .write = bcm47xx_wdt_write, +}; + +static struct miscdevice bcm47xx_wdt_miscdev = { + .minor = WATCHDOG_MINOR, + .name = "watchdog", + .fops = &bcm47xx_wdt_fops, +}; + +static struct notifier_block bcm47xx_wdt_notifier = { + .notifier_call = bcm47xx_wdt_notify_sys, +}; + +static int __init bcm47xx_wdt_init(void) +{ + int ret; + + if (bcm47xx_wdt_hw_stop() < 0) + return -ENODEV; + + setup_timer(&wdt_timer, bcm47xx_timer_tick, 0L); + + if (bcm47xx_wdt_settimeout(wdt_time)) { + bcm47xx_wdt_settimeout(WDT_DEFAULT_TIME); + printk(KERN_INFO DRV_NAME + ": wdt_time value must be 1 <= wdt_time <= 256, using %d\n", + wdt_time); + } + + ret = register_reboot_notifier(&bcm47xx_wdt_notifier); + if (ret) + return ret; + + ret = misc_register(&bcm47xx_wdt_miscdev); + if (ret) { + unregister_reboot_notifier(&bcm47xx_wdt_notifier); + return ret; + } + + printk(KERN_INFO "BCM47xx Watchdog Timer enabled (%d seconds%s)\n", + wdt_time, nowayout ? ", nowayout" : ""); + return 0; +} + +static void __exit bcm47xx_wdt_exit(void) +{ + if (!nowayout) + bcm47xx_wdt_stop(); + + misc_deregister(&bcm47xx_wdt_miscdev); + + unregister_reboot_notifier(&bcm47xx_wdt_notifier); +} + +module_init(bcm47xx_wdt_init); +module_exit(bcm47xx_wdt_exit); + +MODULE_AUTHOR("Aleksandar Radovanovic"); +MODULE_DESCRIPTION("Watchdog driver for Broadcom BCM47xx"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-05 20:30 ` matthieu castet @ 2009-06-08 14:15 ` Florian Fainelli 2009-06-10 17:17 ` Wim Van Sebroeck 0 siblings, 1 reply; 13+ messages in thread From: Florian Fainelli @ 2009-06-08 14:15 UTC (permalink / raw) To: matthieu castet; +Cc: Andrew Morton, wim, linux-kernel, linux-mips, biblbroks Le Friday 05 June 2009 22:30:21 matthieu castet, vous avez écrit : > Andrew Morton wrote: > > On Thu, 04 Jun 2009 22:24:56 +0200 > > > > matthieu castet <castet.matthieu@free.fr> wrote: > >> This add watchdog driver for broadcom 47xx device. > >> It uses the ssb subsytem to access embeded watchdog device. > >> > >> Because the watchdog timeout is very short (about 2s), a soft timer is > >> used to increase the watchdog period. > >> > >> Note : A patch for exporting the ssb_watchdog_timer_set will > >> be submitted on next linux-mips merge. Without this patch it can't > >> be build as a module. It works very well on my Netgear WGT634U, thanks ! Tested-by: Florian Fainelli <florian@openwrt.org> > >> > >> > >> ... > >> > >> --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 > >> 22:22:02.000000000 +0200 +++ > >> linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 > >> @@ -764,6 +764,12 @@ > >> help > >> Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. > >> > >> +config BCM47XX_WDT > >> + tristate "Broadcom BCM47xx Watchdog Timer" > >> + depends on BCM47XX > >> + help > >> + Hardware driver for the Broadcom BCM47xx Watchog Timer. > >> + > > > > Please indent the kconfig body with a single tab character. > > Done > > >> ... > >> > >> +#define DRV_NAME "bcm47xx_wdt" > >> + > >> +#define WDT_DEFAULT_TIME 30 /* seconds */ > >> +#define WDT_MAX_TIME 256 /* seconds */ > >> + > >> +static int wdt_time = WDT_DEFAULT_TIME; > >> +static int nowayout = WATCHDOG_NOWAYOUT; > >> + > >> +module_param(wdt_time, int, 0); > >> +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" > >> + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); > >> + > >> +#ifdef CONFIG_WATCHDOG_NOWAYOUT > >> +module_param(nowayout, int, 0); > >> +MODULE_PARM_DESC(nowayout, > >> + "Watchdog cannot be stopped once started (default=" > >> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > >> +#endif > > > > hm, now what's happening with the third arg to module_param()? > > I don't understand what you mean. > This thing is common in watchdog drivers. For example > drivers/watchdog/at91rm9200_wdt.c does the same thing. > > >> +static struct platform_device *bcm47xx_wdt_platform_device; > >> + > >> +static unsigned long bcm47xx_wdt_busy; > >> +static char expect_release; > >> +static struct timer_list wdt_timer; > >> +static atomic_t ticks; > >> + > >> +static inline void bcm47xx_wdt_hw_start(void) > >> +{ > >> + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ > >> + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); > >> +} > >> + > >> +static inline int bcm47xx_wdt_hw_stop(void) > >> +{ > >> + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); > >> +} > >> + > >> +static void bcm47xx_timer_tick(unsigned long unused) > >> +{ > >> + if(!atomic_dec_and_test(&ticks)) { > > > > Please pass this patch (and all others) through scripts/checkpatch.pl > > and review the resulting output. > > Done, everything is ok, expect a printk line over 80 characters. > > >> + bcm47xx_wdt_hw_start(); > >> + mod_timer(&wdt_timer, jiffies + HZ); > >> + } > >> + else { > >> + printk(KERN_CRIT PFX "Watchdog will fire soon!!!.\n"); > >> + } > >> +} > >> + > >> +static inline void bcm47xx_wdt_pet(void) > >> +{ > >> + atomic_set(&ticks, wdt_time); > >> +} > > > > What does "pet" stand for? > > A watchdog timer is a computer hardware timing device that triggers a > system reset if the main program, due to some fault condition, such as a > hang, neglects to regularly service the watchdog (writing a “service > pulse” to it, also referred to as “petting the dog” [1] > > [1] http://en.wikipedia.org/wiki/Watchdog_timer > > But I can change the name if you want. Note that pet appear in > drivers/watchdog/sb_wdog.c and drivers/watchdog/sbc_epx_c3.c > > >> +static void bcm47xx_wdt_start(void) > >> +{ > >> + bcm47xx_wdt_pet(); > >> + bcm47xx_timer_tick(0); > >> +} > >> + > >> +static void bcm47xx_wdt_pause(void) > >> +{ > >> + del_timer(&wdt_timer); > > > > Should this be del_timer_sync()? The timer callback can still be > > executing after del_timer() returns. > > Yes, changed to del_timer_sync() > > >> +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) > >> +{ > >> + if (expect_release == 42) { > >> + bcm47xx_wdt_stop(); > >> + } else { > >> + printk(KERN_CRIT DRV_NAME ": Unexpected close, not stopping > >> watchdog!\n"); > > > > Can this happen? > > yes : this is a common pattern in watchdog driver (check for example > softdog) : > - expect_release is in bss (set to 0) > - we set expect_release to this magic value, only if we get a write with > a special character and we are not in nowayout. > - So for example doing a "cat /dev/watchdog" should go in this path. > > >> + bcm47xx_wdt_start(); > >> + } > >> + > >> + clear_bit(0, &bcm47xx_wdt_busy); > >> + expect_release = 0; > >> + return 0; > >> +} > >> + > > Thanks for the review. > > I attach a new version. -- Best regards, Florian Fainelli Email : florian@openwrt.org http://openwrt.org ------------------------------- ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-08 14:15 ` Florian Fainelli @ 2009-06-10 17:17 ` Wim Van Sebroeck 2009-06-10 18:47 ` matthieu castet 0 siblings, 1 reply; 13+ messages in thread From: Wim Van Sebroeck @ 2009-06-10 17:17 UTC (permalink / raw) To: Florian Fainelli Cc: matthieu castet, Andrew Morton, linux-kernel, linux-mips, biblbroks Hi Matthieu, Florian, > It works very well on my Netgear WGT634U, thanks ! > > Tested-by: Florian Fainelli <florian@openwrt.org> I incorporated the changes of Andrew and did a clean-up here and there to keep checkpatch happy. Could you test the attached diff to see that it still works. Thanks, Wim. --- diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 443e7e2..8d2fe51 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -729,6 +729,12 @@ config SBC_EPX_C3_WATCHDOG # MIPS Architecture +config BCM47XX_WDT + tristate "Broadcom BCM47xx Watchdog Timer" + depends on BCM47XX + help + Hardware driver for the Broadcom BCM47xx Watchog Timer. + config RC32434_WDT tristate "IDT RC32434 SoC Watchdog Timer" depends on MIKROTIK_RB532 diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index ee8c84c..83ed987 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o # M68KNOMMU Architecture # MIPS Architecture +obj-$(CONFIG_BCM47XX_WDT) += bcm47xx_wdt.o obj-$(CONFIG_RC32434_WDT) += rc32434_wdt.o obj-$(CONFIG_INDYDOG) += indydog.o obj-$(CONFIG_WDT_MTX1) += mtx-1_wdt.o diff --git a/drivers/watchdog/bcm47xx_wdt.c b/drivers/watchdog/bcm47xx_wdt.c new file 100644 --- /dev/null +++ b/drivers/watchdog/bcm47xx_wdt.c @@ -0,0 +1,286 @@ +/* + * Watchdog driver for Broadcom BCM47XX + * + * Copyright (C) 2008 Aleksandar Radovanovic <biblbroks@sezampro.rs> + * Copyright (C) 2009 Matthieu CASTET <castet.matthieu@free.fr> + * + * 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/bitops.h> +#include <linux/errno.h> +#include <linux/fs.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/reboot.h> +#include <linux/types.h> +#include <linux/uaccess.h> +#include <linux/watchdog.h> +#include <linux/timer.h> +#include <linux/jiffies.h> +#include <linux/ssb/ssb_embedded.h> +#include <asm/mach-bcm47xx/bcm47xx.h> + +#define DRV_NAME "bcm47xx_wdt" + +#define WDT_DEFAULT_TIME 30 /* seconds */ +#define WDT_MAX_TIME 255 /* seconds */ + +static int wdt_time = WDT_DEFAULT_TIME; +static int nowayout = WATCHDOG_NOWAYOUT; + +module_param(wdt_time, int, 0); +MODULE_PARM_DESC(wdt_time, "Watchdog time in seconds. (default=" + __MODULE_STRING(WDT_DEFAULT_TIME) ")"); + +#ifdef CONFIG_WATCHDOG_NOWAYOUT +module_param(nowayout, int, 0); +MODULE_PARM_DESC(nowayout, + "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +#endif + +static unsigned long bcm47xx_wdt_busy; +static char expect_release; +static struct timer_list wdt_timer; +static atomic_t ticks; + +static inline void bcm47xx_wdt_hw_start(void) +{ + /* this is 2,5s on 100Mhz clock and 2s on 133 Mhz */ + ssb_watchdog_timer_set(&ssb_bcm47xx, 0xfffffff); +} + +static inline int bcm47xx_wdt_hw_stop(void) +{ + return ssb_watchdog_timer_set(&ssb_bcm47xx, 0); +} + +static void bcm47xx_timer_tick(unsigned long unused) +{ + if (!atomic_dec_and_test(&ticks)) { + bcm47xx_wdt_hw_start(); + mod_timer(&wdt_timer, jiffies + HZ); + } else { + printk(KERN_CRIT DRV_NAME "Watchdog will fire soon!!!\n"); + } +} + +static inline void bcm47xx_wdt_pet(void) +{ + atomic_set(&ticks, wdt_time); +} + +static void bcm47xx_wdt_start(void) +{ + bcm47xx_wdt_pet(); + bcm47xx_timer_tick(0); +} + +static void bcm47xx_wdt_pause(void) +{ + del_timer_sync(&wdt_timer); + bcm47xx_wdt_hw_stop(); +} + +static void bcm47xx_wdt_stop(void) +{ + bcm47xx_wdt_pause(); +} + +static int bcm47xx_wdt_settimeout(int new_time) +{ + if ((new_time <= 0) || (new_time > WDT_MAX_TIME)) + return -EINVAL; + + wdt_time = new_time; + return 0; +} + +static int bcm47xx_wdt_open(struct inode *inode, struct file *file) +{ + if (test_and_set_bit(0, &bcm47xx_wdt_busy)) + return -EBUSY; + + bcm47xx_wdt_start(); + return nonseekable_open(inode, file); +} + +static int bcm47xx_wdt_release(struct inode *inode, struct file *file) +{ + if (expect_release == 42) { + bcm47xx_wdt_stop(); + } else { + printk(KERN_CRIT DRV_NAME + ": Unexpected close, not stopping watchdog!\n"); + bcm47xx_wdt_start(); + } + + clear_bit(0, &bcm47xx_wdt_busy); + expect_release = 0; + return 0; +} + +static ssize_t bcm47xx_wdt_write(struct file *file, const char __user *data, + size_t len, loff_t *ppos) +{ + if (len) { + if (!nowayout) { + size_t i; + + expect_release = 0; + + for (i = 0; i != len; i++) { + char c; + if (get_user(c, data + i)) + return -EFAULT; + if (c == 'V') + expect_release = 42; + } + } + bcm47xx_wdt_pet(); + } + return len; +} + +static struct watchdog_info bcm47xx_wdt_info = { + .identity = DRV_NAME, + .options = WDIOF_SETTIMEOUT | + WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE, +}; + +static long bcm47xx_wdt_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + void __user *argp = (void __user *)arg; + int __user *p = argp; + int new_value, retval = -EINVAL;; + + switch (cmd) { + case WDIOC_GETSUPPORT: + return copy_to_user(argp, &bcm47xx_wdt_info, + sizeof(bcm47xx_wdt_info)) ? -EFAULT : 0; + + case WDIOC_GETSTATUS: + case WDIOC_GETBOOTSTATUS: + return put_user(0, p); + + case WDIOC_SETOPTIONS: + if (get_user(new_value, p)) + return -EFAULT; + + if (new_value & WDIOS_DISABLECARD) { + bcm47xx_wdt_stop(); + retval = 0; + } + + if (new_value & WDIOS_ENABLECARD) { + bcm47xx_wdt_start(); + retval = 0; + } + + return retval; + + case WDIOC_KEEPALIVE: + bcm47xx_wdt_pet(); + return 0; + + case WDIOC_SETTIMEOUT: + if (get_user(new_value, p)) + return -EFAULT; + + if (bcm47xx_wdt_settimeout(new_value)) + return -EINVAL; + + bcm47xx_wdt_pet(); + + case WDIOC_GETTIMEOUT: + return put_user(wdt_time, p); + + default: + return -ENOTTY; + } +} + +static int bcm47xx_wdt_notify_sys(struct notifier_block *this, + nsigned long code, void *unused) +{ + if (code == SYS_DOWN || code == SYS_HALT) + bcm47xx_wdt_stop(); + return NOTIFY_DONE; +} + +static const struct file_operations bcm47xx_wdt_fops = { + .owner = THIS_MODULE, + .llseek = no_llseek, + .unlocked_ioctl = bcm47xx_wdt_ioctl, + .open = bcm47xx_wdt_open, + .release = bcm47xx_wdt_release, + .write = bcm47xx_wdt_write, +}; + +static struct miscdevice bcm47xx_wdt_miscdev = { + .minor = WATCHDOG_MINOR, + .name = "watchdog", + .fops = &bcm47xx_wdt_fops, +}; + +static struct notifier_block bcm47xx_wdt_notifier = { + .notifier_call = bcm47xx_wdt_notify_sys, +}; + +static int __init bcm47xx_wdt_init(void) +{ + int ret; + + if (bcm47xx_wdt_hw_stop() < 0) + return -ENODEV; + + setup_timer(&wdt_timer, bcm47xx_timer_tick, 0L); + + if (bcm47xx_wdt_settimeout(wdt_time)) { + bcm47xx_wdt_settimeout(WDT_DEFAULT_TIME); + printk(KERN_INFO DRV_NAME ": " + "wdt_time value must be 0 < wdt_time < %d, using %d\n", + (WDT_MAX_TIME + 1), wdt_time); + } + + ret = register_reboot_notifier(&bcm47xx_wdt_notifier); + if (ret) + return ret; + + ret = misc_register(&bcm47xx_wdt_miscdev); + if (ret) { + unregister_reboot_notifier(&bcm47xx_wdt_notifier); + return ret; + } + + printk(KERN_INFO "BCM47xx Watchdog Timer enabled (%d seconds%s)\n", + wdt_time, nowayout ? ", nowayout" : ""); + return 0; +} + +static void __exit bcm47xx_wdt_exit(void) +{ + if (!nowayout) + bcm47xx_wdt_stop(); + + misc_deregister(&bcm47xx_wdt_miscdev); + + unregister_reboot_notifier(&bcm47xx_wdt_notifier); +} + +module_init(bcm47xx_wdt_init); +module_exit(bcm47xx_wdt_exit); + +MODULE_AUTHOR("Aleksandar Radovanovic"); +MODULE_DESCRIPTION("Watchdog driver for Broadcom BCM47xx"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-10 17:17 ` Wim Van Sebroeck @ 2009-06-10 18:47 ` matthieu castet 2009-06-10 19:06 ` Wim Van Sebroeck 0 siblings, 1 reply; 13+ messages in thread From: matthieu castet @ 2009-06-10 18:47 UTC (permalink / raw) To: Wim Van Sebroeck Cc: Florian Fainelli, Andrew Morton, linux-kernel, linux-mips, biblbroks Hi, Wim Van Sebroeck wrote: > Hi Matthieu, Florian, > >> It works very well on my Netgear WGT634U, thanks ! >> >> Tested-by: Florian Fainelli <florian@openwrt.org> > > I incorporated the changes of Andrew and did a clean-up here and there to keep checkpatch happy. > Could you test the attached diff to see that it still works. > > Thanks, > Wim. > > + > +#define WDT_DEFAULT_TIME 30 /* seconds */ > +#define WDT_MAX_TIME 255 /* seconds */ Why have changed this from 256 to 255 ? > +} > + > +static int bcm47xx_wdt_notify_sys(struct notifier_block *this, > + nsigned long code, void *unused) ^^^^ Does this build ? Matthieu ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-10 18:47 ` matthieu castet @ 2009-06-10 19:06 ` Wim Van Sebroeck 0 siblings, 0 replies; 13+ messages in thread From: Wim Van Sebroeck @ 2009-06-10 19:06 UTC (permalink / raw) To: matthieu castet Cc: Florian Fainelli, Andrew Morton, linux-kernel, linux-mips, biblbroks Hi Matthieu, >> + >> +#define WDT_DEFAULT_TIME 30 /* seconds */ >> +#define WDT_MAX_TIME 255 /* seconds */ > Why have changed this from 256 to 255 ? Since you use a timer to control the real watchdog, the wdt_time function is arbitraty anyway. Most watchdog drivers use 0xFFFF. Since you only went to 256 it made more sense to have it as 0xFF which is 255. We can make it 65535 also. >> +} >> + >> +static int bcm47xx_wdt_notify_sys(struct notifier_block *this, >> + nsigned long code, void *unused) > ^^^^ > Does this build ? Nope. should be unsigned. Kind regards, Wim. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-04 20:24 add bcm47xx watchdog driver matthieu castet 2009-06-05 13:58 ` Florian Fainelli 2009-06-05 19:48 ` add bcm47xx watchdog driver Andrew Morton @ 2009-06-05 19:50 ` Andrew Morton 2009-06-05 20:03 ` matthieu castet 2 siblings, 1 reply; 13+ messages in thread From: Andrew Morton @ 2009-06-05 19:50 UTC (permalink / raw) To: matthieu castet; +Cc: wim, linux-kernel, linux-mips, biblbroks On Thu, 04 Jun 2009 22:24:56 +0200 matthieu castet <castet.matthieu@free.fr> wrote: > This add watchdog driver for broadcom 47xx device. > It uses the ssb subsytem to access embeded watchdog device. > > ... > > --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 > +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 > @@ -764,6 +764,12 @@ > help > Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. > > +config BCM47XX_WDT > + tristate "Broadcom BCM47xx Watchdog Timer" > + depends on BCM47XX > + help > + Hardware driver for the Broadcom BCM47xx Watchog Timer. Shouldn't there be some SSB dependencies in Kconfig here? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: add bcm47xx watchdog driver 2009-06-05 19:50 ` Andrew Morton @ 2009-06-05 20:03 ` matthieu castet 0 siblings, 0 replies; 13+ messages in thread From: matthieu castet @ 2009-06-05 20:03 UTC (permalink / raw) To: Andrew Morton; +Cc: wim, linux-kernel, linux-mips, biblbroks Andrew Morton wrote: > On Thu, 04 Jun 2009 22:24:56 +0200 > matthieu castet <castet.matthieu@free.fr> wrote: > >> This add watchdog driver for broadcom 47xx device. >> It uses the ssb subsytem to access embeded watchdog device. >> >> ... >> >> --- linux-2.6.orig/drivers/watchdog/Kconfig 2009-05-25 22:22:02.000000000 +0200 >> +++ linux-2.6/drivers/watchdog/Kconfig 2009-05-25 22:26:06.000000000 +0200 >> @@ -764,6 +764,12 @@ >> help >> Hardware driver for the built-in watchdog timer on TXx9 MIPS SoCs. >> >> +config BCM47XX_WDT >> + tristate "Broadcom BCM47xx Watchdog Timer" >> + depends on BCM47XX >> + help >> + Hardware driver for the Broadcom BCM47xx Watchog Timer. > > Shouldn't there be some SSB dependencies in Kconfig here? No, because : config BCM47XX bool "BCM47XX based boards" [...] select SSB select SSB_DRIVER_MIPS select SSB_DRIVER_EXTIF select SSB_EMBEDDED select SSB_PCICORE_HOSTMODE if PCI ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-06-10 19:07 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-06-04 20:24 add bcm47xx watchdog driver matthieu castet 2009-06-05 13:58 ` Florian Fainelli 2009-06-05 14:58 ` castet.matthieu 2009-06-06 10:58 ` Florian Fainelli 2009-06-05 16:57 ` add bcm47xx watchdog driver v2 matthieu castet 2009-06-05 19:48 ` add bcm47xx watchdog driver Andrew Morton 2009-06-05 20:30 ` matthieu castet 2009-06-08 14:15 ` Florian Fainelli 2009-06-10 17:17 ` Wim Van Sebroeck 2009-06-10 18:47 ` matthieu castet 2009-06-10 19:06 ` Wim Van Sebroeck 2009-06-05 19:50 ` Andrew Morton 2009-06-05 20:03 ` matthieu castet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).