IMX Watchdog driver for machs MX2 up to MX51

IMX Watchdog driver for machs MX2 up to MX51

[Wolfram Sang]

Hello,

here is finally the watchdog driver for MX2 and later. It has been rewritten as
the changes from the last version are major:

- seperation from MX1-driver as discussed with Wim
- added the timer for petting a closed watchdog device
- switching to managed resources
- picking up ideas from Vladimir
- lots of simplification on the way

The MX1-driver is not included in this patchset. Mach-updates to affected machs
are included in this patchset. Some of the naming in there has grown
inconsistently (mxc_wdt vs imx_wdt_device0), but this happened to not only the
watchdog device, so the old names are kept for now, waiting for a general
cleanup-patch.

It has been tested on a mx27 and mx35 board. mx25 and mx51 were build-tested.
checkpatch is happy, too.

Please test, review and consider for inclusion.

Kind regards,

   Wolfram

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Vladimir Zapolskiy <vzapolskiy@gmail.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Wim Van Sebroeck <wim@iguana.be>
---
 drivers/watchdog/Kconfig    |   12 ++
 drivers/watchdog/Makefile   |    1 +
 drivers/watchdog/imx2_wdt.c |  279 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 292 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/imx2_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0e8468f..b24baec 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -306,6 +306,18 @@ config MAX63XX_WATCHDOG
 	help
 	  Support for memory mapped max63{69,70,71,72,73,74} watchdog timer.
 
+config IMX2_WDT
+	tristate "IMX2+ Watchdog"
+	depends on ARCH_MX2 || ARCH_MX25 || ARCH_MX3 || ARCH_MX5
+	help
+	  This is the driver for the hardware watchdog
+	  on the Freescale IMX2 and later processors.
+	  If you have one of these processors and wish to have
+	  watchdog support enabled, say Y, otherwise say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called imx2_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5e3cb95..72f3e20 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_STMP3XXX_WATCHDOG) += stmp3xxx_wdt.o
 obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
 obj-$(CONFIG_ADX_WATCHDOG) += adx_wdt.o
 obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
+obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
new file mode 100644
index 0000000..980aa2e
--- /dev/null
+++ b/drivers/watchdog/imx2_wdt.c
@@ -0,0 +1,279 @@
+/*
+ * Watchdog driver for IMX2 and later processors
+ *
+ *  Copyright (C) 2010 Wolfram Sang, Pengutronix e.K. <w.sang@pengutronix.de>
+ *
+ * some parts adapted by similar drivers from Darius Augulis and Vladimir
+ * Zapolskiy.
+ *
+ * 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.
+ *
+ * NOTE: MX1 has a slightly different Watchdog than MX2 and later:
+ *
+ *			MX1:		MX2+:
+ *			----		-----
+ * Registers:		32-bit		16-bit
+ * Stopable timer:	Yes		No
+ * Need to enable clk:	No		Yes
+ * Halt on suspend:	Manual		Can be automatic
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/clk.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <mach/hardware.h>
+
+#define DRIVER_NAME "imx2-wdt"
+
+#define IMX2_WDT_WCR		0x00		/* Control reg */
+#define IMX2_WDT_WCR_WDZST	(1 << 0)
+#define IMX2_WDT_WCR_WDE	(1 << 2)
+#define IMX2_WDT_WSR		0x02		/* Service reg */
+
+#define IMX2_WDT_MAX_TIME	128
+#define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
+#define IMX2_WDT_SEQ1		0x5555
+#define IMX2_WDT_SEQ2		0xAAAA
+
+#define IMX2_WDT_STATUS_OPEN	0
+#define IMX2_WDT_STATUS_STARTED	1
+
+static struct {
+	struct clk *clk;
+	void __iomem *base;
+	unsigned timeout;
+	unsigned long status;
+	struct timer_list timer;	/* Pings the watchdog when closed */
+} imx2_wdt;
+
+static struct miscdevice imx2_wdt_miscdev;
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+
+static unsigned timeout = IMX2_WDT_DEFAULT_TIME;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
+
+static const struct watchdog_info imx2_wdt_info = {
+	.identity = "imx2+ watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static void imx2_wdt_ping(void)
+{
+	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
+	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
+}
+
+static void imx2_wdt_timer_ping(unsigned long arg)
+{
+	imx2_wdt_ping();
+	mod_timer(&imx2_wdt.timer, jiffies + imx2_wdt.timeout * HZ / 2);
+}
+
+static void imx2_wdt_setup(void)
+{
+	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
+
+	val = (val & 0x00ff) | ((imx2_wdt.timeout * 2 - 1) << 8);
+	val |= IMX2_WDT_WCR_WDE;
+	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
+	imx2_wdt_ping();
+}
+
+static int imx2_wdt_open(struct inode *inode, struct file *file)
+{
+	if (test_and_set_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status))
+		return -EBUSY;
+
+	clk_enable(imx2_wdt.clk);
+	imx2_wdt_setup();
+	del_timer_sync(&imx2_wdt.timer);
+	set_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status);
+	return nonseekable_open(inode, file);
+}
+
+static int imx2_wdt_close(struct inode *inode, struct file *file)
+{
+	/* no clk_disable. it cannot be disabled once started */
+	clear_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status);
+	if (nowayout)
+		dev_crit(imx2_wdt_miscdev.parent,
+			"Device closed: Expect reboot!\n");
+	else
+		imx2_wdt_timer_ping(0);
+
+	return 0;
+}
+
+static int imx2_wdt_ioctl(struct inode *inode, struct file *file,
+		unsigned int cmd, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	int __user *p = argp;
+	int new_value;
+
+	switch (cmd) {
+	case WDIOC_KEEPALIVE:
+		imx2_wdt_ping();
+		return 0;
+
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &imx2_wdt_info,
+			sizeof(struct watchdog_info)) ? -EFAULT : 0;
+
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_value, p))
+			return -EFAULT;
+		if ((new_value < 1) || (new_value > IMX2_WDT_MAX_TIME))
+			return -EINVAL;
+
+		imx2_wdt.timeout = new_value;
+		imx2_wdt_setup();
+
+		/* Fallthrough to return current value */
+	case WDIOC_GETTIMEOUT:
+		return put_user(imx2_wdt.timeout, p);
+
+	default:
+		return -ENOTTY;
+	}
+}
+
+static ssize_t imx2_wdt_write(struct file *file, const char *data,
+						size_t len, loff_t *ppos)
+{
+	if (len)
+		imx2_wdt_ping();
+	return len;
+}
+
+static const struct file_operations imx2_wdt_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.ioctl = imx2_wdt_ioctl,
+	.open = imx2_wdt_open,
+	.release = imx2_wdt_close,
+	.write = imx2_wdt_write,
+};
+
+static struct miscdevice imx2_wdt_miscdev = {
+	.minor = WATCHDOG_MINOR,
+	.name = "watchdog",
+	.fops = &imx2_wdt_fops,
+};
+
+static void imx2_wdt_shutdown(struct platform_device *pdev)
+{
+	del_timer_sync(&imx2_wdt.timer);
+	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status))
+		dev_crit(imx2_wdt_miscdev.parent,
+			"Device shutdown: Expect reboot!\n");
+}
+
+static int __devinit imx2_wdt_probe(struct platform_device *pdev)
+{
+	int ret;
+	int res_size;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENODEV;
+	}
+
+	res_size = resource_size(res);
+	if (!devm_request_mem_region(&pdev->dev, res->start, res_size,
+		res->name)) {
+		dev_err(&pdev->dev, "can't allocate %d bytes at %d address\n",
+			res_size, res->start);
+		return -ENOMEM;
+	}
+
+	imx2_wdt.base = devm_ioremap_nocache(&pdev->dev, res->start, res_size);
+	if (!imx2_wdt.base) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		return -ENOMEM;
+	}
+
+	imx2_wdt.clk = clk_get_sys("imx-wdt.0", NULL);
+	if (IS_ERR(imx2_wdt.clk)) {
+		dev_err(&pdev->dev, "can't get Watchdog clock\n");
+		return PTR_ERR(imx2_wdt.clk);
+	}
+
+	imx2_wdt.timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
+	if (imx2_wdt.timeout != timeout)
+		dev_warn(&pdev->dev, "Initial timeout out of range! "
+			"Clamped from %u to %u\n", timeout, imx2_wdt.timeout);
+
+	imx2_wdt_miscdev.parent = &pdev->dev;
+	ret = misc_register(&imx2_wdt_miscdev);
+	if (ret)
+		goto fail;
+
+	setup_timer(&imx2_wdt.timer, imx2_wdt_timer_ping, 0);
+	dev_info(&pdev->dev, "IMX2+ Watchdog Timer enabled\n");
+	return 0;
+
+fail:
+	imx2_wdt_miscdev.parent = NULL;
+	clk_put(imx2_wdt.clk);
+	return ret;
+}
+
+static int __devexit imx2_wdt_remove(struct platform_device *pdev)
+{
+	del_timer_sync(&imx2_wdt.timer);
+	misc_deregister(&imx2_wdt_miscdev);
+	imx2_wdt_miscdev.parent = NULL;
+
+	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status))
+		dev_crit(imx2_wdt_miscdev.parent,
+			"Device removed: Expect reboot!\n");
+	else
+		clk_put(imx2_wdt.clk);
+
+	return 0;
+}
+
+static struct platform_driver imx2_wdt_driver = {
+	.probe		= imx2_wdt_probe,
+	.remove		= __devexit_p(imx2_wdt_remove),
+	.shutdown	= imx2_wdt_shutdown,
+	.driver		= {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init imx2_wdt_init(void)
+{
+	return platform_driver_probe(&imx2_wdt_driver, imx2_wdt_probe);
+}
+module_init(imx2_wdt_init);
+
+static void __exit imx2_wdt_exit(void)
+{
+	platform_driver_unregister(&imx2_wdt_driver);
+}
+module_exit(imx2_wdt_exit);
+
+MODULE_AUTHOR("Wolfram Sang");
+MODULE_DESCRIPTION("Watchdog driver for IMX2 and later");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
1.7.0

[PATCH 2/6] arm/mach-mx2/3: Fix watchdog-devices to match the current driver

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx2/devices.c |    9 ++-------
 arch/arm/mach-mx3/devices.c |    2 +-
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-mx2/devices.c b/arch/arm/mach-mx2/devices.c
index b91e412..a9377ce 100644
--- a/arch/arm/mach-mx2/devices.c
+++ b/arch/arm/mach-mx2/devices.c
@@ -109,12 +109,7 @@ DEFINE_IMX_GPT_DEVICE(4, MX27_GPT5_BASE_ADDR, MX27_INT_GPT5);
 DEFINE_IMX_GPT_DEVICE(5, MX27_GPT6_BASE_ADDR, MX27_INT_GPT6);
 #endif
 
-/*
- * Watchdog:
- * - i.MX1
- * - i.MX21
- * - i.MX27
- */
+/* Watchdog: i.MX1 has seperate driver, i.MX21 and i.MX27 are equal */
 static struct resource mxc_wdt_resources[] = {
 	{
 		.start = MX2x_WDOG_BASE_ADDR,
@@ -124,7 +119,7 @@ static struct resource mxc_wdt_resources[] = {
 };
 
 struct platform_device mxc_wdt = {
-	.name = "mxc_wdt",
+	.name = "imx2-wdt",
 	.id = 0,
 	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
 	.resource = mxc_wdt_resources,
diff --git a/arch/arm/mach-mx3/devices.c b/arch/arm/mach-mx3/devices.c
index 1ffed28..81a1dba 100644
--- a/arch/arm/mach-mx3/devices.c
+++ b/arch/arm/mach-mx3/devices.c
@@ -582,7 +582,7 @@ static struct resource imx_wdt_resources[] = {
 };
 
 struct platform_device imx_wdt_device0 = {
-	.name           = "imx-wdt",
+	.name           = "imx2-wdt",
 	.id             = 0,
 	.num_resources  = ARRAY_SIZE(imx_wdt_resources),
 	.resource       = imx_wdt_resources,
-- 
1.7.0

[PATCH 3/6] arm/mach-mx3: add watchdog device to Phytec-boards

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx3/mach-pcm037.c |    1 +
 arch/arm/mach-mx3/mach-pcm043.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx3/mach-pcm037.c b/arch/arm/mach-mx3/mach-pcm037.c
index 2df1ec5..5024284 100644
--- a/arch/arm/mach-mx3/mach-pcm037.c
+++ b/arch/arm/mach-mx3/mach-pcm037.c
@@ -449,6 +449,7 @@ static int __init pcm037_camera_alloc_dma(const size_t buf_size)
 static struct platform_device *devices[] __initdata = {
 	&pcm037_flash,
 	&pcm037_sram_device,
+	&imx_wdt_device0,
 	&pcm037_mt9t031,
 	&pcm037_mt9v022,
 };
diff --git a/arch/arm/mach-mx3/mach-pcm043.c b/arch/arm/mach-mx3/mach-pcm043.c
index 1bf1ec2..78d9185 100644
--- a/arch/arm/mach-mx3/mach-pcm043.c
+++ b/arch/arm/mach-mx3/mach-pcm043.c
@@ -150,6 +150,7 @@ static struct i2c_board_info pcm043_i2c_devices[] = {
 static struct platform_device *devices[] __initdata = {
 	&pcm043_flash,
 	&mxc_fec_device,
+	&imx_wdt_device0,
 };
 
 static struct pad_desc pcm043_pads[] = {
-- 
1.7.0

[PATCH 4/6] arm/mach-mx2: add watchdog device to Phytec-boards

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx2/mach-pca100.c |    1 +
 arch/arm/mach-mx2/mach-pcm038.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx2/mach-pca100.c b/arch/arm/mach-mx2/mach-pca100.c
index 778fff2..a87422e 100644
--- a/arch/arm/mach-mx2/mach-pca100.c
+++ b/arch/arm/mach-mx2/mach-pca100.c
@@ -145,6 +145,7 @@ static struct mxc_nand_platform_data pca100_nand_board_info = {
 static struct platform_device *platform_devices[] __initdata = {
 	&mxc_w1_master_device,
 	&mxc_fec_device,
+	&mxc_wdt,
 };
 
 static struct imxi2c_platform_data pca100_i2c_1_data = {
diff --git a/arch/arm/mach-mx2/mach-pcm038.c b/arch/arm/mach-mx2/mach-pcm038.c
index 035fbe0..36c8943 100644
--- a/arch/arm/mach-mx2/mach-pcm038.c
+++ b/arch/arm/mach-mx2/mach-pcm038.c
@@ -182,6 +182,7 @@ static struct platform_device *platform_devices[] __initdata = {
 	&mxc_w1_master_device,
 	&mxc_fec_device,
 	&pcm038_sram_mtd_device,
+	&mxc_wdt,
 };
 
 /* On pcm038 there's a sram attached to CS1, we enable the chipselect here and
-- 
1.7.0

[PATCH 5/6] arm/mx25: add watchdog device

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx25/devices.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx25/devices.c b/arch/arm/mach-mx25/devices.c
index 3f4b8a0..3a405fa 100644
--- a/arch/arm/mach-mx25/devices.c
+++ b/arch/arm/mach-mx25/devices.c
@@ -500,3 +500,18 @@ struct platform_device mx25_fb_device = {
 		.coherent_dma_mask = 0xFFFFFFFF,
 	},
 };
+
+static struct resource mxc_wdt_resources[] = {
+	{
+		.start = MX25_WDOG_BASE_ADDR,
+		.end = MX25_WDOG_BASE_ADDR + SZ_16K - 1,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+struct platform_device mxc_wdt = {
+	.name = "imx2-wdt",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
+	.resource = mxc_wdt_resources,
+};
-- 
1.7.0

[PATCH 6/6] arm/mx51: add watchdog device

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx5/devices.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
index 5070ae1..a039f9e 100644
--- a/arch/arm/mach-mx5/devices.c
+++ b/arch/arm/mach-mx5/devices.c
@@ -92,6 +92,21 @@ struct platform_device mxc_fec_device = {
 	.resource = mxc_fec_resources,
 };
 
+static struct resource mxc_wdt_resources[] = {
+	{
+		.start = MX51_WDOG_BASE_ADDR,
+		.end = MX51_WDOG_BASE_ADDR + SZ_16K - 1,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+struct platform_device mxc_wdt = {
+	.name = "imx2-wdt",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
+	.resource = mxc_wdt_resources,
+};
+
 static struct mxc_gpio_port mxc_gpio_ports[] = {
 	{
 		.chip.label = "gpio-0",
-- 
1.7.0

IMX Watchdog driver for machs MX2 up to MX51

[Wolfram Sang]

On Thu, Apr 29, 2010 at 10:03:16AM +0200, Wolfram Sang wrote:
> Hello,
> 
> here is finally the watchdog driver for MX2 and later. It has been rewritten as
> the changes from the last version are major:
> 
> - seperation from MX1-driver as discussed with Wim
> - added the timer for petting a closed watchdog device
> - switching to managed resources
> - picking up ideas from Vladimir
> - lots of simplification on the way
> 
> The MX1-driver is not included in this patchset. Mach-updates to affected machs
> are included in this patchset. Some of the naming in there has grown
> inconsistently (mxc_wdt vs imx_wdt_device0), but this happened to not only the
> watchdog device, so the old names are kept for now, waiting for a general
> cleanup-patch.
> 
> It has been tested on a mx27 and mx35 board. mx25 and mx51 were build-tested.
> checkpatch is happy, too.
> 
> Please test, review and consider for inclusion.
> 
> Kind regards,
> 
>    Wolfram

Adding Wim to cc...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100429/cc2077e1/attachment-0001.sig>

[PATCH 5/6] arm/mx25: add watchdog device

[Juergen Beisert]

Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>

Runtime tested on a i.MX25 CPU based target.

Tested-by: Juergen Beisert <jbe@pengutronix.de>

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wolfram Sang]

On Thu, Apr 29, 2010 at 10:03:17AM +0200, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Vladimir Zapolskiy <vzapolskiy@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Wim Van Sebroeck <wim@iguana.be>

Ping. Wim, did you notice this one? Shall the driver go via your tree or via
the imx-tree along with the resource updates (with your ack)?

> ---
>  drivers/watchdog/Kconfig    |   12 ++
>  drivers/watchdog/Makefile   |    1 +
>  drivers/watchdog/imx2_wdt.c |  279 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 292 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/watchdog/imx2_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0e8468f..b24baec 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -306,6 +306,18 @@ config MAX63XX_WATCHDOG
>  	help
>  	  Support for memory mapped max63{69,70,71,72,73,74} watchdog timer.
>  
> +config IMX2_WDT
> +	tristate "IMX2+ Watchdog"
> +	depends on ARCH_MX2 || ARCH_MX25 || ARCH_MX3 || ARCH_MX5
> +	help
> +	  This is the driver for the hardware watchdog
> +	  on the Freescale IMX2 and later processors.
> +	  If you have one of these processors and wish to have
> +	  watchdog support enabled, say Y, otherwise say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called imx2_wdt.
> +
>  # AVR32 Architecture
>  
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5e3cb95..72f3e20 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_STMP3XXX_WATCHDOG) += stmp3xxx_wdt.o
>  obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
>  obj-$(CONFIG_ADX_WATCHDOG) += adx_wdt.o
>  obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> +obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> new file mode 100644
> index 0000000..980aa2e
> --- /dev/null
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -0,0 +1,279 @@
> +/*
> + * Watchdog driver for IMX2 and later processors
> + *
> + *  Copyright (C) 2010 Wolfram Sang, Pengutronix e.K. <w.sang@pengutronix.de>
> + *
> + * some parts adapted by similar drivers from Darius Augulis and Vladimir
> + * Zapolskiy.
> + *
> + * 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.
> + *
> + * NOTE: MX1 has a slightly different Watchdog than MX2 and later:
> + *
> + *			MX1:		MX2+:
> + *			----		-----
> + * Registers:		32-bit		16-bit
> + * Stopable timer:	Yes		No
> + * Need to enable clk:	No		Yes
> + * Halt on suspend:	Manual		Can be automatic
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/clk.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/timer.h>
> +#include <linux/jiffies.h>
> +#include <mach/hardware.h>
> +
> +#define DRIVER_NAME "imx2-wdt"
> +
> +#define IMX2_WDT_WCR		0x00		/* Control reg */
> +#define IMX2_WDT_WCR_WDZST	(1 << 0)
> +#define IMX2_WDT_WCR_WDE	(1 << 2)
> +#define IMX2_WDT_WSR		0x02		/* Service reg */
> +
> +#define IMX2_WDT_MAX_TIME	128
> +#define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
> +#define IMX2_WDT_SEQ1		0x5555
> +#define IMX2_WDT_SEQ2		0xAAAA
> +
> +#define IMX2_WDT_STATUS_OPEN	0
> +#define IMX2_WDT_STATUS_STARTED	1
> +
> +static struct {
> +	struct clk *clk;
> +	void __iomem *base;
> +	unsigned timeout;
> +	unsigned long status;
> +	struct timer_list timer;	/* Pings the watchdog when closed */
> +} imx2_wdt;
> +
> +static struct miscdevice imx2_wdt_miscdev;
> +
> +static int nowayout = WATCHDOG_NOWAYOUT;
> +
> +static unsigned timeout = IMX2_WDT_DEFAULT_TIME;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
> +
> +static const struct watchdog_info imx2_wdt_info = {
> +	.identity = "imx2+ watchdog",
> +	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static void imx2_wdt_ping(void)
> +{
> +	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> +	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
> +}
> +
> +static void imx2_wdt_timer_ping(unsigned long arg)
> +{
> +	imx2_wdt_ping();
> +	mod_timer(&imx2_wdt.timer, jiffies + imx2_wdt.timeout * HZ / 2);
> +}
> +
> +static void imx2_wdt_setup(void)
> +{
> +	u16 val = __raw_readw(imx2_wdt.base + IMX2_WDT_WCR);
> +
> +	val = (val & 0x00ff) | ((imx2_wdt.timeout * 2 - 1) << 8);
> +	val |= IMX2_WDT_WCR_WDE;
> +	__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> +	imx2_wdt_ping();
> +}
> +
> +static int imx2_wdt_open(struct inode *inode, struct file *file)
> +{
> +	if (test_and_set_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status))
> +		return -EBUSY;
> +
> +	clk_enable(imx2_wdt.clk);
> +	imx2_wdt_setup();
> +	del_timer_sync(&imx2_wdt.timer);
> +	set_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status);
> +	return nonseekable_open(inode, file);
> +}
> +
> +static int imx2_wdt_close(struct inode *inode, struct file *file)
> +{
> +	/* no clk_disable. it cannot be disabled once started */
> +	clear_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status);
> +	if (nowayout)
> +		dev_crit(imx2_wdt_miscdev.parent,
> +			"Device closed: Expect reboot!\n");
> +	else
> +		imx2_wdt_timer_ping(0);
> +
> +	return 0;
> +}
> +
> +static int imx2_wdt_ioctl(struct inode *inode, struct file *file,
> +		unsigned int cmd, unsigned long arg)
> +{
> +	void __user *argp = (void __user *)arg;
> +	int __user *p = argp;
> +	int new_value;
> +
> +	switch (cmd) {
> +	case WDIOC_KEEPALIVE:
> +		imx2_wdt_ping();
> +		return 0;
> +
> +	case WDIOC_GETSUPPORT:
> +		return copy_to_user(argp, &imx2_wdt_info,
> +			sizeof(struct watchdog_info)) ? -EFAULT : 0;
> +
> +	case WDIOC_SETTIMEOUT:
> +		if (get_user(new_value, p))
> +			return -EFAULT;
> +		if ((new_value < 1) || (new_value > IMX2_WDT_MAX_TIME))
> +			return -EINVAL;
> +
> +		imx2_wdt.timeout = new_value;
> +		imx2_wdt_setup();
> +
> +		/* Fallthrough to return current value */
> +	case WDIOC_GETTIMEOUT:
> +		return put_user(imx2_wdt.timeout, p);
> +
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
> +static ssize_t imx2_wdt_write(struct file *file, const char *data,
> +						size_t len, loff_t *ppos)
> +{
> +	if (len)
> +		imx2_wdt_ping();
> +	return len;
> +}
> +
> +static const struct file_operations imx2_wdt_fops = {
> +	.owner = THIS_MODULE,
> +	.llseek = no_llseek,
> +	.ioctl = imx2_wdt_ioctl,
> +	.open = imx2_wdt_open,
> +	.release = imx2_wdt_close,
> +	.write = imx2_wdt_write,
> +};
> +
> +static struct miscdevice imx2_wdt_miscdev = {
> +	.minor = WATCHDOG_MINOR,
> +	.name = "watchdog",
> +	.fops = &imx2_wdt_fops,
> +};
> +
> +static void imx2_wdt_shutdown(struct platform_device *pdev)
> +{
> +	del_timer_sync(&imx2_wdt.timer);
> +	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status))
> +		dev_crit(imx2_wdt_miscdev.parent,
> +			"Device shutdown: Expect reboot!\n");
> +}
> +
> +static int __devinit imx2_wdt_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	int res_size;
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "can't get device resources\n");
> +		return -ENODEV;
> +	}
> +
> +	res_size = resource_size(res);
> +	if (!devm_request_mem_region(&pdev->dev, res->start, res_size,
> +		res->name)) {
> +		dev_err(&pdev->dev, "can't allocate %d bytes at %d address\n",
> +			res_size, res->start);
> +		return -ENOMEM;
> +	}
> +
> +	imx2_wdt.base = devm_ioremap_nocache(&pdev->dev, res->start, res_size);
> +	if (!imx2_wdt.base) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	imx2_wdt.clk = clk_get_sys("imx-wdt.0", NULL);
> +	if (IS_ERR(imx2_wdt.clk)) {
> +		dev_err(&pdev->dev, "can't get Watchdog clock\n");
> +		return PTR_ERR(imx2_wdt.clk);
> +	}
> +
> +	imx2_wdt.timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
> +	if (imx2_wdt.timeout != timeout)
> +		dev_warn(&pdev->dev, "Initial timeout out of range! "
> +			"Clamped from %u to %u\n", timeout, imx2_wdt.timeout);
> +
> +	imx2_wdt_miscdev.parent = &pdev->dev;
> +	ret = misc_register(&imx2_wdt_miscdev);
> +	if (ret)
> +		goto fail;
> +
> +	setup_timer(&imx2_wdt.timer, imx2_wdt_timer_ping, 0);
> +	dev_info(&pdev->dev, "IMX2+ Watchdog Timer enabled\n");
> +	return 0;
> +
> +fail:
> +	imx2_wdt_miscdev.parent = NULL;
> +	clk_put(imx2_wdt.clk);
> +	return ret;
> +}
> +
> +static int __devexit imx2_wdt_remove(struct platform_device *pdev)
> +{
> +	del_timer_sync(&imx2_wdt.timer);
> +	misc_deregister(&imx2_wdt_miscdev);
> +	imx2_wdt_miscdev.parent = NULL;
> +
> +	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status))
> +		dev_crit(imx2_wdt_miscdev.parent,
> +			"Device removed: Expect reboot!\n");
> +	else
> +		clk_put(imx2_wdt.clk);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver imx2_wdt_driver = {
> +	.probe		= imx2_wdt_probe,
> +	.remove		= __devexit_p(imx2_wdt_remove),
> +	.shutdown	= imx2_wdt_shutdown,
> +	.driver		= {
> +		.name	= DRIVER_NAME,
> +		.owner	= THIS_MODULE,
> +	},
> +};
> +
> +static int __init imx2_wdt_init(void)
> +{
> +	return platform_driver_probe(&imx2_wdt_driver, imx2_wdt_probe);
> +}
> +module_init(imx2_wdt_init);
> +
> +static void __exit imx2_wdt_exit(void)
> +{
> +	platform_driver_unregister(&imx2_wdt_driver);
> +}
> +module_exit(imx2_wdt_exit);
> +
> +MODULE_AUTHOR("Wolfram Sang");
> +MODULE_DESCRIPTION("Watchdog driver for IMX2 and later");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> -- 
> 1.7.0
> 

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100514/ebd6a81e/attachment.sig>

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wim Van Sebroeck]

Hi Wolfram,

> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > Cc: Vladimir Zapolskiy <vzapolskiy@gmail.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Wim Van Sebroeck <wim@iguana.be>
> 
> Ping. Wim, did you notice this one? Shall the driver go via your tree or via
> the imx-tree along with the resource updates (with your ack)?

Yes, I saw this one. I started reviewing it, but didn't finish it yet.
Will let you know something when it's done.

Driver can go via my tree but since it's part of a series of other patches, it can also
go via the imx tree (which is probably preferred).

Kind regards,
Wim.

[PATCH 5/6] arm/mx25: add watchdog device

[Sascha Hauer]

Hi Wolfram,

This patch (and the next one) miss the corresponding entry in devices.h

Sascha

On Thu, Apr 29, 2010 at 10:03:21AM +0200, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/mach-mx25/devices.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-mx25/devices.c b/arch/arm/mach-mx25/devices.c
> index 3f4b8a0..3a405fa 100644
> --- a/arch/arm/mach-mx25/devices.c
> +++ b/arch/arm/mach-mx25/devices.c
> @@ -500,3 +500,18 @@ struct platform_device mx25_fb_device = {
>  		.coherent_dma_mask = 0xFFFFFFFF,
>  	},
>  };
> +
> +static struct resource mxc_wdt_resources[] = {
> +	{
> +		.start = MX25_WDOG_BASE_ADDR,
> +		.end = MX25_WDOG_BASE_ADDR + SZ_16K - 1,
> +		.flags = IORESOURCE_MEM,
> +	},
> +};
> +
> +struct platform_device mxc_wdt = {
> +	.name = "imx2-wdt",
> +	.id = 0,
> +	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
> +	.resource = mxc_wdt_resources,
> +};
> -- 
> 1.7.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Sascha Hauer]

On Fri, May 14, 2010 at 05:56:31PM +0200, Wim Van Sebroeck wrote:
> Hi Wolfram,
> 
> > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > > Cc: Vladimir Zapolskiy <vzapolskiy@gmail.com>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Wim Van Sebroeck <wim@iguana.be>
> > 
> > Ping. Wim, did you notice this one? Shall the driver go via your tree or via
> > the imx-tree along with the resource updates (with your ack)?
> 
> Yes, I saw this one. I started reviewing it, but didn't finish it yet.
> Will let you know something when it's done.
> 
> Driver can go via my tree but since it's part of a series of other patches, it can also
> go via the imx tree (which is probably preferred).

They are ortogonal, so feel free to apply the driver once you finished
reviewing it.
Meanwhile I'll take care of the platform patches.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 2/6] arm/mach-mx2/3: Fix watchdog-devices to match the current driver

[Sascha Hauer]

On Thu, Apr 29, 2010 at 10:03:18AM +0200, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/mach-mx2/devices.c |    9 ++-------
>  arch/arm/mach-mx3/devices.c |    2 +-
>  2 files changed, 3 insertions(+), 8 deletions(-)

Applied to mxc-master.

Sascha


> 
> 
> diff --git a/arch/arm/mach-mx2/devices.c b/arch/arm/mach-mx2/devices.c
> index b91e412..a9377ce 100644
> --- a/arch/arm/mach-mx2/devices.c
> +++ b/arch/arm/mach-mx2/devices.c
> @@ -109,12 +109,7 @@ DEFINE_IMX_GPT_DEVICE(4, MX27_GPT5_BASE_ADDR, MX27_INT_GPT5);
>  DEFINE_IMX_GPT_DEVICE(5, MX27_GPT6_BASE_ADDR, MX27_INT_GPT6);
>  #endif
>  
> -/*
> - * Watchdog:
> - * - i.MX1
> - * - i.MX21
> - * - i.MX27
> - */
> +/* Watchdog: i.MX1 has seperate driver, i.MX21 and i.MX27 are equal */
>  static struct resource mxc_wdt_resources[] = {
>  	{
>  		.start = MX2x_WDOG_BASE_ADDR,
> @@ -124,7 +119,7 @@ static struct resource mxc_wdt_resources[] = {
>  };
>  
>  struct platform_device mxc_wdt = {
> -	.name = "mxc_wdt",
> +	.name = "imx2-wdt",
>  	.id = 0,
>  	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
>  	.resource = mxc_wdt_resources,
> diff --git a/arch/arm/mach-mx3/devices.c b/arch/arm/mach-mx3/devices.c
> index 1ffed28..81a1dba 100644
> --- a/arch/arm/mach-mx3/devices.c
> +++ b/arch/arm/mach-mx3/devices.c
> @@ -582,7 +582,7 @@ static struct resource imx_wdt_resources[] = {
>  };
>  
>  struct platform_device imx_wdt_device0 = {
> -	.name           = "imx-wdt",
> +	.name           = "imx2-wdt",
>  	.id             = 0,
>  	.num_resources  = ARRAY_SIZE(imx_wdt_resources),
>  	.resource       = imx_wdt_resources,
> -- 
> 1.7.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/6] arm/mach-mx3: add watchdog device to Phytec-boards

[Sascha Hauer]

On Thu, Apr 29, 2010 at 10:03:19AM +0200, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/mach-mx3/mach-pcm037.c |    1 +
>  arch/arm/mach-mx3/mach-pcm043.c |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)

Applied to mxc-master (also 4/6)

Sascha

> 
> diff --git a/arch/arm/mach-mx3/mach-pcm037.c b/arch/arm/mach-mx3/mach-pcm037.c
> index 2df1ec5..5024284 100644
> --- a/arch/arm/mach-mx3/mach-pcm037.c
> +++ b/arch/arm/mach-mx3/mach-pcm037.c
> @@ -449,6 +449,7 @@ static int __init pcm037_camera_alloc_dma(const size_t buf_size)
>  static struct platform_device *devices[] __initdata = {
>  	&pcm037_flash,
>  	&pcm037_sram_device,
> +	&imx_wdt_device0,
>  	&pcm037_mt9t031,
>  	&pcm037_mt9v022,
>  };
> diff --git a/arch/arm/mach-mx3/mach-pcm043.c b/arch/arm/mach-mx3/mach-pcm043.c
> index 1bf1ec2..78d9185 100644
> --- a/arch/arm/mach-mx3/mach-pcm043.c
> +++ b/arch/arm/mach-mx3/mach-pcm043.c
> @@ -150,6 +150,7 @@ static struct i2c_board_info pcm043_i2c_devices[] = {
>  static struct platform_device *devices[] __initdata = {
>  	&pcm043_flash,
>  	&mxc_fec_device,
> +	&imx_wdt_device0,
>  };
>  
>  static struct pad_desc pcm043_pads[] = {
> -- 
> 1.7.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH V2 5/6] arm/mx25: add watchdog device

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Tested-by: Juergen Beisert <jbe@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx25/devices.c |   15 +++++++++++++++
 arch/arm/mach-mx25/devices.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx25/devices.c b/arch/arm/mach-mx25/devices.c
index 3f4b8a0..3a405fa 100644
--- a/arch/arm/mach-mx25/devices.c
+++ b/arch/arm/mach-mx25/devices.c
@@ -500,3 +500,18 @@ struct platform_device mx25_fb_device = {
 		.coherent_dma_mask = 0xFFFFFFFF,
 	},
 };
+
+static struct resource mxc_wdt_resources[] = {
+	{
+		.start = MX25_WDOG_BASE_ADDR,
+		.end = MX25_WDOG_BASE_ADDR + SZ_16K - 1,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+struct platform_device mxc_wdt = {
+	.name = "imx2-wdt",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
+	.resource = mxc_wdt_resources,
+};
diff --git a/arch/arm/mach-mx25/devices.h b/arch/arm/mach-mx25/devices.h
index 39560e1..cee12c0 100644
--- a/arch/arm/mach-mx25/devices.h
+++ b/arch/arm/mach-mx25/devices.h
@@ -21,3 +21,4 @@ extern struct platform_device mx25_fec_device;
 extern struct platform_device mxc_nand_device;
 extern struct platform_device mx25_rtc_device;
 extern struct platform_device mx25_fb_device;
+extern struct platform_device mxc_wdt;
-- 
1.7.0

[PATCH V2 6/6] arm/mx51: add watchdog device

[Wolfram Sang]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-mx5/devices.c |   15 +++++++++++++++
 arch/arm/mach-mx5/devices.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
index 23850e6..7130449 100644
--- a/arch/arm/mach-mx5/devices.c
+++ b/arch/arm/mach-mx5/devices.c
@@ -153,6 +153,21 @@ struct platform_device mxc_usbh1_device = {
 	},
 };
 
+static struct resource mxc_wdt_resources[] = {
+	{
+		.start = MX51_WDOG_BASE_ADDR,
+		.end = MX51_WDOG_BASE_ADDR + SZ_16K - 1,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+struct platform_device mxc_wdt = {
+	.name = "imx2-wdt",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(mxc_wdt_resources),
+	.resource = mxc_wdt_resources,
+};
+
 static struct mxc_gpio_port mxc_gpio_ports[] = {
 	{
 		.chip.label = "gpio-0",
diff --git a/arch/arm/mach-mx5/devices.h b/arch/arm/mach-mx5/devices.h
index 0494d6b..c879ae7 100644
--- a/arch/arm/mach-mx5/devices.h
+++ b/arch/arm/mach-mx5/devices.h
@@ -5,3 +5,4 @@ extern struct platform_device mxc_fec_device;
 extern struct platform_device mxc_usbdr_host_device;
 extern struct platform_device mxc_usbh1_device;
 extern struct platform_device mxc_usbdr_udc_device;
+extern struct platform_device mxc_wdt;
-- 
1.7.0

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wim Van Sebroeck]

Hi Wolfram,

> Ping. Wim, did you notice this one? Shall the driver go via your tree or via
> the imx-tree along with the resource updates (with your ack)?

I was not to happy with how the close of /dev/watchdog was being done.
So I modified your code so that
* the close is better supported.
* the code is ready for when we change to the generic watchdog api.
* the magic close feature is supported.

I didn't compile test the code. Could you have a look at it? compile it and test it?
Goal is to get this in during this merge window still...

Kind regards,
Wim.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: imx2_wdt.c
Type: text/x-csrc
Size: 10545 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100521/9cb30207/attachment-0001.bin>

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wolfram Sang]

Hi Wim,

thanks for the review!

On Fri, May 21, 2010 at 11:52:11PM +0200, Wim Van Sebroeck wrote:
> Hi Wolfram,
> 
> > Ping. Wim, did you notice this one? Shall the driver go via your tree or via
> > the imx-tree along with the resource updates (with your ack)?
> 
> I was not to happy with how the close of /dev/watchdog was being done.
> So I modified your code so that
> * the close is better supported.
> * the code is ready for when we change to the generic watchdog api.

Cool, any concrete plans for this happening?

> * the magic close feature is supported.

OK, I left it out intentionally, as I expected it to come for free with the new
api at a later stage:)

> 
> I didn't compile test the code. Could you have a look at it? compile it and test it?

Will do. Some initial comments. (An incremental patch might have been easier to
check)

> Goal is to get this in during this merge window still...

The new driver-rule should apply here, no?

> 
> Kind regards,
> Wim.
> 

> /*
>  * Watchdog driver for IMX2 and later processors
>  *
>  *  Copyright (C) 2010 Wolfram Sang, Pengutronix e.K. <w.sang@pengutronix.de>
>  *
>  * some parts adapted by similar drivers from Darius Augulis and Vladimir
>  * Zapolskiy.
>  *
>  * 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.
>  *
>  * NOTE: MX1 has a slightly different Watchdog than MX2 and later:
>  *
>  *			MX1:		MX2+:
>  *			----		-----
>  * Registers:		32-bit		16-bit
>  * Stopable timer:	Yes		No
>  * Need to enable clk:	No		Yes
>  * Halt on suspend:	Manual		Can be automatic
>  */
> 
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/miscdevice.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/platform_device.h>
> #include <linux/watchdog.h>
> #include <linux/clk.h>
> #include <linux/fs.h>
> #include <linux/io.h>
> #include <linux/uaccess.h>
> #include <linux/timer.h>
> #include <linux/jiffies.h>
> #include <mach/hardware.h>
> 
> #define DRIVER_NAME "imx2-wdt"
> 
> #define IMX2_WDT_WCR		0x00		/* Control Register */
> #define IMX2_WDT_WCR_WT		(0xFF << 8)	/* -> Watchdog Timeout Field */
> #define IMX2_WDT_WCR_WDA	(1 << 5)	/* -> !Watchdog Assertion */
> #define IMX2_WDT_WCR_SRS	(1 << 4)	/* -> !Software Reset Signal */
> #define IMX2_WDT_WCR_WRE	(1 << 3)	/* -> WDOG Reset Enable */
> #define IMX2_WDT_WCR_WDE	(1 << 2)	/* -> Watchdog Enable */
> #define IMX2_WDT_WCR_WDBG	(1 << 1)	/* -> Watchdog Debug Enable */
> #define IMX2_WDT_WCR_WDZST	(1 << 0)	/* -> Watchdog Low Power */
> 
> #define IMX2_WDT_WSR		0x02		/* Service Register */
> #define IMX2_WDT_SEQ1		0x5555		/* -> service sequence 1 */
> #define IMX2_WDT_SEQ2		0xAAAA		/* -> service sequence 2 */
> 
> #define IMX2_WDT_WRSR		0x04		/* Reset Status Register */
> #define IMX2_WDT_WRSR_PWR	(1 << 4)	/* -> Power-On Reset */
> #define IMX2_WDT_WRSR_EXT	(1 << 3)	/* -> External Reset */
> #define IMX2_WDT_WRSR_TOUT	(1 << 1)	/* -> WDOG Time-Out Reset */
> #define IMX2_WDT_WRSR_SFTW	(1 << 0)	/* -> Software Reset */
> 
> #define IMX2_WDT_MAX_TIME	128
> #define IMX2_WDT_DEFAULT_TIME	60		/* in seconds */
> 
> #define WDOG_SEC_TO_COUNT(s)	((s * 2) << 8)

Any reason you dropped the '- 1' from my version here?

> 
> #define IMX2_WDT_STATUS_OPEN	0
> #define IMX2_WDT_STATUS_STARTED	1
> #define IMX2_WDT_EXPECT_CLOSE	2
> 
> /* Apply nand and or masks to data read from addr and write back
>  * we nand first so that we can erase the timeout before setting the new one */
> #define IMX2_WDT_CHG_BITS(addr, nand, or) \
> 	__raw_writew((__raw_readw(addr) & ~nand) | or, addr)

I don't like such a macro very much, but well...

> 
> 
> static struct {
> 	struct clk *clk;
> 	void __iomem *base;
> 	unsigned timeout;
> 	unsigned long status;
> 	struct timer_list timer;	/* Pings the watchdog when closed */
> } imx2_wdt;
> 
> static struct miscdevice imx2_wdt_miscdev;
> 
> static int nowayout = WATCHDOG_NOWAYOUT;
> module_param(nowayout, int, 0);
> MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> 				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> 
> 
> static unsigned timeout = IMX2_WDT_DEFAULT_TIME;
> module_param(timeout, uint, 0);
> MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
> 				__MODULE_STRING(IMX2_WDT_DEFAULT_TIME) ")");
> 
> static const struct watchdog_info imx2_wdt_info = {
> 	.identity = "imx2+ watchdog",
> 	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
> };
> 
> static inline void imx2_wdt_init(void)
> {
> 	unsigned int bits_off;
> 	unsigned int bits_on;
> 
> 	/* Strip the old watchdog Time-Out value */
> 	bits_off = IMX2_WDT_WCR_WT;
> 	/* Generate reset if WDOG times out */
> 	bits_off |= IMX2_WDT_WCR_WRE;
> 	/* Keep Watchdog Disabled */
> 	bits_off |= IMX2_WDT_WCR_WDE;
> 	/* Continue timer operation during DEBUG mode */
> 	bits_off |= IMX2_WDT_WCR_WDBG;
> 	/* Continue Watchdog Timer during Low Power modes */
> 	bits_off |= IMX2_WDT_WCR_WDZST;
> 	
> 	/* Set the watchdog's Time-Out value */
> 	bits_on = WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
> 	/* Don't set Watchdog Assertion */
> 	bits_on |= IMX2_WDT_WCR_WDA;
> 	/* Don't set Software Reset Signal */
> 	bits_on |= IMX2_WDT_WCR_SRS;

This is too excessive IMO. The bootloader might already have setup the watchdog
according to the board.

> 
> 	IMX2_WDT_CHG_BITS(imx2_wdt.base + IMX2_WDT_WCR, bits_off, bits_on);
> }
> 
> static inline void imx2_wdt_enable(void)
> {
> 	IMX2_WDT_CHG_BITS(imx2_wdt.base + IMX2_WDT_WCR, 0, IMX2_WDT_WCR_WDE);
> }

I assume this is for the easy migration to the generic watchdog API? Otherwise
I see little use for a seperate function.

> 
> static inline void imx2_wdt_ping(void)
> {
> 	/* When enabled, the Watchdog requires that a service sequence be
> 	 * written to the Watchdog Service Register (WSR) */

IMHO this is maybe over-commenting :)

> 	__raw_writew(IMX2_WDT_SEQ1, imx2_wdt.base + IMX2_WDT_WSR);
> 	__raw_writew(IMX2_WDT_SEQ2, imx2_wdt.base + IMX2_WDT_WSR);
> }
> 
> static void imx2_wdt_timer_ping(void)
> {
> 	/* ping it every imx2_wdt.timeout / 2 seconds to prevent reboot */
> 	imx2_wdt_ping();
> 	mod_timer(&imx2_wdt.timer, jiffies + imx2_wdt.timeout * HZ / 2);
> }
> 
> static void imx2_wdt_start(void)
> {
> 	if (!test_and_set_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status)) {
> 		/* at our first start we enable clock and do initialisations */
> 		clk_enable(imx2_wdt.clk);
> 
> 		imx2_wdt_init();
> 		imx2_wdt_enable();
> 	} else	/* delete the timer that pings the watchdog after close */
> 		del_timer_sync(&imx2_wdt.timer);
> 
> 	/* Watchdog is enabled - time to reload the timeout value */
> 	imx2_wdt_ping();
> }
> 
> static void imx2_wdt_stop(void)
> {
> 	/* we don't need a clk_disable, it cannot be disabled once started.
> 	 * We use a timer to ping the watchdog while /dev/watchdog is closed */
> 	imx2_wdt_timer_ping();
> }
> 
> static void imx2_wdt_set_timeout(int new_timeout)
> {
> 	/* set the new timeout value in the WSR */
> 	IMX2_WDT_CHG_BITS(imx2_wdt.base + IMX2_WDT_WCR, IMX2_WDT_WCR_WT,
> 					WDOG_SEC_TO_COUNT(new_timeout));
> }
> 
> static int imx2_wdt_open(struct inode *inode, struct file *file)
> {
> 	if (test_and_set_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status))
> 		return -EBUSY;
> 
> 	imx2_wdt_start();
> 	return nonseekable_open(inode, file);
> }
> 
> static int imx2_wdt_close(struct inode *inode, struct file *file)
> {
> 	if (test_bit(IMX2_WDT_EXPECT_CLOSE, &imx2_wdt.status) && !nowayout)
> 		imx2_wdt_stop();
> 	else {
> 		dev_crit(imx2_wdt_miscdev.parent,
> 			"Unexpected close: Expect reboot!\n");
> 		imx2_wdt_ping();
> 	}
> 
> 	clear_bit(IMX2_WDT_EXPECT_CLOSE, &imx2_wdt.status);
> 	clear_bit(IMX2_WDT_STATUS_OPEN, &imx2_wdt.status);
> 	return 0;
> }
> 
> static long imx2_wdt_ioctl(struct file *file, unsigned int cmd,
> 							unsigned long arg)
> {
> 	void __user *argp = (void __user *)arg;
> 	int __user *p = argp;
> 	int new_value;
> 
> 	switch (cmd) {
> 	case WDIOC_GETSUPPORT:
> 		return copy_to_user(argp, &imx2_wdt_info,
> 			sizeof(struct watchdog_info)) ? -EFAULT : 0;
> 
> 	case WDIOC_GETSTATUS:
> 	case WDIOC_GETBOOTSTATUS:
> 		return put_user(0, p);
> 
> 	case WDIOC_KEEPALIVE:
> 		imx2_wdt_ping();
> 		return 0;
> 
> 	case WDIOC_SETTIMEOUT:
> 		if (get_user(new_value, p))
> 			return -EFAULT;
> 		if ((new_value < 1) || (new_value > IMX2_WDT_MAX_TIME))
> 			return -EINVAL;
> 		imx2_wdt_set_timeout(new_value);
> 		imx2_wdt.timeout = new_value;
> 		imx2_wdt_ping();
> 
> 		/* Fallthrough to return current value */
> 	case WDIOC_GETTIMEOUT:
> 		return put_user(imx2_wdt.timeout, p);
> 
> 	default:
> 		return -ENOTTY;
> 	}
> }
> 
> static ssize_t imx2_wdt_write(struct file *file, const char __user *data,
> 						size_t len, loff_t *ppos)
> {
> 	size_t i;
> 	char c;
> 
> 	if (len == 0)	/* Can we see this even ? */
> 		return 0;
> 
> 	clear_bit(IMX2_WDT_EXPECT_CLOSE, &imx2_wdt.status);
> 	/* scan to see whether or not we got the magic character */
> 	for (i = 0; i != len; i++) {
> 		if (get_user(c, data + i))
> 			return -EFAULT;
> 		if (c == 'V')
> 			set_bit(IMX2_WDT_EXPECT_CLOSE, &imx2_wdt.status);
> 		}
> 	}
> 	imx2_wdt_ping();
> 	return len;
> }
> 
> static const struct file_operations imx2_wdt_fops = {
> 	.owner = THIS_MODULE,
> 	.llseek = no_llseek,
> 	.unlocked_ioctl = imx2_wdt_ioctl,
> 	.open = imx2_wdt_open,
> 	.release = imx2_wdt_close,
> 	.write = imx2_wdt_write,
> };
> 
> static struct miscdevice imx2_wdt_miscdev = {
> 	.minor = WATCHDOG_MINOR,
> 	.name = "watchdog",
> 	.fops = &imx2_wdt_fops,
> };
> 
> static int __init imx2_wdt_probe(struct platform_device *pdev)
> {
> 	int ret;
> 	int res_size;
> 	struct resource *res;
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 	if (!res) {
> 		dev_err(&pdev->dev, "can't get device resources\n");
> 		return -ENODEV;
> 	}
> 
> 	res_size = resource_size(res);
> 	if (!devm_request_mem_region(&pdev->dev, res->start, res_size,
> 		res->name)) {
> 		dev_err(&pdev->dev, "can't allocate %d bytes at %d address\n",
> 			res_size, res->start);
> 		return -ENOMEM;
> 	}
> 
> 	imx2_wdt.base = devm_ioremap_nocache(&pdev->dev, res->start, res_size);
> 	if (!imx2_wdt.base) {
> 		dev_err(&pdev->dev, "ioremap failed\n");
> 		return -ENOMEM;
> 	}
> 
> 	imx2_wdt.clk = clk_get_sys("imx-wdt.0", NULL);
> 	if (IS_ERR(imx2_wdt.clk)) {
> 		dev_err(&pdev->dev, "can't get Watchdog clock\n");
> 		return PTR_ERR(imx2_wdt.clk);
> 	}
> 
> 	imx2_wdt.timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
> 	if (imx2_wdt.timeout != timeout)
> 		dev_warn(&pdev->dev, "Initial timeout out of range! "
> 			"Clamped from %u to %u\n", timeout, imx2_wdt.timeout);
> 
> 	setup_timer(&imx2_wdt.timer, imx2_wdt_timer_ping, 0);
> 
> 	imx2_wdt_miscdev.parent = &pdev->dev;
> 	ret = misc_register(&imx2_wdt_miscdev);
> 	if (ret)
> 		goto fail;
> 
> 	dev_info(&pdev->dev,
> 		"IMX2+ Watchdog Timer enabled. timeout=%d sec (nowayout=%d)\n",
> 						imx2_wdt.timeout, nowayout);
> 	return 0;
> 
> fail:
> 	imx2_wdt_miscdev.parent = NULL;
> 	clk_put(imx2_wdt.clk);
> 	return ret;
> }
> 
> static int __exit imx2_wdt_remove(struct platform_device *pdev)
> {
> 	misc_deregister(&imx2_wdt_miscdev);
> 
> 	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status)) {
> 		del_timer_sync(&imx2_wdt.timer);
> 
> 		dev_crit(imx2_wdt_miscdev.parent,
> 			"Device removed: Expect reboot!\n");
> 	} else
> 		clk_put(imx2_wdt.clk);
> 
> 	imx2_wdt_miscdev.parent = NULL;
> 	return 0;
> }
> 
> static void imx2_wdt_shutdown(struct platform_device *pdev)
> {
> 	if (test_bit(IMX2_WDT_STATUS_STARTED, &imx2_wdt.status)) {
> 		/* we are running, we need to delete the timer but will give
> 		 * max timeout before reboot will take place */
> 		del_timer_sync(&imx2_wdt.timer);
> 		imx2_wdt_set_timeout(IMX2_WDT_MAX_TIME);
> 		imx2_wdt_ping();
> 
> 		dev_crit(imx2_wdt_miscdev.parent,
> 			"Device shutdown: Expect reboot!\n");
> 	}
> }
> 
> static struct platform_driver imx2_wdt_driver = {
> 	.probe		= imx2_wdt_probe,
> 	.remove		= __exit_p(imx2_wdt_remove),
> 	.shutdown	= imx2_wdt_shutdown,
> 	.driver		= {
> 		.name	= DRIVER_NAME,
> 		.owner	= THIS_MODULE,
> 	},
> };
> 
> static int __init imx2_wdt_init(void)
> {
> 	return platform_driver_probe(&imx2_wdt_driver, imx2_wdt_probe);
> }
> module_init(imx2_wdt_init);
> 
> static void __exit imx2_wdt_exit(void)
> {
> 	platform_driver_unregister(&imx2_wdt_driver);
> }
> module_exit(imx2_wdt_exit);
> 
> MODULE_AUTHOR("Wolfram Sang");
> MODULE_DESCRIPTION("Watchdog driver for IMX2 and later");
> MODULE_LICENSE("GPL v2");
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> MODULE_ALIAS("platform:" DRIVER_NAME);

The rest looks good, thanks for that. Some functions looks a bit too trivial
(and are called just once) for my taste, but I guess this is for easier
migration later? Oh, and a few whitespace issues, easily fixed.

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100525/736f6d78/attachment-0001.sig>

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wim Van Sebroeck]

Hi Wolfram,

> > I was not to happy with how the close of /dev/watchdog was being done.
> > So I modified your code so that
> > * the close is better supported.
> > * the code is ready for when we change to the generic watchdog api.
> 
> Cool, any concrete plans for this happening?

I'm reviewing it with Alan, we discussed some smaal changes. Once they are applied, Alan will review the code again.
After that we will sent it out for review on the mailing-lists. So I expect within 2 weeks.

> > I didn't compile test the code. Could you have a look at it? compile it and test it?
> 
> Will do. Some initial comments. (An incremental patch might have been easier to
> check)

Correct :-)

> > Goal is to get this in during this merge window still...
> 
> The new driver-rule should apply here, no?

Normally yes, but this should have been reviewed earlier by myself. If we have something working and after all the reviews we allready did, I think it is time that user's can start using this watchdog.

> > #define WDOG_SEC_TO_COUNT(s)	((s * 2) << 8)
> 
> Any reason you dropped the '- 1' from my version here?

If you want a timeout of 1 second, and you subtract the -1 then you only get 0.5 seconds according to the reference manual.

> > /* Apply nand and or masks to data read from addr and write back
> >  * we nand first so that we can erase the timeout before setting the new one */
> > #define IMX2_WDT_CHG_BITS(addr, nand, or) \
> > 	__raw_writew((__raw_readw(addr) & ~nand) | or, addr)
> 
> I don't like such a macro very much, but well...

Can be changed easily. Just did this, I'll sent you the new file.

> > static inline void imx2_wdt_init(void)
> > {
> > 	unsigned int bits_off;
> > 	unsigned int bits_on;
> > 
> > 	/* Strip the old watchdog Time-Out value */
> > 	bits_off = IMX2_WDT_WCR_WT;
> > 	/* Generate reset if WDOG times out */
> > 	bits_off |= IMX2_WDT_WCR_WRE;
> > 	/* Keep Watchdog Disabled */
> > 	bits_off |= IMX2_WDT_WCR_WDE;
> > 	/* Continue timer operation during DEBUG mode */
> > 	bits_off |= IMX2_WDT_WCR_WDBG;
> > 	/* Continue Watchdog Timer during Low Power modes */
> > 	bits_off |= IMX2_WDT_WCR_WDZST;
> > 	
> > 	/* Set the watchdog's Time-Out value */
> > 	bits_on = WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
> > 	/* Don't set Watchdog Assertion */
> > 	bits_on |= IMX2_WDT_WCR_WDA;
> > 	/* Don't set Software Reset Signal */
> > 	bits_on |= IMX2_WDT_WCR_SRS;
> 
> This is too excessive IMO. The bootloader might already have setup the watchdog
> according to the board.

imho timeout should be set, watchdog should reset and not generate an interrupt and WDE bit should be used.
All the rest should indeed be set by a boot-loader.

> > 	IMX2_WDT_CHG_BITS(imx2_wdt.base + IMX2_WDT_WCR, bits_off, bits_on);
> > }
> > 
> > static inline void imx2_wdt_enable(void)
> > {
> > 	IMX2_WDT_CHG_BITS(imx2_wdt.base + IMX2_WDT_WCR, 0, IMX2_WDT_WCR_WDE);
> > }
> 
> I assume this is for the easy migration to the generic watchdog API? Otherwise
> I see little use for a seperate function.

Can indeed go in imx2_wdt_init.

> > 
> > static inline void imx2_wdt_ping(void)
> > {
> > 	/* When enabled, the Watchdog requires that a service sequence be
> > 	 * written to the Watchdog Service Register (WSR) */
> 
> IMHO this is maybe over-commenting :)

Deleted :-)

> The rest looks good, thanks for that. Some functions looks a bit too trivial
> (and are called just once) for my taste, but I guess this is for easier
> migration later? Oh, and a few whitespace issues, easily fixed.

Found that whitespace also allready.

Will sent you the changed code and a test-program (so that I can see what the output is :-) ).

Kind regards,
Wim.

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wolfram Sang]

> > > * the code is ready for when we change to the generic watchdog api.
> > 
> > Cool, any concrete plans for this happening?
> 
> I'm reviewing it with Alan, we discussed some smaal changes. Once they are
> applied, Alan will review the code again. After that we will sent it out for
> review on the mailing-lists. So I expect within 2 weeks.

Awesome news! \o/

> > The new driver-rule should apply here, no?
> 
> Normally yes, but this should have been reviewed earlier by myself. If we
> have something working and after all the reviews we allready did, I think it
	> is time that user's can start using this watchdog.

Uhm, yes, but they can still have it with rc2? Doesn't matter, the sooner the
better ;)

> > > #define WDOG_SEC_TO_COUNT(s)	((s * 2) << 8)
> > 
> > Any reason you dropped the '- 1' from my version here?
> 
> If you want a timeout of 1 second, and you subtract the -1 then you only get
> 0.5 seconds according to the reference manual.

Which one? MX27 and MX31 refman say the range is 0.5 to 128 seconds.
So, 0x00 = 0.5s, 0x01 = 1.0s... The MX35 refman even has a table confirming this.

> > This is too excessive IMO. The bootloader might already have setup the
> > watchdog according to the board.
> 
> imho timeout should be set, watchdog should reset and not generate an
> interrupt and WDE bit should be used. All the rest should indeed be set by a
> boot-loader.

ACK.

> Will sent you the changed code and a test-program (so that I can see what the
> output is :-) ).

Okay.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100525/18dff0a8/attachment.sig>

[PATCH 1/6] Driver for the watchdog timer on Freescale IMX2 (and later) processors.

[Wim Van Sebroeck]

Hi Wolfram,

> > > > #define WDOG_SEC_TO_COUNT(s)	((s * 2) << 8)
> > > 
> > > Any reason you dropped the '- 1' from my version here?
> > 
> > If you want a timeout of 1 second, and you subtract the -1 then you only get
> > 0.5 seconds according to the reference manual.
> 
> Which one? MX27 and MX31 refman say the range is 0.5 to 128 seconds.
> So, 0x00 = 0.5s, 0x01 = 1.0s... The MX35 refman even has a table confirming this.

My fault. I interpreted it incorrectly. if you have 128 seconds then you indeed have value 0xFF << 8...
so -1 should indeed be there. This make the macro:
#define WDOG_SEC_TO_COUNT(s)	(((s * 2) - 1) << 8)

Thanks,
Wim.