[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Viresh KUMAR]

Technical Reference Manual can be found at:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/watchdog/Kconfig     |    7 +
 drivers/watchdog/Makefile    |    1 +
 drivers/watchdog/sp805_wdt.c |  386 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 394 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/sp805_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index b87ba23..2759a91 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -73,6 +73,13 @@ config WM8350_WATCHDOG
 
 # ARM Architecture
 
+config ARM_SP805_WATCHDOG
+	tristate "ARM SP805 Watchdog"
+	depends on ARM_AMBA
+	help
+	  ARM Primecell SP805 Watchdog timer. This will reboot your system when
+	  the timeout is reached.
+
 config AT91RM9200_WATCHDOG
 	tristate "AT91RM9200 watchdog"
 	depends on ARCH_AT91RM9200
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5e3cb95..28af87a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
 # ALPHA Architecture
 
 # ARM Architecture
+obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
 obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
 obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
 obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
new file mode 100644
index 0000000..85d86ac
--- /dev/null
+++ b/drivers/watchdog/sp805_wdt.c
@@ -0,0 +1,386 @@
+/*
+ * drivers/char/watchdog/sp805-wdt.c
+ *
+ * Watchdog driver for ARM SP805 watchdog module
+ *
+ * Copyright (C) 2010 ST Microelectronics
+ * Viresh Kumar<viresh.kumar@st.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2 or later. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/device.h>
+#include <linux/resource.h>
+#include <linux/amba/bus.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/watchdog.h>
+
+/* default timeout in seconds */
+#define DEFAULT_TIMEOUT		60
+
+#define MODULE_NAME		"sp805-wdt"
+
+/* watchdog register offsets and masks */
+#define WDTLOAD			0x000
+	#define LOAD_MIN	0x00000001
+	#define LOAD_MAX	0xFFFFFFFF
+#define WDTVALUE		0x004
+#define WDTCONTROL		0x008
+	/* control register masks */
+	#define	INT_ENABLE	(1 << 0)
+	#define	RESET_ENABLE	(1 << 1)
+#define WDTINTCLR		0x00C
+#define WDTRIS			0x010
+#define WDTMIS			0x014
+	#define INT_MASK	(1 << 0)
+#define WDTLOCK			0xC00
+	#define	UNLOCK		0x1ACCE551
+	#define	LOCK		0x00000001
+
+/**
+ * struct sp805_wdt: sp805 wdt device structure
+ *
+ * lock: spin lock protecting dev structure and io access
+ * base: base address of wdt
+ * clk: clock structure of wdt
+ * dev: amba device structure of wdt
+ * status: current status of wdt
+ * load_val: load value to be set for current timeout
+ * timeout: current programmed timeout
+ */
+struct sp805_wdt {
+	spinlock_t			lock;
+	void __iomem			*base;
+	struct clk			*clk;
+	struct amba_device		*adev;
+	unsigned long			status;
+	#define WDT_BUSY		0
+	#define WDT_CAN_BE_CLOSED	1
+	unsigned int			load_val;
+	unsigned int			timeout;
+};
+
+/* local variables */
+static struct sp805_wdt *wdt;
+static int nowayout = WATCHDOG_NOWAYOUT;
+
+/* This routine finds load value that will reset system in required timout */
+static void wdt_setload(unsigned int timeout)
+{
+	u64 load, rate;
+
+	rate = clk_get_rate(wdt->clk);
+
+	/*
+	 * sp805 runs counter with given value twice, after the end of first
+	 * counter it gives an interrupt and then starts counter again. If
+	 * interrupt already occured then it resets the system. This is why
+	 * load is half of what should be required.
+	 */
+	load = div_u64(rate, 2) * timeout - 1;
+
+	load = (load > LOAD_MAX) ? LOAD_MAX : load;
+	load = (load < LOAD_MIN) ? LOAD_MIN : load;
+
+	spin_lock(&wdt->lock);
+	wdt->load_val = load;
+	/* roundup timeout to closest positive integer value */
+	wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
+	spin_unlock(&wdt->lock);
+}
+
+/* returns number of seconds left for reset to occur */
+static u32 wdt_timeleft(void)
+{
+	u64 load, rate;
+
+	rate = clk_get_rate(wdt->clk);
+
+	spin_lock(&wdt->lock);
+	load = readl(wdt->base + WDTVALUE);
+
+	/*If the interrupt is inactive then time left is WDTValue + WDTLoad. */
+	if (!(readl(wdt->base + WDTRIS) & INT_MASK))
+		load += wdt->load_val + 1;
+	spin_unlock(&wdt->lock);
+
+	return div_u64(load, rate);
+}
+
+/* enables watchdog timers reset */
+static void wdt_enable(void)
+{
+	spin_lock(&wdt->lock);
+
+	writel(UNLOCK, wdt->base + WDTLOCK);
+	writel(wdt->load_val, wdt->base + WDTLOAD);
+	writel(INT_MASK, wdt->base + WDTINTCLR);
+	writel(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+	writel(LOCK, wdt->base + WDTLOCK);
+
+	spin_unlock(&wdt->lock);
+}
+
+/* disables watchdog timers reset */
+static void wdt_disable(void)
+{
+	spin_lock(&wdt->lock);
+
+	writel(UNLOCK, wdt->base + WDTLOCK);
+	writel(0, wdt->base + WDTCONTROL);
+	writel(0, wdt->base + WDTLOAD);
+	writel(LOCK, wdt->base + WDTLOCK);
+
+	spin_unlock(&wdt->lock);
+}
+
+static ssize_t sp805_wdt_write(struct file *file, const char *data,
+		size_t len, loff_t *ppos)
+{
+	if (len) {
+		if (!nowayout) {
+			size_t i;
+
+			clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
+
+			for (i = 0; i != len; i++) {
+				char c;
+
+				if (get_user(c, data + i))
+					return -EFAULT;
+				/* Check for Magic Close character */
+				if (c == 'V') {
+					set_bit(WDT_CAN_BE_CLOSED,
+							&wdt->status);
+					break;
+				}
+			}
+		}
+		wdt_enable();
+	}
+	return len;
+}
+
+static const struct watchdog_info ident = {
+	.options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = MODULE_NAME,
+};
+
+static long sp805_wdt_ioctl(struct file *file, unsigned int cmd,
+		unsigned long arg)
+{
+	int ret = -ENOTTY;
+	unsigned int timeout;
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		ret = copy_to_user((struct watchdog_info *)arg, &ident,
+				sizeof(ident)) ? -EFAULT : 0;
+		break;
+
+	case WDIOC_GETSTATUS:
+		ret = put_user(0, (int *)arg);
+		break;
+
+	case WDIOC_KEEPALIVE:
+		wdt_enable();
+		ret = 0;
+		break;
+
+	case WDIOC_SETTIMEOUT:
+		ret = get_user(timeout, (unsigned int *)arg);
+		if (ret)
+			break;
+
+		wdt_setload(timeout);
+
+		wdt_enable();
+		/* Fall through */
+
+	case WDIOC_GETTIMEOUT:
+		ret = put_user(wdt->timeout, (unsigned int *)arg);
+		break;
+	case WDIOC_GETTIMELEFT:
+		ret = put_user(wdt_timeleft(), (unsigned int *)arg);
+		break;
+	}
+	return ret;
+}
+
+static int sp805_wdt_open(struct inode *inode, struct file *file)
+{
+	int ret = 0;
+
+	if (test_and_set_bit(WDT_BUSY, &wdt->status))
+		return -EBUSY;
+
+	ret = clk_enable(wdt->clk);
+	if (ret) {
+		dev_err(&wdt->adev->dev, "clock enable fail");
+		goto err;
+	}
+
+	wdt_enable();
+
+	/* can not be closed, once enabled */
+	clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
+	return nonseekable_open(inode, file);
+
+err:
+	clear_bit(WDT_BUSY, &wdt->status);
+	return ret;
+}
+
+static int sp805_wdt_release(struct inode *inode, struct file *file)
+{
+	if (!test_bit(WDT_CAN_BE_CLOSED, &wdt->status)) {
+		clear_bit(WDT_BUSY, &wdt->status);
+		dev_warn(&wdt->adev->dev, "Device closed unexpectedly\n");
+		return 0;
+	}
+
+	wdt_disable();
+	clk_disable(wdt->clk);
+	clear_bit(WDT_BUSY, &wdt->status);
+
+	return 0;
+}
+
+static const struct file_operations sp805_wdt_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.write = sp805_wdt_write,
+	.unlocked_ioctl = sp805_wdt_ioctl,
+	.open = sp805_wdt_open,
+	.release = sp805_wdt_release,
+};
+
+static struct miscdevice sp805_wdt_miscdev = {
+	.minor = WATCHDOG_MINOR,
+	.name = "watchdog",
+	.fops = &sp805_wdt_fops,
+};
+
+static int __devinit
+sp805_wdt_probe(struct amba_device *adev, struct amba_id *id)
+{
+	int ret = 0;
+
+	if (!request_mem_region(adev->res.start, resource_size(&adev->res),
+				"sp805_wdt")) {
+		dev_warn(&adev->dev, "Failed to get memory region resource\n");
+		ret = -ENOENT;
+		goto err;
+	}
+
+	wdt = kzalloc(sizeof(*wdt), GFP_KERNEL);
+	if (!wdt) {
+		dev_warn(&adev->dev, "Kzalloc failed\n");
+		ret = -ENOMEM;
+		goto err_kzalloc;
+	}
+
+	wdt->clk = clk_get(&adev->dev, NULL);
+	if (IS_ERR(wdt->clk)) {
+		dev_warn(&adev->dev, "Clock not found\n");
+		ret = PTR_ERR(wdt->clk);
+		goto err_clk_get;
+	}
+
+	wdt->base = ioremap(adev->res.start, resource_size(&adev->res));
+	if (!wdt->base) {
+		ret = -ENOMEM;
+		dev_warn(&adev->dev, "ioremap fail\n");
+		goto err_ioremap;
+	}
+
+	wdt->adev = adev;
+	spin_lock_init(&wdt->lock);
+	wdt_setload(DEFAULT_TIMEOUT);
+
+	ret = misc_register(&sp805_wdt_miscdev);
+	if (ret < 0) {
+		dev_warn(&adev->dev, "cannot register misc device\n");
+		goto err_misc_register;
+	}
+
+	dev_info(&adev->dev, "registration successful\n");
+	return 0;
+
+err_misc_register:
+	iounmap(wdt->base);
+err_ioremap:
+	clk_put(wdt->clk);
+err_clk_get:
+	kfree(wdt);
+	wdt = NULL;
+err_kzalloc:
+	release_mem_region(adev->res.start, resource_size(&adev->res));
+err:
+	dev_err(&adev->dev, "Probe Failed!!!\n");
+	return ret;
+}
+
+static int __devexit sp805_wdt_remove(struct amba_device *adev)
+{
+	misc_deregister(&sp805_wdt_miscdev);
+	iounmap(wdt->base);
+	clk_put(wdt->clk);
+	kfree(wdt);
+	release_mem_region(adev->res.start, resource_size(&adev->res));
+
+	return 0;
+}
+
+static struct amba_id sp805_wdt_ids[] __initdata = {
+	{
+		.id	= 0x00141805,
+		.mask	= 0x00ffffff,
+	},
+	{ 0, 0 },
+};
+
+static struct amba_driver sp805_wdt_driver = {
+	.drv = {
+		.name	= MODULE_NAME,
+	},
+	.id_table	= sp805_wdt_ids,
+	.probe		= sp805_wdt_probe,
+	.remove = __devexit_p(sp805_wdt_remove),
+};
+
+static int __init sp805_wdt_init(void)
+{
+	return amba_driver_register(&sp805_wdt_driver);
+}
+module_init(sp805_wdt_init);
+
+static void __exit sp805_wdt_exit(void)
+{
+	amba_driver_unregister(&sp805_wdt_driver);
+}
+module_exit(sp805_wdt_exit);
+
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout,
+		"Set to 1 to keep watchdog running after device release");
+
+MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
+MODULE_DESCRIPTION("ARM SP805 Watchdog Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.6.0.2

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Russell King - ARM Linux]

On Thu, May 20, 2010 at 02:03:48PM +0530, Viresh KUMAR wrote:
> Technical Reference Manual can be found at:
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf

Please include Wim Van Sebroeck for watchdog stuff - his address is in
MAINTAINERS.  Please also ensure that in future you're sending your
patches to relevant people - there isn't the capacity to review
every bit of code on this list.

There is a script - scripts/get_maintainer.pl - to help with this.

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Viresh KUMAR]

On 6/2/2010 1:18 PM, Russell King - ARM Linux wrote:
> On Thu, May 20, 2010 at 02:03:48PM +0530, Viresh KUMAR wrote:
>> Technical Reference Manual can be found at:
>> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf
> 
> Please include Wim Van Sebroeck for watchdog stuff - his address is in
> MAINTAINERS.  Please also ensure that in future you're sending your
> patches to relevant people - there isn't the capacity to review
> every bit of code on this list.
> 

Thanks Russell.

I will send patch to Wim Van.

viresh

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Linus Walleij]

Sorry for late review.

I was bored so I compiled this driver in the latest -next
and tested on ARM PB1176JZF-S that I've brought for
entertainment this vacation...

First: add
#include <linux/slab.h>
or compilation fails on newer codebases.

Then I get this:
sp805-wdt dev:wdog: Clock not found
sp805-wdt dev:wdog: Probe Failed!!!
sp805-wdt: probe of dev:wdog failed with error -2

So I added it to the realviews, even though the clocking
of the watchdog is not mentioned in the reference manual
it appears to be using the same 24MHz clock as the UARTs
and the RTC. (ARM people, can you confirm this?)

After this it seems to work...
Tested-by: Linus Walleij <linus.walleij@stericsson.com>

2010/5/20 Viresh KUMAR <viresh.kumar@st.com>:
> Technical Reference Manual can be found at:
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf
>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
> ?drivers/watchdog/Kconfig ? ? | ? ?7 +
> ?drivers/watchdog/Makefile ? ?| ? ?1 +
> ?drivers/watchdog/sp805_wdt.c | ?386 ++++++++++++++++++++++++++++++++++++++++++
> ?3 files changed, 394 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/watchdog/sp805_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index b87ba23..2759a91 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -73,6 +73,13 @@ config WM8350_WATCHDOG
>
> ?# ARM Architecture
>
> +config ARM_SP805_WATCHDOG
> + ? ? ? tristate "ARM SP805 Watchdog"

Why would you want this as a module... Can't it
be just bool?

> + ? ? ? depends on ARM_AMBA
> + ? ? ? help
> + ? ? ? ? ARM Primecell SP805 Watchdog timer. This will reboot your system when
> + ? ? ? ? the timeout is reached.
> +
> ?config AT91RM9200_WATCHDOG
> ? ? ? ?tristate "AT91RM9200 watchdog"
> ? ? ? ?depends on ARCH_AT91RM9200
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5e3cb95..28af87a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
> ?# ALPHA Architecture
>
> ?# ARM Architecture
> +obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
> ?obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
> ?obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
> ?obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> new file mode 100644
> index 0000000..85d86ac
> --- /dev/null
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -0,0 +1,386 @@
> +/*
> + * drivers/char/watchdog/sp805-wdt.c
> + *
> + * Watchdog driver for ARM SP805 watchdog module
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2 or later. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/resource.h>
> +#include <linux/amba/bus.h>
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/math64.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/watchdog.h>

#include <linux/slab.h>

> +
> +/* default timeout in seconds */
> +#define DEFAULT_TIMEOUT ? ? ? ? ? ? ? ?60
> +
> +#define MODULE_NAME ? ? ? ? ? ?"sp805-wdt"
> +
> +/* watchdog register offsets and masks */
> +#define WDTLOAD ? ? ? ? ? ? ? ? ? ? ? ?0x000
> + ? ? ? #define LOAD_MIN ? ? ? ?0x00000001
> + ? ? ? #define LOAD_MAX ? ? ? ?0xFFFFFFFF

I haven't seen this style of indenting the #defines for
stuff you can put into the registers before, but I sort of
like it. Is it common in the kernel?

> +#define WDTVALUE ? ? ? ? ? ? ? 0x004
> +#define WDTCONTROL ? ? ? ? ? ? 0x008
> + ? ? ? /* control register masks */
> + ? ? ? #define INT_ENABLE ? ? ?(1 << 0)
> + ? ? ? #define RESET_ENABLE ? ?(1 << 1)
> +#define WDTINTCLR ? ? ? ? ? ? ?0x00C
> +#define WDTRIS ? ? ? ? ? ? ? ? 0x010
> +#define WDTMIS ? ? ? ? ? ? ? ? 0x014
> + ? ? ? #define INT_MASK ? ? ? ?(1 << 0)
> +#define WDTLOCK ? ? ? ? ? ? ? ? ? ? ? ?0xC00
> + ? ? ? #define UNLOCK ? ? ? ? ?0x1ACCE551
> + ? ? ? #define LOCK ? ? ? ? ? ?0x00000001
> +
> +/**
> + * struct sp805_wdt: sp805 wdt device structure
> + *
> + * lock: spin lock protecting dev structure and io access
> + * base: base address of wdt
> + * clk: clock structure of wdt
> + * dev: amba device structure of wdt
> + * status: current status of wdt
> + * load_val: load value to be set for current timeout
> + * timeout: current programmed timeout
> + */
> +struct sp805_wdt {
> + ? ? ? spinlock_t ? ? ? ? ? ? ? ? ? ? ?lock;
> + ? ? ? void __iomem ? ? ? ? ? ? ? ? ? ?*base;
> + ? ? ? struct clk ? ? ? ? ? ? ? ? ? ? ?*clk;
> + ? ? ? struct amba_device ? ? ? ? ? ? ?*adev;
> + ? ? ? unsigned long ? ? ? ? ? ? ? ? ? status;
> + ? ? ? #define WDT_BUSY ? ? ? ? ? ? ? ?0
> + ? ? ? #define WDT_CAN_BE_CLOSED ? ? ? 1
> + ? ? ? unsigned int ? ? ? ? ? ? ? ? ? ?load_val;
> + ? ? ? unsigned int ? ? ? ? ? ? ? ? ? ?timeout;
> +};
> +
> +/* local variables */
> +static struct sp805_wdt *wdt;
> +static int nowayout = WATCHDOG_NOWAYOUT;
> +
> +/* This routine finds load value that will reset system in required timout */
> +static void wdt_setload(unsigned int timeout)
> +{
> + ? ? ? u64 load, rate;
> +
> + ? ? ? rate = clk_get_rate(wdt->clk);
> +
> + ? ? ? /*
> + ? ? ? ?* sp805 runs counter with given value twice, after the end of first
> + ? ? ? ?* counter it gives an interrupt and then starts counter again. If
> + ? ? ? ?* interrupt already occured then it resets the system. This is why
> + ? ? ? ?* load is half of what should be required.
> + ? ? ? ?*/
> + ? ? ? load = div_u64(rate, 2) * timeout - 1;
> +
> + ? ? ? load = (load > LOAD_MAX) ? LOAD_MAX : load;
> + ? ? ? load = (load < LOAD_MIN) ? LOAD_MIN : load;
> +
> + ? ? ? spin_lock(&wdt->lock);
> + ? ? ? wdt->load_val = load;
> + ? ? ? /* roundup timeout to closest positive integer value */
> + ? ? ? wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);

Look in linux/kernel.h, use the
DIV_ROUND_CLOSEST() macro instead of this.

> + ? ? ? spin_unlock(&wdt->lock);
> +}
> +
> +/* returns number of seconds left for reset to occur */
> +static u32 wdt_timeleft(void)
> +{
> + ? ? ? u64 load, rate;
> +
> + ? ? ? rate = clk_get_rate(wdt->clk);
> +
> + ? ? ? spin_lock(&wdt->lock);
> + ? ? ? load = readl(wdt->base + WDTVALUE);
> +
> + ? ? ? /*If the interrupt is inactive then time left is WDTValue + WDTLoad. */
> + ? ? ? if (!(readl(wdt->base + WDTRIS) & INT_MASK))
> + ? ? ? ? ? ? ? load += wdt->load_val + 1;
> + ? ? ? spin_unlock(&wdt->lock);
> +
> + ? ? ? return div_u64(load, rate);
> +}
> +
> +/* enables watchdog timers reset */
> +static void wdt_enable(void)
> +{
> + ? ? ? spin_lock(&wdt->lock);
> +
> + ? ? ? writel(UNLOCK, wdt->base + WDTLOCK);
> + ? ? ? writel(wdt->load_val, wdt->base + WDTLOAD);
> + ? ? ? writel(INT_MASK, wdt->base + WDTINTCLR);
> + ? ? ? writel(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
> + ? ? ? writel(LOCK, wdt->base + WDTLOCK);
> +
> + ? ? ? spin_unlock(&wdt->lock);
> +}
> +
> +/* disables watchdog timers reset */
> +static void wdt_disable(void)
> +{
> + ? ? ? spin_lock(&wdt->lock);
> +
> + ? ? ? writel(UNLOCK, wdt->base + WDTLOCK);
> + ? ? ? writel(0, wdt->base + WDTCONTROL);
> + ? ? ? writel(0, wdt->base + WDTLOAD);
> + ? ? ? writel(LOCK, wdt->base + WDTLOCK);
> +
> + ? ? ? spin_unlock(&wdt->lock);
> +}
> +
> +static ssize_t sp805_wdt_write(struct file *file, const char *data,
> + ? ? ? ? ? ? ? size_t len, loff_t *ppos)
> +{
> + ? ? ? if (len) {
> + ? ? ? ? ? ? ? if (!nowayout) {
> + ? ? ? ? ? ? ? ? ? ? ? size_t i;
> +
> + ? ? ? ? ? ? ? ? ? ? ? clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
> +
> + ? ? ? ? ? ? ? ? ? ? ? for (i = 0; i != len; i++) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? char c;
> +
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (get_user(c, data + i))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* Check for Magic Close character */
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (c == 'V') {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? set_bit(WDT_CAN_BE_CLOSED,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &wdt->status);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? wdt_enable();
> + ? ? ? }
> + ? ? ? return len;
> +}
> +
> +static const struct watchdog_info ident = {
> + ? ? ? .options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
> + ? ? ? .identity = MODULE_NAME,
> +};
> +
> +static long sp805_wdt_ioctl(struct file *file, unsigned int cmd,
> + ? ? ? ? ? ? ? unsigned long arg)
> +{
> + ? ? ? int ret = -ENOTTY;
> + ? ? ? unsigned int timeout;
> +
> + ? ? ? switch (cmd) {
> + ? ? ? case WDIOC_GETSUPPORT:
> + ? ? ? ? ? ? ? ret = copy_to_user((struct watchdog_info *)arg, &ident,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(ident)) ? -EFAULT : 0;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case WDIOC_GETSTATUS:
> + ? ? ? ? ? ? ? ret = put_user(0, (int *)arg);
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case WDIOC_KEEPALIVE:
> + ? ? ? ? ? ? ? wdt_enable();
> + ? ? ? ? ? ? ? ret = 0;
> + ? ? ? ? ? ? ? break;
> +
> + ? ? ? case WDIOC_SETTIMEOUT:
> + ? ? ? ? ? ? ? ret = get_user(timeout, (unsigned int *)arg);
> + ? ? ? ? ? ? ? if (ret)
> + ? ? ? ? ? ? ? ? ? ? ? break;
> +
> + ? ? ? ? ? ? ? wdt_setload(timeout);
> +
> + ? ? ? ? ? ? ? wdt_enable();
> + ? ? ? ? ? ? ? /* Fall through */
> +
> + ? ? ? case WDIOC_GETTIMEOUT:
> + ? ? ? ? ? ? ? ret = put_user(wdt->timeout, (unsigned int *)arg);
> + ? ? ? ? ? ? ? break;
> + ? ? ? case WDIOC_GETTIMELEFT:
> + ? ? ? ? ? ? ? ret = put_user(wdt_timeleft(), (unsigned int *)arg);
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
> + ? ? ? return ret;
> +}
> +
> +static int sp805_wdt_open(struct inode *inode, struct file *file)
> +{
> + ? ? ? int ret = 0;
> +
> + ? ? ? if (test_and_set_bit(WDT_BUSY, &wdt->status))
> + ? ? ? ? ? ? ? return -EBUSY;
> +
> + ? ? ? ret = clk_enable(wdt->clk);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(&wdt->adev->dev, "clock enable fail");
> + ? ? ? ? ? ? ? goto err;
> + ? ? ? }
> +
> + ? ? ? wdt_enable();
> +
> + ? ? ? /* can not be closed, once enabled */
> + ? ? ? clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
> + ? ? ? return nonseekable_open(inode, file);
> +
> +err:
> + ? ? ? clear_bit(WDT_BUSY, &wdt->status);
> + ? ? ? return ret;
> +}
> +
> +static int sp805_wdt_release(struct inode *inode, struct file *file)
> +{
> + ? ? ? if (!test_bit(WDT_CAN_BE_CLOSED, &wdt->status)) {
> + ? ? ? ? ? ? ? clear_bit(WDT_BUSY, &wdt->status);
> + ? ? ? ? ? ? ? dev_warn(&wdt->adev->dev, "Device closed unexpectedly\n");
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? }
> +
> + ? ? ? wdt_disable();
> + ? ? ? clk_disable(wdt->clk);
> + ? ? ? clear_bit(WDT_BUSY, &wdt->status);
> +
> + ? ? ? return 0;
> +}
> +
> +static const struct file_operations sp805_wdt_fops = {
> + ? ? ? .owner = THIS_MODULE,
> + ? ? ? .llseek = no_llseek,
> + ? ? ? .write = sp805_wdt_write,
> + ? ? ? .unlocked_ioctl = sp805_wdt_ioctl,
> + ? ? ? .open = sp805_wdt_open,
> + ? ? ? .release = sp805_wdt_release,
> +};
> +
> +static struct miscdevice sp805_wdt_miscdev = {
> + ? ? ? .minor = WATCHDOG_MINOR,
> + ? ? ? .name = "watchdog",
> + ? ? ? .fops = &sp805_wdt_fops,
> +};
> +
> +static int __devinit
> +sp805_wdt_probe(struct amba_device *adev, struct amba_id *id)
> +{
> + ? ? ? int ret = 0;
> +
> + ? ? ? if (!request_mem_region(adev->res.start, resource_size(&adev->res),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "sp805_wdt")) {
> + ? ? ? ? ? ? ? dev_warn(&adev->dev, "Failed to get memory region resource\n");
> + ? ? ? ? ? ? ? ret = -ENOENT;
> + ? ? ? ? ? ? ? goto err;
> + ? ? ? }
> +
> + ? ? ? wdt = kzalloc(sizeof(*wdt), GFP_KERNEL);
> + ? ? ? if (!wdt) {
> + ? ? ? ? ? ? ? dev_warn(&adev->dev, "Kzalloc failed\n");
> + ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? goto err_kzalloc;
> + ? ? ? }
> +
> + ? ? ? wdt->clk = clk_get(&adev->dev, NULL);
> + ? ? ? if (IS_ERR(wdt->clk)) {
> + ? ? ? ? ? ? ? dev_warn(&adev->dev, "Clock not found\n");
> + ? ? ? ? ? ? ? ret = PTR_ERR(wdt->clk);
> + ? ? ? ? ? ? ? goto err_clk_get;
> + ? ? ? }
> +
> + ? ? ? wdt->base = ioremap(adev->res.start, resource_size(&adev->res));
> + ? ? ? if (!wdt->base) {
> + ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? dev_warn(&adev->dev, "ioremap fail\n");
> + ? ? ? ? ? ? ? goto err_ioremap;
> + ? ? ? }
> +
> + ? ? ? wdt->adev = adev;
> + ? ? ? spin_lock_init(&wdt->lock);
> + ? ? ? wdt_setload(DEFAULT_TIMEOUT);
> +
> + ? ? ? ret = misc_register(&sp805_wdt_miscdev);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? dev_warn(&adev->dev, "cannot register misc device\n");
> + ? ? ? ? ? ? ? goto err_misc_register;
> + ? ? ? }
> +
> + ? ? ? dev_info(&adev->dev, "registration successful\n");
> + ? ? ? return 0;
> +
> +err_misc_register:
> + ? ? ? iounmap(wdt->base);
> +err_ioremap:
> + ? ? ? clk_put(wdt->clk);
> +err_clk_get:
> + ? ? ? kfree(wdt);
> + ? ? ? wdt = NULL;
> +err_kzalloc:
> + ? ? ? release_mem_region(adev->res.start, resource_size(&adev->res));
> +err:
> + ? ? ? dev_err(&adev->dev, "Probe Failed!!!\n");

Drop the exclamation marks, that it's dev_err() is serious enough.

> + ? ? ? return ret;
> +}
> +
> +static int __devexit sp805_wdt_remove(struct amba_device *adev)
> +{
> + ? ? ? misc_deregister(&sp805_wdt_miscdev);
> + ? ? ? iounmap(wdt->base);
> + ? ? ? clk_put(wdt->clk);
> + ? ? ? kfree(wdt);
> + ? ? ? release_mem_region(adev->res.start, resource_size(&adev->res));
> +
> + ? ? ? return 0;
> +}
> +
> +static struct amba_id sp805_wdt_ids[] __initdata = {
> + ? ? ? {
> + ? ? ? ? ? ? ? .id ? ? = 0x00141805,
> + ? ? ? ? ? ? ? .mask ? = 0x00ffffff,
> + ? ? ? },
> + ? ? ? { 0, 0 },
> +};
> +
> +static struct amba_driver sp805_wdt_driver = {
> + ? ? ? .drv = {
> + ? ? ? ? ? ? ? .name ? = MODULE_NAME,
> + ? ? ? },
> + ? ? ? .id_table ? ? ? = sp805_wdt_ids,
> + ? ? ? .probe ? ? ? ? ?= sp805_wdt_probe,
> + ? ? ? .remove = __devexit_p(sp805_wdt_remove),
> +};
> +
> +static int __init sp805_wdt_init(void)
> +{
> + ? ? ? return amba_driver_register(&sp805_wdt_driver);
> +}
> +module_init(sp805_wdt_init);
> +
> +static void __exit sp805_wdt_exit(void)
> +{
> + ? ? ? amba_driver_unregister(&sp805_wdt_driver);
> +}
> +module_exit(sp805_wdt_exit);
> +
> +module_param(nowayout, int, 0);
> +MODULE_PARM_DESC(nowayout,
> + ? ? ? ? ? ? ? "Set to 1 to keep watchdog running after device release");
> +
> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
> +MODULE_DESCRIPTION("ARM SP805 Watchdog Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

Yours,
Linus Walleij

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Viresh KUMAR]

On 7/19/2010 1:59 AM, Linus Walleij wrote:
> Sorry for late review.
> 
> I was bored so I compiled this driver in the latest -next
> and tested on ARM PB1176JZF-S that I've brought for
> entertainment this vacation...
> 
> First: add
> #include <linux/slab.h>
> or compilation fails on newer codebases.

OK.

> 
> Then I get this:
> sp805-wdt dev:wdog: Clock not found
> sp805-wdt dev:wdog: Probe Failed!!!
> sp805-wdt: probe of dev:wdog failed with error -2
> 
> So I added it to the realviews, even though the clocking
> of the watchdog is not mentioned in the reference manual
> it appears to be using the same 24MHz clock as the UARTs
> and the RTC. (ARM people, can you confirm this?)

Yes, it is using 24 Mhz on spear and is its functional clock.
So this time we need clock support in driver.

> 
> After this it seems to work...
> Tested-by: Linus Walleij <linus.walleij@stericsson.com>
> 
> 2010/5/20 Viresh KUMAR <viresh.kumar@st.com>:
>> Technical Reference Manual can be found at:
>> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>>  drivers/watchdog/Kconfig     |    7 +
>>  drivers/watchdog/Makefile    |    1 +
>>  drivers/watchdog/sp805_wdt.c |  386 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 394 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/watchdog/sp805_wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index b87ba23..2759a91 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -73,6 +73,13 @@ config WM8350_WATCHDOG
>>
>>  # ARM Architecture
>>
>> +config ARM_SP805_WATCHDOG
>> +       tristate "ARM SP805 Watchdog"
> 
> Why would you want this as a module... Can't it
> be just bool?

I think watchdog can be present as a module. It can be inserted at
runtime. Other drivers in drivers/watchdog/Kconfig are also present
in modules.

> 
>> +       depends on ARM_AMBA
>> +       help
>> +         ARM Primecell SP805 Watchdog timer. This will reboot your system when
>> +         the timeout is reached.
>> +
>>  config AT91RM9200_WATCHDOG
>>        tristate "AT91RM9200 watchdog"
>>        depends on ARCH_AT91RM9200
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 5e3cb95..28af87a 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -25,6 +25,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
>>  # ALPHA Architecture
>>
>>  # ARM Architecture
>> +obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
>>  obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
>>  obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
>>  obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> new file mode 100644
>> index 0000000..85d86ac
>> --- /dev/null
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -0,0 +1,386 @@
>> +/*
>> + * drivers/char/watchdog/sp805-wdt.c
>> + *
>> + * Watchdog driver for ARM SP805 watchdog module
>> + *
>> + * Copyright (C) 2010 ST Microelectronics
>> + * Viresh Kumar<viresh.kumar@st.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2 or later. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/resource.h>
>> +#include <linux/amba/bus.h>
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/fs.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/math64.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/watchdog.h>
> 
> #include <linux/slab.h>

OK.

> 
>> +
>> +/* default timeout in seconds */
>> +#define DEFAULT_TIMEOUT                60
>> +
>> +#define MODULE_NAME            "sp805-wdt"
>> +
>> +/* watchdog register offsets and masks */
>> +#define WDTLOAD                        0x000
>> +       #define LOAD_MIN        0x00000001
>> +       #define LOAD_MAX        0xFFFFFFFF
> 
> I haven't seen this style of indenting the #defines for
> stuff you can put into the registers before, but I sort of
> like it. Is it common in the kernel?

I am not sure if it is common, but i thought it would be much more readable
now. so did it.

> 
>> +#define WDTVALUE               0x004
>> +#define WDTCONTROL             0x008
>> +       /* control register masks */
>> +       #define INT_ENABLE      (1 << 0)
>> +       #define RESET_ENABLE    (1 << 1)
>> +#define WDTINTCLR              0x00C
>> +#define WDTRIS                 0x010
>> +#define WDTMIS                 0x014
>> +       #define INT_MASK        (1 << 0)
>> +#define WDTLOCK                        0xC00
>> +       #define UNLOCK          0x1ACCE551
>> +       #define LOCK            0x00000001
>> +
>> +/**
>> + * struct sp805_wdt: sp805 wdt device structure
>> + *
>> + * lock: spin lock protecting dev structure and io access
>> + * base: base address of wdt
>> + * clk: clock structure of wdt
>> + * dev: amba device structure of wdt
>> + * status: current status of wdt
>> + * load_val: load value to be set for current timeout
>> + * timeout: current programmed timeout
>> + */
>> +struct sp805_wdt {
>> +       spinlock_t                      lock;
>> +       void __iomem                    *base;
>> +       struct clk                      *clk;
>> +       struct amba_device              *adev;
>> +       unsigned long                   status;
>> +       #define WDT_BUSY                0
>> +       #define WDT_CAN_BE_CLOSED       1
>> +       unsigned int                    load_val;
>> +       unsigned int                    timeout;
>> +};
>> +
>> +/* local variables */
>> +static struct sp805_wdt *wdt;
>> +static int nowayout = WATCHDOG_NOWAYOUT;
>> +
>> +/* This routine finds load value that will reset system in required timout */
>> +static void wdt_setload(unsigned int timeout)
>> +{
>> +       u64 load, rate;
>> +
>> +       rate = clk_get_rate(wdt->clk);
>> +
>> +       /*
>> +        * sp805 runs counter with given value twice, after the end of first
>> +        * counter it gives an interrupt and then starts counter again. If
>> +        * interrupt already occured then it resets the system. This is why
>> +        * load is half of what should be required.
>> +        */
>> +       load = div_u64(rate, 2) * timeout - 1;
>> +
>> +       load = (load > LOAD_MAX) ? LOAD_MAX : load;
>> +       load = (load < LOAD_MIN) ? LOAD_MIN : load;
>> +
>> +       spin_lock(&wdt->lock);
>> +       wdt->load_val = load;
>> +       /* roundup timeout to closest positive integer value */
>> +       wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
> 
> Look in linux/kernel.h, use the
> DIV_ROUND_CLOSEST() macro instead of this.

OK.

> 
>> +       spin_unlock(&wdt->lock);
>> +}
>> +
>> +/* returns number of seconds left for reset to occur */
>> +static u32 wdt_timeleft(void)
>> +{
>> +       u64 load, rate;
>> +
>> +       rate = clk_get_rate(wdt->clk);
>> +
>> +       spin_lock(&wdt->lock);
>> +       load = readl(wdt->base + WDTVALUE);
>> +
>> +       /*If the interrupt is inactive then time left is WDTValue + WDTLoad. */
>> +       if (!(readl(wdt->base + WDTRIS) & INT_MASK))
>> +               load += wdt->load_val + 1;
>> +       spin_unlock(&wdt->lock);
>> +
>> +       return div_u64(load, rate);
>> +}
>> +
>> +/* enables watchdog timers reset */
>> +static void wdt_enable(void)
>> +{
>> +       spin_lock(&wdt->lock);
>> +
>> +       writel(UNLOCK, wdt->base + WDTLOCK);
>> +       writel(wdt->load_val, wdt->base + WDTLOAD);
>> +       writel(INT_MASK, wdt->base + WDTINTCLR);
>> +       writel(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
>> +       writel(LOCK, wdt->base + WDTLOCK);
>> +
>> +       spin_unlock(&wdt->lock);
>> +}
>> +
>> +/* disables watchdog timers reset */
>> +static void wdt_disable(void)
>> +{
>> +       spin_lock(&wdt->lock);
>> +
>> +       writel(UNLOCK, wdt->base + WDTLOCK);
>> +       writel(0, wdt->base + WDTCONTROL);
>> +       writel(0, wdt->base + WDTLOAD);
>> +       writel(LOCK, wdt->base + WDTLOCK);
>> +
>> +       spin_unlock(&wdt->lock);
>> +}
>> +
>> +static ssize_t sp805_wdt_write(struct file *file, const char *data,
>> +               size_t len, loff_t *ppos)
>> +{
>> +       if (len) {
>> +               if (!nowayout) {
>> +                       size_t i;
>> +
>> +                       clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
>> +
>> +                       for (i = 0; i != len; i++) {
>> +                               char c;
>> +
>> +                               if (get_user(c, data + i))
>> +                                       return -EFAULT;
>> +                               /* Check for Magic Close character */
>> +                               if (c == 'V') {
>> +                                       set_bit(WDT_CAN_BE_CLOSED,
>> +                                                       &wdt->status);
>> +                                       break;
>> +                               }
>> +                       }
>> +               }
>> +               wdt_enable();
>> +       }
>> +       return len;
>> +}
>> +
>> +static const struct watchdog_info ident = {
>> +       .options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
>> +       .identity = MODULE_NAME,
>> +};
>> +
>> +static long sp805_wdt_ioctl(struct file *file, unsigned int cmd,
>> +               unsigned long arg)
>> +{
>> +       int ret = -ENOTTY;
>> +       unsigned int timeout;
>> +
>> +       switch (cmd) {
>> +       case WDIOC_GETSUPPORT:
>> +               ret = copy_to_user((struct watchdog_info *)arg, &ident,
>> +                               sizeof(ident)) ? -EFAULT : 0;
>> +               break;
>> +
>> +       case WDIOC_GETSTATUS:
>> +               ret = put_user(0, (int *)arg);
>> +               break;
>> +
>> +       case WDIOC_KEEPALIVE:
>> +               wdt_enable();
>> +               ret = 0;
>> +               break;
>> +
>> +       case WDIOC_SETTIMEOUT:
>> +               ret = get_user(timeout, (unsigned int *)arg);
>> +               if (ret)
>> +                       break;
>> +
>> +               wdt_setload(timeout);
>> +
>> +               wdt_enable();
>> +               /* Fall through */
>> +
>> +       case WDIOC_GETTIMEOUT:
>> +               ret = put_user(wdt->timeout, (unsigned int *)arg);
>> +               break;
>> +       case WDIOC_GETTIMELEFT:
>> +               ret = put_user(wdt_timeleft(), (unsigned int *)arg);
>> +               break;
>> +       }
>> +       return ret;
>> +}
>> +
>> +static int sp805_wdt_open(struct inode *inode, struct file *file)
>> +{
>> +       int ret = 0;
>> +
>> +       if (test_and_set_bit(WDT_BUSY, &wdt->status))
>> +               return -EBUSY;
>> +
>> +       ret = clk_enable(wdt->clk);
>> +       if (ret) {
>> +               dev_err(&wdt->adev->dev, "clock enable fail");
>> +               goto err;
>> +       }
>> +
>> +       wdt_enable();
>> +
>> +       /* can not be closed, once enabled */
>> +       clear_bit(WDT_CAN_BE_CLOSED, &wdt->status);
>> +       return nonseekable_open(inode, file);
>> +
>> +err:
>> +       clear_bit(WDT_BUSY, &wdt->status);
>> +       return ret;
>> +}
>> +
>> +static int sp805_wdt_release(struct inode *inode, struct file *file)
>> +{
>> +       if (!test_bit(WDT_CAN_BE_CLOSED, &wdt->status)) {
>> +               clear_bit(WDT_BUSY, &wdt->status);
>> +               dev_warn(&wdt->adev->dev, "Device closed unexpectedly\n");
>> +               return 0;
>> +       }
>> +
>> +       wdt_disable();
>> +       clk_disable(wdt->clk);
>> +       clear_bit(WDT_BUSY, &wdt->status);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct file_operations sp805_wdt_fops = {
>> +       .owner = THIS_MODULE,
>> +       .llseek = no_llseek,
>> +       .write = sp805_wdt_write,
>> +       .unlocked_ioctl = sp805_wdt_ioctl,
>> +       .open = sp805_wdt_open,
>> +       .release = sp805_wdt_release,
>> +};
>> +
>> +static struct miscdevice sp805_wdt_miscdev = {
>> +       .minor = WATCHDOG_MINOR,
>> +       .name = "watchdog",
>> +       .fops = &sp805_wdt_fops,
>> +};
>> +
>> +static int __devinit
>> +sp805_wdt_probe(struct amba_device *adev, struct amba_id *id)
>> +{
>> +       int ret = 0;
>> +
>> +       if (!request_mem_region(adev->res.start, resource_size(&adev->res),
>> +                               "sp805_wdt")) {
>> +               dev_warn(&adev->dev, "Failed to get memory region resource\n");
>> +               ret = -ENOENT;
>> +               goto err;
>> +       }
>> +
>> +       wdt = kzalloc(sizeof(*wdt), GFP_KERNEL);
>> +       if (!wdt) {
>> +               dev_warn(&adev->dev, "Kzalloc failed\n");
>> +               ret = -ENOMEM;
>> +               goto err_kzalloc;
>> +       }
>> +
>> +       wdt->clk = clk_get(&adev->dev, NULL);
>> +       if (IS_ERR(wdt->clk)) {
>> +               dev_warn(&adev->dev, "Clock not found\n");
>> +               ret = PTR_ERR(wdt->clk);
>> +               goto err_clk_get;
>> +       }
>> +
>> +       wdt->base = ioremap(adev->res.start, resource_size(&adev->res));
>> +       if (!wdt->base) {
>> +               ret = -ENOMEM;
>> +               dev_warn(&adev->dev, "ioremap fail\n");
>> +               goto err_ioremap;
>> +       }
>> +
>> +       wdt->adev = adev;
>> +       spin_lock_init(&wdt->lock);
>> +       wdt_setload(DEFAULT_TIMEOUT);
>> +
>> +       ret = misc_register(&sp805_wdt_miscdev);
>> +       if (ret < 0) {
>> +               dev_warn(&adev->dev, "cannot register misc device\n");
>> +               goto err_misc_register;
>> +       }
>> +
>> +       dev_info(&adev->dev, "registration successful\n");
>> +       return 0;
>> +
>> +err_misc_register:
>> +       iounmap(wdt->base);
>> +err_ioremap:
>> +       clk_put(wdt->clk);
>> +err_clk_get:
>> +       kfree(wdt);
>> +       wdt = NULL;
>> +err_kzalloc:
>> +       release_mem_region(adev->res.start, resource_size(&adev->res));
>> +err:
>> +       dev_err(&adev->dev, "Probe Failed!!!\n");
> 
> Drop the exclamation marks, that it's dev_err() is serious enough.
> 

OK.

>> +       return ret;
>> +}
>> +
>> +static int __devexit sp805_wdt_remove(struct amba_device *adev)
>> +{
>> +       misc_deregister(&sp805_wdt_miscdev);
>> +       iounmap(wdt->base);
>> +       clk_put(wdt->clk);
>> +       kfree(wdt);
>> +       release_mem_region(adev->res.start, resource_size(&adev->res));
>> +
>> +       return 0;
>> +}
>> +
>> +static struct amba_id sp805_wdt_ids[] __initdata = {
>> +       {
>> +               .id     = 0x00141805,
>> +               .mask   = 0x00ffffff,
>> +       },
>> +       { 0, 0 },
>> +};
>> +
>> +static struct amba_driver sp805_wdt_driver = {
>> +       .drv = {
>> +               .name   = MODULE_NAME,
>> +       },
>> +       .id_table       = sp805_wdt_ids,
>> +       .probe          = sp805_wdt_probe,
>> +       .remove = __devexit_p(sp805_wdt_remove),
>> +};
>> +
>> +static int __init sp805_wdt_init(void)
>> +{
>> +       return amba_driver_register(&sp805_wdt_driver);
>> +}
>> +module_init(sp805_wdt_init);
>> +
>> +static void __exit sp805_wdt_exit(void)
>> +{
>> +       amba_driver_unregister(&sp805_wdt_driver);
>> +}
>> +module_exit(sp805_wdt_exit);
>> +
>> +module_param(nowayout, int, 0);
>> +MODULE_PARM_DESC(nowayout,
>> +               "Set to 1 to keep watchdog running after device release");
>> +
>> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
>> +MODULE_DESCRIPTION("ARM SP805 Watchdog Driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> 

I will send V2 after Wim's review comments.

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Viresh KUMAR]

On 7/19/2010 10:37 AM, vireshk wrote:
>>> +/* This routine finds load value that will reset system in required timout */
>>> >> +static void wdt_setload(unsigned int timeout)
>>> >> +{
>>> >> +       u64 load, rate;
>>> >> +
>>> >> +       rate = clk_get_rate(wdt->clk);
>>> >> +
>>> >> +       /*
>>> >> +        * sp805 runs counter with given value twice, after the end of first
>>> >> +        * counter it gives an interrupt and then starts counter again. If
>>> >> +        * interrupt already occured then it resets the system. This is why
>>> >> +        * load is half of what should be required.
>>> >> +        */
>>> >> +       load = div_u64(rate, 2) * timeout - 1;
>>> >> +
>>> >> +       load = (load > LOAD_MAX) ? LOAD_MAX : load;
>>> >> +       load = (load < LOAD_MIN) ? LOAD_MIN : load;
>>> >> +
>>> >> +       spin_lock(&wdt->lock);
>>> >> +       wdt->load_val = load;
>>> >> +       /* roundup timeout to closest positive integer value */
>>> >> +       wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
>> > 
>> > Look in linux/kernel.h, use the
>> > DIV_ROUND_CLOSEST() macro instead of this.
> OK.
> 

Linus,

Now i remember why i didn't choose DIV_ROUND_CLOSEST in V1 of this patch.
As it is a u64 division, i get following compilation error if i use
DIV_ROUND_CLOSEST. I wasn't sure if inclusion of some header file can remove
this error, and so i used div_u64.

error:
"undefined reference to '__aeabi_uldivmod'"

Do you have some other solution to fix this error??

viresh.

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Linus Walleij]

2010/7/27 Viresh KUMAR <viresh.kumar@st.com>:

>>>> >> + ? ? ? /* roundup timeout to closest positive integer value */
>>>> >> + ? ? ? wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
>>> >
>>> > Look in linux/kernel.h, use the
>>> > DIV_ROUND_CLOSEST() macro instead of this.
>
> Now i remember why i didn't choose DIV_ROUND_CLOSEST in V1 of this patch.
> As it is a u64 division, i get following compilation error if i use
> DIV_ROUND_CLOSEST. I wasn't sure if inclusion of some header file can remove
> this error, and so i used div_u64.

Aha, I have no solution to that, but I'd recommend adding the comment
/* Cannot use DIV_ROUND_CLOSEST() due to the 64bit size */

Yours,
Linus Walleij

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Viresh KUMAR]

On 7/28/2010 3:55 AM, Linus Walleij wrote:
> 2010/7/27 Viresh KUMAR <viresh.kumar@st.com>:
> 
>>>>>>> +       /* roundup timeout to closest positive integer value */
>>>>>>> +       wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
>>>>>
>>>>> Look in linux/kernel.h, use the
>>>>> DIV_ROUND_CLOSEST() macro instead of this.
>>
>> Now i remember why i didn't choose DIV_ROUND_CLOSEST in V1 of this patch.
>> As it is a u64 division, i get following compilation error if i use
>> DIV_ROUND_CLOSEST. I wasn't sure if inclusion of some header file can remove
>> this error, and so i used div_u64.
> 
> Aha, I have no solution to that, but I'd recommend adding the comment
> /* Cannot use DIV_ROUND_CLOSEST() due to the 64bit size */
> 

I thought that also, but i didn't like that. There is nothing wrong or
special in code. It is fine to do divisions the way it is done. So i didn't
give any comment here. You still want me to give a comment there?

viresh.

[PATCH] Watchdog: Adding support for ARM Primecell SP805 Watchdog

[Linus Walleij]

2010/7/28 Viresh KUMAR <viresh.kumar@st.com>:
> On 7/28/2010 3:55 AM, Linus Walleij wrote:
>> 2010/7/27 Viresh KUMAR <viresh.kumar@st.com>:
>>
>>>>>>>> + ? ? ? /* roundup timeout to closest positive integer value */
>>>>>>>> + ? ? ? wdt->timeout = div_u64((load + 1) * 2 + (rate / 2), rate);
>>>>>>
>>>>>> Look in linux/kernel.h, use the
>>>>>> DIV_ROUND_CLOSEST() macro instead of this.
>>>
>>> Now i remember why i didn't choose DIV_ROUND_CLOSEST in V1 of this patch.
>>> As it is a u64 division, i get following compilation error if i use
>>> DIV_ROUND_CLOSEST. I wasn't sure if inclusion of some header file can remove
>>> this error, and so i used div_u64.
>>
>> Aha, I have no solution to that, but I'd recommend adding the comment
>> /* Cannot use DIV_ROUND_CLOSEST() due to the 64bit size */
>>
>
> I thought that also, but i didn't like that. There is nothing wrong or
> special in code. It is fine to do divisions the way it is done. So i didn't
> give any comment here. You still want me to give a comment there?

The comment would not be stating that there is something wrong
with the division, merely that the standard macro isn't used for this
reason, but it's not important, feel free to drop it if you think it's nitpicky.

Yours,
Linus Walleij