Watchdog: Adding support for ARM Primecell SP805 Watchdog

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

Quoted 2 lines · Viresh KUMAR

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.

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

Signed-off-by Viresh Kumar <[email protected]>

---

?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

drivers/watchdog/Kconfig +0 -0

index b87ba23..2759a91 100644
@@ -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

drivers/watchdog/Makefile +0 -0

index 5e3cb95..28af87a 100644
@@ -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

drivers/watchdog/sp805_wdt.c new file +21 -0

new file mode 100644
index 0000000..85d86ac
@@ -0,0 +1,386 @@
+/*
+ * drivers/char/watchdog/sp805-wdt.c
+ *
+ * Watchdog driver for ARM SP805 watchdog module
+ *
+ * Copyright (C) 2010 ST Microelectronics
+ * Viresh Kumar<[email protected]>
+ *
+ * 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>

drivers/watchdog/sp805_wdt.c new file +52 -0

+#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.

drivers/watchdog/sp805_wdt.c new file +222 -0

+ ? ? ? 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:

drivers/watchdog/sp805_wdt.c new file +41 -0

+ ? ? ? 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);
+}

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 <[email protected]>

2010/5/20 Viresh KUMAR <[email protected]>:

Quoted 17 lines

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

Signed-off-by Viresh Kumar <[email protected]>

---
 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

drivers/watchdog/Kconfig +0 -0

index b87ba23..2759a91 100644
@@ -73,6 +73,13 @@ config WM8350_WATCHDOG
 

drivers/watchdog/Kconfig config WM8350_WATCHDOG · line 77

Quoted 4 lines

drivers/watchdog/Kconfig +2 -0

 # ARM Architecture
 
+config ARM_SP805_WATCHDOG
+       tristate "ARM SP805 Watchdog"

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

Quoted 52 lines

drivers/watchdog/Kconfig +5 -0

+       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

drivers/watchdog/Makefile +1 -0

index 5e3cb95..28af87a 100644
@@ -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

drivers/watchdog/sp805_wdt.c new file +25 -0

new file mode 100644
index 0000000..85d86ac
@@ -0,0 +1,386 @@
+/*
+ * drivers/char/watchdog/sp805-wdt.c
+ *
+ * Watchdog driver for ARM SP805 watchdog module
+ *
+ * Copyright (C) 2010 ST Microelectronics
+ * Viresh Kumar<[email protected]>
+ *
+ * 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>

drivers/watchdog/sp805_wdt.c line 31

Quoted 6 lines

drivers/watchdog/sp805_wdt.c new file +6 -0

+#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>

drivers/watchdog/sp805_wdt.c line 41

Quoted 11 lines

drivers/watchdog/sp805_wdt.c new file +10 -0

+
+/* 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?

Quoted 58 lines

drivers/watchdog/sp805_wdt.c new file +57 -0

+#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;

drivers/watchdog/sp805_wdt.c line 103

Quoted 5 lines

drivers/watchdog/sp805_wdt.c new file +5 -0

+
+       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.

Quoted 227 lines

drivers/watchdog/sp805_wdt.c new file +226 -0

+       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:

drivers/watchdog/sp805_wdt.c line 335

Quoted 6 lines

drivers/watchdog/sp805_wdt.c new file +6 -0

+       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.

Quoted 43 lines

drivers/watchdog/sp805_wdt.c new file +43 -0

+       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);
+

drivers/watchdog/sp805_wdt.c line 386

Quoted 8 lines

drivers/watchdog/sp805_wdt.c new file +8 -0

+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout,
+               "Set to 1 to keep watchdog running after device release");
+
+MODULE_AUTHOR("Viresh Kumar <[email protected]>");
+MODULE_DESCRIPTION("ARM SP805 Watchdog Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

Quoted 17 lines

Quoted 17 lines

+/* This routine finds load value that will reset system in required timout */

Quoted 16 lines

Quoted 16 lines

drivers/watchdog/sp805_wdt.c new file +16 -0

+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;

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

drivers/watchdog/sp805_wdt.c line 103

Quoted 5 lines

drivers/watchdog/sp805_wdt.c new file +5 -0

+
+       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.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

+ ? ? ? /* 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.

2010/7/27 Viresh KUMAR <[email protected]>:

Quoted 6 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

drivers/watchdog/sp805_wdt.c line 103

Quoted 2 lines

drivers/watchdog/sp805_wdt.c new file +2 -0

+       /* 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.

Quoted 4 lines

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 */

Quoted 13 lines

2010/7/27 Viresh KUMAR <[email protected]>:

Quoted 10 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

+ ? ? ? /* 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.

Quoted 3 lines

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?