[RFC] [PATCH 0/5] omap watchdog fixes

[RFC] [PATCH 0/5] omap watchdog fixes

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

**** THIS IS NOT MEANT FOR INTEGRATION ****

Before sending to mainline I need a little help from you
people.

The main issue would be that mainline doesn't have yet the
omap_prcm_get_reset_sources() which means patch 5 would break
support for omap2/3 if applied to linux-omap.

I left the command commented out with a REVISIT note so when
those pieces go upstream, that comment should be removed.

I don't have hw rigth now, but I'll test the patches tomorrow
at work on omap2 (n810) and omap3 (sdp).

Please, give it a good review, specially on the clock handling
as I'm not really sure if I did it right.

Felipe Balbi (5):
  watchdog: sync linux-omap changes
  watchdog: another ioremap() fix
  watchdog: move omap_wdt.h to include/linux/watchdog
  watchdog: cleanup a bit omap_wdt.c
  watchdog: introduce platform_data and remove cpu conditional code

 arch/arm/plat-omap/devices.c      |   76 +++++++-
 drivers/watchdog/omap_wdt.c       |  369 ++++++++++++++++++++++---------------
 drivers/watchdog/omap_wdt.h       |   64 -------
 include/linux/watchdog/omap_wdt.h |   83 +++++++++
 4 files changed, 367 insertions(+), 225 deletions(-)
 delete mode 100644 drivers/watchdog/omap_wdt.h
 create mode 100644 include/linux/watchdog/omap_wdt.h

[PATCH 1/5] watchdog: sync linux-omap changes

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

These are changes that have been sitting in linux-omap
and were never sent upstream.

Hopefully, it'll never happen again at least for this
driver.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/devices.c |   21 ++-
 drivers/watchdog/omap_wdt.c  |  314 ++++++++++++++++++++++++++----------------
 drivers/watchdog/omap_wdt.h  |   28 ++---
 3 files changed, 214 insertions(+), 149 deletions(-)

diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index bc1cf30..f1eaa44 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -454,16 +454,8 @@ static inline void omap_init_uwire(void) {}
 
 #if	defined(CONFIG_OMAP_WATCHDOG) || defined(CONFIG_OMAP_WATCHDOG_MODULE)
 
-#ifdef CONFIG_ARCH_OMAP24XX
-#define	OMAP_WDT_BASE		0x48022000
-#else
-#define	OMAP_WDT_BASE		0xfffeb000
-#endif
-
 static struct resource wdt_resources[] = {
 	{
-		.start		= OMAP_WDT_BASE,
-		.end		= OMAP_WDT_BASE + 0x4f,
 		.flags		= IORESOURCE_MEM,
 	},
 };
@@ -477,6 +469,19 @@ static struct platform_device omap_wdt_device = {
 
 static void omap_init_wdt(void)
 {
+	if (cpu_is_omap16xx())
+		wdt_resources[0].start = 0xfffeb000;
+	else if (cpu_is_omap2420())
+		wdt_resources[0].start = 0x48022000; /* WDT2 */
+	else if (cpu_is_omap2430())
+		wdt_resources[0].start = 0x49016000; /* WDT2 */
+	else if (cpu_is_omap343x())
+		wdt_resources[0].start = 0x48314000; /* WDT2 */
+	else
+		return;
+
+	wdt_resources[0].end = wdt_resources[0].start + 0x4f;
+
 	(void) platform_device_register(&omap_wdt_device);
 }
 #else
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 3a11dad..b111dec 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -1,7 +1,7 @@
 /*
- * linux/drivers/char/watchdog/omap_wdt.c
+ * linux/drivers/watchdog/omap_wdt.c
  *
- * Watchdog driver for the TI OMAP 16xx & 24xx 32KHz (non-secure) watchdog
+ * Watchdog driver for the TI OMAP 16xx & 24xx/34xx 32KHz (non-secure) watchdog
  *
  * Author: MontaVista Software, Inc.
  *	 <gdavis@mvista.com> or <source@mvista.com>
@@ -39,58 +39,71 @@
 #include <linux/platform_device.h>
 #include <linux/moduleparam.h>
 #include <linux/clk.h>
-#include <linux/bitops.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
+
+#include <asm/io.h>
+#include <asm/uaccess.h>
 #include <mach/hardware.h>
+#include <asm/bitops.h>
+
 #include <mach/prcm.h>
 
 #include "omap_wdt.h"
 
+static struct platform_device *omap_wdt_dev;
+
 static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 
-static int omap_wdt_users;
-static struct clk *armwdt_ck;
-static struct clk *mpu_wdt_ick;
-static struct clk *mpu_wdt_fck;
-
 static unsigned int wdt_trgr_pattern = 0x1234;
-static spinlock_t wdt_lock;
+struct omap_wdt_dev {
+	void __iomem    *base;          /* physical */
+	struct device   *dev;
+	int             omap_wdt_users;
+	struct clk      *armwdt_ck;
+	struct clk      *mpu_wdt_ick;
+	struct clk      *mpu_wdt_fck;
+	struct resource *mem;
+	struct miscdevice omap_wdt_miscdev;
+};
 
-static void omap_wdt_ping(void)
+static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
+	void __iomem    *base = wdev->base;
 	/* wait for posted write to complete */
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	wdt_trgr_pattern = ~wdt_trgr_pattern;
-	omap_writel(wdt_trgr_pattern, (OMAP_WATCHDOG_TGR));
+	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
 	/* wait for posted write to complete */
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	/* reloaded WCRR from WLDR */
 }
 
-static void omap_wdt_enable(void)
+static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 {
+	void __iomem *base;
+	base = wdev->base;
 	/* Sequence to enable the watchdog */
-	omap_writel(0xBBBB, OMAP_WATCHDOG_SPR);
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x10)
+	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
-	omap_writel(0x4444, OMAP_WATCHDOG_SPR);
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x10)
+	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
 }
 
-static void omap_wdt_disable(void)
+static void omap_wdt_disable(struct omap_wdt_dev *wdev)
 {
+	void __iomem *base;
+	base = wdev->base;
 	/* sequence required to disable watchdog */
-	omap_writel(0xAAAA, OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x10)
+	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
-	omap_writel(0x5555, OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x10)
+	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
 }
 
@@ -103,15 +116,17 @@ static void omap_wdt_adjust_timeout(unsigned new_timeout)
 	timer_margin = new_timeout;
 }
 
-static void omap_wdt_set_timeout(void)
+static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 {
 	u32 pre_margin = GET_WLDR_VAL(timer_margin);
+	void __iomem *base;
+	base = wdev->base;
 
 	/* just count up at 32 KHz */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x04)
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
-	omap_writel(pre_margin, OMAP_WATCHDOG_LDR);
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x04)
+	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
 }
 
@@ -121,81 +136,88 @@ static void omap_wdt_set_timeout(void)
 
 static int omap_wdt_open(struct inode *inode, struct file *file)
 {
-	if (test_and_set_bit(1, (unsigned long *)&omap_wdt_users))
+	struct omap_wdt_dev *wdev;
+	void __iomem *base;
+	wdev = platform_get_drvdata(omap_wdt_dev);
+	base = wdev->base;
+	if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
 		return -EBUSY;
 
 	if (cpu_is_omap16xx())
-		clk_enable(armwdt_ck);	/* Enable the clock */
+		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
 
-	if (cpu_is_omap24xx()) {
-		clk_enable(mpu_wdt_ick);    /* Enable the interface clock */
-		clk_enable(mpu_wdt_fck);    /* Enable the functional clock */
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
+		clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
 	}
 
 	/* initialize prescaler */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x01)
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
-	omap_writel((1 << 5) | (PTV << 2), OMAP_WATCHDOG_CNTRL);
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x01)
+	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
 
-	omap_wdt_set_timeout();
-	omap_wdt_enable();
+	file->private_data = (void *) wdev;
+
+	omap_wdt_set_timeout(wdev);
+	omap_wdt_enable(wdev);
 	return nonseekable_open(inode, file);
 }
 
 static int omap_wdt_release(struct inode *inode, struct file *file)
 {
+	struct omap_wdt_dev *wdev;
+	wdev = file->private_data;
 	/*
 	 *      Shut off the timer unless NOWAYOUT is defined.
 	 */
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
-	omap_wdt_disable();
 
-	if (cpu_is_omap16xx()) {
-		clk_disable(armwdt_ck);	/* Disable the clock */
-		clk_put(armwdt_ck);
-		armwdt_ck = NULL;
-	}
+	omap_wdt_disable(wdev);
 
-	if (cpu_is_omap24xx()) {
-		clk_disable(mpu_wdt_ick);	/* Disable the clock */
-		clk_disable(mpu_wdt_fck);	/* Disable the clock */
-		clk_put(mpu_wdt_ick);
-		clk_put(mpu_wdt_fck);
-		mpu_wdt_ick = NULL;
-		mpu_wdt_fck = NULL;
+	if (cpu_is_omap16xx())
+		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
+
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
+		clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
 	}
 #else
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
-	omap_wdt_users = 0;
+	wdev->omap_wdt_users = 0;
 	return 0;
 }
 
-static ssize_t omap_wdt_write(struct file *file, const char __user *data,
+static ssize_t
+omap_wdt_write(struct file *file, const char __user *data,
 		size_t len, loff_t *ppos)
 {
+	struct omap_wdt_dev *wdev;
+	wdev = file->private_data;
 	/* Refresh LOAD_TIME. */
-	if (len) {
-		spin_lock(&wdt_lock);
-		omap_wdt_ping();
-		spin_unlock(&wdt_lock);
-	}
+	if (len)
+		omap_wdt_ping(wdev);
 	return len;
 }
 
-static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
-						unsigned long arg)
+static int
+omap_wdt_ioctl(struct inode *inode, struct file *file,
+	unsigned int cmd, unsigned long arg)
 {
+	struct omap_wdt_dev *wdev;
 	int new_margin;
-	static const struct watchdog_info ident = {
+	static struct watchdog_info ident = {
 		.identity = "OMAP Watchdog",
 		.options = WDIOF_SETTIMEOUT,
 		.firmware_version = 0,
 	};
+	wdev = file->private_data;
 
 	switch (cmd) {
+	default:
+		return -ENOTTY;
 	case WDIOC_GETSUPPORT:
 		return copy_to_user((struct watchdog_info __user *)arg, &ident,
 				sizeof(ident));
@@ -209,128 +231,173 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
 			return put_user(omap_prcm_get_reset_sources(),
 					(int __user *)arg);
 	case WDIOC_KEEPALIVE:
-		spin_lock(&wdt_lock);
-		omap_wdt_ping();
-		spin_unlock(&wdt_lock);
+		omap_wdt_ping(wdev);
 		return 0;
 	case WDIOC_SETTIMEOUT:
 		if (get_user(new_margin, (int __user *)arg))
 			return -EFAULT;
 		omap_wdt_adjust_timeout(new_margin);
 
-		spin_lock(&wdt_lock);
-		omap_wdt_disable();
-		omap_wdt_set_timeout();
-		omap_wdt_enable();
+		omap_wdt_disable(wdev);
+		omap_wdt_set_timeout(wdev);
+		omap_wdt_enable(wdev);
 
-		omap_wdt_ping();
-		spin_unlock(&wdt_lock);
+		omap_wdt_ping(wdev);
 		/* Fall */
 	case WDIOC_GETTIMEOUT:
 		return put_user(timer_margin, (int __user *)arg);
-	default:
-		return -ENOTTY;
 	}
+	return 0;
 }
 
 static const struct file_operations omap_wdt_fops = {
 	.owner = THIS_MODULE,
 	.write = omap_wdt_write,
-	.unlocked_ioctl = omap_wdt_ioctl,
+	.ioctl = omap_wdt_ioctl,
 	.open = omap_wdt_open,
 	.release = omap_wdt_release,
 };
 
-static struct miscdevice omap_wdt_miscdev = {
-	.minor = WATCHDOG_MINOR,
-	.name = "watchdog",
-	.fops = &omap_wdt_fops,
-};
 
 static int __init omap_wdt_probe(struct platform_device *pdev)
 {
 	struct resource *res, *mem;
 	int ret;
+	struct omap_wdt_dev *wdev;
 
 	/* reserve static register mappings */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -ENOENT;
 
+	if (omap_wdt_dev)
+		return -EBUSY;
+
 	mem = request_mem_region(res->start, res->end - res->start + 1,
 				 pdev->name);
 	if (mem == NULL)
 		return -EBUSY;
 
-	platform_set_drvdata(pdev, mem);
-
-	omap_wdt_users = 0;
+	wdev = kzalloc(sizeof(struct omap_wdt_dev), GFP_KERNEL);
+	if (!wdev) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	wdev->omap_wdt_users = 0;
+	wdev->mem = mem;
 
 	if (cpu_is_omap16xx()) {
-		armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
-		if (IS_ERR(armwdt_ck)) {
-			ret = PTR_ERR(armwdt_ck);
-			armwdt_ck = NULL;
+		wdev->armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
+		if (IS_ERR(wdev->armwdt_ck)) {
+			ret = PTR_ERR(wdev->armwdt_ck);
+			wdev->armwdt_ck = NULL;
 			goto fail;
 		}
 	}
 
 	if (cpu_is_omap24xx()) {
-		mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
-		if (IS_ERR(mpu_wdt_ick)) {
-			ret = PTR_ERR(mpu_wdt_ick);
-			mpu_wdt_ick = NULL;
+		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
+		if (IS_ERR(wdev->mpu_wdt_ick)) {
+			ret = PTR_ERR(wdev->mpu_wdt_ick);
+			wdev->mpu_wdt_ick = NULL;
+			goto fail;
+		}
+		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
+		if (IS_ERR(wdev->mpu_wdt_fck)) {
+			ret = PTR_ERR(wdev->mpu_wdt_fck);
+			wdev->mpu_wdt_fck = NULL;
+			goto fail;
+		}
+	}
+
+	if (cpu_is_omap34xx()) {
+		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "wdt2_ick");
+		if (IS_ERR(wdev->mpu_wdt_ick)) {
+			ret = PTR_ERR(wdev->mpu_wdt_ick);
+			wdev->mpu_wdt_ick = NULL;
 			goto fail;
 		}
-		mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
-		if (IS_ERR(mpu_wdt_fck)) {
-			ret = PTR_ERR(mpu_wdt_fck);
-			mpu_wdt_fck = NULL;
+		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
+		if (IS_ERR(wdev->mpu_wdt_fck)) {
+			ret = PTR_ERR(wdev->mpu_wdt_fck);
+			wdev->mpu_wdt_fck = NULL;
 			goto fail;
 		}
 	}
+	wdev->base = (void __iomem *) (mem->start);
+	platform_set_drvdata(pdev, wdev);
 
-	omap_wdt_disable();
+	omap_wdt_disable(wdev);
 	omap_wdt_adjust_timeout(timer_margin);
 
-	omap_wdt_miscdev.parent = &pdev->dev;
-	ret = misc_register(&omap_wdt_miscdev);
+	wdev->omap_wdt_miscdev.parent = &pdev->dev;
+	wdev->omap_wdt_miscdev.minor = WATCHDOG_MINOR;
+	wdev->omap_wdt_miscdev.name = "watchdog";
+	wdev->omap_wdt_miscdev.fops = &omap_wdt_fops;
+
+	ret = misc_register(&(wdev->omap_wdt_miscdev));
 	if (ret)
 		goto fail;
 
-	pr_info("OMAP Watchdog Timer: initial timeout %d sec\n", timer_margin);
+	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
+		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
+		timer_margin);
 
 	/* autogate OCP interface clock */
-	omap_writel(0x01, OMAP_WATCHDOG_SYS_CONFIG);
+	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
+
+	omap_wdt_dev = pdev;
+
 	return 0;
 
 fail:
-	if (armwdt_ck)
-		clk_put(armwdt_ck);
-	if (mpu_wdt_ick)
-		clk_put(mpu_wdt_ick);
-	if (mpu_wdt_fck)
-		clk_put(mpu_wdt_fck);
-	release_resource(mem);
+	if (wdev) {
+		platform_set_drvdata(pdev, NULL);
+		if (wdev->armwdt_ck)
+			clk_put(wdev->armwdt_ck);
+		if (wdev->mpu_wdt_ick)
+			clk_put(wdev->mpu_wdt_ick);
+		if (wdev->mpu_wdt_fck)
+			clk_put(wdev->mpu_wdt_fck);
+		kfree(wdev);
+	}
+	if (mem) {
+		release_resource(mem);
+	}
 	return ret;
 }
 
 static void omap_wdt_shutdown(struct platform_device *pdev)
 {
-	omap_wdt_disable();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+
+	if (wdev->omap_wdt_users)
+		omap_wdt_disable(wdev);
 }
 
 static int omap_wdt_remove(struct platform_device *pdev)
 {
-	struct resource *mem = platform_get_drvdata(pdev);
-	misc_deregister(&omap_wdt_miscdev);
-	release_resource(mem);
-	if (armwdt_ck)
-		clk_put(armwdt_ck);
-	if (mpu_wdt_ick)
-		clk_put(mpu_wdt_ick);
-	if (mpu_wdt_fck)
-		clk_put(mpu_wdt_fck);
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+
+	misc_deregister(&(wdev->omap_wdt_miscdev));
+	release_resource(wdev->mem);
+	platform_set_drvdata(pdev, NULL);
+	if (wdev->armwdt_ck) {
+		clk_put(wdev->armwdt_ck);
+		wdev->armwdt_ck = NULL;
+	}
+	if (wdev->mpu_wdt_ick) {
+		clk_put(wdev->mpu_wdt_ick);
+		wdev->mpu_wdt_ick = NULL;
+	}
+	if (wdev->mpu_wdt_fck) {
+		clk_put(wdev->mpu_wdt_fck);
+		wdev->mpu_wdt_fck = NULL;
+	}
+	kfree(wdev);
+	omap_wdt_dev = NULL;
 	return 0;
 }
 
@@ -344,16 +411,20 @@ static int omap_wdt_remove(struct platform_device *pdev)
 
 static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	if (omap_wdt_users)
-		omap_wdt_disable();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+	if (wdev->omap_wdt_users)
+		omap_wdt_disable(wdev);
 	return 0;
 }
 
 static int omap_wdt_resume(struct platform_device *pdev)
 {
-	if (omap_wdt_users) {
-		omap_wdt_enable();
-		omap_wdt_ping();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+	if (wdev->omap_wdt_users) {
+		omap_wdt_enable(wdev);
+		omap_wdt_ping(wdev);
 	}
 	return 0;
 }
@@ -377,7 +448,6 @@ static struct platform_driver omap_wdt_driver = {
 
 static int __init omap_wdt_init(void)
 {
-	spin_lock_init(&wdt_lock);
 	return platform_driver_register(&omap_wdt_driver);
 }
 
diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
index 52a532a..fc02ec6 100644
--- a/drivers/watchdog/omap_wdt.h
+++ b/drivers/watchdog/omap_wdt.h
@@ -30,25 +30,15 @@
 #ifndef _OMAP_WATCHDOG_H
 #define _OMAP_WATCHDOG_H
 
-#define OMAP1610_WATCHDOG_BASE		0xfffeb000
-#define OMAP2420_WATCHDOG_BASE		0x48022000	/*WDT Timer 2 */
-
-#ifdef CONFIG_ARCH_OMAP24XX
-#define OMAP_WATCHDOG_BASE 		OMAP2420_WATCHDOG_BASE
-#else
-#define OMAP_WATCHDOG_BASE 		OMAP1610_WATCHDOG_BASE
-#define RM_RSTST_WKUP			0
-#endif
-
-#define OMAP_WATCHDOG_REV		(OMAP_WATCHDOG_BASE + 0x00)
-#define OMAP_WATCHDOG_SYS_CONFIG	(OMAP_WATCHDOG_BASE + 0x10)
-#define OMAP_WATCHDOG_STATUS		(OMAP_WATCHDOG_BASE + 0x14)
-#define OMAP_WATCHDOG_CNTRL		(OMAP_WATCHDOG_BASE + 0x24)
-#define OMAP_WATCHDOG_CRR		(OMAP_WATCHDOG_BASE + 0x28)
-#define OMAP_WATCHDOG_LDR		(OMAP_WATCHDOG_BASE + 0x2c)
-#define OMAP_WATCHDOG_TGR		(OMAP_WATCHDOG_BASE + 0x30)
-#define OMAP_WATCHDOG_WPS		(OMAP_WATCHDOG_BASE + 0x34)
-#define OMAP_WATCHDOG_SPR		(OMAP_WATCHDOG_BASE + 0x48)
+#define OMAP_WATCHDOG_REV		(0x00)
+#define OMAP_WATCHDOG_SYS_CONFIG	(0x10)
+#define OMAP_WATCHDOG_STATUS		(0x14)
+#define OMAP_WATCHDOG_CNTRL		(0x24)
+#define OMAP_WATCHDOG_CRR		(0x28)
+#define OMAP_WATCHDOG_LDR		(0x2c)
+#define OMAP_WATCHDOG_TGR		(0x30)
+#define OMAP_WATCHDOG_WPS		(0x34)
+#define OMAP_WATCHDOG_SPR		(0x48)
 
 /* Using the prescaler, the OMAP watchdog could go for many
  * months before firing.  These limits work without scaling,
-- 
1.6.0.1.196.g01914

[PATCH 2/5] watchdog: another ioremap() fix

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

convert to use ioremap() and __raw_{read/write} friends.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
Signed-off-by: George G. Davis <gdavis@mvista.com>
---
 drivers/watchdog/omap_wdt.c |   52 +++++++++++++++++++++++++-----------------
 1 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index b111dec..e395743 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -71,12 +71,12 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
 	void __iomem    *base = wdev->base;
 	/* wait for posted write to complete */
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	wdt_trgr_pattern = ~wdt_trgr_pattern;
-	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
+	__raw_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
 	/* wait for posted write to complete */
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	/* reloaded WCRR from WLDR */
 }
@@ -86,11 +86,11 @@ static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 	void __iomem *base;
 	base = wdev->base;
 	/* Sequence to enable the watchdog */
-	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
+	__raw_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
-	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
+	__raw_writel(0x4444, base + OMAP_WATCHDOG_SPR);
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
 }
 
@@ -99,11 +99,11 @@ static void omap_wdt_disable(struct omap_wdt_dev *wdev)
 	void __iomem *base;
 	base = wdev->base;
 	/* sequence required to disable watchdog */
-	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
+	__raw_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
-	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
+	__raw_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
 }
 
@@ -123,10 +123,10 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 	base = wdev->base;
 
 	/* just count up at 32 KHz */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
-	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
+	__raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
 }
 
@@ -152,10 +152,10 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	}
 
 	/* initialize prescaler */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
-	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
+	__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
 
 	file->private_data = (void *) wdev;
@@ -225,7 +225,7 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
 		return put_user(0, (int __user *)arg);
 	case WDIOC_GETBOOTSTATUS:
 		if (cpu_is_omap16xx())
-			return put_user(omap_readw(ARM_SYSST),
+			return put_user(__raw_readw(ARM_SYSST),
 					(int __user *)arg);
 		if (cpu_is_omap24xx())
 			return put_user(omap_prcm_get_reset_sources(),
@@ -324,7 +324,12 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 			goto fail;
 		}
 	}
-	wdev->base = (void __iomem *) (mem->start);
+	wdev->base = ioremap(res->start, res->end - res->start + 1);
+	if (!wdev->base) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
 	platform_set_drvdata(pdev, wdev);
 
 	omap_wdt_disable(wdev);
@@ -340,11 +345,11 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		goto fail;
 
 	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
-		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
+		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
 		timer_margin);
 
 	/* autogate OCP interface clock */
-	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
+	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
 	omap_wdt_dev = pdev;
 
@@ -359,6 +364,8 @@ fail:
 			clk_put(wdev->mpu_wdt_ick);
 		if (wdev->mpu_wdt_fck)
 			clk_put(wdev->mpu_wdt_fck);
+		if (wdev->base)
+			iounmap(wdev->base);
 		kfree(wdev);
 	}
 	if (mem) {
@@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
 		clk_put(wdev->mpu_wdt_fck);
 		wdev->mpu_wdt_fck = NULL;
 	}
+	if (wdev->base)
+		iounmap(wdev->base);
+
 	kfree(wdev);
 	omap_wdt_dev = NULL;
 	return 0;
-- 
1.6.0.1.196.g01914

[PATCH 3/5] watchdog: move omap_wdt.h to include/linux/watchdog

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Create a new include/linux/watchdog directory
for holding watchdog chips headers, move omap_wdt.h
to the new location and update the include patch in
the driver source.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 drivers/watchdog/omap_wdt.c                    |    3 +--
 {drivers => include/linux}/watchdog/omap_wdt.h |    4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)
 rename {drivers => include/linux}/watchdog/omap_wdt.h (95%)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index e395743..f21fd94 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -33,6 +33,7 @@
 #include <linux/mm.h>
 #include <linux/miscdevice.h>
 #include <linux/watchdog.h>
+#include <linux/watchdog/omap_wdt.h>
 #include <linux/reboot.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -47,8 +48,6 @@
 
 #include <mach/prcm.h>
 
-#include "omap_wdt.h"
-
 static struct platform_device *omap_wdt_dev;
 
 static unsigned timer_margin;
diff --git a/drivers/watchdog/omap_wdt.h b/include/linux/watchdog/omap_wdt.h
similarity index 95%
rename from drivers/watchdog/omap_wdt.h
rename to include/linux/watchdog/omap_wdt.h
index fc02ec6..62e6a74 100644
--- a/drivers/watchdog/omap_wdt.h
+++ b/include/linux/watchdog/omap_wdt.h
@@ -1,5 +1,5 @@
 /*
- *  linux/drivers/char/watchdog/omap_wdt.h
+ *  include/linux/watchdog/omap_wdt.h
  *
  *  BRIEF MODULE DESCRIPTION
  *      OMAP Watchdog timer register definitions
@@ -44,7 +44,7 @@
  * months before firing.  These limits work without scaling,
  * with the 60 second default assumed by most tools and docs.
  */
-#define TIMER_MARGIN_MAX    	(24 * 60 * 60)	/* 1 day */
+#define TIMER_MARGIN_MAX	(24 * 60 * 60)	/* 1 day */
 #define TIMER_MARGIN_DEFAULT	60	/* 60 secs */
 #define TIMER_MARGIN_MIN	1
 
-- 
1.6.0.1.196.g01914

[PATCH 4/5] watchdog: cleanup a bit omap_wdt.c

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Trivial cleanup patch.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 drivers/watchdog/omap_wdt.c |  143 +++++++++++++++++++++++++------------------
 1 files changed, 84 insertions(+), 59 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index f21fd94..7700a0a 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -69,11 +69,14 @@ struct omap_wdt_dev {
 static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
 	void __iomem    *base = wdev->base;
+
 	/* wait for posted write to complete */
 	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
+
 	wdt_trgr_pattern = ~wdt_trgr_pattern;
 	__raw_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
+
 	/* wait for posted write to complete */
 	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
@@ -82,12 +85,13 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 
 static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 {
-	void __iomem *base;
-	base = wdev->base;
+	void __iomem *base = wdev->base;
+
 	/* Sequence to enable the watchdog */
 	__raw_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
 	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
+
 	__raw_writel(0x4444, base + OMAP_WATCHDOG_SPR);
 	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
@@ -95,12 +99,13 @@ static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 
 static void omap_wdt_disable(struct omap_wdt_dev *wdev)
 {
-	void __iomem *base;
-	base = wdev->base;
+	void __iomem *base = wdev->base;
+
 	/* sequence required to disable watchdog */
 	__raw_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
+
 	__raw_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
@@ -118,12 +123,12 @@ static void omap_wdt_adjust_timeout(unsigned new_timeout)
 static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 {
 	u32 pre_margin = GET_WLDR_VAL(timer_margin);
-	void __iomem *base;
-	base = wdev->base;
+	void __iomem *base = wdev->base;
 
 	/* just count up at 32 KHz */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
+
 	__raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
@@ -132,13 +137,11 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 /*
  *	Allow only one task to hold it open
  */
-
 static int omap_wdt_open(struct inode *inode, struct file *file)
 {
-	struct omap_wdt_dev *wdev;
-	void __iomem *base;
-	wdev = platform_get_drvdata(omap_wdt_dev);
-	base = wdev->base;
+	struct omap_wdt_dev *wdev = platform_get_drvdata(omap_wdt_dev);
+	void __iomem *base = wdev->base;
+
 	if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
 		return -EBUSY;
 
@@ -153,6 +156,7 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	/* initialize prescaler */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
+
 	__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
@@ -161,13 +165,14 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 
 	omap_wdt_set_timeout(wdev);
 	omap_wdt_enable(wdev);
+
 	return nonseekable_open(inode, file);
 }
 
 static int omap_wdt_release(struct inode *inode, struct file *file)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = file->private_data;
+	struct omap_wdt_dev *wdev = file->private_data;
+
 	/*
 	 *      Shut off the timer unless NOWAYOUT is defined.
 	 */
@@ -186,6 +191,7 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
 	wdev->omap_wdt_users = 0;
+
 	return 0;
 }
 
@@ -193,11 +199,12 @@ static ssize_t
 omap_wdt_write(struct file *file, const char __user *data,
 		size_t len, loff_t *ppos)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = file->private_data;
+	struct omap_wdt_dev *wdev = file->private_data;
+
 	/* Refresh LOAD_TIME. */
 	if (len)
 		omap_wdt_ping(wdev);
+
 	return len;
 }
 
@@ -207,11 +214,13 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
 {
 	struct omap_wdt_dev *wdev;
 	int new_margin;
+
 	static struct watchdog_info ident = {
 		.identity = "OMAP Watchdog",
 		.options = WDIOF_SETTIMEOUT,
 		.firmware_version = 0,
 	};
+
 	wdev = file->private_data;
 
 	switch (cmd) {
@@ -246,6 +255,7 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
 	case WDIOC_GETTIMEOUT:
 		return put_user(timer_margin, (int __user *)arg);
 	}
+
 	return 0;
 }
 
@@ -257,31 +267,37 @@ static const struct file_operations omap_wdt_fops = {
 	.release = omap_wdt_release,
 };
 
-
 static int __init omap_wdt_probe(struct platform_device *pdev)
 {
 	struct resource *res, *mem;
-	int ret;
 	struct omap_wdt_dev *wdev;
+	int ret;
 
 	/* reserve static register mappings */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENOENT;
+	if (!res) {
+		ret = -ENOENT;
+		goto err_get_resource;
+	}
 
-	if (omap_wdt_dev)
-		return -EBUSY;
+	if (omap_wdt_dev) {
+		ret - EBUSY;
+		goto err_busy;
+	}
 
 	mem = request_mem_region(res->start, res->end - res->start + 1,
 				 pdev->name);
-	if (mem == NULL)
-		return -EBUSY;
+	if (!mem) {
+		ret = -EBUSY;
+		goto err_busy;
+	}
 
 	wdev = kzalloc(sizeof(struct omap_wdt_dev), GFP_KERNEL);
 	if (!wdev) {
 		ret = -ENOMEM;
-		goto fail;
+		goto err_kzalloc;
 	}
+
 	wdev->omap_wdt_users = 0;
 	wdev->mem = mem;
 
@@ -290,7 +306,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		if (IS_ERR(wdev->armwdt_ck)) {
 			ret = PTR_ERR(wdev->armwdt_ck);
 			wdev->armwdt_ck = NULL;
-			goto fail;
+			goto err_clk;
 		}
 	}
 
@@ -299,13 +315,13 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		if (IS_ERR(wdev->mpu_wdt_ick)) {
 			ret = PTR_ERR(wdev->mpu_wdt_ick);
 			wdev->mpu_wdt_ick = NULL;
-			goto fail;
+			goto err_clk;
 		}
 		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
 		if (IS_ERR(wdev->mpu_wdt_fck)) {
 			ret = PTR_ERR(wdev->mpu_wdt_fck);
 			wdev->mpu_wdt_fck = NULL;
-			goto fail;
+			goto err_clk;
 		}
 	}
 
@@ -314,19 +330,19 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		if (IS_ERR(wdev->mpu_wdt_ick)) {
 			ret = PTR_ERR(wdev->mpu_wdt_ick);
 			wdev->mpu_wdt_ick = NULL;
-			goto fail;
+			goto err_clk;
 		}
 		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
 		if (IS_ERR(wdev->mpu_wdt_fck)) {
 			ret = PTR_ERR(wdev->mpu_wdt_fck);
 			wdev->mpu_wdt_fck = NULL;
-			goto fail;
+			goto err_clk;
 		}
 	}
 	wdev->base = ioremap(res->start, res->end - res->start + 1);
 	if (!wdev->base) {
 		ret = -ENOMEM;
-		goto fail;
+		goto err_ioremap;
 	}
 
 	platform_set_drvdata(pdev, wdev);
@@ -341,7 +357,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 
 	ret = misc_register(&(wdev->omap_wdt_miscdev));
 	if (ret)
-		goto fail;
+		goto err_misc;
 
 	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
 		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
@@ -354,29 +370,34 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 
 	return 0;
 
-fail:
-	if (wdev) {
-		platform_set_drvdata(pdev, NULL);
-		if (wdev->armwdt_ck)
-			clk_put(wdev->armwdt_ck);
-		if (wdev->mpu_wdt_ick)
-			clk_put(wdev->mpu_wdt_ick);
-		if (wdev->mpu_wdt_fck)
-			clk_put(wdev->mpu_wdt_fck);
-		if (wdev->base)
-			iounmap(wdev->base);
-		kfree(wdev);
-	}
-	if (mem) {
-		release_resource(mem);
-	}
+err_misc:
+	platform_set_drvdata(pdev, NULL);
+	iounmap(wdev->base);
+
+err_ioremap:
+	wdev->base = NULL;
+
+err_clk:
+	if (wdev->armwdt_ck)
+		clk_put(wdev->armwdt_ck);
+	if (wdev->mpu_wdt_ick)
+		clk_put(wdev->mpu_wdt_ick);
+	if (wdev->mpu_wdt_fck)
+		clk_put(wdev->mpu_wdt_fck);
+	kfree(wdev);
+
+err_kzalloc:
+	release_resource(mem);
+
+err_busy:
+err_get_resource:
+
 	return ret;
 }
 
 static void omap_wdt_shutdown(struct platform_device *pdev)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = platform_get_drvdata(pdev);
+	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
 	if (wdev->omap_wdt_users)
 		omap_wdt_disable(wdev);
@@ -384,29 +405,33 @@ static void omap_wdt_shutdown(struct platform_device *pdev)
 
 static int omap_wdt_remove(struct platform_device *pdev)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = platform_get_drvdata(pdev);
+	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
 	misc_deregister(&(wdev->omap_wdt_miscdev));
 	release_resource(wdev->mem);
 	platform_set_drvdata(pdev, NULL);
+
 	if (wdev->armwdt_ck) {
 		clk_put(wdev->armwdt_ck);
 		wdev->armwdt_ck = NULL;
 	}
+
 	if (wdev->mpu_wdt_ick) {
 		clk_put(wdev->mpu_wdt_ick);
 		wdev->mpu_wdt_ick = NULL;
 	}
+
 	if (wdev->mpu_wdt_fck) {
 		clk_put(wdev->mpu_wdt_fck);
 		wdev->mpu_wdt_fck = NULL;
 	}
+
 	if (wdev->base)
 		iounmap(wdev->base);
 
 	kfree(wdev);
 	omap_wdt_dev = NULL;
+
 	return 0;
 }
 
@@ -420,34 +445,34 @@ static int omap_wdt_remove(struct platform_device *pdev)
 
 static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = platform_get_drvdata(pdev);
+	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
+
 	if (wdev->omap_wdt_users)
 		omap_wdt_disable(wdev);
+
 	return 0;
 }
 
 static int omap_wdt_resume(struct platform_device *pdev)
 {
-	struct omap_wdt_dev *wdev;
-	wdev = platform_get_drvdata(pdev);
+	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
+
 	if (wdev->omap_wdt_users) {
 		omap_wdt_enable(wdev);
 		omap_wdt_ping(wdev);
 	}
+
 	return 0;
 }
-
-#else
-#define	omap_wdt_suspend	NULL
-#define	omap_wdt_resume		NULL
 #endif
 
 static struct platform_driver omap_wdt_driver = {
 	.probe		= omap_wdt_probe,
 	.remove		= omap_wdt_remove,
+#ifdef	CONFIG_PM
 	.shutdown	= omap_wdt_shutdown,
 	.suspend	= omap_wdt_suspend,
+#endif
 	.resume		= omap_wdt_resume,
 	.driver		= {
 		.owner	= THIS_MODULE,
-- 
1.6.0.1.196.g01914

[PATCH 5/5] watchdog: introduce platform_data and remove cpu conditional code

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Get rid of cpu conditional code.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/devices.c      |   65 +++++++++++++++++--
 drivers/watchdog/omap_wdt.c       |  127 +++++++++++++------------------------
 include/linux/watchdog/omap_wdt.h |   29 +++++++++
 3 files changed, 132 insertions(+), 89 deletions(-)

diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index f1eaa44..d756743 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/watchdog/omap_wdt.h>
 
 #include <mach/hardware.h>
 #include <asm/io.h>
@@ -460,6 +461,51 @@ static struct resource wdt_resources[] = {
 	},
 };
 
+static int omap_wdt_set_clock(struct omap_wdt_dev *wdev, int status)
+{
+	if (status) {
+		if (cpu_is_omap16xx())
+			clk_disable(wdev->fck);
+
+		if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+			clk_disable(wdev->ick);
+			clk_disable(wdev->fck);
+		}
+	} else {
+		if (cpu_is_omap16xx())
+			clk_disable(wdev->fck);
+
+		if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+			clk_disable(wdev->ick);
+			clk_disable(wdev->fck);
+		}
+	}
+
+	return 0;
+}
+
+static int omap_wdt_get_bootstatus(void)
+{
+	if (cpu_is_omap16xx())
+		return __raw_readw(ARM_SYSST);
+
+#if 0
+	/* REVISIT: When the patch introducing the following function
+	 * gets merged upstream, uncomment this code to enable omap2
+	 * support on omap watchdog driver.
+	 */
+	if (cpu_is_omap24xx())
+		return omap_prcm_get_reset_sources();
+#endif
+
+	return 0;
+}
+
+static struct omap_wdt_platform_data omap_wdt_pdata = {
+	.set_clock	= omap_wdt_set_clock,
+	.get_bootstatus	= omap_wdt_get_bootstatus,
+};
+
 static struct platform_device omap_wdt_device = {
 	.name	   = "omap_wdt",
 	.id	     = -1,
@@ -469,17 +515,26 @@ static struct platform_device omap_wdt_device = {
 
 static void omap_init_wdt(void)
 {
-	if (cpu_is_omap16xx())
+	if (cpu_is_omap16xx()) {
+		omap_wdt_pdata.fck = "armwdt_ck";
 		wdt_resources[0].start = 0xfffeb000;
-	else if (cpu_is_omap2420())
+	} else if (cpu_is_omap2420()) {
+		omap_wdt_pdata.fck = "mpu_wdt_ick";
+		omap_wdt_pdata.ick = "mpu_wdt_fck";
 		wdt_resources[0].start = 0x48022000; /* WDT2 */
-	else if (cpu_is_omap2430())
+	} else if (cpu_is_omap2430()) {
+		omap_wdt_pdata.fck = "mpu_wdt_ick";
+		omap_wdt_pdata.ick = "mpu_wdt_fck";
 		wdt_resources[0].start = 0x49016000; /* WDT2 */
-	else if (cpu_is_omap343x())
+	} else if (cpu_is_omap343x()) {
+		omap_wdt_pdata.fck = "wdt2_ick";
+		omap_wdt_pdata.ick = "wdt2_fck";
 		wdt_resources[0].start = 0x48314000; /* WDT2 */
-	else
+	} else {
 		return;
+	}
 
+	omap_wdt_device.dev.platform_data = &omap_wdt_pdata;
 	wdt_resources[0].end = wdt_resources[0].start + 0x4f;
 
 	(void) platform_device_register(&omap_wdt_device);
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 7700a0a..52c597c 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -48,23 +48,20 @@
 
 #include <mach/prcm.h>
 
-static struct platform_device *omap_wdt_dev;
-
 static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 
+static struct platform_device *omap_wdt_dev;
 static unsigned int wdt_trgr_pattern = 0x1234;
-struct omap_wdt_dev {
-	void __iomem    *base;          /* physical */
-	struct device   *dev;
-	int             omap_wdt_users;
-	struct clk      *armwdt_ck;
-	struct clk      *mpu_wdt_ick;
-	struct clk      *mpu_wdt_fck;
-	struct resource *mem;
-	struct miscdevice omap_wdt_miscdev;
-};
+
+static inline int omap_wdt_set_clock(struct omap_wdt_dev *wdev, int status)
+{
+	if (wdev->set_clock)
+		return wdev->set_clock(wdev, status);
+
+	return 0;
+}
 
 static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
@@ -145,13 +142,7 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
 		return -EBUSY;
 
-	if (cpu_is_omap16xx())
-		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
-		clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
-	}
+	omap_wdt_set_clock(wdev, 1);
 
 	/* initialize prescaler */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
@@ -177,16 +168,8 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 	 *      Shut off the timer unless NOWAYOUT is defined.
 	 */
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
-
 	omap_wdt_disable(wdev);
-
-	if (cpu_is_omap16xx())
-		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
-		clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
-	}
+	omap_wdt_set_clock(wdev, 1);
 #else
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
@@ -232,12 +215,8 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
 	case WDIOC_GETSTATUS:
 		return put_user(0, (int __user *)arg);
 	case WDIOC_GETBOOTSTATUS:
-		if (cpu_is_omap16xx())
-			return put_user(__raw_readw(ARM_SYSST),
-					(int __user *)arg);
-		if (cpu_is_omap24xx())
-			return put_user(omap_prcm_get_reset_sources(),
-					(int __user *)arg);
+		return put_user(wdev->get_bootstatus(),
+				(int __user *)arg);
 	case WDIOC_KEEPALIVE:
 		omap_wdt_ping(wdev);
 		return 0;
@@ -269,6 +248,7 @@ static const struct file_operations omap_wdt_fops = {
 
 static int __init omap_wdt_probe(struct platform_device *pdev)
 {
+	struct omap_wdt_platform_data *pdata = pdev->dev.platform_data;
 	struct resource *res, *mem;
 	struct omap_wdt_dev *wdev;
 	int ret;
@@ -281,7 +261,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	}
 
 	if (omap_wdt_dev) {
-		ret - EBUSY;
+		ret = EBUSY;
 		goto err_busy;
 	}
 
@@ -300,45 +280,30 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 
 	wdev->omap_wdt_users = 0;
 	wdev->mem = mem;
+	wdev->get_bootstatus = pdata->get_bootstatus;
+	wdev->set_clock = pdata->set_clock;
 
-	if (cpu_is_omap16xx()) {
-		wdev->armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
-		if (IS_ERR(wdev->armwdt_ck)) {
-			ret = PTR_ERR(wdev->armwdt_ck);
-			wdev->armwdt_ck = NULL;
-			goto err_clk;
-		}
+	/* omap1 has only the functional clock so we believe everybody
+	 * will pass that pointer correctly. For the interface clock,
+	 * we check whether that pointer is true to avoid null pointer
+	 * dereferences
+	 */
+	wdev->fck = clk_get(&pdev->dev, pdata->fck);
+	if (IS_ERR(wdev->fck)) {
+		ret = PTR_ERR(wdev->fck);
+		wdev->fck = NULL;
+		goto err_clk;
 	}
 
-	if (cpu_is_omap24xx()) {
-		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
-		if (IS_ERR(wdev->mpu_wdt_ick)) {
-			ret = PTR_ERR(wdev->mpu_wdt_ick);
-			wdev->mpu_wdt_ick = NULL;
-			goto err_clk;
-		}
-		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
-		if (IS_ERR(wdev->mpu_wdt_fck)) {
-			ret = PTR_ERR(wdev->mpu_wdt_fck);
-			wdev->mpu_wdt_fck = NULL;
+	if (pdata->ick) {
+		wdev->ick = clk_get(&pdev->dev, pdata->ick);
+		if (IS_ERR(wdev->ick)) {
+			ret = PTR_ERR(wdev->ick);
+			wdev->ick = NULL;
 			goto err_clk;
 		}
 	}
 
-	if (cpu_is_omap34xx()) {
-		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "wdt2_ick");
-		if (IS_ERR(wdev->mpu_wdt_ick)) {
-			ret = PTR_ERR(wdev->mpu_wdt_ick);
-			wdev->mpu_wdt_ick = NULL;
-			goto err_clk;
-		}
-		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
-		if (IS_ERR(wdev->mpu_wdt_fck)) {
-			ret = PTR_ERR(wdev->mpu_wdt_fck);
-			wdev->mpu_wdt_fck = NULL;
-			goto err_clk;
-		}
-	}
 	wdev->base = ioremap(res->start, res->end - res->start + 1);
 	if (!wdev->base) {
 		ret = -ENOMEM;
@@ -378,12 +343,11 @@ err_ioremap:
 	wdev->base = NULL;
 
 err_clk:
-	if (wdev->armwdt_ck)
-		clk_put(wdev->armwdt_ck);
-	if (wdev->mpu_wdt_ick)
-		clk_put(wdev->mpu_wdt_ick);
-	if (wdev->mpu_wdt_fck)
-		clk_put(wdev->mpu_wdt_fck);
+	if (wdev->fck)
+		clk_put(wdev->fck);
+	if (wdev->ick)
+		clk_put(wdev->ick);
+
 	kfree(wdev);
 
 err_kzalloc:
@@ -411,19 +375,14 @@ static int omap_wdt_remove(struct platform_device *pdev)
 	release_resource(wdev->mem);
 	platform_set_drvdata(pdev, NULL);
 
-	if (wdev->armwdt_ck) {
-		clk_put(wdev->armwdt_ck);
-		wdev->armwdt_ck = NULL;
-	}
-
-	if (wdev->mpu_wdt_ick) {
-		clk_put(wdev->mpu_wdt_ick);
-		wdev->mpu_wdt_ick = NULL;
+	if (wdev->fck) {
+		clk_put(wdev->fck);
+		wdev->fck = NULL;
 	}
 
-	if (wdev->mpu_wdt_fck) {
-		clk_put(wdev->mpu_wdt_fck);
-		wdev->mpu_wdt_fck = NULL;
+	if (wdev->ick) {
+		clk_put(wdev->ick);
+		wdev->ick = NULL;
 	}
 
 	if (wdev->base)
diff --git a/include/linux/watchdog/omap_wdt.h b/include/linux/watchdog/omap_wdt.h
index 62e6a74..858d9a4 100644
--- a/include/linux/watchdog/omap_wdt.h
+++ b/include/linux/watchdog/omap_wdt.h
@@ -30,6 +30,9 @@
 #ifndef _OMAP_WATCHDOG_H
 #define _OMAP_WATCHDOG_H
 
+#include <linux/clk.h>
+#include <linux/miscdevice.h>
+
 #define OMAP_WATCHDOG_REV		(0x00)
 #define OMAP_WATCHDOG_SYS_CONFIG	(0x10)
 #define OMAP_WATCHDOG_STATUS		(0x14)
@@ -51,4 +54,30 @@
 #define PTV			0	/* prescale */
 #define GET_WLDR_VAL(secs)	(0xffffffff - ((secs) * (32768/(1<<PTV))) + 1)
 
+struct omap_wdt_dev {
+	void __iomem    *base;          /* physical */
+	struct device   *dev;
+	int             omap_wdt_users;
+
+	struct clk      *ick;
+	struct clk      *fck;
+
+	struct resource *mem;
+	struct miscdevice omap_wdt_miscdev;
+
+	/* clock handling */
+	int	(*set_clock)(struct omap_wdt_dev *wdev, int status);
+
+	/* used for put_user() */
+	int	(*get_bootstatus)(void);
+};
+
+struct omap_wdt_platform_data {
+	const char	*ick;
+	const char	*fck;
+
+	int	(*set_clock)(struct omap_wdt_dev *wdev, int status);
+	int	(*get_bootstatus)(void);
+};
+
 #endif				/* _OMAP_WATCHDOG_H */
-- 
1.6.0.1.196.g01914

Re: [PATCH 5/5] watchdog: introduce platform_data and remove cpu conditional code

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 01:23:42AM +0300, Felipe Balbi wrote:
> +struct omap_wdt_platform_data {
> +	const char	*ick;
> +	const char	*fck;

Grumble.  The clk API is supposed to make this stuff completely
unnecessary - but OMAP doesn't implement the clk API correctly -
by using the 'name' as the sole key.  The 'name' argument is
supposed to be used to disambiguate between multiple clock
inputs on a single device. ;(

Re: [PATCH 5/5] watchdog: introduce platform_data and remove cpu conditional code

[Felipe Balbi]

On Fri, Sep 19, 2008 at 12:20:06AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 01:23:42AM +0300, Felipe Balbi wrote:
> > +struct omap_wdt_platform_data {
> > +	const char	*ick;
> > +	const char	*fck;
> 
> Grumble.  The clk API is supposed to make this stuff completely
> unnecessary - but OMAP doesn't implement the clk API correctly -
> by using the 'name' as the sole key.  The 'name' argument is
> supposed to be used to disambiguate between multiple clock
> inputs on a single device. ;(

Yes, and we have two clocks for the same device_id on the bus. Right ?

-- 
balbi

Re: [PATCH 5/5] watchdog: introduce platform_data and remove cpu conditional code

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 02:23:25AM +0300, Felipe Balbi wrote:
> On Fri, Sep 19, 2008 at 12:20:06AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 19, 2008 at 01:23:42AM +0300, Felipe Balbi wrote:
> > > +struct omap_wdt_platform_data {
> > > +	const char	*ick;
> > > +	const char	*fck;
> > 
> > Grumble.  The clk API is supposed to make this stuff completely
> > unnecessary - but OMAP doesn't implement the clk API correctly -
> > by using the 'name' as the sole key.  The 'name' argument is
> > supposed to be used to disambiguate between multiple clock
> > inputs on a single device. ;(
> 
> Yes, and we have two clocks for the same device_id on the bus. Right ?

That's not a problem.  I have an solution to solve this, but for the
time being, let's just keep the logic inside the omap_wdt driver.

Re: [PATCH 5/5] watchdog: introduce platform_data and remove cpu conditional code

[Felipe Balbi]

On Fri, Sep 19, 2008 at 08:46:34AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 02:23:25AM +0300, Felipe Balbi wrote:
> > On Fri, Sep 19, 2008 at 12:20:06AM +0100, Russell King - ARM Linux wrote:
> > > On Fri, Sep 19, 2008 at 01:23:42AM +0300, Felipe Balbi wrote:
> > > > +struct omap_wdt_platform_data {
> > > > +	const char	*ick;
> > > > +	const char	*fck;
> > > 
> > > Grumble.  The clk API is supposed to make this stuff completely
> > > unnecessary - but OMAP doesn't implement the clk API correctly -
> > > by using the 'name' as the sole key.  The 'name' argument is
> > > supposed to be used to disambiguate between multiple clock
> > > inputs on a single device. ;(
> > 
> > Yes, and we have two clocks for the same device_id on the bus. Right ?
> 
> That's not a problem.  I have an solution to solve this, but for the
> time being, let's just keep the logic inside the omap_wdt driver.

Ok, I'll wait on this patch then. The sync with mainline and ioremap()
fix will be going to Wim soon, I just need to test for a while.

-- 
balbi

Re: [PATCH 4/5] watchdog: cleanup a bit omap_wdt.c

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 01:23:41AM +0300, Felipe Balbi wrote:
> +
>  	return 0;
>  }
> -
> -#else
> -#define	omap_wdt_suspend	NULL
> -#define	omap_wdt_resume		NULL
>  #endif
>  
>  static struct platform_driver omap_wdt_driver = {
>  	.probe		= omap_wdt_probe,
>  	.remove		= omap_wdt_remove,
> +#ifdef	CONFIG_PM
>  	.shutdown	= omap_wdt_shutdown,
>  	.suspend	= omap_wdt_suspend,
> +#endif

I prefer to see the function names defined to NULL rather than adding
extra ifdefs into drivers.

Re: [PATCH 4/5] watchdog: cleanup a bit omap_wdt.c

[Felipe Balbi]

On Fri, Sep 19, 2008 at 12:10:21AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 01:23:41AM +0300, Felipe Balbi wrote:
> > +
> >  	return 0;
> >  }
> > -
> > -#else
> > -#define	omap_wdt_suspend	NULL
> > -#define	omap_wdt_resume		NULL
> >  #endif
> >  
> >  static struct platform_driver omap_wdt_driver = {
> >  	.probe		= omap_wdt_probe,
> >  	.remove		= omap_wdt_remove,
> > +#ifdef	CONFIG_PM
> >  	.shutdown	= omap_wdt_shutdown,
> >  	.suspend	= omap_wdt_suspend,
> > +#endif
> 
> I prefer to see the function names defined to NULL rather than adding
> extra ifdefs into drivers.

Reverted.

-- 
balbi

Re: [PATCH 3/5] watchdog: move omap_wdt.h to include/linux/watchdog

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 01:23:40AM +0300, Felipe Balbi wrote:
> From: Felipe Balbi <felipe.balbi@nokia.com>
> 
> Create a new include/linux/watchdog directory
> for holding watchdog chips headers, move omap_wdt.h
> to the new location and update the include patch in
> the driver source.

What's the reasoning for this change?  As far as I can see, omap_wdt.h
is private to omap_wdt.c, and shouldn't be exposed to anyone else.
In which case, leaving it along side omap_wdt.c is the right thing to
do.

We don't have any rules which say all header files must be in the
include/ sub-tree.

Re: [PATCH 3/5] watchdog: move omap_wdt.h to include/linux/watchdog

[Felipe Balbi]

On Fri, Sep 19, 2008 at 12:10:34AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 01:23:40AM +0300, Felipe Balbi wrote:
> > From: Felipe Balbi <felipe.balbi@nokia.com>
> > 
> > Create a new include/linux/watchdog directory
> > for holding watchdog chips headers, move omap_wdt.h
> > to the new location and update the include patch in
> > the driver source.
> 
> What's the reasoning for this change?  As far as I can see, omap_wdt.h
> is private to omap_wdt.c, and shouldn't be exposed to anyone else.
> In which case, leaving it along side omap_wdt.c is the right thing to
> do.
> 
> We don't have any rules which say all header files must be in the
> include/ sub-tree.

It's gonna be needed on patch 5/5. Maybe I should meld it in the same
patch ?

-- 
balbi

Re: [PATCH 2/5] watchdog: another ioremap() fix

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 01:23:39AM +0300, Felipe Balbi wrote:
> @@ -359,6 +364,8 @@ fail:
>  			clk_put(wdev->mpu_wdt_ick);
>  		if (wdev->mpu_wdt_fck)
>  			clk_put(wdev->mpu_wdt_fck);
> +		if (wdev->base)
> +			iounmap(wdev->base);
>  		kfree(wdev);
>  	}
>  	if (mem) {
> @@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
>  		clk_put(wdev->mpu_wdt_fck);
>  		wdev->mpu_wdt_fck = NULL;
>  	}
> +	if (wdev->base)
> +		iounmap(wdev->base);
> +

For both of these... iounmap on NULL pointers is allowed, so the
test is redundant.

Re: [PATCH 2/5] watchdog: another ioremap() fix

[Felipe Balbi]

On Fri, Sep 19, 2008 at 12:06:53AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 01:23:39AM +0300, Felipe Balbi wrote:
> > @@ -359,6 +364,8 @@ fail:
> >  			clk_put(wdev->mpu_wdt_ick);
> >  		if (wdev->mpu_wdt_fck)
> >  			clk_put(wdev->mpu_wdt_fck);
> > +		if (wdev->base)
> > +			iounmap(wdev->base);
> >  		kfree(wdev);
> >  	}
> >  	if (mem) {
> > @@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
> >  		clk_put(wdev->mpu_wdt_fck);
> >  		wdev->mpu_wdt_fck = NULL;
> >  	}
> > +	if (wdev->base)
> > +		iounmap(wdev->base);
> > +
> 
> For both of these... iounmap on NULL pointers is allowed, so the
> test is redundant.

Good to know. Thanks

-- 
balbi

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 01:23:38AM +0300, Felipe Balbi wrote:
> From: Felipe Balbi <felipe.balbi@nokia.com>
> 
> These are changes that have been sitting in linux-omap
> and were never sent upstream.
> 
> Hopefully, it'll never happen again at least for this
> driver.

Great, thanks for looking at this.

Unfortunately, it looks like we have two entirely separate forks - one
in mainline which has been worked on, and one in the omap tree which
has been separately worked on.  The two sets of changes have never been
merged, so it's not going to just be a case of updating mainline's to
what's in OMAP.

> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index 3a11dad..b111dec 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -1,7 +1,7 @@
>  /*
> - * linux/drivers/char/watchdog/omap_wdt.c
> + * linux/drivers/watchdog/omap_wdt.c
>   *
> - * Watchdog driver for the TI OMAP 16xx & 24xx 32KHz (non-secure) watchdog
> + * Watchdog driver for the TI OMAP 16xx & 24xx/34xx 32KHz (non-secure) watchdog
>   *
>   * Author: MontaVista Software, Inc.
>   *	 <gdavis@mvista.com> or <source@mvista.com>
> @@ -39,58 +39,71 @@
>  #include <linux/platform_device.h>
>  #include <linux/moduleparam.h>
>  #include <linux/clk.h>
> -#include <linux/bitops.h>
> -#include <linux/io.h>
> -#include <linux/uaccess.h>
> +
> +#include <asm/io.h>
> +#include <asm/uaccess.h>
>  #include <mach/hardware.h>
> +#include <asm/bitops.h>
> +

Please drop this change; I suspect it's a result of a mis-merge, or
maybe the mainline code is ahead in this respect of the omap tree.

> -static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
> -						unsigned long arg)
> +static int
> +omap_wdt_ioctl(struct inode *inode, struct file *file,
> +	unsigned int cmd, unsigned long arg)

This probably wants dropping as well, since it's backing out Alan's
updates to make watchdogs use the unlocked ioctl method.

>  {
> +	struct omap_wdt_dev *wdev;
>  	int new_margin;
> -	static const struct watchdog_info ident = {
> +	static struct watchdog_info ident = {

This backs out a change to add const.

>  		.identity = "OMAP Watchdog",
>  		.options = WDIOF_SETTIMEOUT,
>  		.firmware_version = 0,
>  	};
> +	wdev = file->private_data;
>  
>  	switch (cmd) {
> +	default:
> +		return -ENOTTY;

This backs out changes from 0c06090c9472db0525cb6fe229c3bea33bbbbb3c.

>  	case WDIOC_GETSUPPORT:
>  		return copy_to_user((struct watchdog_info __user *)arg, &ident,
>  				sizeof(ident));
> @@ -209,128 +231,173 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
>  			return put_user(omap_prcm_get_reset_sources(),
>  					(int __user *)arg);
>  	case WDIOC_KEEPALIVE:
> -		spin_lock(&wdt_lock);
> -		omap_wdt_ping();
> -		spin_unlock(&wdt_lock);
> +		omap_wdt_ping(wdev);

Backs out the addition of the spin locking.

>  		return 0;
>  	case WDIOC_SETTIMEOUT:
>  		if (get_user(new_margin, (int __user *)arg))
>  			return -EFAULT;
>  		omap_wdt_adjust_timeout(new_margin);
>  
> -		spin_lock(&wdt_lock);
> -		omap_wdt_disable();
> -		omap_wdt_set_timeout();
> -		omap_wdt_enable();
> +		omap_wdt_disable(wdev);
> +		omap_wdt_set_timeout(wdev);
> +		omap_wdt_enable(wdev);
>  
> -		omap_wdt_ping();
> -		spin_unlock(&wdt_lock);
> +		omap_wdt_ping(wdev);

Ditto.

>  		/* Fall */
>  	case WDIOC_GETTIMEOUT:
>  		return put_user(timer_margin, (int __user *)arg);
> -	default:
> -		return -ENOTTY;

This backs out changes from 0c06090c9472db0525cb6fe229c3bea33bbbbb3c.

>  	}
> +	return 0;

Not needed.

>  }
>  
>  static const struct file_operations omap_wdt_fops = {
>  	.owner = THIS_MODULE,
>  	.write = omap_wdt_write,
> -	.unlocked_ioctl = omap_wdt_ioctl,
> +	.ioctl = omap_wdt_ioctl,

Alan's unlocked_ioctl change backed out.

>  	.open = omap_wdt_open,
>  	.release = omap_wdt_release,
>  };
>  
> -static struct miscdevice omap_wdt_miscdev = {
> -	.minor = WATCHDOG_MINOR,
> -	.name = "watchdog",
> -	.fops = &omap_wdt_fops,
> -};
>  
>  static int __init omap_wdt_probe(struct platform_device *pdev)
>  {
>  	struct resource *res, *mem;
>  	int ret;
> +	struct omap_wdt_dev *wdev;
>  
>  	/* reserve static register mappings */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
>  		return -ENOENT;
>  
> +	if (omap_wdt_dev)
> +		return -EBUSY;
> +
>  	mem = request_mem_region(res->start, res->end - res->start + 1,
>  				 pdev->name);
>  	if (mem == NULL)
>  		return -EBUSY;
>  
> -	platform_set_drvdata(pdev, mem);
> -
> -	omap_wdt_users = 0;
> +	wdev = kzalloc(sizeof(struct omap_wdt_dev), GFP_KERNEL);
> +	if (!wdev) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +	wdev->omap_wdt_users = 0;
> +	wdev->mem = mem;
>  
>  	if (cpu_is_omap16xx()) {
> -		armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
> -		if (IS_ERR(armwdt_ck)) {
> -			ret = PTR_ERR(armwdt_ck);
> -			armwdt_ck = NULL;
> +		wdev->armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
> +		if (IS_ERR(wdev->armwdt_ck)) {
> +			ret = PTR_ERR(wdev->armwdt_ck);
> +			wdev->armwdt_ck = NULL;
>  			goto fail;
>  		}
>  	}
>  
>  	if (cpu_is_omap24xx()) {
> -		mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
> -		if (IS_ERR(mpu_wdt_ick)) {
> -			ret = PTR_ERR(mpu_wdt_ick);
> -			mpu_wdt_ick = NULL;
> +		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
> +		if (IS_ERR(wdev->mpu_wdt_ick)) {
> +			ret = PTR_ERR(wdev->mpu_wdt_ick);
> +			wdev->mpu_wdt_ick = NULL;
> +			goto fail;
> +		}
> +		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
> +		if (IS_ERR(wdev->mpu_wdt_fck)) {
> +			ret = PTR_ERR(wdev->mpu_wdt_fck);
> +			wdev->mpu_wdt_fck = NULL;
> +			goto fail;
> +		}
> +	}
> +
> +	if (cpu_is_omap34xx()) {
> +		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "wdt2_ick");
> +		if (IS_ERR(wdev->mpu_wdt_ick)) {
> +			ret = PTR_ERR(wdev->mpu_wdt_ick);
> +			wdev->mpu_wdt_ick = NULL;
>  			goto fail;
>  		}
> -		mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
> -		if (IS_ERR(mpu_wdt_fck)) {
> -			ret = PTR_ERR(mpu_wdt_fck);
> -			mpu_wdt_fck = NULL;
> +		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
> +		if (IS_ERR(wdev->mpu_wdt_fck)) {
> +			ret = PTR_ERR(wdev->mpu_wdt_fck);
> +			wdev->mpu_wdt_fck = NULL;
>  			goto fail;
>  		}
>  	}
> +	wdev->base = (void __iomem *) (mem->start);
> +	platform_set_drvdata(pdev, wdev);
>  
> -	omap_wdt_disable();
> +	omap_wdt_disable(wdev);
>  	omap_wdt_adjust_timeout(timer_margin);
>  
> -	omap_wdt_miscdev.parent = &pdev->dev;
> -	ret = misc_register(&omap_wdt_miscdev);
> +	wdev->omap_wdt_miscdev.parent = &pdev->dev;
> +	wdev->omap_wdt_miscdev.minor = WATCHDOG_MINOR;
> +	wdev->omap_wdt_miscdev.name = "watchdog";
> +	wdev->omap_wdt_miscdev.fops = &omap_wdt_fops;
> +
> +	ret = misc_register(&(wdev->omap_wdt_miscdev));
>  	if (ret)
>  		goto fail;
>  
> -	pr_info("OMAP Watchdog Timer: initial timeout %d sec\n", timer_margin);
> +	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
> +		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
> +		timer_margin);
>  
>  	/* autogate OCP interface clock */
> -	omap_writel(0x01, OMAP_WATCHDOG_SYS_CONFIG);
> +	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
> +
> +	omap_wdt_dev = pdev;
> +
>  	return 0;
>  
>  fail:
> -	if (armwdt_ck)
> -		clk_put(armwdt_ck);
> -	if (mpu_wdt_ick)
> -		clk_put(mpu_wdt_ick);
> -	if (mpu_wdt_fck)
> -		clk_put(mpu_wdt_fck);
> -	release_resource(mem);
> +	if (wdev) {
> +		platform_set_drvdata(pdev, NULL);
> +		if (wdev->armwdt_ck)
> +			clk_put(wdev->armwdt_ck);
> +		if (wdev->mpu_wdt_ick)
> +			clk_put(wdev->mpu_wdt_ick);
> +		if (wdev->mpu_wdt_fck)
> +			clk_put(wdev->mpu_wdt_fck);
> +		kfree(wdev);
> +	}
> +	if (mem) {
> +		release_resource(mem);
> +	}
>  	return ret;
>  }
>  
>  static void omap_wdt_shutdown(struct platform_device *pdev)
>  {
> -	omap_wdt_disable();
> +	struct omap_wdt_dev *wdev;
> +	wdev = platform_get_drvdata(pdev);
> +
> +	if (wdev->omap_wdt_users)
> +		omap_wdt_disable(wdev);
>  }
>  
>  static int omap_wdt_remove(struct platform_device *pdev)
>  {
> -	struct resource *mem = platform_get_drvdata(pdev);
> -	misc_deregister(&omap_wdt_miscdev);
> -	release_resource(mem);
> -	if (armwdt_ck)
> -		clk_put(armwdt_ck);
> -	if (mpu_wdt_ick)
> -		clk_put(mpu_wdt_ick);
> -	if (mpu_wdt_fck)
> -		clk_put(mpu_wdt_fck);
> +	struct omap_wdt_dev *wdev;
> +	wdev = platform_get_drvdata(pdev);
> +
> +	misc_deregister(&(wdev->omap_wdt_miscdev));
> +	release_resource(wdev->mem);

I wish I'd seen this code before it went in... release_resource()
leaks the memory associated with the resource that was allocated
in request_mem_region().  request_mem_region()'s counterpart is
release_mem_region() not release_resource().

> +	platform_set_drvdata(pdev, NULL);
> +	if (wdev->armwdt_ck) {
> +		clk_put(wdev->armwdt_ck);
> +		wdev->armwdt_ck = NULL;
> +	}
> +	if (wdev->mpu_wdt_ick) {
> +		clk_put(wdev->mpu_wdt_ick);
> +		wdev->mpu_wdt_ick = NULL;
> +	}
> +	if (wdev->mpu_wdt_fck) {
> +		clk_put(wdev->mpu_wdt_fck);
> +		wdev->mpu_wdt_fck = NULL;
> +	}
> +	kfree(wdev);
> +	omap_wdt_dev = NULL;
>  	return 0;
>  }
>  
> @@ -344,16 +411,20 @@ static int omap_wdt_remove(struct platform_device *pdev)
>  
>  static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
>  {
> -	if (omap_wdt_users)
> -		omap_wdt_disable();
> +	struct omap_wdt_dev *wdev;
> +	wdev = platform_get_drvdata(pdev);
> +	if (wdev->omap_wdt_users)
> +		omap_wdt_disable(wdev);
>  	return 0;
>  }
>  
>  static int omap_wdt_resume(struct platform_device *pdev)
>  {
> -	if (omap_wdt_users) {
> -		omap_wdt_enable();
> -		omap_wdt_ping();
> +	struct omap_wdt_dev *wdev;
> +	wdev = platform_get_drvdata(pdev);
> +	if (wdev->omap_wdt_users) {
> +		omap_wdt_enable(wdev);
> +		omap_wdt_ping(wdev);
>  	}
>  	return 0;
>  }
> @@ -377,7 +448,6 @@ static struct platform_driver omap_wdt_driver = {
>  
>  static int __init omap_wdt_init(void)
>  {
> -	spin_lock_init(&wdt_lock);

This wants to be kept.

>  	return platform_driver_register(&omap_wdt_driver);
>  }
>  

If you want to see the changes that have happened in mainline to this
driver:

git diff -u b7e04f8c61a46d742de23af5d7ca2b41b33e40ac..origin \
  drivers/watchdog/omap_wdt.c |less

where 'origin' is your latest mainline commit id or branch name.

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[Felipe Balbi]

On Fri, Sep 19, 2008 at 12:05:29AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 19, 2008 at 01:23:38AM +0300, Felipe Balbi wrote:
> > From: Felipe Balbi <felipe.balbi@nokia.com>
> > 
> > These are changes that have been sitting in linux-omap
> > and were never sent upstream.
> > 
> > Hopefully, it'll never happen again at least for this
> > driver.
> 
> Great, thanks for looking at this.

And thanks for the review. I'll fix your comments and resend the series
;-)

-- 
balbi

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[Russell King - ARM Linux]

On Fri, Sep 19, 2008 at 02:11:20AM +0300, Felipe Balbi wrote:
> On Fri, Sep 19, 2008 at 12:05:29AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 19, 2008 at 01:23:38AM +0300, Felipe Balbi wrote:
> > > From: Felipe Balbi <felipe.balbi@nokia.com>
> > > 
> > > These are changes that have been sitting in linux-omap
> > > and were never sent upstream.
> > > 
> > > Hopefully, it'll never happen again at least for this
> > > driver.
> > 
> > Great, thanks for looking at this.
> 
> And thanks for the review. I'll fix your comments and resend the series
> ;-)

Thanks - I'll take another look on Saturday.

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[Wim Van Sebroeck]

Hi Balbi,

> And thanks for the review. I'll fix your comments and resend the series
> ;-)

Thanks because I had the same objections then Russell.

Greetings,
Wim.

[PATCH 0/5] omap watchdog updaes

[Felipe Balbi]

Hi all,

Following are patches syncing, cleaning up and fixing omap watchdog
driver.

Patches 1-3 could be applied as is. Patches 4 and 5, on the other
hand, needs some discussion since I'd be creating a new directory
(include/linux/watchdog) and changing the clock handling.

Russel King told me, he'd like to hold on patch 5/5 as he'd have a
better solution for the clock handling soon. I'm sending that patch
away anyway so we can start the discussion on the mainling list about
that.

Felipe Balbi (5):
  watchdog: sync linux-omap changes
  watchdog: another ioremap() fix
  watchdog: cleanup a bit omap_wdt.c
  watchdog: move omap_wdt.h to include/linux/watchdog
  watchdog: introduce platform_data and remove cpu conditional code

 arch/arm/plat-omap/devices.c      |   76 ++++++++-
 drivers/watchdog/omap_wdt.c       |  333 ++++++++++++++++++++++---------------
 drivers/watchdog/omap_wdt.h       |   64 -------
 include/linux/watchdog/omap_wdt.h |   83 +++++++++
 4 files changed, 352 insertions(+), 204 deletions(-)
 delete mode 100644 drivers/watchdog/omap_wdt.h
 create mode 100644 include/linux/watchdog/omap_wdt.h

[PATCH 1/5] watchdog: sync linux-omap changes

[Felipe Balbi]

These are changes that have been sitting in linux-omap
and were never sent upstream.

Hopefully, it'll never happen again at least for this
driver.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/devices.c |   21 ++--
 drivers/watchdog/omap_wdt.c  |  287 +++++++++++++++++++++++++++---------------
 drivers/watchdog/omap_wdt.h  |   28 ++---
 3 files changed, 205 insertions(+), 131 deletions(-)

diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index bc1cf30..f1eaa44 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -454,16 +454,8 @@ static inline void omap_init_uwire(void) {}
 
 #if	defined(CONFIG_OMAP_WATCHDOG) || defined(CONFIG_OMAP_WATCHDOG_MODULE)
 
-#ifdef CONFIG_ARCH_OMAP24XX
-#define	OMAP_WDT_BASE		0x48022000
-#else
-#define	OMAP_WDT_BASE		0xfffeb000
-#endif
-
 static struct resource wdt_resources[] = {
 	{
-		.start		= OMAP_WDT_BASE,
-		.end		= OMAP_WDT_BASE + 0x4f,
 		.flags		= IORESOURCE_MEM,
 	},
 };
@@ -477,6 +469,19 @@ static struct platform_device omap_wdt_device = {
 
 static void omap_init_wdt(void)
 {
+	if (cpu_is_omap16xx())
+		wdt_resources[0].start = 0xfffeb000;
+	else if (cpu_is_omap2420())
+		wdt_resources[0].start = 0x48022000; /* WDT2 */
+	else if (cpu_is_omap2430())
+		wdt_resources[0].start = 0x49016000; /* WDT2 */
+	else if (cpu_is_omap343x())
+		wdt_resources[0].start = 0x48314000; /* WDT2 */
+	else
+		return;
+
+	wdt_resources[0].end = wdt_resources[0].start + 0x4f;
+
 	(void) platform_device_register(&omap_wdt_device);
 }
 #else
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 3a11dad..e55f2cc 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -1,7 +1,7 @@
 /*
- * linux/drivers/char/watchdog/omap_wdt.c
+ * linux/drivers/watchdog/omap_wdt.c
  *
- * Watchdog driver for the TI OMAP 16xx & 24xx 32KHz (non-secure) watchdog
+ * Watchdog driver for the TI OMAP 16xx & 24xx/34xx 32KHz (non-secure) watchdog
  *
  * Author: MontaVista Software, Inc.
  *	 <gdavis@mvista.com> or <source@mvista.com>
@@ -39,6 +39,7 @@
 #include <linux/platform_device.h>
 #include <linux/moduleparam.h>
 #include <linux/clk.h>
+
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/uaccess.h>
@@ -47,50 +48,63 @@
 
 #include "omap_wdt.h"
 
+static struct platform_device *omap_wdt_dev;
+
 static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 
-static int omap_wdt_users;
-static struct clk *armwdt_ck;
-static struct clk *mpu_wdt_ick;
-static struct clk *mpu_wdt_fck;
-
 static unsigned int wdt_trgr_pattern = 0x1234;
 static spinlock_t wdt_lock;
 
-static void omap_wdt_ping(void)
+struct omap_wdt_dev {
+	void __iomem    *base;          /* physical */
+	struct device   *dev;
+	int             omap_wdt_users;
+	struct clk      *armwdt_ck;
+	struct clk      *mpu_wdt_ick;
+	struct clk      *mpu_wdt_fck;
+	struct resource *mem;
+	struct miscdevice omap_wdt_miscdev;
+};
+
+static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
+	void __iomem    *base = wdev->base;
 	/* wait for posted write to complete */
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	wdt_trgr_pattern = ~wdt_trgr_pattern;
-	omap_writel(wdt_trgr_pattern, (OMAP_WATCHDOG_TGR));
+	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
 	/* wait for posted write to complete */
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	/* reloaded WCRR from WLDR */
 }
 
-static void omap_wdt_enable(void)
+static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 {
+	void __iomem *base;
+	base = wdev->base;
 	/* Sequence to enable the watchdog */
-	omap_writel(0xBBBB, OMAP_WATCHDOG_SPR);
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x10)
+	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
-	omap_writel(0x4444, OMAP_WATCHDOG_SPR);
-	while ((omap_readl(OMAP_WATCHDOG_WPS)) & 0x10)
+	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
+	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
 }
 
-static void omap_wdt_disable(void)
+static void omap_wdt_disable(struct omap_wdt_dev *wdev)
 {
+	void __iomem *base;
+	base = wdev->base;
 	/* sequence required to disable watchdog */
-	omap_writel(0xAAAA, OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x10)
+	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
-	omap_writel(0x5555, OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x10)
+	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
 }
 
@@ -103,15 +117,17 @@ static void omap_wdt_adjust_timeout(unsigned new_timeout)
 	timer_margin = new_timeout;
 }
 
-static void omap_wdt_set_timeout(void)
+static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 {
 	u32 pre_margin = GET_WLDR_VAL(timer_margin);
+	void __iomem *base;
+	base = wdev->base;
 
 	/* just count up at 32 KHz */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x04)
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
-	omap_writel(pre_margin, OMAP_WATCHDOG_LDR);
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x04)
+	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
 }
 
@@ -121,65 +137,69 @@ static void omap_wdt_set_timeout(void)
 
 static int omap_wdt_open(struct inode *inode, struct file *file)
 {
-	if (test_and_set_bit(1, (unsigned long *)&omap_wdt_users))
+	struct omap_wdt_dev *wdev;
+	void __iomem *base;
+	wdev = platform_get_drvdata(omap_wdt_dev);
+	base = wdev->base;
+	if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
 		return -EBUSY;
 
 	if (cpu_is_omap16xx())
-		clk_enable(armwdt_ck);	/* Enable the clock */
+		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
 
-	if (cpu_is_omap24xx()) {
-		clk_enable(mpu_wdt_ick);    /* Enable the interface clock */
-		clk_enable(mpu_wdt_fck);    /* Enable the functional clock */
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
+		clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
 	}
 
 	/* initialize prescaler */
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x01)
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
-	omap_writel((1 << 5) | (PTV << 2), OMAP_WATCHDOG_CNTRL);
-	while (omap_readl(OMAP_WATCHDOG_WPS) & 0x01)
+	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
+	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
 
-	omap_wdt_set_timeout();
-	omap_wdt_enable();
+	file->private_data = (void *) wdev;
+
+	omap_wdt_set_timeout(wdev);
+	omap_wdt_enable(wdev);
 	return nonseekable_open(inode, file);
 }
 
 static int omap_wdt_release(struct inode *inode, struct file *file)
 {
+	struct omap_wdt_dev *wdev;
+	wdev = file->private_data;
 	/*
 	 *      Shut off the timer unless NOWAYOUT is defined.
 	 */
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
-	omap_wdt_disable();
 
-	if (cpu_is_omap16xx()) {
-		clk_disable(armwdt_ck);	/* Disable the clock */
-		clk_put(armwdt_ck);
-		armwdt_ck = NULL;
-	}
+	omap_wdt_disable(wdev);
 
-	if (cpu_is_omap24xx()) {
-		clk_disable(mpu_wdt_ick);	/* Disable the clock */
-		clk_disable(mpu_wdt_fck);	/* Disable the clock */
-		clk_put(mpu_wdt_ick);
-		clk_put(mpu_wdt_fck);
-		mpu_wdt_ick = NULL;
-		mpu_wdt_fck = NULL;
+	if (cpu_is_omap16xx())
+		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
+
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
+		clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
 	}
 #else
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
-	omap_wdt_users = 0;
+	wdev->omap_wdt_users = 0;
 	return 0;
 }
 
 static ssize_t omap_wdt_write(struct file *file, const char __user *data,
 		size_t len, loff_t *ppos)
 {
+	struct omap_wdt_dev *wdev;
+	wdev = file->private_data;
 	/* Refresh LOAD_TIME. */
 	if (len) {
 		spin_lock(&wdt_lock);
-		omap_wdt_ping();
+		omap_wdt_ping(wdev);
 		spin_unlock(&wdt_lock);
 	}
 	return len;
@@ -188,12 +208,14 @@ static ssize_t omap_wdt_write(struct file *file, const char __user *data,
 static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
 						unsigned long arg)
 {
+	struct omap_wdt_dev *wdev;
 	int new_margin;
 	static const struct watchdog_info ident = {
 		.identity = "OMAP Watchdog",
 		.options = WDIOF_SETTIMEOUT,
 		.firmware_version = 0,
 	};
+	wdev = file->private_data;
 
 	switch (cmd) {
 	case WDIOC_GETSUPPORT:
@@ -210,7 +232,7 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
 					(int __user *)arg);
 	case WDIOC_KEEPALIVE:
 		spin_lock(&wdt_lock);
-		omap_wdt_ping();
+		omap_wdt_ping(wdev);
 		spin_unlock(&wdt_lock);
 		return 0;
 	case WDIOC_SETTIMEOUT:
@@ -218,19 +240,18 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 		omap_wdt_adjust_timeout(new_margin);
 
-		spin_lock(&wdt_lock);
-		omap_wdt_disable();
-		omap_wdt_set_timeout();
-		omap_wdt_enable();
+		omap_wdt_disable(wdev);
+		omap_wdt_set_timeout(wdev);
+		omap_wdt_enable(wdev);
 
-		omap_wdt_ping();
-		spin_unlock(&wdt_lock);
+		omap_wdt_ping(wdev);
 		/* Fall */
 	case WDIOC_GETTIMEOUT:
 		return put_user(timer_margin, (int __user *)arg);
 	default:
 		return -ENOTTY;
 	}
+	return 0;
 }
 
 static const struct file_operations omap_wdt_fops = {
@@ -241,96 +262,150 @@ static const struct file_operations omap_wdt_fops = {
 	.release = omap_wdt_release,
 };
 
-static struct miscdevice omap_wdt_miscdev = {
-	.minor = WATCHDOG_MINOR,
-	.name = "watchdog",
-	.fops = &omap_wdt_fops,
-};
 
 static int __init omap_wdt_probe(struct platform_device *pdev)
 {
 	struct resource *res, *mem;
 	int ret;
+	struct omap_wdt_dev *wdev;
 
 	/* reserve static register mappings */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -ENOENT;
 
+	if (omap_wdt_dev)
+		return -EBUSY;
+
 	mem = request_mem_region(res->start, res->end - res->start + 1,
 				 pdev->name);
 	if (mem == NULL)
 		return -EBUSY;
 
-	platform_set_drvdata(pdev, mem);
-
-	omap_wdt_users = 0;
+	wdev = kzalloc(sizeof(struct omap_wdt_dev), GFP_KERNEL);
+	if (!wdev) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	wdev->omap_wdt_users = 0;
+	wdev->mem = mem;
 
 	if (cpu_is_omap16xx()) {
-		armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
-		if (IS_ERR(armwdt_ck)) {
-			ret = PTR_ERR(armwdt_ck);
-			armwdt_ck = NULL;
+		wdev->armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
+		if (IS_ERR(wdev->armwdt_ck)) {
+			ret = PTR_ERR(wdev->armwdt_ck);
+			wdev->armwdt_ck = NULL;
 			goto fail;
 		}
 	}
 
 	if (cpu_is_omap24xx()) {
-		mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
-		if (IS_ERR(mpu_wdt_ick)) {
-			ret = PTR_ERR(mpu_wdt_ick);
-			mpu_wdt_ick = NULL;
+		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
+		if (IS_ERR(wdev->mpu_wdt_ick)) {
+			ret = PTR_ERR(wdev->mpu_wdt_ick);
+			wdev->mpu_wdt_ick = NULL;
+			goto fail;
+		}
+		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
+		if (IS_ERR(wdev->mpu_wdt_fck)) {
+			ret = PTR_ERR(wdev->mpu_wdt_fck);
+			wdev->mpu_wdt_fck = NULL;
+			goto fail;
+		}
+	}
+
+	if (cpu_is_omap34xx()) {
+		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "wdt2_ick");
+		if (IS_ERR(wdev->mpu_wdt_ick)) {
+			ret = PTR_ERR(wdev->mpu_wdt_ick);
+			wdev->mpu_wdt_ick = NULL;
 			goto fail;
 		}
-		mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
-		if (IS_ERR(mpu_wdt_fck)) {
-			ret = PTR_ERR(mpu_wdt_fck);
-			mpu_wdt_fck = NULL;
+		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
+		if (IS_ERR(wdev->mpu_wdt_fck)) {
+			ret = PTR_ERR(wdev->mpu_wdt_fck);
+			wdev->mpu_wdt_fck = NULL;
 			goto fail;
 		}
 	}
+	wdev->base = (void __iomem *) (mem->start);
+	platform_set_drvdata(pdev, wdev);
 
-	omap_wdt_disable();
+	omap_wdt_disable(wdev);
 	omap_wdt_adjust_timeout(timer_margin);
 
-	omap_wdt_miscdev.parent = &pdev->dev;
-	ret = misc_register(&omap_wdt_miscdev);
+	wdev->omap_wdt_miscdev.parent = &pdev->dev;
+	wdev->omap_wdt_miscdev.minor = WATCHDOG_MINOR;
+	wdev->omap_wdt_miscdev.name = "watchdog";
+	wdev->omap_wdt_miscdev.fops = &omap_wdt_fops;
+
+	ret = misc_register(&(wdev->omap_wdt_miscdev));
 	if (ret)
 		goto fail;
 
-	pr_info("OMAP Watchdog Timer: initial timeout %d sec\n", timer_margin);
+	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
+		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
+		timer_margin);
 
 	/* autogate OCP interface clock */
-	omap_writel(0x01, OMAP_WATCHDOG_SYS_CONFIG);
+	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
+
+	omap_wdt_dev = pdev;
+
 	return 0;
 
 fail:
-	if (armwdt_ck)
-		clk_put(armwdt_ck);
-	if (mpu_wdt_ick)
-		clk_put(mpu_wdt_ick);
-	if (mpu_wdt_fck)
-		clk_put(mpu_wdt_fck);
-	release_resource(mem);
+	if (wdev) {
+		platform_set_drvdata(pdev, NULL);
+		if (wdev->armwdt_ck)
+			clk_put(wdev->armwdt_ck);
+		if (wdev->mpu_wdt_ick)
+			clk_put(wdev->mpu_wdt_ick);
+		if (wdev->mpu_wdt_fck)
+			clk_put(wdev->mpu_wdt_fck);
+		kfree(wdev);
+	}
+	if (mem) {
+		release_mem_region(res->start, res->end - res->start + 1);
+	}
 	return ret;
 }
 
 static void omap_wdt_shutdown(struct platform_device *pdev)
 {
-	omap_wdt_disable();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+
+	if (wdev->omap_wdt_users)
+		omap_wdt_disable(wdev);
 }
 
 static int omap_wdt_remove(struct platform_device *pdev)
 {
-	struct resource *mem = platform_get_drvdata(pdev);
-	misc_deregister(&omap_wdt_miscdev);
-	release_resource(mem);
-	if (armwdt_ck)
-		clk_put(armwdt_ck);
-	if (mpu_wdt_ick)
-		clk_put(mpu_wdt_ick);
-	if (mpu_wdt_fck)
-		clk_put(mpu_wdt_fck);
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (!res)
+		return -ENOENT;
+
+	misc_deregister(&(wdev->omap_wdt_miscdev));
+	release_mem_region(res->start, res->end - res->start + 1);
+	platform_set_drvdata(pdev, NULL);
+	if (wdev->armwdt_ck) {
+		clk_put(wdev->armwdt_ck);
+		wdev->armwdt_ck = NULL;
+	}
+	if (wdev->mpu_wdt_ick) {
+		clk_put(wdev->mpu_wdt_ick);
+		wdev->mpu_wdt_ick = NULL;
+	}
+	if (wdev->mpu_wdt_fck) {
+		clk_put(wdev->mpu_wdt_fck);
+		wdev->mpu_wdt_fck = NULL;
+	}
+	kfree(wdev);
+	omap_wdt_dev = NULL;
 	return 0;
 }
 
@@ -344,16 +419,20 @@ static int omap_wdt_remove(struct platform_device *pdev)
 
 static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	if (omap_wdt_users)
-		omap_wdt_disable();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+	if (wdev->omap_wdt_users)
+		omap_wdt_disable(wdev);
 	return 0;
 }
 
 static int omap_wdt_resume(struct platform_device *pdev)
 {
-	if (omap_wdt_users) {
-		omap_wdt_enable();
-		omap_wdt_ping();
+	struct omap_wdt_dev *wdev;
+	wdev = platform_get_drvdata(pdev);
+	if (wdev->omap_wdt_users) {
+		omap_wdt_enable(wdev);
+		omap_wdt_ping(wdev);
 	}
 	return 0;
 }
diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
index 52a532a..fc02ec6 100644
--- a/drivers/watchdog/omap_wdt.h
+++ b/drivers/watchdog/omap_wdt.h
@@ -30,25 +30,15 @@
 #ifndef _OMAP_WATCHDOG_H
 #define _OMAP_WATCHDOG_H
 
-#define OMAP1610_WATCHDOG_BASE		0xfffeb000
-#define OMAP2420_WATCHDOG_BASE		0x48022000	/*WDT Timer 2 */
-
-#ifdef CONFIG_ARCH_OMAP24XX
-#define OMAP_WATCHDOG_BASE 		OMAP2420_WATCHDOG_BASE
-#else
-#define OMAP_WATCHDOG_BASE 		OMAP1610_WATCHDOG_BASE
-#define RM_RSTST_WKUP			0
-#endif
-
-#define OMAP_WATCHDOG_REV		(OMAP_WATCHDOG_BASE + 0x00)
-#define OMAP_WATCHDOG_SYS_CONFIG	(OMAP_WATCHDOG_BASE + 0x10)
-#define OMAP_WATCHDOG_STATUS		(OMAP_WATCHDOG_BASE + 0x14)
-#define OMAP_WATCHDOG_CNTRL		(OMAP_WATCHDOG_BASE + 0x24)
-#define OMAP_WATCHDOG_CRR		(OMAP_WATCHDOG_BASE + 0x28)
-#define OMAP_WATCHDOG_LDR		(OMAP_WATCHDOG_BASE + 0x2c)
-#define OMAP_WATCHDOG_TGR		(OMAP_WATCHDOG_BASE + 0x30)
-#define OMAP_WATCHDOG_WPS		(OMAP_WATCHDOG_BASE + 0x34)
-#define OMAP_WATCHDOG_SPR		(OMAP_WATCHDOG_BASE + 0x48)
+#define OMAP_WATCHDOG_REV		(0x00)
+#define OMAP_WATCHDOG_SYS_CONFIG	(0x10)
+#define OMAP_WATCHDOG_STATUS		(0x14)
+#define OMAP_WATCHDOG_CNTRL		(0x24)
+#define OMAP_WATCHDOG_CRR		(0x28)
+#define OMAP_WATCHDOG_LDR		(0x2c)
+#define OMAP_WATCHDOG_TGR		(0x30)
+#define OMAP_WATCHDOG_WPS		(0x34)
+#define OMAP_WATCHDOG_SPR		(0x48)
 
 /* Using the prescaler, the OMAP watchdog could go for many
  * months before firing.  These limits work without scaling,
-- 
1.6.0.1.308.gede4c

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[Russell King - ARM Linux]

You're getting there with this patch, but still not completely up to
snuff.

On Fri, Sep 19, 2008 at 01:32:35PM +0300, Felipe Balbi wrote:
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index 3a11dad..e55f2cc 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -39,6 +39,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/moduleparam.h>
>  #include <linux/clk.h>
> +
>  #include <linux/bitops.h>
>  #include <linux/io.h>
>  #include <linux/uaccess.h>

This hunk is unnecessary.

> @@ -218,19 +240,18 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  		omap_wdt_adjust_timeout(new_margin);
>  
> -		spin_lock(&wdt_lock);
> -		omap_wdt_disable();
> -		omap_wdt_set_timeout();
> -		omap_wdt_enable();
> +		omap_wdt_disable(wdev);
> +		omap_wdt_set_timeout(wdev);
> +		omap_wdt_enable(wdev);
>  
> -		omap_wdt_ping();
> -		spin_unlock(&wdt_lock);
> +		omap_wdt_ping(wdev);

This is removing the spin lock which should remain.

>  		/* Fall */
>  	case WDIOC_GETTIMEOUT:
>  		return put_user(timer_margin, (int __user *)arg);
>  	default:
>  		return -ENOTTY;
>  	}
> +	return 0;

And this return statement shouldn't be required.

Apart from those three points, nice work.

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[David Brownell]

On Friday 19 September 2008, Felipe Balbi wrote:
> - * linux/drivers/char/watchdog/omap_wdt.c
> + * linux/drivers/watchdog/omap_wdt.c

current style omits the paths (just use "omap_wdt.c")
since they change periodically...

Re: [PATCH 1/5] watchdog: sync linux-omap changes

[David Brownell]

On Friday 19 September 2008, Felipe Balbi wrote:
> +static struct platform_device *omap_wdt_dev;
> +
>  ...
> +struct omap_wdt_dev {
> +       void __iomem    *base;          /* physical */
> +       struct device   *dev;


You don't need both omap_wdt_dev (platform device)
and omap_wdt_dev.dev (hmm, never used).  In fact
the former isn't needed either ... its role seems
to be ensure only one watchdog device gets bound,
which is more naturally done by not registering
more than one such platform device.
 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html