[PATCH] watchdog: Add driver for Altera Watchdog Timer

[PATCH] watchdog: Add driver for Altera Watchdog Timer

[Tobias Klauser]

This driver adds support for the Altera Timer in the Watchdog Timer
configuration. This component is usually found on SOPC (System on
Programmable Chip) for Altera FPGAs containing a Nios2 softcore
processor.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/watchdog/Kconfig      |   11 ++
 drivers/watchdog/Makefile     |    3 +
 drivers/watchdog/altera_wdt.c |  229 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 243 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/altera_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 62e743d..be4cb39 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -972,6 +972,17 @@ config BCM63XX_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called bcm63xx_wdt.
 
+# NIOS2 Architecture
+
+config ALTERA_WDT
+	tristate "Altera Watchdog Timer"
+	select WATCHDOG_NOWAYOUT
+	help
+	  Watchdog driver for the Altera Watchdog Timer found on Altera
+	  Nios2 SOPC systems.
+
+	  Once the watchdog timer is started it cannot be stopped anymore.
+
 # PARISC Architecture
 
 # POWERPC Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index dd77665..0e7cf8f 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -122,6 +122,9 @@ obj-$(CONFIG_TXX9_WDT) += txx9wdt.o
 obj-$(CONFIG_OCTEON_WDT) += octeon-wdt.o
 octeon-wdt-y := octeon-wdt-main.o octeon-wdt-nmi.o
 
+# NIOS2 Architecture
+obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
+
 # PARISC Architecture
 
 # POWERPC Architecture
diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
new file mode 100644
index 0000000..a7bf7ce
--- /dev/null
+++ b/drivers/watchdog/altera_wdt.c
@@ -0,0 +1,229 @@
+/*
+ * Driver for the Altera Watchdog Timer
+ *
+ * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2005 Walter Goossens
+ * Copyright (C) 1996 Alan Cox <alan@redhat.com>
+ *
+ * Originally based on wdt.c
+ *
+ * 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/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/uaccess.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+
+#define WATCHDOG_NAME	"altera_wdt"
+
+/* Register offsets */
+#define ALTERA_WDT_SIZE		0x18
+#define ALTERA_WDT_STATUS	0x00
+#define ALTERA_WDT_CONTROL	0x01
+#define ALTERA_WDT_PERIODL	0x02
+#define ALTERA_WDT_PERIODH	0x03
+
+#define ALTERA_WDT_RUN_BIT	0x04
+
+static struct platform_device *altera_wdt_pdev;
+
+struct altera_wdt_dev {
+	void __iomem *base;
+	unsigned long wdt_users;
+};
+
+static int altera_wdt_start(struct altera_wdt_dev *wdev)
+{
+	u32 control = readl(wdev->base + ALTERA_WDT_CONTROL);
+
+	pr_debug("%s: Starting watchdog timer\n", WATCHDOG_NAME);
+	writel(control | ALTERA_WDT_RUN_BIT, wdev->base + ALTERA_WDT_CONTROL);
+	return 0;
+}
+
+static void altera_wdt_ping(struct altera_wdt_dev *wdev)
+{
+	/* It doesn't matter what value we write */
+	writel(1, wdev->base + ALTERA_WDT_PERIODL);
+}
+
+static ssize_t altera_wdt_write(struct file *file, const char __user *buf,
+		size_t len, loff_t *ppos)
+{
+	struct altera_wdt_dev *wdev = file->private_data;
+
+	if (len)
+		altera_wdt_ping(wdev);
+	return len;
+}
+
+static const struct watchdog_info altera_wdt_info = {
+	.identity		= "Altera Watchdog",
+	.options		= WDIOF_KEEPALIVEPING,
+	.firmware_version	= 1,
+};
+
+static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	struct altera_wdt_dev *wdev = file->private_data;
+	void __user *argp = (void __user *) arg;
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
+	case WDIOC_KEEPALIVE:
+		altera_wdt_ping(wdev);
+		return 0;
+	default:
+		return -ENOTTY;
+	}
+}
+
+static int altera_wdt_open(struct inode *inode, struct file *file)
+{
+	struct altera_wdt_dev *wdev = platform_get_drvdata(altera_wdt_pdev);
+
+	if (test_and_set_bit(1, &wdev->wdt_users))
+		return -EBUSY;
+
+	file->private_data = wdev;
+
+	altera_wdt_start(wdev);
+	altera_wdt_ping(wdev);
+
+	return nonseekable_open(inode, file);
+}
+
+static int altera_wdt_release(struct inode *inode, struct file *file)
+{
+	struct altera_wdt_dev *wdev = file->private_data;
+
+	/* It is not possible to stop the watchdog timer, once started */
+	pr_crit("%s: Unexpected close, not stopping!\n", WATCHDOG_NAME);
+	wdev->wdt_users = 0;
+
+	return 0;
+}
+
+static const struct file_operations altera_wdt_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.write		= altera_wdt_write,
+	.unlocked_ioctl	= altera_wdt_ioctl,
+	.open		= altera_wdt_open,
+	.release	= altera_wdt_release,
+};
+
+static struct miscdevice altera_wdt_miscdev = {
+	.minor	= WATCHDOG_MINOR,
+	.name	= "watchdog",
+	.fops	= &altera_wdt_fops,
+};
+
+static int __devinit altera_wdt_probe(struct platform_device *pdev)
+{
+	struct altera_wdt_dev *wdev;
+	struct resource *res, *mem;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	if (altera_wdt_pdev)
+		return -EBUSY;
+
+	mem = request_mem_region(res->start, resource_size(res), pdev->name);
+	if (!mem)
+		return -EBUSY;
+
+	wdev = kzalloc(sizeof(struct altera_wdt_dev), GFP_KERNEL);
+	if (!wdev) {
+		ret = -ENOMEM;
+		goto err_kzalloc;
+	}
+
+	wdev->base = ioremap(mem->start, resource_size(mem));
+	if (!wdev->base) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
+
+	platform_set_drvdata(pdev, wdev);
+
+	ret = misc_register(&altera_wdt_miscdev);
+	if (ret)
+		goto err_misc;
+
+	altera_wdt_pdev = pdev;
+
+	return 0;
+
+err_misc:
+	iounmap(wdev->base);
+err_ioremap:
+	kfree(wdev);
+err_kzalloc:
+	release_mem_region(res->start, resource_size(res));
+
+	return ret;
+}
+
+static int __devexit altera_wdt_remove(struct platform_device *pdev)
+{
+	struct altera_wdt_dev *wdev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	misc_deregister(&altera_wdt_miscdev);
+	platform_set_drvdata(pdev, NULL);
+	iounmap(wdev->base);
+	release_mem_region(res->start, resource_size(res));
+	kfree(wdev);
+	altera_wdt_pdev = NULL;
+
+	return 0;
+}
+
+static void altera_wdt_shutdown(struct platform_device *pdev)
+{
+	/* Do nothing. It is not possible to stop the watchdog timer, once
+	 * started */
+}
+
+static struct platform_driver altera_wdt_driver = {
+	.probe		= altera_wdt_probe,
+	.remove		= __devexit_p(altera_wdt_remove),
+	.shutdown	= altera_wdt_shutdown,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= WATCHDOG_NAME,
+	},
+};
+
+static int __init altera_wdt_init(void)
+{
+	return platform_driver_register(&altera_wdt_driver);
+}
+
+static void __exit altera_wdt_exit(void)
+{
+	platform_driver_unregister(&altera_wdt_driver);
+}
+
+module_init(altera_wdt_init);
+module_exit(altera_wdt_exit);
+
+MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
+MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.0.4

Re: [PATCH] watchdog: Add driver for Altera Watchdog Timer

[Jamie Iles]

Hi Tobias,

On Wed, Jan 12, 2011 at 11:43:47AM +0100, Tobias Klauser wrote:
> This driver adds support for the Altera Timer in the Watchdog Timer
> configuration. This component is usually found on SOPC (System on
> Programmable Chip) for Altera FPGAs containing a Nios2 softcore
> processor.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  drivers/watchdog/Kconfig      |   11 ++
>  drivers/watchdog/Makefile     |    3 +
>  drivers/watchdog/altera_wdt.c |  229 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 243 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/watchdog/altera_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 62e743d..be4cb39 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -972,6 +972,17 @@ config BCM63XX_WDT
>  	  To compile this driver as a loadable module, choose M here.
>  	  The module will be called bcm63xx_wdt.
>  
> +# NIOS2 Architecture
> +
> +config ALTERA_WDT
> +	tristate "Altera Watchdog Timer"
> +	select WATCHDOG_NOWAYOUT
> +	help
> +	  Watchdog driver for the Altera Watchdog Timer found on Altera
> +	  Nios2 SOPC systems.
> +
> +	  Once the watchdog timer is started it cannot be stopped anymore.
> +
>  # PARISC Architecture

I've been writing a driver for a watchdog that also cannot be stopped 
and Wim pointed out that selecting WATCHDOG_NOWAYOUT is the wrong way to 
do this as it will select this for the system and not the single driver.  
The correct approach is to have a software timer to implement a 
heartbeat to kick the watchdog when it is not open.  There are examples 
in at91sam9_wdt.c and pika_wdt.c

> diff --git a/drivers/watchdog/altera_wdt.c 
> b/drivers/watchdog/altera_wdt.c
> new file mode 100644
> index 0000000..a7bf7ce
> --- /dev/null
> +++ b/drivers/watchdog/altera_wdt.c
> @@ -0,0 +1,229 @@
> +/*
> + * Driver for the Altera Watchdog Timer
> + *
> + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> + * Copyright (C) 2005 Walter Goossens
> + * Copyright (C) 1996 Alan Cox <alan@redhat.com>
> + *
> + * Originally based on wdt.c
> + *
> + * 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/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +
> +#define WATCHDOG_NAME	"altera_wdt"
> +
> +/* Register offsets */
> +#define ALTERA_WDT_SIZE		0x18
> +#define ALTERA_WDT_STATUS	0x00
> +#define ALTERA_WDT_CONTROL	0x01
> +#define ALTERA_WDT_PERIODL	0x02
> +#define ALTERA_WDT_PERIODH	0x03
> +
> +#define ALTERA_WDT_RUN_BIT	0x04
> +
> +static struct platform_device *altera_wdt_pdev;
> +
> +struct altera_wdt_dev {
> +	void __iomem *base;
> +	unsigned long wdt_users;
> +};

As you support a single watchdog, do you need the altera_wdt_pdev 
pointer?  This requires you to set the drvdata then keep retrieving it, 
but you could just have a static instance of altera_wdt_dev.

> +
> +static int altera_wdt_start(struct altera_wdt_dev *wdev)
> +{
> +	u32 control = readl(wdev->base + ALTERA_WDT_CONTROL);
> +
> +	pr_debug("%s: Starting watchdog timer\n", WATCHDOG_NAME);
> +	writel(control | ALTERA_WDT_RUN_BIT, wdev->base + ALTERA_WDT_CONTROL);
> +	return 0;
> +}
> +
> +static void altera_wdt_ping(struct altera_wdt_dev *wdev)
> +{
> +	/* It doesn't matter what value we write */
> +	writel(1, wdev->base + ALTERA_WDT_PERIODL);
> +}
> +
> +static ssize_t altera_wdt_write(struct file *file, const char __user *buf,
> +		size_t len, loff_t *ppos)
> +{
> +	struct altera_wdt_dev *wdev = file->private_data;
> +
> +	if (len)
> +		altera_wdt_ping(wdev);
> +	return len;
> +}
> +
> +static const struct watchdog_info altera_wdt_info = {
> +	.identity		= "Altera Watchdog",
> +	.options		= WDIOF_KEEPALIVEPING,
> +	.firmware_version	= 1,
> +};
> +
> +static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
> +		unsigned long arg)
> +{
> +	struct altera_wdt_dev *wdev = file->private_data;
> +	void __user *argp = (void __user *) arg;
> +
> +	switch (cmd) {
> +	case WDIOC_GETSUPPORT:
> +		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
> +	case WDIOC_KEEPALIVE:
> +		altera_wdt_ping(wdev);
> +		return 0;
> +	default:
> +		return -ENOTTY;
> +	}
> +}

Can you set/get the timeout period on this watchdog?  Even if you can't 
set it then it would be good to allow the user to read the constant 
timeout that the watchdog uses so it knows how often to kick it.  If you 
use the software timeout heartbeat then in the future you could emulate 
longer timeouts.

Can you set/get the timeout period on this watchdog?  Even if you can't 
set it then it would be good to allow the user to read the constant 
timeout that the watchdog uses so it knows how often to kick it.  If you 
use the software timeout heartbeat then in the future you could emulate 
longer timeouts.

> +static int altera_wdt_open(struct inode *inode, struct file *file)
> +{
> +	struct altera_wdt_dev *wdev = platform_get_drvdata(altera_wdt_pdev);
> +
> +	if (test_and_set_bit(1, &wdev->wdt_users))
> +		return -EBUSY;
> +
> +	file->private_data = wdev;
> +
> +	altera_wdt_start(wdev);
> +	altera_wdt_ping(wdev);
> +
> +	return nonseekable_open(inode, file);
> +}
> +
> +static int altera_wdt_release(struct inode *inode, struct file *file)
> +{
> +	struct altera_wdt_dev *wdev = file->private_data;
> +
> +	/* It is not possible to stop the watchdog timer, once started */
> +	pr_crit("%s: Unexpected close, not stopping!\n", WATCHDOG_NAME);
> +	wdev->wdt_users = 0;
> +
> +	return 0;
> +}
> +
> +static const struct file_operations altera_wdt_fops = {
> +	.owner		= THIS_MODULE,
> +	.llseek		= no_llseek,
> +	.write		= altera_wdt_write,
> +	.unlocked_ioctl	= altera_wdt_ioctl,
> +	.open		= altera_wdt_open,
> +	.release	= altera_wdt_release,
> +};
> +
> +static struct miscdevice altera_wdt_miscdev = {
> +	.minor	= WATCHDOG_MINOR,
> +	.name	= "watchdog",
> +	.fops	= &altera_wdt_fops,
> +};
> +
> +static int __devinit altera_wdt_probe(struct platform_device *pdev)
> +{
> +	struct altera_wdt_dev *wdev;
> +	struct resource *res, *mem;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENOENT;
> +
> +	if (altera_wdt_pdev)
> +		return -EBUSY;
> +
> +	mem = request_mem_region(res->start, resource_size(res), pdev->name);
> +	if (!mem)
> +		return -EBUSY;
> +
> +	wdev = kzalloc(sizeof(struct altera_wdt_dev), GFP_KERNEL);
> +	if (!wdev) {
> +		ret = -ENOMEM;
> +		goto err_kzalloc;
> +	}
> +
> +	wdev->base = ioremap(mem->start, resource_size(mem));
> +	if (!wdev->base) {
> +		ret = -ENOMEM;
> +		goto err_ioremap;
> +	}

You can use devm_request_mem_region(), devm_kzalloc() and devm_ioremap() 
here to make the cleanup and error handling a bit simpler 
(Documentation/driver-model/devres.txt)

> +
> +	platform_set_drvdata(pdev, wdev);
> +
> +	ret = misc_register(&altera_wdt_miscdev);
> +	if (ret)
> +		goto err_misc;
> +
> +	altera_wdt_pdev = pdev;
> +
> +	return 0;
> +
> +err_misc:
> +	iounmap(wdev->base);
> +err_ioremap:
> +	kfree(wdev);
> +err_kzalloc:
> +	release_mem_region(res->start, resource_size(res));
> +
> +	return ret;
> +}
> +
> +static int __devexit altera_wdt_remove(struct platform_device *pdev)
> +{
> +	struct altera_wdt_dev *wdev = platform_get_drvdata(pdev);
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	misc_deregister(&altera_wdt_miscdev);
> +	platform_set_drvdata(pdev, NULL);
> +	iounmap(wdev->base);
> +	release_mem_region(res->start, resource_size(res));
> +	kfree(wdev);
> +	altera_wdt_pdev = NULL;
> +
> +	return 0;
> +}
> +
> +static void altera_wdt_shutdown(struct platform_device *pdev)
> +{
> +	/* Do nothing. It is not possible to stop the watchdog timer, once
> +	 * started */
> +}

Can you just remove this function and not define it in alter_wdt_driver?

> +
> +static struct platform_driver altera_wdt_driver = {
> +	.probe		= altera_wdt_probe,
> +	.remove		= __devexit_p(altera_wdt_remove),
> +	.shutdown	= altera_wdt_shutdown,
> +	.driver		= {
> +		.owner	= THIS_MODULE,
> +		.name	= WATCHDOG_NAME,
> +	},
> +};
> +
> +static int __init altera_wdt_init(void)
> +{
> +	return platform_driver_register(&altera_wdt_driver);
> +}
> +
> +static void __exit altera_wdt_exit(void)
> +{
> +	platform_driver_unregister(&altera_wdt_driver);
> +}
> +
> +module_init(altera_wdt_init);
> +module_exit(altera_wdt_exit);
> +
> +MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
> +MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

Jamie

Re: [PATCH] watchdog: Add driver for Altera Watchdog Timer

[Tobias Klauser]

Hi Jamie

Thanks a lot for your review of the driver.

On 2011-01-12 at 12:48:00 +0100, Jamie Iles <jamie@jamieiles.com> wrote:
> On Wed, Jan 12, 2011 at 11:43:47AM +0100, Tobias Klauser wrote:
> > This driver adds support for the Altera Timer in the Watchdog Timer
> > configuration. This component is usually found on SOPC (System on
> > Programmable Chip) for Altera FPGAs containing a Nios2 softcore
> > processor.
> > 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  drivers/watchdog/Kconfig      |   11 ++
> >  drivers/watchdog/Makefile     |    3 +
> >  drivers/watchdog/altera_wdt.c |  229 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 243 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/watchdog/altera_wdt.c
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 62e743d..be4cb39 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -972,6 +972,17 @@ config BCM63XX_WDT
> >  	  To compile this driver as a loadable module, choose M here.
> >  	  The module will be called bcm63xx_wdt.
> >  
> > +# NIOS2 Architecture
> > +
> > +config ALTERA_WDT
> > +	tristate "Altera Watchdog Timer"
> > +	select WATCHDOG_NOWAYOUT
> > +	help
> > +	  Watchdog driver for the Altera Watchdog Timer found on Altera
> > +	  Nios2 SOPC systems.
> > +
> > +	  Once the watchdog timer is started it cannot be stopped anymore.
> > +
> >  # PARISC Architecture
> 
> I've been writing a driver for a watchdog that also cannot be stopped 
> and Wim pointed out that selecting WATCHDOG_NOWAYOUT is the wrong way to 
> do this as it will select this for the system and not the single driver.  
> The correct approach is to have a software timer to implement a 
> heartbeat to kick the watchdog when it is not open.  There are examples 
> in at91sam9_wdt.c and pika_wdt.c

I did not know that, but it sounds reasonable. I'll have a look at the
two drivers you mention and add a software timer in the altera_wdt
driver.

> > diff --git a/drivers/watchdog/altera_wdt.c 
> > b/drivers/watchdog/altera_wdt.c
> > new file mode 100644
> > index 0000000..a7bf7ce
> > --- /dev/null
> > +++ b/drivers/watchdog/altera_wdt.c
> > @@ -0,0 +1,229 @@
> > +/*
> > + * Driver for the Altera Watchdog Timer
> > + *
> > + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> > + * Copyright (C) 2005 Walter Goossens
> > + * Copyright (C) 1996 Alan Cox <alan@redhat.com>
> > + *
> > + * Originally based on wdt.c
> > + *
> > + * 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/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/fs.h>
> > +#include <linux/init.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/slab.h>
> > +#include <linux/io.h>
> > +
> > +#define WATCHDOG_NAME	"altera_wdt"
> > +
> > +/* Register offsets */
> > +#define ALTERA_WDT_SIZE		0x18
> > +#define ALTERA_WDT_STATUS	0x00
> > +#define ALTERA_WDT_CONTROL	0x01
> > +#define ALTERA_WDT_PERIODL	0x02
> > +#define ALTERA_WDT_PERIODH	0x03
> > +
> > +#define ALTERA_WDT_RUN_BIT	0x04
> > +
> > +static struct platform_device *altera_wdt_pdev;
> > +
> > +struct altera_wdt_dev {
> > +	void __iomem *base;
> > +	unsigned long wdt_users;
> > +};
> 
> As you support a single watchdog, do you need the altera_wdt_pdev 
> pointer?  This requires you to set the drvdata then keep retrieving it, 
> but you could just have a static instance of altera_wdt_dev.

In principle it would be possible to have multiple instances of this
timer on one system (depending on how you configure your FPGA), though
that would not make much sense of course.

> > +
> > +static int altera_wdt_start(struct altera_wdt_dev *wdev)
> > +{
> > +	u32 control = readl(wdev->base + ALTERA_WDT_CONTROL);
> > +
> > +	pr_debug("%s: Starting watchdog timer\n", WATCHDOG_NAME);
> > +	writel(control | ALTERA_WDT_RUN_BIT, wdev->base + ALTERA_WDT_CONTROL);
> > +	return 0;
> > +}
> > +
> > +static void altera_wdt_ping(struct altera_wdt_dev *wdev)
> > +{
> > +	/* It doesn't matter what value we write */
> > +	writel(1, wdev->base + ALTERA_WDT_PERIODL);
> > +}
> > +
> > +static ssize_t altera_wdt_write(struct file *file, const char __user *buf,
> > +		size_t len, loff_t *ppos)
> > +{
> > +	struct altera_wdt_dev *wdev = file->private_data;
> > +
> > +	if (len)
> > +		altera_wdt_ping(wdev);
> > +	return len;
> > +}
> > +
> > +static const struct watchdog_info altera_wdt_info = {
> > +	.identity		= "Altera Watchdog",
> > +	.options		= WDIOF_KEEPALIVEPING,
> > +	.firmware_version	= 1,
> > +};
> > +
> > +static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
> > +		unsigned long arg)
> > +{
> > +	struct altera_wdt_dev *wdev = file->private_data;
> > +	void __user *argp = (void __user *) arg;
> > +
> > +	switch (cmd) {
> > +	case WDIOC_GETSUPPORT:
> > +		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
> > +	case WDIOC_KEEPALIVE:
> > +		altera_wdt_ping(wdev);
> > +		return 0;
> > +	default:
> > +		return -ENOTTY;
> > +	}
> > +}
> 
> Can you set/get the timeout period on this watchdog?  Even if you can't 
> set it then it would be good to allow the user to read the constant 
> timeout that the watchdog uses so it knows how often to kick it.  If you 
> use the software timeout heartbeat then in the future you could emulate 
> longer timeouts.

This is a very primitive watchdog timer which does only allow the
timeout period to be set when instantiating the timer in the FPGA, so
from a software point-of-view the timer has a fixed timeout. It doesn't
even allow to read the timeout value directly from the "hardware".

But we could use the additional information (including the timeout value
among a lot of other configurable values) generated when building the
FPGA firmware and store that in the device tree (support for device tree
was recently added to the Nios2 Linux port). So we could get it (and
the base address) from there in the altera_wdt driver. Does that sound
reasonable?

> > +static int altera_wdt_open(struct inode *inode, struct file *file)
> > +{
> > +	struct altera_wdt_dev *wdev = platform_get_drvdata(altera_wdt_pdev);
> > +
> > +	if (test_and_set_bit(1, &wdev->wdt_users))
> > +		return -EBUSY;
> > +
> > +	file->private_data = wdev;
> > +
> > +	altera_wdt_start(wdev);
> > +	altera_wdt_ping(wdev);
> > +
> > +	return nonseekable_open(inode, file);
> > +}
> > +
> > +static int altera_wdt_release(struct inode *inode, struct file *file)
> > +{
> > +	struct altera_wdt_dev *wdev = file->private_data;
> > +
> > +	/* It is not possible to stop the watchdog timer, once started */
> > +	pr_crit("%s: Unexpected close, not stopping!\n", WATCHDOG_NAME);
> > +	wdev->wdt_users = 0;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct file_operations altera_wdt_fops = {
> > +	.owner		= THIS_MODULE,
> > +	.llseek		= no_llseek,
> > +	.write		= altera_wdt_write,
> > +	.unlocked_ioctl	= altera_wdt_ioctl,
> > +	.open		= altera_wdt_open,
> > +	.release	= altera_wdt_release,
> > +};
> > +
> > +static struct miscdevice altera_wdt_miscdev = {
> > +	.minor	= WATCHDOG_MINOR,
> > +	.name	= "watchdog",
> > +	.fops	= &altera_wdt_fops,
> > +};
> > +
> > +static int __devinit altera_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct altera_wdt_dev *wdev;
> > +	struct resource *res, *mem;
> > +	int ret;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -ENOENT;
> > +
> > +	if (altera_wdt_pdev)
> > +		return -EBUSY;
> > +
> > +	mem = request_mem_region(res->start, resource_size(res), pdev->name);
> > +	if (!mem)
> > +		return -EBUSY;
> > +
> > +	wdev = kzalloc(sizeof(struct altera_wdt_dev), GFP_KERNEL);
> > +	if (!wdev) {
> > +		ret = -ENOMEM;
> > +		goto err_kzalloc;
> > +	}
> > +
> > +	wdev->base = ioremap(mem->start, resource_size(mem));
> > +	if (!wdev->base) {
> > +		ret = -ENOMEM;
> > +		goto err_ioremap;
> > +	}
> 
> You can use devm_request_mem_region(), devm_kzalloc() and devm_ioremap() 
> here to make the cleanup and error handling a bit simpler 
> (Documentation/driver-model/devres.txt)

I didn't know about these functions. I'll have a look at them and
convert the driver to use them.

> > +
> > +	platform_set_drvdata(pdev, wdev);
> > +
> > +	ret = misc_register(&altera_wdt_miscdev);
> > +	if (ret)
> > +		goto err_misc;
> > +
> > +	altera_wdt_pdev = pdev;
> > +
> > +	return 0;
> > +
> > +err_misc:
> > +	iounmap(wdev->base);
> > +err_ioremap:
> > +	kfree(wdev);
> > +err_kzalloc:
> > +	release_mem_region(res->start, resource_size(res));
> > +
> > +	return ret;
> > +}
> > +
> > +static int __devexit altera_wdt_remove(struct platform_device *pdev)
> > +{
> > +	struct altera_wdt_dev *wdev = platform_get_drvdata(pdev);
> > +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > +	misc_deregister(&altera_wdt_miscdev);
> > +	platform_set_drvdata(pdev, NULL);
> > +	iounmap(wdev->base);
> > +	release_mem_region(res->start, resource_size(res));
> > +	kfree(wdev);
> > +	altera_wdt_pdev = NULL;
> > +
> > +	return 0;
> > +}
> > +
> > +static void altera_wdt_shutdown(struct platform_device *pdev)
> > +{
> > +	/* Do nothing. It is not possible to stop the watchdog timer, once
> > +	 * started */
> > +}
> 
> Can you just remove this function and not define it in alter_wdt_driver?

Ok, will do that.

> > +
> > +static struct platform_driver altera_wdt_driver = {
> > +	.probe		= altera_wdt_probe,
> > +	.remove		= __devexit_p(altera_wdt_remove),
> > +	.shutdown	= altera_wdt_shutdown,
> > +	.driver		= {
> > +		.owner	= THIS_MODULE,
> > +		.name	= WATCHDOG_NAME,
> > +	},
> > +};
> > +
> > +static int __init altera_wdt_init(void)
> > +{
> > +	return platform_driver_register(&altera_wdt_driver);
> > +}
> > +
> > +static void __exit altera_wdt_exit(void)
> > +{
> > +	platform_driver_unregister(&altera_wdt_driver);
> > +}
> > +
> > +module_init(altera_wdt_init);
> > +module_exit(altera_wdt_exit);
> > +
> > +MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
> > +MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
> > +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

Re: [PATCH] watchdog: Add driver for Altera Watchdog Timer

[Jamie Iles]

On Wed, Jan 12, 2011 at 01:36:37PM +0100, Tobias Klauser wrote:
[...]
> > > +static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
> > > +		unsigned long arg)
> > > +{
> > > +	struct altera_wdt_dev *wdev = file->private_data;
> > > +	void __user *argp = (void __user *) arg;
> > > +
> > > +	switch (cmd) {
> > > +	case WDIOC_GETSUPPORT:
> > > +		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
> > > +	case WDIOC_KEEPALIVE:
> > > +		altera_wdt_ping(wdev);
> > > +		return 0;
> > > +	default:
> > > +		return -ENOTTY;
> > > +	}
> > > +}
> > 
> > Can you set/get the timeout period on this watchdog?  Even if you can't 
> > set it then it would be good to allow the user to read the constant 
> > timeout that the watchdog uses so it knows how often to kick it.  If you 
> > use the software timeout heartbeat then in the future you could emulate 
> > longer timeouts.
> 
> This is a very primitive watchdog timer which does only allow the
> timeout period to be set when instantiating the timer in the FPGA, so
> from a software point-of-view the timer has a fixed timeout. It doesn't
> even allow to read the timeout value directly from the "hardware".
> 
> But we could use the additional information (including the timeout value
> among a lot of other configurable values) generated when building the
> FPGA firmware and store that in the device tree (support for device tree
> was recently added to the Nios2 Linux port). So we could get it (and
> the base address) from there in the altera_wdt driver. Does that sound
> reasonable?

That sounds like a very good idea to me.  I can see that allowing 
userspace to know the timeout period would be useful.

Jamie

[PATCH v2] watchdog: Add driver for Altera Watchdog Timer

[Tobias Klauser]

This driver adds support for the Altera Timer in the Watchdog Timer
configuration. This component is usually found on SOPC (System on
Programmable Chip) for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---

Thanks a lot to Jamie Iles for reviewing the first submitted version of this
driver.

changes for v2:
 - Get driver properties from devicetree (and document them), thus the
   driver depends on OF
 - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
   timer resetting the watchdog timer when the device is not open
 - Only support a single instance of the watchdog, so the private data
   can be kept in static data
 - Use devm_* functions in altera_wdt_probe to simplify cleanup and
   error handling
 - Remove empty altera_wdt_shutdown function
 - Add MODULE_LICENSE (GPL)

 .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
 drivers/watchdog/Kconfig                           |    8 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/altera_wdt.c                      |  310 ++++++++++++++++++++
 4 files changed, 326 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
 create mode 100644 drivers/watchdog/altera_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
new file mode 100644
index 0000000..16ac949
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
@@ -0,0 +1,7 @@
+Altera Watchdog Timer
+
+Required properties:
+- compatible : should be "ALTR,wdt-1.0"
+- clock-frequency : frequency of the clock input
+- timeout : load value of the timer (number of clock ticks until watchdog
+            resets)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index bd6a33d..ebcafb0 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -69,6 +69,14 @@ config WM8350_WATCHDOG
 	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
 	  the watchdog triggers the system will be reset.
 
+config ALTERA_WDT
+	tristate "Altera Watchdog Timer"
+	depends on OF
+	help
+	  Support for the Altera Timer in the Watchdog Timer configuration. This
+	  component is found on SOPC (System on Programmable Chip) for Altera
+	  FPGAs.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index fd9c8c0..48a2167 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)		+= cpwd.o
 obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
+obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
new file mode 100644
index 0000000..dbced1e
--- /dev/null
+++ b/drivers/watchdog/altera_wdt.c
@@ -0,0 +1,310 @@
+/*
+ * Driver for the Altera Watchdog Timer
+ *
+ * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2005 Walter Goossens
+ *
+ * Originally based on wdt.c which is
+ *
+ * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
+ *
+ * Software timeout heartbeat code based on pika_wdt.c which is
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *
+ * 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/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/timer.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/of_platform.h>
+
+#define WATCHDOG_NAME	"altera_wdt"
+
+/* Register offsets */
+#define ALTERA_WDT_STATUS	0x00
+#define ALTERA_WDT_CONTROL	0x04
+#define ALTERA_WDT_PERIODL	0x08
+#define ALTERA_WDT_PERIODH	0x0C
+
+#define ALTERA_WDT_RUN_BIT	0x04
+
+/* User land timeout */
+#define WDT_HEARTBEAT 15
+static int heartbeat = WDT_HEARTBEAT;
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
+	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
+
+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) ")");
+
+static struct {
+	void __iomem *base;
+	unsigned long wdt_timeout;	/* timeout of the hardware timer */
+
+	unsigned long next_heartbeat;	/* the next_heartbeat for the timer */
+	unsigned long is_open;
+	char expect_close;
+	struct timer_list timer;	/* software timer that pings the watchdog */
+} altera_wdt_priv;
+
+/*
+ * Start the watchdog. Once it has been started, it cannot be stopped anymore.
+ */
+static int altera_wdt_setup(void)
+{
+	u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
+
+	writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
+	return 0;
+}
+
+/*
+ * Tickle the watchdog (reset the watchdog timer)
+ */
+static void altera_wdt_reset(void)
+{
+	/* It doesn't matter what value we write */
+	writel(1, altera_wdt_priv.base + ALTERA_WDT_PERIODL);
+}
+
+/*
+ * Software timer tick
+ */
+static void altera_wdt_ping(unsigned long data)
+{
+	if (time_before(jiffies, altera_wdt_priv.next_heartbeat) ||
+			(!nowayout && !altera_wdt_priv.is_open)) {
+		altera_wdt_reset();
+		mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+	} else
+		pr_crit(WATCHDOG_NAME ": I will reset your machine!\n");
+}
+
+static void altera_wdt_keepalive(void)
+{
+	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
+}
+
+static void altera_wdt_start(void)
+{
+	altera_wdt_keepalive();
+	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+}
+
+static int altera_wdt_open(struct inode *inode, struct file *file)
+{
+	/* /dev/watchdog can only be opened once */
+	if (test_and_set_bit(0, &altera_wdt_priv.is_open))
+		return -EBUSY;
+
+	altera_wdt_start();
+
+	return nonseekable_open(inode, file);
+}
+
+static int altera_wdt_release(struct inode *inode, struct file *file)
+{
+	/* stop internal ping */
+	if (!altera_wdt_priv.expect_close)
+		del_timer(&altera_wdt_priv.timer);
+
+	clear_bit(0, &altera_wdt_priv.is_open);
+	altera_wdt_priv.expect_close = 0;
+
+	return 0;
+}
+
+static ssize_t altera_wdt_write(struct file *file, const char __user *data,
+		size_t len, loff_t *ppos)
+{
+	if (!len)
+		return 0;
+
+	/* Scan for magic character */
+	if (!nowayout) {
+		size_t i;
+
+		altera_wdt_priv.expect_close = 0;
+
+		for (i = 0; i < len; i++) {
+			char c;
+			if (get_user(c, data + i))
+				return -EFAULT;
+			if (c == 'V') {
+				altera_wdt_priv.expect_close = 42;
+				break;
+			}
+		}
+	}
+
+	altera_wdt_keepalive();
+
+	return len;
+}
+
+static const struct watchdog_info altera_wdt_info = {
+	.identity		= "Altera Watchdog",
+	.options		= WDIOF_SETTIMEOUT |
+				  WDIOF_KEEPALIVEPING |
+				  WDIOF_MAGICCLOSE,
+	.firmware_version	= 1,
+};
+
+static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	void __user *argp = (void __user *) arg;
+	int __user *p = argp;
+	int new_value;
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
+
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+
+	case WDIOC_KEEPALIVE:
+		altera_wdt_keepalive();
+		return 0;
+
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_value, p))
+			return -EFAULT;
+
+		heartbeat = new_value;
+		altera_wdt_keepalive();
+
+		return put_user(new_value, p);  /* return current value */
+
+	case WDIOC_GETTIMEOUT:
+		return put_user(heartbeat, p);
+
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations altera_wdt_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.open		= altera_wdt_open,
+	.release	= altera_wdt_release,
+	.write		= altera_wdt_write,
+	.unlocked_ioctl	= altera_wdt_ioctl,
+};
+
+static struct miscdevice altera_wdt_miscdev = {
+	.minor	= WATCHDOG_MINOR,
+	.name	= "watchdog",
+	.fops	= &altera_wdt_fops,
+};
+
+static int __devinit altera_wdt_probe(struct platform_device *pdev)
+{
+	struct resource *res, *mem;
+	const u32 *freq_prop, *timeout_prop;
+	unsigned long timeout;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	mem = devm_request_mem_region(&pdev->dev, res->start,
+				      resource_size(res), pdev->name);
+	if (!mem)
+		return -EBUSY;
+
+	altera_wdt_priv.base = devm_ioremap_nocache(&pdev->dev, mem->start,
+						    resource_size(mem));
+	if (!altera_wdt_priv.base)
+		return -ENOMEM;
+
+	freq_prop = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
+	if (!freq_prop)
+		return -ENODEV;
+
+	timeout_prop = of_get_property(pdev->dev.of_node, "timeout", NULL);
+	if (!timeout_prop)
+		return -ENODEV;
+
+	/* Add 1 as the timeout property actually holds the load value */
+	timeout = be32_to_cpup(timeout_prop) + 1;
+	/* Convert timeout to msecs */
+	timeout = timeout / (be32_to_cpup(freq_prop) / MSEC_PER_SEC);
+	/* Tickle the watchdog twice per timeout period */
+	altera_wdt_priv.wdt_timeout = msecs_to_jiffies(timeout / 2);
+
+	ret = misc_register(&altera_wdt_miscdev);
+	if (ret)
+		return ret;
+
+	altera_wdt_setup();
+	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
+	setup_timer(&altera_wdt_priv.timer, altera_wdt_ping, 0);
+	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+
+	pr_info(WATCHDOG_NAME " enabled (heartbeat=%d sec, nowayout=%d)\n",
+		heartbeat, nowayout);
+
+	return 0;
+}
+
+static int __devexit altera_wdt_remove(struct platform_device *pdev)
+{
+	misc_deregister(&altera_wdt_miscdev);
+	return 0;
+}
+
+static struct of_device_id altera_wdt_match[] = {
+	{ .compatible = "ALTR,wdt-1.0", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, altera_wdt_match);
+
+static struct platform_driver altera_wdt_driver = {
+	.probe		= altera_wdt_probe,
+	.remove		= __devexit_p(altera_wdt_remove),
+	.driver		= {
+		.owner		= THIS_MODULE,
+		.name		= WATCHDOG_NAME,
+		.of_match_table	= altera_wdt_match,
+	},
+};
+
+static int __init altera_wdt_init(void)
+{
+	return platform_driver_register(&altera_wdt_driver);
+}
+
+static void __exit altera_wdt_exit(void)
+{
+	platform_driver_unregister(&altera_wdt_driver);
+}
+
+module_init(altera_wdt_init);
+module_exit(altera_wdt_exit);
+
+MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
+MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_ALIAS("platform:" WATCHDOG_NAME);
-- 
1.7.0.4

Re: [PATCH v2] watchdog: Add driver for Altera Watchdog Timer

[Jamie Iles]

Hi Tobias,

One very pedantic comment, otherwise looks great!

On Tue, Feb 08, 2011 at 02:22:02PM +0100, Tobias Klauser wrote:
> This driver adds support for the Altera Timer in the Watchdog Timer
> configuration. This component is usually found on SOPC (System on
> Programmable Chip) for Altera FPGAs.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

Reviewed-by: Jamie Iles <jamie@jamieiles.com>.

> ---
> 
> Thanks a lot to Jamie Iles for reviewing the first submitted version of this
> driver.
> 
> changes for v2:
>  - Get driver properties from devicetree (and document them), thus the
>    driver depends on OF
>  - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
>    timer resetting the watchdog timer when the device is not open
>  - Only support a single instance of the watchdog, so the private data
>    can be kept in static data
>  - Use devm_* functions in altera_wdt_probe to simplify cleanup and
>    error handling
>  - Remove empty altera_wdt_shutdown function
>  - Add MODULE_LICENSE (GPL)
> 
>  .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
>  drivers/watchdog/Kconfig                           |    8 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/altera_wdt.c                      |  310 ++++++++++++++++++++
>  4 files changed, 326 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
>  create mode 100644 drivers/watchdog/altera_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> new file mode 100644
> index 0000000..16ac949
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> @@ -0,0 +1,7 @@
> +Altera Watchdog Timer
> +
> +Required properties:
> +- compatible : should be "ALTR,wdt-1.0"
> +- clock-frequency : frequency of the clock input
> +- timeout : load value of the timer (number of clock ticks until watchdog
> +            resets)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bd6a33d..ebcafb0 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -69,6 +69,14 @@ config WM8350_WATCHDOG
>  	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>  	  the watchdog triggers the system will be reset.
>  
> +config ALTERA_WDT
> +	tristate "Altera Watchdog Timer"
> +	depends on OF
> +	help
> +	  Support for the Altera Timer in the Watchdog Timer configuration. This
> +	  component is found on SOPC (System on Programmable Chip) for Altera
> +	  FPGAs.
> +
>  # ALPHA Architecture
>  
>  # ARM Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index fd9c8c0..48a2167 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)		+= cpwd.o
>  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> +obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
> diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
> new file mode 100644
> index 0000000..dbced1e
> --- /dev/null
> +++ b/drivers/watchdog/altera_wdt.c
> @@ -0,0 +1,310 @@
> +/*
> + * Driver for the Altera Watchdog Timer
> + *
> + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> + * Copyright (C) 2005 Walter Goossens
> + *
> + * Originally based on wdt.c which is
> + *
> + * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
> + *
> + * Software timeout heartbeat code based on pika_wdt.c which is
> + *
> + * Copyright (c) 2008 PIKA Technologies
> + *
> + * 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/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/timer.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/of_platform.h>
> +
> +#define WATCHDOG_NAME	"altera_wdt"
> +
> +/* Register offsets */
> +#define ALTERA_WDT_STATUS	0x00
> +#define ALTERA_WDT_CONTROL	0x04
> +#define ALTERA_WDT_PERIODL	0x08
> +#define ALTERA_WDT_PERIODH	0x0C
> +
> +#define ALTERA_WDT_RUN_BIT	0x04
> +
> +/* User land timeout */
> +#define WDT_HEARTBEAT 15
> +static int heartbeat = WDT_HEARTBEAT;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
> +	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
> +
> +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) ")");
> +
> +static struct {
> +	void __iomem *base;
> +	unsigned long wdt_timeout;	/* timeout of the hardware timer */
> +
> +	unsigned long next_heartbeat;	/* the next_heartbeat for the timer */
> +	unsigned long is_open;
> +	char expect_close;
> +	struct timer_list timer;	/* software timer that pings the watchdog */
> +} altera_wdt_priv;
> +
> +/*
> + * Start the watchdog. Once it has been started, it cannot be stopped anymore.
> + */
> +static int altera_wdt_setup(void)
> +{
> +	u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +
> +	writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +	return 0;
> +}

Looks like this could be made static void.

Jamie

Re: [Nios2-dev] [PATCH v2] watchdog: Add driver for Altera Watchdog Timer

[Walter Goossens]

Hi Tobias,

One small remark below, looks ok otherwise.
Thanks for the work you put into this!

On 2/8/11 2:22 PM, Tobias Klauser wrote:
[...]
> +static struct miscdevice altera_wdt_miscdev = {
> +	.minor	= WATCHDOG_MINOR,
> +	.name	= "watchdog",
> +	.fops	=&altera_wdt_fops,
> +};
> +
> +static int __devinit altera_wdt_probe(struct platform_device *pdev)
> +{
> +	struct resource *res, *mem;
> +	const u32 *freq_prop, *timeout_prop;
Type should be const __be32 * here.
> +	unsigned long timeout;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENOENT;
> +
> +	mem = devm_request_mem_region(&pdev->dev, res->start,
> +				      resource_size(res), pdev->name);
> +	if (!mem)
> +		return -EBUSY;
> +
> +	altera_wdt_priv.base = devm_ioremap_nocache(&pdev->dev, mem->start,
> +						    resource_size(mem));
> +	if (!altera_wdt_priv.base)
> +		return -ENOMEM;
> +
> +	freq_prop = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> +	if (!freq_prop)
> +		return -ENODEV;
> +
> +	timeout_prop = of_get_property(pdev->dev.of_node, "timeout", NULL);
> +	if (!timeout_prop)
> +		return -ENODEV;
> +
> +	/* Add 1 as the timeout property actually holds the load value */
> +	timeout = be32_to_cpup(timeout_prop) + 1;
> +	/* Convert timeout to msecs */
> +	timeout = timeout / (be32_to_cpup(freq_prop) / MSEC_PER_SEC);
> +	/* Tickle the watchdog twice per timeout period */
> +	altera_wdt_priv.wdt_timeout = msecs_to_jiffies(timeout / 2);
> +
> +	ret = misc_register(&altera_wdt_miscdev);
> +	if (ret)
> +		return ret;
> +
> +	altera_wdt_setup();
> +	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> +	setup_timer(&altera_wdt_priv.timer, altera_wdt_ping, 0);
> +	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +
> +	pr_info(WATCHDOG_NAME " enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		heartbeat, nowayout);
> +
> +	return 0;
> +}
> +
> +static int __devexit altera_wdt_remove(struct platform_device *pdev)
> +{
> +	misc_deregister(&altera_wdt_miscdev);
> +	return 0;
> +}
> +
> +static struct of_device_id altera_wdt_match[] = {
> +	{ .compatible = "ALTR,wdt-1.0", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, altera_wdt_match);
> +
> +static struct platform_driver altera_wdt_driver = {
> +	.probe		= altera_wdt_probe,
> +	.remove		= __devexit_p(altera_wdt_remove),
> +	.driver		= {
> +		.owner		= THIS_MODULE,
> +		.name		= WATCHDOG_NAME,
> +		.of_match_table	= altera_wdt_match,
> +	},
> +};
> +
> +static int __init altera_wdt_init(void)
> +{
> +	return platform_driver_register(&altera_wdt_driver);
> +}
> +
> +static void __exit altera_wdt_exit(void)
> +{
> +	platform_driver_unregister(&altera_wdt_driver);
> +}
> +
> +module_init(altera_wdt_init);
> +module_exit(altera_wdt_exit);
> +
> +MODULE_AUTHOR("Walter Goossens, Tobias Klauser<tklauser@distanz.ch>");
> +MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> +MODULE_ALIAS("platform:" WATCHDOG_NAME);

[PATCH v3] watchdog: Add driver for Altera Watchdog Timer

[Tobias Klauser]

This driver adds support for the Altera Timer in the Watchdog Timer
configuration. This component is usually found on SOPC (System on
Programmable Chip) for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Reviewed-by: Jamie Iles <jamie@jamieiles.com>
---
Thanks a lot to Jamie Iles and Walter Goossens for their feedback on v2.

changes for v3:
 - Make altera_wdt_setup static void (as suggested by Jamie)
 - Use const __be32 * for devicetree properties (as suggested by Walter)
changes for v2:
 - Get driver properties from devicetree (and document them), thus the
   driver depends on OF
 - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
   timer resetting the watchdog timer when the device is not open
 - Only support a single instance of the watchdog, so the private data
   can be kept in static data
 - Use devm_* functions in altera_wdt_probe to simplify cleanup and
   error handling
 - Remove empty altera_wdt_shutdown function
 - Add MODULE_LICENSE (GPL)


 .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
 drivers/watchdog/Kconfig                           |    8 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/altera_wdt.c                      |  309 ++++++++++++++++++++
 4 files changed, 325 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
 create mode 100644 drivers/watchdog/altera_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
new file mode 100644
index 0000000..16ac949
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
@@ -0,0 +1,7 @@
+Altera Watchdog Timer
+
+Required properties:
+- compatible : should be "ALTR,wdt-1.0"
+- clock-frequency : frequency of the clock input
+- timeout : load value of the timer (number of clock ticks until watchdog
+            resets)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index bd6a33d..ebcafb0 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -69,6 +69,14 @@ config WM8350_WATCHDOG
 	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
 	  the watchdog triggers the system will be reset.
 
+config ALTERA_WDT
+	tristate "Altera Watchdog Timer"
+	depends on OF
+	help
+	  Support for the Altera Timer in the Watchdog Timer configuration. This
+	  component is found on SOPC (System on Programmable Chip) for Altera
+	  FPGAs.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index fd9c8c0..48a2167 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)		+= cpwd.o
 obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
+obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
new file mode 100644
index 0000000..2a7f14a
--- /dev/null
+++ b/drivers/watchdog/altera_wdt.c
@@ -0,0 +1,309 @@
+/*
+ * Driver for the Altera Watchdog Timer
+ *
+ * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2005 Walter Goossens
+ *
+ * Originally based on wdt.c which is
+ *
+ * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
+ *
+ * Software timeout heartbeat code based on pika_wdt.c which is
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *
+ * 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/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/timer.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/of_platform.h>
+
+#define WATCHDOG_NAME	"altera_wdt"
+
+/* Register offsets */
+#define ALTERA_WDT_STATUS	0x00
+#define ALTERA_WDT_CONTROL	0x04
+#define ALTERA_WDT_PERIODL	0x08
+#define ALTERA_WDT_PERIODH	0x0C
+
+#define ALTERA_WDT_RUN_BIT	0x04
+
+/* User land timeout */
+#define WDT_HEARTBEAT 15
+static int heartbeat = WDT_HEARTBEAT;
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
+	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
+
+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) ")");
+
+static struct {
+	void __iomem *base;
+	unsigned long wdt_timeout;	/* timeout of the hardware timer */
+
+	unsigned long next_heartbeat;	/* the next_heartbeat for the timer */
+	unsigned long is_open;
+	char expect_close;
+	struct timer_list timer;	/* software timer that pings the watchdog */
+} altera_wdt_priv;
+
+/*
+ * Start the watchdog. Once it has been started, it cannot be stopped anymore.
+ */
+static void altera_wdt_setup(void)
+{
+	u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
+
+	writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
+}
+
+/*
+ * Tickle the watchdog (reset the watchdog timer)
+ */
+static void altera_wdt_reset(void)
+{
+	/* It doesn't matter what value we write */
+	writel(1, altera_wdt_priv.base + ALTERA_WDT_PERIODL);
+}
+
+/*
+ * Software timer tick
+ */
+static void altera_wdt_ping(unsigned long data)
+{
+	if (time_before(jiffies, altera_wdt_priv.next_heartbeat) ||
+			(!nowayout && !altera_wdt_priv.is_open)) {
+		altera_wdt_reset();
+		mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+	} else
+		pr_crit(WATCHDOG_NAME ": I will reset your machine!\n");
+}
+
+static void altera_wdt_keepalive(void)
+{
+	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
+}
+
+static void altera_wdt_start(void)
+{
+	altera_wdt_keepalive();
+	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+}
+
+static int altera_wdt_open(struct inode *inode, struct file *file)
+{
+	/* /dev/watchdog can only be opened once */
+	if (test_and_set_bit(0, &altera_wdt_priv.is_open))
+		return -EBUSY;
+
+	altera_wdt_start();
+
+	return nonseekable_open(inode, file);
+}
+
+static int altera_wdt_release(struct inode *inode, struct file *file)
+{
+	/* stop internal ping */
+	if (!altera_wdt_priv.expect_close)
+		del_timer(&altera_wdt_priv.timer);
+
+	clear_bit(0, &altera_wdt_priv.is_open);
+	altera_wdt_priv.expect_close = 0;
+
+	return 0;
+}
+
+static ssize_t altera_wdt_write(struct file *file, const char __user *data,
+		size_t len, loff_t *ppos)
+{
+	if (!len)
+		return 0;
+
+	/* Scan for magic character */
+	if (!nowayout) {
+		size_t i;
+
+		altera_wdt_priv.expect_close = 0;
+
+		for (i = 0; i < len; i++) {
+			char c;
+			if (get_user(c, data + i))
+				return -EFAULT;
+			if (c == 'V') {
+				altera_wdt_priv.expect_close = 42;
+				break;
+			}
+		}
+	}
+
+	altera_wdt_keepalive();
+
+	return len;
+}
+
+static const struct watchdog_info altera_wdt_info = {
+	.identity		= "Altera Watchdog",
+	.options		= WDIOF_SETTIMEOUT |
+				  WDIOF_KEEPALIVEPING |
+				  WDIOF_MAGICCLOSE,
+	.firmware_version	= 1,
+};
+
+static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	void __user *argp = (void __user *) arg;
+	int __user *p = argp;
+	int new_value;
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
+
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+
+	case WDIOC_KEEPALIVE:
+		altera_wdt_keepalive();
+		return 0;
+
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_value, p))
+			return -EFAULT;
+
+		heartbeat = new_value;
+		altera_wdt_keepalive();
+
+		return put_user(new_value, p);  /* return current value */
+
+	case WDIOC_GETTIMEOUT:
+		return put_user(heartbeat, p);
+
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations altera_wdt_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.open		= altera_wdt_open,
+	.release	= altera_wdt_release,
+	.write		= altera_wdt_write,
+	.unlocked_ioctl	= altera_wdt_ioctl,
+};
+
+static struct miscdevice altera_wdt_miscdev = {
+	.minor	= WATCHDOG_MINOR,
+	.name	= "watchdog",
+	.fops	= &altera_wdt_fops,
+};
+
+static int __devinit altera_wdt_probe(struct platform_device *pdev)
+{
+	struct resource *res, *mem;
+	const __be32 *freq_prop, *timeout_prop;
+	unsigned long timeout;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	mem = devm_request_mem_region(&pdev->dev, res->start,
+				      resource_size(res), pdev->name);
+	if (!mem)
+		return -EBUSY;
+
+	altera_wdt_priv.base = devm_ioremap_nocache(&pdev->dev, mem->start,
+						    resource_size(mem));
+	if (!altera_wdt_priv.base)
+		return -ENOMEM;
+
+	freq_prop = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
+	if (!freq_prop)
+		return -ENODEV;
+
+	timeout_prop = of_get_property(pdev->dev.of_node, "timeout", NULL);
+	if (!timeout_prop)
+		return -ENODEV;
+
+	/* Add 1 as the timeout property actually holds the load value */
+	timeout = be32_to_cpup(timeout_prop) + 1;
+	/* Convert timeout to msecs */
+	timeout = timeout / (be32_to_cpup(freq_prop) / MSEC_PER_SEC);
+	/* Tickle the watchdog twice per timeout period */
+	altera_wdt_priv.wdt_timeout = msecs_to_jiffies(timeout / 2);
+
+	ret = misc_register(&altera_wdt_miscdev);
+	if (ret)
+		return ret;
+
+	altera_wdt_setup();
+	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
+	setup_timer(&altera_wdt_priv.timer, altera_wdt_ping, 0);
+	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
+
+	pr_info(WATCHDOG_NAME " enabled (heartbeat=%d sec, nowayout=%d)\n",
+		heartbeat, nowayout);
+
+	return 0;
+}
+
+static int __devexit altera_wdt_remove(struct platform_device *pdev)
+{
+	misc_deregister(&altera_wdt_miscdev);
+	return 0;
+}
+
+static struct of_device_id altera_wdt_match[] = {
+	{ .compatible = "altr,wdt-1.0", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, altera_wdt_match);
+
+static struct platform_driver altera_wdt_driver = {
+	.probe		= altera_wdt_probe,
+	.remove		= __devexit_p(altera_wdt_remove),
+	.driver		= {
+		.owner		= THIS_MODULE,
+		.name		= WATCHDOG_NAME,
+		.of_match_table	= altera_wdt_match,
+	},
+};
+
+static int __init altera_wdt_init(void)
+{
+	return platform_driver_register(&altera_wdt_driver);
+}
+
+static void __exit altera_wdt_exit(void)
+{
+	platform_driver_unregister(&altera_wdt_driver);
+}
+
+module_init(altera_wdt_init);
+module_exit(altera_wdt_exit);
+
+MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
+MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_ALIAS("platform:" WATCHDOG_NAME);
-- 
1.7.0.4

Re: [PATCH v3] watchdog: Add driver for Altera Watchdog Timer

[Belisko Marek]

On Wed, Feb 9, 2011 at 8:19 AM, Tobias Klauser <tklauser@distanz.ch> wrote:
> This driver adds support for the Altera Timer in the Watchdog Timer
> configuration. This component is usually found on SOPC (System on
> Programmable Chip) for Altera FPGAs.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> Reviewed-by: Jamie Iles <jamie@jamieiles.com>
> ---
> Thanks a lot to Jamie Iles and Walter Goossens for their feedback on v2.
>
> changes for v3:
>  - Make altera_wdt_setup static void (as suggested by Jamie)
>  - Use const __be32 * for devicetree properties (as suggested by Walter)
> changes for v2:
>  - Get driver properties from devicetree (and document them), thus the
>   driver depends on OF
>  - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
>   timer resetting the watchdog timer when the device is not open
>  - Only support a single instance of the watchdog, so the private data
>   can be kept in static data
>  - Use devm_* functions in altera_wdt_probe to simplify cleanup and
>   error handling
>  - Remove empty altera_wdt_shutdown function
>  - Add MODULE_LICENSE (GPL)
>
>
>  .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
>  drivers/watchdog/Kconfig                           |    8 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/altera_wdt.c                      |  309 ++++++++++++++++++++
>  4 files changed, 325 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
>  create mode 100644 drivers/watchdog/altera_wdt.c
>
> diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> new file mode 100644
> index 0000000..16ac949
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> @@ -0,0 +1,7 @@
> +Altera Watchdog Timer
> +
> +Required properties:
> +- compatible : should be "ALTR,wdt-1.0"
> +- clock-frequency : frequency of the clock input
> +- timeout : load value of the timer (number of clock ticks until watchdog
> +            resets)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bd6a33d..ebcafb0 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -69,6 +69,14 @@ config WM8350_WATCHDOG
>          Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>          the watchdog triggers the system will be reset.
>
> +config ALTERA_WDT
> +       tristate "Altera Watchdog Timer"
> +       depends on OF
> +       help
> +         Support for the Altera Timer in the Watchdog Timer configuration. This
> +         component is found on SOPC (System on Programmable Chip) for Altera
> +         FPGAs.
> +
>  # ALPHA Architecture
>
>  # ARM Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index fd9c8c0..48a2167 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)               += cpwd.o
>  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> +obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
> diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
> new file mode 100644
> index 0000000..2a7f14a
> --- /dev/null
> +++ b/drivers/watchdog/altera_wdt.c
> @@ -0,0 +1,309 @@
> +/*
> + * Driver for the Altera Watchdog Timer
> + *
> + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> + * Copyright (C) 2005 Walter Goossens
> + *
> + * Originally based on wdt.c which is
> + *
> + * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
> + *
> + * Software timeout heartbeat code based on pika_wdt.c which is
> + *
> + * Copyright (c) 2008 PIKA Technologies
> + *
> + * 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/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/timer.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/of_platform.h>
> +
> +#define WATCHDOG_NAME  "altera_wdt"
> +
> +/* Register offsets */
> +#define ALTERA_WDT_STATUS      0x00
> +#define ALTERA_WDT_CONTROL     0x04
> +#define ALTERA_WDT_PERIODL     0x08
> +#define ALTERA_WDT_PERIODH     0x0C
> +
> +#define ALTERA_WDT_RUN_BIT     0x04
> +
> +/* User land timeout */
> +#define WDT_HEARTBEAT 15
> +static int heartbeat = WDT_HEARTBEAT;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
> +       "(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
> +
> +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) ")");
> +
> +static struct {
> +       void __iomem *base;
> +       unsigned long wdt_timeout;      /* timeout of the hardware timer */
> +
> +       unsigned long next_heartbeat;   /* the next_heartbeat for the timer */
> +       unsigned long is_open;
> +       char expect_close;
> +       struct timer_list timer;        /* software timer that pings the watchdog */
> +} altera_wdt_priv;
> +
> +/*
> + * Start the watchdog. Once it has been started, it cannot be stopped anymore.
> + */
> +static void altera_wdt_setup(void)
> +{
> +       u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +
> +       writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +}
> +
> +/*
> + * Tickle the watchdog (reset the watchdog timer)
> + */
> +static void altera_wdt_reset(void)
> +{
> +       /* It doesn't matter what value we write */
> +       writel(1, altera_wdt_priv.base + ALTERA_WDT_PERIODL);
> +}
> +
> +/*
> + * Software timer tick
> + */
> +static void altera_wdt_ping(unsigned long data)
> +{
> +       if (time_before(jiffies, altera_wdt_priv.next_heartbeat) ||
> +                       (!nowayout && !altera_wdt_priv.is_open)) {
> +               altera_wdt_reset();
> +               mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +       } else
> +               pr_crit(WATCHDOG_NAME ": I will reset your machine!\n");
> +}
> +
> +static void altera_wdt_keepalive(void)
> +{
> +       altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> +}
> +
> +static void altera_wdt_start(void)
> +{
> +       altera_wdt_keepalive();
> +       mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +}
> +
> +static int altera_wdt_open(struct inode *inode, struct file *file)
> +{
> +       /* /dev/watchdog can only be opened once */
> +       if (test_and_set_bit(0, &altera_wdt_priv.is_open))
> +               return -EBUSY;
> +
> +       altera_wdt_start();
> +
> +       return nonseekable_open(inode, file);
> +}
> +
> +static int altera_wdt_release(struct inode *inode, struct file *file)
> +{
> +       /* stop internal ping */
> +       if (!altera_wdt_priv.expect_close)
> +               del_timer(&altera_wdt_priv.timer);
> +
> +       clear_bit(0, &altera_wdt_priv.is_open);
> +       altera_wdt_priv.expect_close = 0;
> +
> +       return 0;
> +}
> +
> +static ssize_t altera_wdt_write(struct file *file, const char __user *data,
> +               size_t len, loff_t *ppos)
> +{
> +       if (!len)
> +               return 0;
> +
> +       /* Scan for magic character */
> +       if (!nowayout) {
> +               size_t i;
> +
> +               altera_wdt_priv.expect_close = 0;
> +
> +               for (i = 0; i < len; i++) {
> +                       char c;
> +                       if (get_user(c, data + i))
> +                               return -EFAULT;
> +                       if (c == 'V') {
> +                               altera_wdt_priv.expect_close = 42;
For what this value is used? Why 42? It's never checked in code to such a value.
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       altera_wdt_keepalive();
> +
> +       return len;
> +}
> +
> +static const struct watchdog_info altera_wdt_info = {
> +       .identity               = "Altera Watchdog",
> +       .options                = WDIOF_SETTIMEOUT |
> +                                 WDIOF_KEEPALIVEPING |
> +                                 WDIOF_MAGICCLOSE,
> +       .firmware_version       = 1,
> +};
> +
> +static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
> +               unsigned long arg)
> +{
> +       void __user *argp = (void __user *) arg;
> +       int __user *p = argp;
> +       int new_value;
> +
> +       switch (cmd) {
> +       case WDIOC_GETSUPPORT:
> +               return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
> +
> +       case WDIOC_GETSTATUS:
> +       case WDIOC_GETBOOTSTATUS:
> +               return put_user(0, p);
> +
> +       case WDIOC_KEEPALIVE:
> +               altera_wdt_keepalive();
> +               return 0;
> +
> +       case WDIOC_SETTIMEOUT:
> +               if (get_user(new_value, p))
> +                       return -EFAULT;
> +
> +               heartbeat = new_value;
> +               altera_wdt_keepalive();
> +
> +               return put_user(new_value, p);  /* return current value */
> +
> +       case WDIOC_GETTIMEOUT:
> +               return put_user(heartbeat, p);
> +
> +       default:
> +               return -ENOTTY;
> +       }
> +}
> +
> +static const struct file_operations altera_wdt_fops = {
> +       .owner          = THIS_MODULE,
> +       .llseek         = no_llseek,
> +       .open           = altera_wdt_open,
> +       .release        = altera_wdt_release,
> +       .write          = altera_wdt_write,
> +       .unlocked_ioctl = altera_wdt_ioctl,
> +};
> +
> +static struct miscdevice altera_wdt_miscdev = {
> +       .minor  = WATCHDOG_MINOR,
> +       .name   = "watchdog",
> +       .fops   = &altera_wdt_fops,
> +};
> +
> +static int __devinit altera_wdt_probe(struct platform_device *pdev)
> +{
> +       struct resource *res, *mem;
> +       const __be32 *freq_prop, *timeout_prop;
> +       unsigned long timeout;
> +       int ret;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -ENOENT;
> +
> +       mem = devm_request_mem_region(&pdev->dev, res->start,
> +                                     resource_size(res), pdev->name);
> +       if (!mem)
> +               return -EBUSY;
> +
> +       altera_wdt_priv.base = devm_ioremap_nocache(&pdev->dev, mem->start,
> +                                                   resource_size(mem));
> +       if (!altera_wdt_priv.base)
> +               return -ENOMEM;
> +
> +       freq_prop = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> +       if (!freq_prop)
> +               return -ENODEV;
> +
> +       timeout_prop = of_get_property(pdev->dev.of_node, "timeout", NULL);
> +       if (!timeout_prop)
> +               return -ENODEV;
> +
> +       /* Add 1 as the timeout property actually holds the load value */
> +       timeout = be32_to_cpup(timeout_prop) + 1;
> +       /* Convert timeout to msecs */
> +       timeout = timeout / (be32_to_cpup(freq_prop) / MSEC_PER_SEC);
> +       /* Tickle the watchdog twice per timeout period */
> +       altera_wdt_priv.wdt_timeout = msecs_to_jiffies(timeout / 2);
> +
> +       ret = misc_register(&altera_wdt_miscdev);
> +       if (ret)
> +               return ret;
> +
> +       altera_wdt_setup();
> +       altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> +       setup_timer(&altera_wdt_priv.timer, altera_wdt_ping, 0);
> +       mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +
> +       pr_info(WATCHDOG_NAME " enabled (heartbeat=%d sec, nowayout=%d)\n",
> +               heartbeat, nowayout);
> +
> +       return 0;
> +}
> +
> +static int __devexit altera_wdt_remove(struct platform_device *pdev)
> +{
> +       misc_deregister(&altera_wdt_miscdev);
> +       return 0;
> +}
> +
> +static struct of_device_id altera_wdt_match[] = {
> +       { .compatible = "altr,wdt-1.0", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, altera_wdt_match);
> +
> +static struct platform_driver altera_wdt_driver = {
> +       .probe          = altera_wdt_probe,
> +       .remove         = __devexit_p(altera_wdt_remove),
> +       .driver         = {
> +               .owner          = THIS_MODULE,
> +               .name           = WATCHDOG_NAME,
> +               .of_match_table = altera_wdt_match,
> +       },
> +};
> +
> +static int __init altera_wdt_init(void)
> +{
> +       return platform_driver_register(&altera_wdt_driver);
> +}
> +
> +static void __exit altera_wdt_exit(void)
> +{
> +       platform_driver_unregister(&altera_wdt_driver);
> +}
> +
> +module_init(altera_wdt_init);
> +module_exit(altera_wdt_exit);
> +
> +MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
> +MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> +MODULE_ALIAS("platform:" WATCHDOG_NAME);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

Re: [PATCH v3] watchdog: Add driver for Altera Watchdog Timer

[Tobias Klauser]

On 2011-02-09 at 08:56:48 +0100, Belisko Marek <marek.belisko@gmail.com> wrote:
> On Wed, Feb 9, 2011 at 8:19 AM, Tobias Klauser <tklauser@distanz.ch> wrote:
> > This driver adds support for the Altera Timer in the Watchdog Timer
> > configuration. This component is usually found on SOPC (System on
> > Programmable Chip) for Altera FPGAs.
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > Reviewed-by: Jamie Iles <jamie@jamieiles.com>
> > ---
> > Thanks a lot to Jamie Iles and Walter Goossens for their feedback on v2.
> >
> > changes for v3:
> >  - Make altera_wdt_setup static void (as suggested by Jamie)
> >  - Use const __be32 * for devicetree properties (as suggested by Walter)
> > changes for v2:
> >  - Get driver properties from devicetree (and document them), thus the
> >   driver depends on OF
> >  - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
> >   timer resetting the watchdog timer when the device is not open
> >  - Only support a single instance of the watchdog, so the private data
> >   can be kept in static data
> >  - Use devm_* functions in altera_wdt_probe to simplify cleanup and
> >   error handling
> >  - Remove empty altera_wdt_shutdown function
> >  - Add MODULE_LICENSE (GPL)
> >
> >
> >  .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
> >  drivers/watchdog/Kconfig                           |    8 +
> >  drivers/watchdog/Makefile                          |    1 +
> >  drivers/watchdog/altera_wdt.c                      |  309 ++++++++++++++++++++
> >  4 files changed, 325 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> >  create mode 100644 drivers/watchdog/altera_wdt.c
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> > new file mode 100644
> > index 0000000..16ac949
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> > @@ -0,0 +1,7 @@
> > +Altera Watchdog Timer
> > +
> > +Required properties:
> > +- compatible : should be "ALTR,wdt-1.0"
> > +- clock-frequency : frequency of the clock input
> > +- timeout : load value of the timer (number of clock ticks until watchdog
> > +            resets)
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index bd6a33d..ebcafb0 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -69,6 +69,14 @@ config WM8350_WATCHDOG
> >          Support for the watchdog in the WM8350 AudioPlus PMIC.  When
> >          the watchdog triggers the system will be reset.
> >
> > +config ALTERA_WDT
> > +       tristate "Altera Watchdog Timer"
> > +       depends on OF
> > +       help
> > +         Support for the Altera Timer in the Watchdog Timer configuration. This
> > +         component is found on SOPC (System on Programmable Chip) for Altera
> > +         FPGAs.
> > +
> >  # ALPHA Architecture
> >
> >  # ARM Architecture
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index fd9c8c0..48a2167 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)               += cpwd.o
> >  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
> >  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
> >  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> > +obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
> >  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
> > diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
> > new file mode 100644
> > index 0000000..2a7f14a
> > --- /dev/null
> > +++ b/drivers/watchdog/altera_wdt.c
> > @@ -0,0 +1,309 @@
> > +/*
> > + * Driver for the Altera Watchdog Timer
> > + *
> > + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> > + * Copyright (C) 2005 Walter Goossens
> > + *
> > + * Originally based on wdt.c which is
> > + *
> > + * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
> > + *
> > + * Software timeout heartbeat code based on pika_wdt.c which is
> > + *
> > + * Copyright (c) 2008 PIKA Technologies
> > + *
> > + * 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/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/fs.h>
> > +#include <linux/init.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/timer.h>
> > +#include <linux/io.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/of_platform.h>
> > +
> > +#define WATCHDOG_NAME  "altera_wdt"
> > +
> > +/* Register offsets */
> > +#define ALTERA_WDT_STATUS      0x00
> > +#define ALTERA_WDT_CONTROL     0x04
> > +#define ALTERA_WDT_PERIODL     0x08
> > +#define ALTERA_WDT_PERIODH     0x0C
> > +
> > +#define ALTERA_WDT_RUN_BIT     0x04
> > +
> > +/* User land timeout */
> > +#define WDT_HEARTBEAT 15
> > +static int heartbeat = WDT_HEARTBEAT;
> > +module_param(heartbeat, int, 0);
> > +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
> > +       "(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
> > +
> > +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) ")");
> > +
> > +static struct {
> > +       void __iomem *base;
> > +       unsigned long wdt_timeout;      /* timeout of the hardware timer */
> > +
> > +       unsigned long next_heartbeat;   /* the next_heartbeat for the timer */
> > +       unsigned long is_open;
> > +       char expect_close;
> > +       struct timer_list timer;        /* software timer that pings the watchdog */
> > +} altera_wdt_priv;
> > +
> > +/*
> > + * Start the watchdog. Once it has been started, it cannot be stopped anymore.
> > + */
> > +static void altera_wdt_setup(void)
> > +{
> > +       u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> > +
> > +       writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> > +}
> > +
> > +/*
> > + * Tickle the watchdog (reset the watchdog timer)
> > + */
> > +static void altera_wdt_reset(void)
> > +{
> > +       /* It doesn't matter what value we write */
> > +       writel(1, altera_wdt_priv.base + ALTERA_WDT_PERIODL);
> > +}
> > +
> > +/*
> > + * Software timer tick
> > + */
> > +static void altera_wdt_ping(unsigned long data)
> > +{
> > +       if (time_before(jiffies, altera_wdt_priv.next_heartbeat) ||
> > +                       (!nowayout && !altera_wdt_priv.is_open)) {
> > +               altera_wdt_reset();
> > +               mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> > +       } else
> > +               pr_crit(WATCHDOG_NAME ": I will reset your machine!\n");
> > +}
> > +
> > +static void altera_wdt_keepalive(void)
> > +{
> > +       altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> > +}
> > +
> > +static void altera_wdt_start(void)
> > +{
> > +       altera_wdt_keepalive();
> > +       mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> > +}
> > +
> > +static int altera_wdt_open(struct inode *inode, struct file *file)
> > +{
> > +       /* /dev/watchdog can only be opened once */
> > +       if (test_and_set_bit(0, &altera_wdt_priv.is_open))
> > +               return -EBUSY;
> > +
> > +       altera_wdt_start();
> > +
> > +       return nonseekable_open(inode, file);
> > +}
> > +
> > +static int altera_wdt_release(struct inode *inode, struct file *file)
> > +{
> > +       /* stop internal ping */
> > +       if (!altera_wdt_priv.expect_close)
> > +               del_timer(&altera_wdt_priv.timer);
> > +
> > +       clear_bit(0, &altera_wdt_priv.is_open);
> > +       altera_wdt_priv.expect_close = 0;
> > +
> > +       return 0;
> > +}
> > +
> > +static ssize_t altera_wdt_write(struct file *file, const char __user *data,
> > +               size_t len, loff_t *ppos)
> > +{
> > +       if (!len)
> > +               return 0;
> > +
> > +       /* Scan for magic character */
> > +       if (!nowayout) {
> > +               size_t i;
> > +
> > +               altera_wdt_priv.expect_close = 0;
> > +
> > +               for (i = 0; i < len; i++) {
> > +                       char c;
> > +                       if (get_user(c, data + i))
> > +                               return -EFAULT;
> > +                       if (c == 'V') {
> > +                               altera_wdt_priv.expect_close = 42;
> For what this value is used? Why 42? It's never checked in code to such a value.

The value 42 is arbitrary, I used it because other watchdog drivers
implementing software timers to tickle the watchdog are using it that
way too. Its value is not explicitely checked for, it's only checked in
altera_wdt_release for being 0/not 0 to determine whether to stop the
software timer.

Thanks
Tobias

Re: [PATCH v3] watchdog: Add driver for Altera Watchdog Timer

[Grant Likely]

On Wed, Feb 09, 2011 at 08:19:12AM +0100, Tobias Klauser wrote:
> This driver adds support for the Altera Timer in the Watchdog Timer
> configuration. This component is usually found on SOPC (System on
> Programmable Chip) for Altera FPGAs.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> Reviewed-by: Jamie Iles <jamie@jamieiles.com>

Acked-by: Grant Likely <grant.likely@secretlab.ca> for the devicetree binding.

g.

> ---
> Thanks a lot to Jamie Iles and Walter Goossens for their feedback on v2.
> 
> changes for v3:
>  - Make altera_wdt_setup static void (as suggested by Jamie)
>  - Use const __be32 * for devicetree properties (as suggested by Walter)
> changes for v2:
>  - Get driver properties from devicetree (and document them), thus the
>    driver depends on OF
>  - Do not select WATCHDOG_NOWAYOUT, but instead implement a software
>    timer resetting the watchdog timer when the device is not open
>  - Only support a single instance of the watchdog, so the private data
>    can be kept in static data
>  - Use devm_* functions in altera_wdt_probe to simplify cleanup and
>    error handling
>  - Remove empty altera_wdt_shutdown function
>  - Add MODULE_LICENSE (GPL)
> 
> 
>  .../devicetree/bindings/watchdog/altera_wdt.txt    |    7 +
>  drivers/watchdog/Kconfig                           |    8 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/altera_wdt.c                      |  309 ++++++++++++++++++++
>  4 files changed, 325 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/altera_wdt.txt
>  create mode 100644 drivers/watchdog/altera_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/altera_wdt.txt b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> new file mode 100644
> index 0000000..16ac949
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/altera_wdt.txt
> @@ -0,0 +1,7 @@
> +Altera Watchdog Timer
> +
> +Required properties:
> +- compatible : should be "ALTR,wdt-1.0"
> +- clock-frequency : frequency of the clock input
> +- timeout : load value of the timer (number of clock ticks until watchdog
> +            resets)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bd6a33d..ebcafb0 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -69,6 +69,14 @@ config WM8350_WATCHDOG
>  	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>  	  the watchdog triggers the system will be reset.
>  
> +config ALTERA_WDT
> +	tristate "Altera Watchdog Timer"
> +	depends on OF
> +	help
> +	  Support for the Altera Timer in the Watchdog Timer configuration. This
> +	  component is found on SOPC (System on Programmable Chip) for Altera
> +	  FPGAs.
> +
>  # ALPHA Architecture
>  
>  # ARM Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index fd9c8c0..48a2167 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -153,4 +153,5 @@ obj-$(CONFIG_WATCHDOG_CP1XXX)		+= cpwd.o
>  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> +obj-$(CONFIG_ALTERA_WDT) += altera_wdt.o
>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
> diff --git a/drivers/watchdog/altera_wdt.c b/drivers/watchdog/altera_wdt.c
> new file mode 100644
> index 0000000..2a7f14a
> --- /dev/null
> +++ b/drivers/watchdog/altera_wdt.c
> @@ -0,0 +1,309 @@
> +/*
> + * Driver for the Altera Watchdog Timer
> + *
> + * Copyright (C) 2011 Tobias Klauser <tklauser@distanz.ch>
> + * Copyright (C) 2005 Walter Goossens
> + *
> + * Originally based on wdt.c which is
> + *
> + * Copyright (C) 1995-1997 Alan Cox <alan@lxorguk.ukuu.org.uk>
> + *
> + * Software timeout heartbeat code based on pika_wdt.c which is
> + *
> + * Copyright (c) 2008 PIKA Technologies
> + *
> + * 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/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/timer.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/of_platform.h>
> +
> +#define WATCHDOG_NAME	"altera_wdt"
> +
> +/* Register offsets */
> +#define ALTERA_WDT_STATUS	0x00
> +#define ALTERA_WDT_CONTROL	0x04
> +#define ALTERA_WDT_PERIODL	0x08
> +#define ALTERA_WDT_PERIODH	0x0C
> +
> +#define ALTERA_WDT_RUN_BIT	0x04
> +
> +/* User land timeout */
> +#define WDT_HEARTBEAT 15
> +static int heartbeat = WDT_HEARTBEAT;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
> +	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
> +
> +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) ")");
> +
> +static struct {
> +	void __iomem *base;
> +	unsigned long wdt_timeout;	/* timeout of the hardware timer */
> +
> +	unsigned long next_heartbeat;	/* the next_heartbeat for the timer */
> +	unsigned long is_open;
> +	char expect_close;
> +	struct timer_list timer;	/* software timer that pings the watchdog */
> +} altera_wdt_priv;
> +
> +/*
> + * Start the watchdog. Once it has been started, it cannot be stopped anymore.
> + */
> +static void altera_wdt_setup(void)
> +{
> +	u32 control = readl(altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +
> +	writel(control | ALTERA_WDT_RUN_BIT, altera_wdt_priv.base + ALTERA_WDT_CONTROL);
> +}
> +
> +/*
> + * Tickle the watchdog (reset the watchdog timer)
> + */
> +static void altera_wdt_reset(void)
> +{
> +	/* It doesn't matter what value we write */
> +	writel(1, altera_wdt_priv.base + ALTERA_WDT_PERIODL);
> +}
> +
> +/*
> + * Software timer tick
> + */
> +static void altera_wdt_ping(unsigned long data)
> +{
> +	if (time_before(jiffies, altera_wdt_priv.next_heartbeat) ||
> +			(!nowayout && !altera_wdt_priv.is_open)) {
> +		altera_wdt_reset();
> +		mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +	} else
> +		pr_crit(WATCHDOG_NAME ": I will reset your machine!\n");
> +}
> +
> +static void altera_wdt_keepalive(void)
> +{
> +	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> +}
> +
> +static void altera_wdt_start(void)
> +{
> +	altera_wdt_keepalive();
> +	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +}
> +
> +static int altera_wdt_open(struct inode *inode, struct file *file)
> +{
> +	/* /dev/watchdog can only be opened once */
> +	if (test_and_set_bit(0, &altera_wdt_priv.is_open))
> +		return -EBUSY;
> +
> +	altera_wdt_start();
> +
> +	return nonseekable_open(inode, file);
> +}
> +
> +static int altera_wdt_release(struct inode *inode, struct file *file)
> +{
> +	/* stop internal ping */
> +	if (!altera_wdt_priv.expect_close)
> +		del_timer(&altera_wdt_priv.timer);
> +
> +	clear_bit(0, &altera_wdt_priv.is_open);
> +	altera_wdt_priv.expect_close = 0;
> +
> +	return 0;
> +}
> +
> +static ssize_t altera_wdt_write(struct file *file, const char __user *data,
> +		size_t len, loff_t *ppos)
> +{
> +	if (!len)
> +		return 0;
> +
> +	/* Scan for magic character */
> +	if (!nowayout) {
> +		size_t i;
> +
> +		altera_wdt_priv.expect_close = 0;
> +
> +		for (i = 0; i < len; i++) {
> +			char c;
> +			if (get_user(c, data + i))
> +				return -EFAULT;
> +			if (c == 'V') {
> +				altera_wdt_priv.expect_close = 42;
> +				break;
> +			}
> +		}
> +	}
> +
> +	altera_wdt_keepalive();
> +
> +	return len;
> +}
> +
> +static const struct watchdog_info altera_wdt_info = {
> +	.identity		= "Altera Watchdog",
> +	.options		= WDIOF_SETTIMEOUT |
> +				  WDIOF_KEEPALIVEPING |
> +				  WDIOF_MAGICCLOSE,
> +	.firmware_version	= 1,
> +};
> +
> +static long altera_wdt_ioctl(struct file *file, unsigned int cmd,
> +		unsigned long arg)
> +{
> +	void __user *argp = (void __user *) arg;
> +	int __user *p = argp;
> +	int new_value;
> +
> +	switch (cmd) {
> +	case WDIOC_GETSUPPORT:
> +		return copy_to_user(argp, &altera_wdt_info, sizeof(altera_wdt_info));
> +
> +	case WDIOC_GETSTATUS:
> +	case WDIOC_GETBOOTSTATUS:
> +		return put_user(0, p);
> +
> +	case WDIOC_KEEPALIVE:
> +		altera_wdt_keepalive();
> +		return 0;
> +
> +	case WDIOC_SETTIMEOUT:
> +		if (get_user(new_value, p))
> +			return -EFAULT;
> +
> +		heartbeat = new_value;
> +		altera_wdt_keepalive();
> +
> +		return put_user(new_value, p);  /* return current value */
> +
> +	case WDIOC_GETTIMEOUT:
> +		return put_user(heartbeat, p);
> +
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
> +static const struct file_operations altera_wdt_fops = {
> +	.owner		= THIS_MODULE,
> +	.llseek		= no_llseek,
> +	.open		= altera_wdt_open,
> +	.release	= altera_wdt_release,
> +	.write		= altera_wdt_write,
> +	.unlocked_ioctl	= altera_wdt_ioctl,
> +};
> +
> +static struct miscdevice altera_wdt_miscdev = {
> +	.minor	= WATCHDOG_MINOR,
> +	.name	= "watchdog",
> +	.fops	= &altera_wdt_fops,
> +};
> +
> +static int __devinit altera_wdt_probe(struct platform_device *pdev)
> +{
> +	struct resource *res, *mem;
> +	const __be32 *freq_prop, *timeout_prop;
> +	unsigned long timeout;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENOENT;
> +
> +	mem = devm_request_mem_region(&pdev->dev, res->start,
> +				      resource_size(res), pdev->name);
> +	if (!mem)
> +		return -EBUSY;
> +
> +	altera_wdt_priv.base = devm_ioremap_nocache(&pdev->dev, mem->start,
> +						    resource_size(mem));
> +	if (!altera_wdt_priv.base)
> +		return -ENOMEM;
> +
> +	freq_prop = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> +	if (!freq_prop)
> +		return -ENODEV;
> +
> +	timeout_prop = of_get_property(pdev->dev.of_node, "timeout", NULL);
> +	if (!timeout_prop)
> +		return -ENODEV;
> +
> +	/* Add 1 as the timeout property actually holds the load value */
> +	timeout = be32_to_cpup(timeout_prop) + 1;
> +	/* Convert timeout to msecs */
> +	timeout = timeout / (be32_to_cpup(freq_prop) / MSEC_PER_SEC);
> +	/* Tickle the watchdog twice per timeout period */
> +	altera_wdt_priv.wdt_timeout = msecs_to_jiffies(timeout / 2);
> +
> +	ret = misc_register(&altera_wdt_miscdev);
> +	if (ret)
> +		return ret;
> +
> +	altera_wdt_setup();
> +	altera_wdt_priv.next_heartbeat = jiffies + heartbeat * HZ;
> +	setup_timer(&altera_wdt_priv.timer, altera_wdt_ping, 0);
> +	mod_timer(&altera_wdt_priv.timer, jiffies + altera_wdt_priv.wdt_timeout);
> +
> +	pr_info(WATCHDOG_NAME " enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		heartbeat, nowayout);
> +
> +	return 0;
> +}
> +
> +static int __devexit altera_wdt_remove(struct platform_device *pdev)
> +{
> +	misc_deregister(&altera_wdt_miscdev);
> +	return 0;
> +}
> +
> +static struct of_device_id altera_wdt_match[] = {
> +	{ .compatible = "altr,wdt-1.0", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, altera_wdt_match);
> +
> +static struct platform_driver altera_wdt_driver = {
> +	.probe		= altera_wdt_probe,
> +	.remove		= __devexit_p(altera_wdt_remove),
> +	.driver		= {
> +		.owner		= THIS_MODULE,
> +		.name		= WATCHDOG_NAME,
> +		.of_match_table	= altera_wdt_match,
> +	},
> +};
> +
> +static int __init altera_wdt_init(void)
> +{
> +	return platform_driver_register(&altera_wdt_driver);
> +}
> +
> +static void __exit altera_wdt_exit(void)
> +{
> +	platform_driver_unregister(&altera_wdt_driver);
> +}
> +
> +module_init(altera_wdt_init);
> +module_exit(altera_wdt_exit);
> +
> +MODULE_AUTHOR("Walter Goossens, Tobias Klauser <tklauser@distanz.ch>");
> +MODULE_DESCRIPTION("Driver for Altera Watchdog Timer");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> +MODULE_ALIAS("platform:" WATCHDOG_NAME);
> -- 
> 1.7.0.4
>