watchdog driver for embedded systems with a supervisor watchdog (MAX823 or so)
connected to a gpio. This is the platform_driver and needs platform_data for
defining the gpio pin and the watchdog timeout.
Signed-off-by: Marc Pignat <[email protected]>
---
Hi!
Changes form take1:
* corrected includes
* fixed default ioctl to respond -ENOTTY
* fixed CONFIG_PM
* other small changes
* simplified banner display (simpler)
This problem will probably go away when the watchdog-core will be merged
and the memory gain is very small.
* changed from __init to __devinit
More robust and flexible
Thanks for all comments!
If you have already read take1, you can safely go to the patch.
If you've got a max823, 824 or any cpu supervisor like this connected to your
cpu by a gpio pin, you can use this watchdog driver.
Simply add this to your board specific setup file:
#include <linux/gpio_wdt.h>
static struct gpio_wdt_pdata my_wdt = {
????????/* WDI input connected to this gpio */
????????.pin????????????= AT91_PIN_PD27,
????????/* The *min* timeout */
????????.timeout_ms?????= 1140
};
static struct platform_device my_watchdog_device =
{
????????.name = "gpio_wdt",
????????.id ? = 0,
????????.dev ?=?{
????????????????.platform_data = &my_wdt,
????????}
};
then
platform_device_register(&my_watchdog_device);
This patch is against 2.6.24-rc7-git7
Comments are welcome
Regards
Marc
diff -urN a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
--- a/drivers/watchdog/gpio_wdt.c 1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/watchdog/gpio_wdt.c 2008-01-14 10:40:46.000000000 +0100
@@ -0,0 +1,301 @@
+/*
+ * Generic GPIO based watchdog driver
+ *
+ * Copyright (C) 2007 Marc Pignat for http://www.hevs.ch
+ *
+ * This is a driver for the MAXIM MAX823 (and similar watchdog chips) connected
+ * on a generic gpio line.
+ *
+ * Principle of operation:
+ *
+ * When the WDI input is not driven (or in high impedance state), the
+ * watchdog is disabled. This mode of operations supposes that the gpio line
+ * can be put in high-impedance state (input) and is not driving too much
+ * current from the WDI pin.
+ *
+ * When the WDI input is driven, it must be toggled before the timeout,
+ * otherwise a reset is generated.
+ *
+ * Other supported chips:
+ * MAXIM: MAX824, ...
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/pm.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/gpio_watchdog.h>
+#include <linux/fs.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+
+#include <asm/uaccess.h>
+#include <asm/gpio.h>
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct gpio_wdt {
+ struct gpio_wdt_pdata *pdata;
+ unsigned long driver_open;
+ char expect_close;
+};
+
+static struct gpio_wdt *watchdog;
+
+#define GPIO_WDT_DRIVER_NAME "gpio_wdt"
+#define PFX GPIO_WDT_DRIVER_NAME":"
+
+static int gpio_wdt_keepalive(struct gpio_wdt *wdt)
+{
+ gpio_set_value(wdt->pdata->pin, 0);
+ gpio_set_value(wdt->pdata->pin, 1);
+ return 0;
+}
+
+static int gpio_wdt_stop(struct gpio_wdt *wdt)
+{
+ gpio_wdt_keepalive(wdt);
+ gpio_direction_input(wdt->pdata->pin);
+ return 0;
+}
+
+static int gpio_wdt_start(struct gpio_wdt *wdt)
+{
+ gpio_direction_output(wdt->pdata->pin, 0);
+ gpio_wdt_keepalive(wdt);
+ return 0;
+}
+
+static int gpio_wdt_open(struct inode *inode, struct file *file)
+{
+ if (test_and_set_bit(0, &watchdog->driver_open))
+ return -EBUSY;
+
+ gpio_wdt_start(watchdog);
+ gpio_wdt_keepalive(watchdog);
+
+ return nonseekable_open(inode, file);
+}
+
+static int gpio_wdt_release(struct inode *inode, struct file *file)
+{
+ if (watchdog->expect_close == 42) {
+ gpio_wdt_stop(watchdog);
+ module_put(THIS_MODULE);
+ } else {
+ printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
+ gpio_wdt_keepalive(watchdog);
+ }
+ clear_bit(0, &watchdog->driver_open);
+ watchdog->expect_close = 0;
+ return 0;
+}
+
+static ssize_t gpio_wdt_write(struct file *file, const char __user *data, size_t len, loff_t *ppos)
+{
+ /*
+ * Refresh the timer.
+ */
+ if (len) {
+ if (!nowayout) {
+ size_t i;
+
+ /* In case it was set long ago */
+ watchdog->expect_close = 0;
+
+ for (i = 0; i != len; i++) {
+ char c;
+
+ if (get_user(c, data + i))
+ return -EFAULT;
+ if (c == 'V')
+ watchdog->expect_close = 42;
+ }
+ }
+ gpio_wdt_keepalive(watchdog);
+ }
+ return len;
+
+ gpio_wdt_keepalive(watchdog);
+ return len;
+}
+
+static struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .firmware_version = 0,
+ .identity = GPIO_WDT_DRIVER_NAME,
+};
+
+static int gpio_wdt_ioctl(struct inode *inode, struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ u32 timeout = watchdog->pdata->timeout_ms;
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+ case WDIOC_GETSTATUS:
+ case WDIOC_GETBOOTSTATUS:
+ return put_user(0, p);
+ case WDIOC_KEEPALIVE:
+ gpio_wdt_keepalive(watchdog);
+ return 0;
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout, p);
+ case WDIOC_SETTIMEOUT:
+ return -EINVAL;
+ default:
+ return -ENOTTY;
+ }
+}
+
+static int gpio_wdt_notify_sys(struct notifier_block *this,
+ unsigned long code, void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ gpio_wdt_stop(watchdog);
+
+ return NOTIFY_DONE;
+}
+
+static struct file_operations gpio_wdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .write = gpio_wdt_write,
+ .ioctl = gpio_wdt_ioctl,
+ .open = gpio_wdt_open,
+ .release = gpio_wdt_release,
+};
+
+static struct miscdevice gpio_wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &gpio_wdt_fops,
+};
+
+static struct notifier_block gpio_wdt_notifier = {
+ .notifier_call = gpio_wdt_notify_sys,
+};
+
+static int __devinit gpio_wdt_probe(struct platform_device *pdev)
+{
+ struct gpio_wdt_pdata *pdata = (struct gpio_wdt_pdata *)pdev->dev.platform_data;
+ int ret;
+
+ if (watchdog) {
+ printk(KERN_ERR PFX "only one device supported\n");
+ return -ENODEV;
+ }
+
+ if (!pdata) {
+ printk(KERN_ERR PFX "no platform data\n");
+ return -ENODEV;
+ }
+
+ watchdog = kzalloc(sizeof(*watchdog), GFP_KERNEL);
+
+ if (!watchdog)
+ return -ENOMEM;
+
+ watchdog->pdata = pdata;
+
+ if (watchdog->pdata->pin == 0 || watchdog->pdata->timeout_ms == 0) {
+ kfree(watchdog);
+ printk(KERN_ERR PFX "invalid platform data\n");
+ return -ENODEV;
+ }
+
+ ret = register_reboot_notifier(&gpio_wdt_notifier);
+ if (ret) {
+ kfree(watchdog);
+ printk(KERN_ERR PFX "cannot register reboot notifier (err=%d)\n", ret);
+ return ret;
+ }
+
+ ret = misc_register(&gpio_wdt_miscdev);
+ if (ret) {
+ unregister_reboot_notifier(&gpio_wdt_notifier);
+ kfree(watchdog);
+ printk(KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
+ WATCHDOG_MINOR, ret);
+ return ret;
+ }
+
+ printk(KERN_INFO PFX "fixed %d.%03d seconds timeout (nowayout= %d)\n",
+ watchdog->pdata->timeout_ms/1000,
+ watchdog->pdata->timeout_ms%1000,
+ nowayout);
+
+ return 0;
+}
+
+static int __devexit gpio_wdt_remove(struct platform_device *pdev)
+{
+ misc_deregister(&gpio_wdt_miscdev);
+ unregister_reboot_notifier(&gpio_wdt_notifier);
+ kfree(watchdog);
+ watchdog = NULL;
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+int gpio_wdt_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ gpio_wdt_stop(watchdog);
+ return 0;
+}
+
+int gpio_wdt_resume(struct platform_device *pdev)
+{
+ if (test_bit(0, &watchdog->driver_open))
+ gpio_wdt_start(watchdog);
+ return 0;
+}
+
+#else
+#define gpio_wdt_suspend NULL
+#define gpio_wdt_resume NULL
+#endif
+
+/* the device driver */
+static struct platform_driver gpio_wdt_driver = {
+ .probe = gpio_wdt_probe,
+ .remove = __devexit_p(gpio_wdt_remove),
+ .suspend = gpio_wdt_suspend,
+ .resume = gpio_wdt_resume,
+ .driver = {
+ .name = GPIO_WDT_DRIVER_NAME,
+ .owner = THIS_MODULE,
+ },
+};
+
+/* module init or kerel entry point */
+static int __init gpio_wdt_driver_init(void)
+{
+ return platform_driver_register(&gpio_wdt_driver);
+}
+
+/* module exit */
+static void __exit gpio_wdt_driver_exit(void)
+{
+ platform_driver_unregister(&gpio_wdt_driver);
+}
+
+module_init(gpio_wdt_driver_init);
+module_exit(gpio_wdt_driver_exit);
+
+MODULE_AUTHOR("Marc Pignat");
+MODULE_DESCRIPTION("Watchdog driver on a generic gpio line");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
diff -urN a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
--- a/drivers/watchdog/Kconfig 2008-01-08 17:30:30.000000000 +0100
+++ b/drivers/watchdog/Kconfig 2008-01-14 10:14:32.000000000 +0100
@@ -55,6 +55,16 @@
To compile this driver as a module, choose M here: the
module will be called softdog.
+config GPIO_WATCHDOG
+ tristate "Support for GPIO connected watchdog"
+ depends on GENERIC_GPIO
+ help
+ This option enables support for a watchdog connected via a GPIO line
+ (such as the MAX823).
+
+ To compile this driver as a module, choose M here: the module will be
+ called gpio_wdt.
+
# ALPHA Architecture
# ARM Architecture
diff -urN a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
--- a/drivers/watchdog/Makefile 2008-01-08 17:30:30.000000000 +0100
+++ b/drivers/watchdog/Makefile 2008-01-10 14:01:32.000000000 +0100
@@ -121,3 +121,4 @@
# Architecture Independant
obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
+obj-$(CONFIG_GPIO_WATCHDOG) += gpio_wdt.o
Hi Marc,
Le lundi 14 janvier 2008, Marc Pignat a ?crit?:
> +static int gpio_wdt_keepalive(struct gpio_wdt *wdt)
> +{
> + gpio_set_value(wdt->pdata->pin, 0);
> + gpio_set_value(wdt->pdata->pin, 1);
> + return 0;
> +}
I would like this function to be supplied by the platform_data structure
because as I mentioned before, not all GPIO connected watchdog will simply
need a single bit to be toggled, but also sometimes a full GPIO line.
If we do not supply any specific callback to keepalive the watchdog let's just
assume we need a single bit to be toggled if this is ok with you ?
Thank you !
--
Cordialement, Florian Fainelli
------------------------------
Hi Florian!
On Monday 14 January 2008, Florian Fainelli wrote:
...
> I would like this function to be supplied by the platform_data structure
> because as I mentioned before, not all GPIO connected watchdog will simply
> need a single bit to be toggled, but also sometimes a full GPIO line.
I understand your wish, but...
You told me that your plaform doesn't implement the generic gpio interface
(yet?), so this driver can't work for you.
...
>
> If we do not supply any specific callback to keepalive the watchdog let's just
> assume we need a single bit to be toggled if this is ok with you ?
This is not the only function to be changed, start, stop and keepalive (and
probably suspend/resume) should be changed, and when the 'Uniform Watchdog
Device Driver' will be merged, writing your driver dealing with more gpio will
be almost trivial.
Please make sure to look a the right place, the git tree for the unified
driver you have given is 17 month old... here is the latest:
http://git.kernel.org/?p=linux/kernel/git/wim/linux-2.6-watchdog-mm.git;a=summary
I'm trying to get this work merged because I use it, it works for me and I'm
almost sure I haven't got the only embedded system using a max823 connected
to a gpio.
Let me know if you want some help implementing your driver!
Regards
Marc
Le lundi 14 janvier 2008, Marc Pignat a ?crit?:
> Hi Florian!
> I understand your wish, but...
> You told me that your plaform doesn't implement the generic gpio interface
> (yet?), so this driver can't work for you.
You understood me wrong, I told you that not all platforms, actually only
AVR32 and ARM seem to make use of David Brownell's gpiolib, but the MIPS
board I am working with supports the "old" generic GPIO API which required
you to define your own wrappers for gpio_set/get_value and such. For both
implementations the config symbol is GENERIC_GPIO, which can somehow be
confusing.
>
> This is not the only function to be changed, start, stop and keepalive (and
> probably suspend/resume) should be changed, and when the 'Uniform Watchdog
> Device Driver' will be merged, writing your driver dealing with more gpio
> will be almost trivial.
I use only one gpio bit as well, only the keepalive function should be
changed. And even if we need the other ones to be changed, let's assume we
can pass the necessary callback to the platform driver, and checking wether
they are defined and call them or not is just trivial. This will not bloat
the driver.
>
> Please make sure to look a the right place, the git tree for the unified
> driver you have given is 17 month old... here is the latest:
> http://git.kernel.org/?p=linux/kernel/git/wim/linux-2.6-watchdog-mm.git;a=s
>ummary
>
> I'm trying to get this work merged because I use it, it works for me and
> I'm almost sure I haven't got the only embedded system using a max823
> connected to a gpio.
Yes, I understand, but having a max823-centric driver does not make it as
general as it could be.
>
> Let me know if you want some help implementing your driver!
It already exists and his merged as mtx1_wdt, I just want to get rid of it if
any generic gpio wathchdog driver can fit my needs and others.
As a general comments, Wim did not answer yet, I would like to hear from him.
--
Cordialement, Florian Fainelli
------------------------------
On Monday 14 January 2008, Florian Fainelli wrote:
> Le lundi 14 janvier 2008, Marc Pignat a ?crit?:
> > Hi Florian!
> > I understand your wish, but...
> > You told me that your plaform doesn't implement the generic gpio interface
> > (yet?), so this driver can't work for you.
>
> You understood me wrong, I told you that not all platforms, actually only
> AVR32 and ARM seem to make use of David Brownell's gpiolib, but the MIPS
> board I am working with supports the "old" generic GPIO API which required
> you to define your own wrappers for gpio_set/get_value and such. For both
> implementations the config symbol is GENERIC_GPIO, which can somehow be
> confusing.
The gpiolib isn't merged yet and it's purpose it to let use any gpio in the
system independently of how they are connected (directly, or through spi, ...).
So I don't use it!
...
> I use only one gpio bit as well, only the keepalive function should be
> changed. And even if we need the other ones to be changed, let's assume we
> can pass the necessary callback to the platform driver, and checking wether
> they are defined and call them or not is just trivial. This will not bloat
> the driver.
Okay, so what do you do with this single pin?
...
> Yes, I understand, but having a max823-centric driver does not make it as
> general as it could be.
Sure. Another solution is to make add a .type in the gpio_wdt struct to choose
which function to use for keepalive -> support for more chips in the driver
itself.
>
> >
> > Let me know if you want some help implementing your driver!
>
> It already exists and his merged as mtx1_wdt, I just want to get rid of it if
> any generic gpio wathchdog driver can fit my needs and others.
I just had a look at it. It has additionnal restrictions like 'It should not
be triggered more often than 1.6 seconds', ..., and works with a timer, it
is really very differrent of what I'm doing...
Can you please tell me what is connected to this gpio?
>
> As a general comments, Wim did not answer yet, I would like to hear from him.
Me too :)
Regards
Marc
On Mon, 14 Jan 2008 15:03:14 +0100
Florian Fainelli <[email protected]> wrote:
> Le lundi 14 janvier 2008, Marc Pignat a écrit :
> > Hi Florian!
> > I understand your wish, but...
> > You told me that your plaform doesn't implement the generic gpio
> > interface (yet?), so this driver can't work for you.
>
> You understood me wrong, I told you that not all platforms, actually
> only AVR32 and ARM seem to make use of David Brownell's gpiolib, but
> the MIPS board I am working with supports the "old" generic GPIO API
> which required you to define your own wrappers for gpio_set/get_value
> and such. For both implementations the config symbol is GENERIC_GPIO,
> which can somehow be confusing.
GENERIC_GPIO means that some implementation of the generic GPIO API is
present. Gpiolib is a specific implementation of that API which
architectures may choose to utilize -- it doesn't change anything that
drivers should care about.
Haavard