[PATCH 0/4] watchdog: new driver for STMP3xxx/MX23/MX28

[PATCH 0/4] watchdog: new driver for STMP3xxx/MX23/MX28

[Wolfram Sang]

Introduce a new user of the watchdog framework :) This is a rewritten driver
for STMP and imx23/28 (which was not compilable anymore due to the missing
mach). Please read the patch descriptions to understand why I think a rewrite
was necessary. The first patch adds an accessor routine to the RTC-driver which
can hopefully go via the watchdog-tree as well. The second patch is a minor
enhancement to the watchdog framework. The third patch adds the new driver,
the fourth removes the old one.

I hope this can make it for 3.2; the old driver was broken (= not compilable)
anyhow, so there cannot be any regression.

Regards,

   Wolfram

Wolfram Sang (4):
  rtc: stmp3xxx: add wdt-accessor function
  watchdog: add define to setup nowayout feature in the status-variable
  watchdog: add new driver for STMP3xxx and i.MX23/28
  watchdog: remove old STMP3XXX driver

 drivers/rtc/rtc-stmp3xxx.c          |   44 +++++
 drivers/watchdog/Kconfig            |   13 +-
 drivers/watchdog/Makefile           |    2 +-
 drivers/watchdog/stmp3xxx_rtc_wdt.c |  147 +++++++++++++++++
 drivers/watchdog/stmp3xxx_wdt.c     |  296 -----------------------------------
 include/linux/watchdog.h            |   14 +-
 6 files changed, 207 insertions(+), 309 deletions(-)
 create mode 100644 drivers/watchdog/stmp3xxx_rtc_wdt.c
 delete mode 100644 drivers/watchdog/stmp3xxx_wdt.c

-- 
1.7.6.3

[PATCH 1/4] rtc: stmp3xxx: add wdt-accessor function

[Wolfram Sang]

This RTC also includes a watchdog timer. Provide an accessor function
for it to be picked up by a watchdog driver. Also register the
platform_device here to get proper boot-time dependencies.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Alessandro Zummo <alessandro.zummo@towertech.it>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: rtc-linux@googlegroups.com
---

Andrew, Wim: Alessandro was not around in the last weeks, so I'd suggest this
patch goes via the watchdog-tree together with the rest of this series if it
passes the review of the watchdog-list? All changes here are in preparation for
the watchdog driver and do not affect the general RTC. Is that okay?

 drivers/rtc/rtc-stmp3xxx.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
index 7315068..379ceb6 100644
--- a/drivers/rtc/rtc-stmp3xxx.c
+++ b/drivers/rtc/rtc-stmp3xxx.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 
 #include <mach/common.h>
+#include <mach/mxs.h>
 
 #define STMP3XXX_RTC_CTRL			0x0
 #define STMP3XXX_RTC_CTRL_SET			0x4
@@ -34,6 +35,7 @@
 #define STMP3XXX_RTC_CTRL_ALARM_IRQ_EN		0x00000001
 #define STMP3XXX_RTC_CTRL_ONEMSEC_IRQ_EN	0x00000002
 #define STMP3XXX_RTC_CTRL_ALARM_IRQ		0x00000004
+#define STMP3XXX_RTC_CTRL_WATCHDOGEN		0x00000010
 
 #define STMP3XXX_RTC_STAT			0x10
 #define STMP3XXX_RTC_STAT_STALE_SHIFT		16
@@ -43,6 +45,8 @@
 
 #define STMP3XXX_RTC_ALARM			0x40
 
+#define STMP3XXX_RTC_WATCHDOG			0x50
+
 #define STMP3XXX_RTC_PERSISTENT0		0x60
 #define STMP3XXX_RTC_PERSISTENT0_SET		0x64
 #define STMP3XXX_RTC_PERSISTENT0_CLR		0x68
@@ -50,12 +54,51 @@
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_EN	0x00000004
 #define STMP3XXX_RTC_PERSISTENT0_ALARM_WAKE	0x00000080
 
+#define STMP3XXX_RTC_PERSISTENT1		0x70
+/* missing bitmask in headers */
+#define STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER	0x80000000
+
 struct stmp3xxx_rtc_data {
 	struct rtc_device *rtc;
 	void __iomem *io;
 	int irq_alarm;
 };
 
+#ifdef CONFIG_STMP3XXX_RTC_WATCHDOG
+struct platform_device stmp3xxx_wdt_pdev = {
+	.name = "stmp3xxx_rtc_wdt",
+	.id = -1,
+};
+
+void stmp3xxx_wdt_setup(struct device *dev, u32 timeout)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct stmp3xxx_rtc_data *rtc_data = platform_get_drvdata(pdev);
+	void __iomem *base;
+
+	if (timeout) {
+		writel(timeout, rtc_data->io + STMP3XXX_RTC_WATCHDOG);
+		base = rtc_data->io + MXS_SET_ADDR;
+	} else {
+		base = rtc_data->io + MXS_CLR_ADDR;
+	}
+	writel(STMP3XXX_RTC_CTRL_WATCHDOGEN, base + STMP3XXX_RTC_CTRL);
+	writel(STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER,
+		base + STMP3XXX_RTC_PERSISTENT1);
+}
+EXPORT_SYMBOL_GPL(stmp3xxx_wdt_setup);
+
+static void stmp3xxx_wdt_register(struct device *dev)
+{
+	stmp3xxx_wdt_pdev.dev.parent = dev;
+	platform_device_register(&stmp3xxx_wdt_pdev);
+}
+#else
+static void stmp3xxx_wdt_register(struct device *dev)
+{
+}
+#endif /* CONFIG_STMP3XXX_RTC_WATCHDOG */
+
 static void stmp3xxx_wait_time(struct stmp3xxx_rtc_data *rtc_data)
 {
 	/*
@@ -231,6 +274,7 @@ static int stmp3xxx_rtc_probe(struct platform_device *pdev)
 		goto out_irq_alarm;
 	}
 
+	stmp3xxx_wdt_register(&pdev->dev);
 	return 0;
 
 out_irq_alarm:
-- 
1.7.6.3

[PATCH 2/4] watchdog: add define to setup nowayout feature in the status-variable

[Wolfram Sang]

Some drivers do not support 'nowayout' as a module parameter but simply
adhere to the global setting (which has pros and cons). Provide
additional defines, so this behaviour can easily be migrated to the
watchdog framework by adding

	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,

to the watchdog_device struct.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 include/linux/watchdog.h |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 111843f..4659bf8 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -53,12 +53,6 @@ struct watchdog_info {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-#define WATCHDOG_NOWAYOUT	1
-#else
-#define WATCHDOG_NOWAYOUT	0
-#endif
-
 struct watchdog_ops;
 struct watchdog_device;
 
@@ -137,6 +131,14 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+#define WATCHDOG_NOWAYOUT		1
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
+#else
+#define WATCHDOG_NOWAYOUT		0
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
+#endif
+
 #endif	/* __KERNEL__ */
 
 #endif  /* ifndef _LINUX_WATCHDOG_H */
-- 
1.7.6.3

[PATCH 3/4] watchdog: add new driver for STMP3xxx and i.MX23/28

[Wolfram Sang]

Replace the existing STMP3xxx driver because it has enough drawbacks
that a rewrite is apropriate. The new driver is designed to use the
watchdog framework which makes it a lot smaller. It now uses an exported
function from the RTC driver to set up its registers (the old driver
silently reused the hopefully already remapped RTC registers, uuuh).
Also, this driver is mach independent, while the old one still depends
on a removed mach.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/watchdog/Kconfig            |   10 +++
 drivers/watchdog/Makefile           |    1 +
 drivers/watchdog/stmp3xxx_rtc_wdt.c |  147 +++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/stmp3xxx_rtc_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1d8cbe4..5aad0dc 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -305,6 +305,16 @@ config STMP3XXX_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called stmp3xxx_wdt.
 
+config STMP3XXX_RTC_WATCHDOG
+	tristate "Freescale STMP3XXX & i.MX23/28 watchdog"
+	depends on RTC_DRV_STMP
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the watchdog timer inside
+	  the RTC for the STMP37XX/378X or i.MX23/28 SoC.
+	  To compile this driver as a module, choose M here: the
+	  module will be called stmp3xxx_rtc_wdt.
+
 config NUC900_WATCHDOG
 	tristate "Nuvoton NUC900 watchdog"
 	depends on ARCH_W90X900
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 55bd574..042076a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o
 obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o
 obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o
 obj-$(CONFIG_STMP3XXX_WATCHDOG) += stmp3xxx_wdt.o
+obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o
 obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
 obj-$(CONFIG_ADX_WATCHDOG) += adx_wdt.o
 obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
diff --git a/drivers/watchdog/stmp3xxx_rtc_wdt.c b/drivers/watchdog/stmp3xxx_rtc_wdt.c
new file mode 100644
index 0000000..55645ad
--- /dev/null
+++ b/drivers/watchdog/stmp3xxx_rtc_wdt.c
@@ -0,0 +1,147 @@
+/*
+ * Watchdog driver for the RTC based watchdog in STMP3xxx and i.MX23/28
+ *
+ * Author: Wolfram Sang <w.sang@pengutronix.de>
+ *
+ * Copyright (C) 2011 Pengutronix e.K.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+
+#define WDOG_TICK_RATE 1000 /* 1 kHz clock */
+#define STMP3XXX_DEFAULT_TIMEOUT 60
+#define STMP3XXX_MAX_TIMEOUT (UINT_MAX / WDOG_TICK_RATE)
+
+static int timeout = STMP3XXX_DEFAULT_TIMEOUT;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout period in seconds from 1 to "
+		 __MODULE_STRING(STMP3XXX_MAX_TIMEOUT) ", default "
+		 __MODULE_STRING(STMP3XXX_DEFAULT_TIMEOUT));
+
+/* this function is in the RTC domain */
+extern void stmp3xxx_wdt_setup(struct device *dev, u32 val);
+
+static int wdt_start(struct watchdog_device *wdd)
+{
+	struct platform_device *pdev = watchdog_get_drvdata(wdd);
+
+	stmp3xxx_wdt_setup(pdev->dev.parent, wdd->timeout * WDOG_TICK_RATE);
+	return 0;
+}
+
+static int wdt_stop(struct watchdog_device *wdd)
+{
+	struct platform_device *pdev = watchdog_get_drvdata(wdd);
+
+	stmp3xxx_wdt_setup(pdev->dev.parent, 0);
+	return 0;
+}
+
+static int wdt_set_timeout(struct watchdog_device *wdd, unsigned new_timeout)
+{
+	struct platform_device *pdev = watchdog_get_drvdata(wdd);
+
+	stmp3xxx_wdt_setup(pdev->dev.parent, new_timeout * WDOG_TICK_RATE);
+	return 0;
+}
+
+static const struct watchdog_info stmp3xxx_wdt_ident = {
+	.options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+	.identity = "STMP3XXX RTC Watchdog",
+};
+
+static const struct watchdog_ops stmp3xxx_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdt_start,
+	.stop = wdt_stop,
+	.set_timeout = wdt_set_timeout,
+};
+
+static struct watchdog_device stmp3xxx_wdd = {
+	.info = &stmp3xxx_wdt_ident,
+	.ops = &stmp3xxx_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = STMP3XXX_MAX_TIMEOUT,
+	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,
+};
+
+static int __devinit stmp3xxx_wdt_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	watchdog_set_drvdata(&stmp3xxx_wdd, pdev);
+
+	stmp3xxx_wdd.timeout = clamp_t(unsigned, timeout, 1, STMP3XXX_MAX_TIMEOUT);
+
+	ret = watchdog_register_device(&stmp3xxx_wdd);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "cannot register watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "initialized watchdog with timeout %ds\n", timeout);
+	return 0;
+}
+
+static int __devexit stmp3xxx_wdt_remove(struct platform_device *pdev)
+{
+	watchdog_unregister_device(&stmp3xxx_wdd);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int stmp3xxx_wdt_suspend(struct platform_device *pdev,
+				pm_message_t state)
+{
+	if (stmp3xxx_wdd.status & WDOG_ACTIVE)
+		wdt_stop(&stmp3xxx_wdd);
+
+	return 0;
+}
+
+static int stmp3xxx_wdt_resume(struct platform_device *pdev)
+{
+	if (stmp3xxx_wdd.status & WDOG_ACTIVE)
+		wdt_start(&stmp3xxx_wdd);
+
+	return 0;
+}
+#else
+#define stmp3xxx_wdt_suspend	NULL
+#define stmp3xxx_wdt_resume	NULL
+#endif
+
+static struct platform_driver stmp3xxx_wdt_driver = {
+	.driver = {
+		.name = "stmp3xxx_rtc_wdt",
+	},
+	.probe = stmp3xxx_wdt_probe,
+	.remove = __devexit_p(stmp3xxx_wdt_remove),
+	.suspend = stmp3xxx_wdt_suspend,
+	.resume = stmp3xxx_wdt_resume,
+};
+
+static int __init stmp3xxx_wdt_init(void)
+{
+	return platform_driver_register(&stmp3xxx_wdt_driver);
+}
+module_init(stmp3xxx_wdt_init);
+
+static void __exit stmp3xxx_wdt_exit(void)
+{
+	return platform_driver_unregister(&stmp3xxx_wdt_driver);
+}
+module_exit(stmp3xxx_wdt_exit);
+
+MODULE_DESCRIPTION("STMP3XXX RTC Watchdog Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Wolfram Sang <w.sang@pengutronix.de>");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.6.3

[PATCH 4/4] watchdog: remove old STMP driver

[Wolfram Sang]

Now that the new driver is in place, we can remove the old one.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---

Plain resend, because vger rejected the mail because of the triple X in
STMP3XXX :(

 drivers/watchdog/Kconfig        |    9 --
 drivers/watchdog/Makefile       |    1 -
 drivers/watchdog/stmp3xxx_wdt.c |  296 ---------------------------------------
 3 files changed, 0 insertions(+), 306 deletions(-)
 delete mode 100644 drivers/watchdog/stmp3xxx_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 5aad0dc..eb839de 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -296,15 +296,6 @@ config TWL4030_WATCHDOG
 	  Support for TI TWL4030 watchdog.  Say 'Y' here to enable the
 	  watchdog timer support for TWL4030 chips.
 
-config STMP3XXX_WATCHDOG
-	tristate "Freescale STMP3XXX watchdog"
-	depends on ARCH_STMP3XXX
-	help
-	  Say Y here if to include support for the watchdog timer
-	  for the Sigmatel STMP37XX/378X SoC.
-	  To compile this driver as a module, choose M here: the
-	  module will be called stmp3xxx_wdt.
-
 config STMP3XXX_RTC_WATCHDOG
 	tristate "Freescale STMP3XXX & i.MX23/28 watchdog"
 	depends on RTC_DRV_STMP
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 042076a..00f43d2 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -49,7 +49,6 @@ obj-$(CONFIG_IOP_WATCHDOG) += iop_wdt.o
 obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o
 obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o
 obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o
-obj-$(CONFIG_STMP3XXX_WATCHDOG) += stmp3xxx_wdt.o
 obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o
 obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
 obj-$(CONFIG_ADX_WATCHDOG) += adx_wdt.o
diff --git a/drivers/watchdog/stmp3xxx_wdt.c b/drivers/watchdog/stmp3xxx_wdt.c
deleted file mode 100644
index b3421fd..0000000
--- a/drivers/watchdog/stmp3xxx_wdt.c
+++ /dev/null
@@ -1,296 +0,0 @@
-/*
- * Watchdog driver for Freescale STMP37XX/STMP378X
- *
- * Author: Vitaly Wool <vital@embeddedalley.com>
- *
- * Copyright 2008 Freescale Semiconductor, Inc. All Rights Reserved.
- * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved.
- */
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/fs.h>
-#include <linux/miscdevice.h>
-#include <linux/watchdog.h>
-#include <linux/platform_device.h>
-#include <linux/spinlock.h>
-#include <linux/uaccess.h>
-
-#include <mach/platform.h>
-#include <mach/regs-rtc.h>
-
-#define DEFAULT_HEARTBEAT	19
-#define MAX_HEARTBEAT		(0x10000000 >> 6)
-
-/* missing bitmask in headers */
-#define BV_RTC_PERSISTENT1_GENERAL__RTC_FORCE_UPDATER     0x80000000
-
-#define WDT_IN_USE		0
-#define WDT_OK_TO_CLOSE		1
-
-#define WDOG_COUNTER_RATE	1000 /* 1 kHz clock */
-
-static DEFINE_SPINLOCK(stmp3xxx_wdt_io_lock);
-static unsigned long wdt_status;
-static const int nowayout = WATCHDOG_NOWAYOUT;
-static int heartbeat = DEFAULT_HEARTBEAT;
-static unsigned long boot_status;
-
-static void wdt_enable(u32 value)
-{
-	spin_lock(&stmp3xxx_wdt_io_lock);
-	__raw_writel(value, REGS_RTC_BASE + HW_RTC_WATCHDOG);
-	stmp3xxx_setl(BM_RTC_CTRL_WATCHDOGEN, REGS_RTC_BASE + HW_RTC_CTRL);
-	stmp3xxx_setl(BV_RTC_PERSISTENT1_GENERAL__RTC_FORCE_UPDATER,
-			REGS_RTC_BASE + HW_RTC_PERSISTENT1);
-	spin_unlock(&stmp3xxx_wdt_io_lock);
-}
-
-static void wdt_disable(void)
-{
-	spin_lock(&stmp3xxx_wdt_io_lock);
-	stmp3xxx_clearl(BV_RTC_PERSISTENT1_GENERAL__RTC_FORCE_UPDATER,
-			REGS_RTC_BASE + HW_RTC_PERSISTENT1);
-	stmp3xxx_clearl(BM_RTC_CTRL_WATCHDOGEN, REGS_RTC_BASE + HW_RTC_CTRL);
-	spin_unlock(&stmp3xxx_wdt_io_lock);
-}
-
-static void wdt_ping(void)
-{
-	wdt_enable(heartbeat * WDOG_COUNTER_RATE);
-}
-
-static int stmp3xxx_wdt_open(struct inode *inode, struct file *file)
-{
-	if (test_and_set_bit(WDT_IN_USE, &wdt_status))
-		return -EBUSY;
-
-	clear_bit(WDT_OK_TO_CLOSE, &wdt_status);
-	wdt_ping();
-
-	return nonseekable_open(inode, file);
-}
-
-static ssize_t stmp3xxx_wdt_write(struct file *file, const char __user *data,
-	size_t len, loff_t *ppos)
-{
-	if (len) {
-		if (!nowayout) {
-			size_t i;
-
-			clear_bit(WDT_OK_TO_CLOSE, &wdt_status);
-
-			for (i = 0; i != len; i++) {
-				char c;
-
-				if (get_user(c, data + i))
-					return -EFAULT;
-				if (c == 'V')
-					set_bit(WDT_OK_TO_CLOSE, &wdt_status);
-			}
-		}
-		wdt_ping();
-	}
-
-	return len;
-}
-
-static const struct watchdog_info ident = {
-	.options	= WDIOF_CARDRESET |
-			  WDIOF_MAGICCLOSE |
-			  WDIOF_SETTIMEOUT |
-			  WDIOF_KEEPALIVEPING,
-	.identity	= "STMP3XXX Watchdog",
-};
-
-static long stmp3xxx_wdt_ioctl(struct file *file, unsigned int cmd,
-	unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = argp;
-	int new_heartbeat, opts;
-	int ret = -ENOTTY;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		ret = copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
-		break;
-
-	case WDIOC_GETSTATUS:
-		ret = put_user(0, p);
-		break;
-
-	case WDIOC_GETBOOTSTATUS:
-		ret = put_user(boot_status, p);
-		break;
-
-	case WDIOC_SETOPTIONS:
-		if (get_user(opts, p)) {
-			ret = -EFAULT;
-			break;
-		}
-		if (opts & WDIOS_DISABLECARD)
-			wdt_disable();
-		else if (opts & WDIOS_ENABLECARD)
-			wdt_ping();
-		else {
-			pr_debug("%s: unknown option 0x%x\n", __func__, opts);
-			ret = -EINVAL;
-			break;
-		}
-		ret = 0;
-		break;
-
-	case WDIOC_KEEPALIVE:
-		wdt_ping();
-		ret = 0;
-		break;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_heartbeat, p)) {
-			ret = -EFAULT;
-			break;
-		}
-		if (new_heartbeat <= 0 || new_heartbeat > MAX_HEARTBEAT) {
-			ret = -EINVAL;
-			break;
-		}
-
-		heartbeat = new_heartbeat;
-		wdt_ping();
-		/* Fall through */
-
-	case WDIOC_GETTIMEOUT:
-		ret = put_user(heartbeat, p);
-		break;
-	}
-	return ret;
-}
-
-static int stmp3xxx_wdt_release(struct inode *inode, struct file *file)
-{
-	int ret = 0;
-
-	if (!nowayout) {
-		if (!test_bit(WDT_OK_TO_CLOSE, &wdt_status)) {
-			wdt_ping();
-			pr_debug("%s: Device closed unexpectdly\n", __func__);
-			ret = -EINVAL;
-		} else {
-			wdt_disable();
-			clear_bit(WDT_OK_TO_CLOSE, &wdt_status);
-		}
-	}
-	clear_bit(WDT_IN_USE, &wdt_status);
-
-	return ret;
-}
-
-static const struct file_operations stmp3xxx_wdt_fops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.write = stmp3xxx_wdt_write,
-	.unlocked_ioctl = stmp3xxx_wdt_ioctl,
-	.open = stmp3xxx_wdt_open,
-	.release = stmp3xxx_wdt_release,
-};
-
-static struct miscdevice stmp3xxx_wdt_miscdev = {
-	.minor = WATCHDOG_MINOR,
-	.name = "watchdog",
-	.fops = &stmp3xxx_wdt_fops,
-};
-
-static int __devinit stmp3xxx_wdt_probe(struct platform_device *pdev)
-{
-	int ret = 0;
-
-	if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
-		heartbeat = DEFAULT_HEARTBEAT;
-
-	boot_status = __raw_readl(REGS_RTC_BASE + HW_RTC_PERSISTENT1) &
-			BV_RTC_PERSISTENT1_GENERAL__RTC_FORCE_UPDATER;
-	boot_status = !!boot_status;
-	stmp3xxx_clearl(BV_RTC_PERSISTENT1_GENERAL__RTC_FORCE_UPDATER,
-			REGS_RTC_BASE + HW_RTC_PERSISTENT1);
-	wdt_disable();		/* disable for now */
-
-	ret = misc_register(&stmp3xxx_wdt_miscdev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "cannot register misc device\n");
-		return ret;
-	}
-
-	printk(KERN_INFO "stmp3xxx watchdog: initialized, heartbeat %d sec\n",
-		heartbeat);
-
-	return ret;
-}
-
-static int __devexit stmp3xxx_wdt_remove(struct platform_device *pdev)
-{
-	misc_deregister(&stmp3xxx_wdt_miscdev);
-	return 0;
-}
-
-#ifdef CONFIG_PM
-static int wdt_suspended;
-static u32 wdt_saved_time;
-
-static int stmp3xxx_wdt_suspend(struct platform_device *pdev,
-				pm_message_t state)
-{
-	if (__raw_readl(REGS_RTC_BASE + HW_RTC_CTRL) &
-		BM_RTC_CTRL_WATCHDOGEN) {
-		wdt_suspended = 1;
-		wdt_saved_time = __raw_readl(REGS_RTC_BASE + HW_RTC_WATCHDOG);
-		wdt_disable();
-	}
-	return 0;
-}
-
-static int stmp3xxx_wdt_resume(struct platform_device *pdev)
-{
-	if (wdt_suspended) {
-		wdt_enable(wdt_saved_time);
-		wdt_suspended = 0;
-	}
-	return 0;
-}
-#else
-#define stmp3xxx_wdt_suspend	NULL
-#define stmp3xxx_wdt_resume	NULL
-#endif
-
-static struct platform_driver platform_wdt_driver = {
-	.driver = {
-		.name = "stmp3xxx_wdt",
-	},
-	.probe = stmp3xxx_wdt_probe,
-	.remove = __devexit_p(stmp3xxx_wdt_remove),
-	.suspend = stmp3xxx_wdt_suspend,
-	.resume = stmp3xxx_wdt_resume,
-};
-
-static int __init stmp3xxx_wdt_init(void)
-{
-	return platform_driver_register(&platform_wdt_driver);
-}
-
-static void __exit stmp3xxx_wdt_exit(void)
-{
-	return platform_driver_unregister(&platform_wdt_driver);
-}
-
-module_init(stmp3xxx_wdt_init);
-module_exit(stmp3xxx_wdt_exit);
-
-MODULE_DESCRIPTION("STMP3XXX Watchdog Driver");
-MODULE_LICENSE("GPL");
-
-module_param(heartbeat, int, 0);
-MODULE_PARM_DESC(heartbeat,
-		 "Watchdog heartbeat period in seconds from 1 to "
-		 __MODULE_STRING(MAX_HEARTBEAT) ", default "
-		 __MODULE_STRING(DEFAULT_HEARTBEAT));
-
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.6.3

Re: [PATCH 1/4] rtc: stmp3xxx: add wdt-accessor function

[Andrew Morton]

On Wed, 28 Sep 2011 14:29:54 +0200
Wolfram Sang <w.sang@pengutronix.de> wrote:

> This RTC also includes a watchdog timer. Provide an accessor function
> for it to be picked up by a watchdog driver. Also register the
> platform_device here to get proper boot-time dependencies.
> 
> ...
>
> Andrew, Wim: Alessandro was not around in the last weeks, so I'd suggest this
> patch goes via the watchdog-tree together with the rest of this series if it
> passes the review of the watchdog-list? All changes here are in preparation for
> the watchdog driver and do not affect the general RTC. Is that okay?

OK by me.  Be careful about the Kconfig setup when doing
cross-subsystem dependencies like this.  Otherwise you get sad emails
from Randy when his build breaks.

>
> ...
>
>  struct stmp3xxx_rtc_data {
>  	struct rtc_device *rtc;
>  	void __iomem *io;
>  	int irq_alarm;
>  };
>  
> +#ifdef CONFIG_STMP3XXX_RTC_WATCHDOG

Should it be

#if defined(CONFIG_STMP3XXX_RTC_WATCHDOG) || defined(CONFIG_STMP3XXX_RTC_WATCHDOG_MODULE)

?

> +struct platform_device stmp3xxx_wdt_pdev = {

static?

> +	.name = "stmp3xxx_rtc_wdt",
> +	.id = -1,
> +};
> +
> +void stmp3xxx_wdt_setup(struct device *dev, u32 timeout)

I'd suggest documenting `timeout' at least.  Why would one want to set
it to zero and what effect does that have?  What are its units?

> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct stmp3xxx_rtc_data *rtc_data = platform_get_drvdata(pdev);
> +	void __iomem *base;
> +
> +	if (timeout) {
> +		writel(timeout, rtc_data->io + STMP3XXX_RTC_WATCHDOG);
> +		base = rtc_data->io + MXS_SET_ADDR;
> +	} else {
> +		base = rtc_data->io + MXS_CLR_ADDR;
> +	}
> +	writel(STMP3XXX_RTC_CTRL_WATCHDOGEN, base + STMP3XXX_RTC_CTRL);
> +	writel(STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER,
> +		base + STMP3XXX_RTC_PERSISTENT1);
> +}
> +EXPORT_SYMBOL_GPL(stmp3xxx_wdt_setup);

The patch adds a global symbol but fails to declare that symbol in a
header file.  This is always wrong.

> +static void stmp3xxx_wdt_register(struct device *dev)
> +{
> +	stmp3xxx_wdt_pdev.dev.parent = dev;
> +	platform_device_register(&stmp3xxx_wdt_pdev);
> +}
> +#else
> +static void stmp3xxx_wdt_register(struct device *dev)
> +{
> +}
> +#endif /* CONFIG_STMP3XXX_RTC_WATCHDOG */
> +
>  static void stmp3xxx_wait_time(struct stmp3xxx_rtc_data *rtc_data)
>  {
>  	/*
> @@ -231,6 +274,7 @@ static int stmp3xxx_rtc_probe(struct platform_device *pdev)
>  		goto out_irq_alarm;
>  	}
>  
> +	stmp3xxx_wdt_register(&pdev->dev);
>  	return 0;

Re: [PATCH 1/4] rtc: stmp3xxx: add wdt-accessor function

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

Hi Andrew,

> > Andrew, Wim: Alessandro was not around in the last weeks, so I'd suggest this
> > patch goes via the watchdog-tree together with the rest of this series if it
> > passes the review of the watchdog-list? All changes here are in preparation for
> > the watchdog driver and do not affect the general RTC. Is that okay?
> 
> OK by me.  Be careful about the Kconfig setup when doing
> cross-subsystem dependencies like this.  Otherwise you get sad emails
> from Randy when his build breaks.

Yes, the watchdog driver depends on the RTC.

> > +	.name = "stmp3xxx_rtc_wdt",
> > +	.id = -1,
> > +};
> > +
> > +void stmp3xxx_wdt_setup(struct device *dev, u32 timeout)
> 
> I'd suggest documenting `timeout' at least.  Why would one want to set
> it to zero and what effect does that have?  What are its units?

See below.

> The patch adds a global symbol but fails to declare that symbol in a
> header file.  This is always wrong.

I admit the aproach was quite wrong. I exported the function but wanted the
watchdog driver to be the only user (and so the parameters of the function were
quite specific to the watchdog driver). Thus, I kind of tried to hide its
interface for other users. I will try to come up with something better,
probably passing the function via the platform_device created at runtime. I
also played with sharing resources, yet that would require a top-level driver
managing the resources and two sub-level drivers (RTC and watchdog) which feels
unneccessary complex to me.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]