[PATCH 0/6] OMAP: omap_wdt: clocks and NOWAYOUT fixes.

[PATCH 0/6] OMAP: omap_wdt: clocks and NOWAYOUT fixes.

[Atal Shargorodsky]

This patchset reorganizes slightly the code of omap_wdt driver,
introduces proper management of clocks and
CONFIG_WATCHDOG_NOWAYOUT + PM support, which was broken.
Also, inspired by numerous watchdog drivers using this technique, 
this patchset makes the NOWAYOUT behavior not to be restricted by kernel
configuration, but to be controlled by a module parameter.


It's not expected to be applied as is, as it's not utilizing the clkdev,
so part of it is obsolete. So I'm posting it because of the other changes
it introduced to be discussed.

Also the first patch in the patch set cannot apply because of the
current driver in linux-omap git tree being broken.
It will apply after applying the inlined patch provided by Felipe Balbi:


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

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 7bcbb7f..0fb9fd7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -294,7 +294,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
                goto err_busy;
        }
 
-       wdev = kzalloc(sizeof(struct omap_wdt_dev), GFP_KERNEL);
+       wdev = kzalloc(sizeof(*wdev), GFP_KERNEL);
        if (!wdev) {
                ret = -ENOMEM;
                goto err_kzalloc;
@@ -347,8 +347,18 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
                goto err_ioremap;
        }
 
+       spin_lock_init(&wdt_lock);
        platform_set_drvdata(pdev, wdev);
 
+       /* enable clocks for register access */
+       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_disable(wdev);
        omap_wdt_adjust_timeout(timer_margin);
 
@@ -368,6 +378,15 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
        /* autogate OCP interface clock */
        __raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
+       /* disable clocks since we don't need them now */
+       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_dev = pdev;
 
        return 0;
@@ -488,7 +507,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);
 }
 
-- 



And another thing to be noted: Imre Deak proposed to stay with
cpu_is_* macros for a better optimization.
Or to use a (!cpu_class_is_omap1()) instead of 
        (cpu_is_omap24xx() || cpu_is_omap34xx()).
I think it's the right thing to do, but it's not included in the patchset,
as this patchset is not expected to be applied as is anyway, 
but rather to be discussed.

[PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure.

[Atal Shargorodsky]

No need to hold three clocks, when for each paricular platform only
two are used - finctional and interface.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   51 ++++++++++++------------------------------
 1 files changed, 15 insertions(+), 36 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index b04f650..5b72c7c 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -60,7 +60,6 @@ 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;
@@ -146,13 +145,10 @@ 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()) {
+	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 */
-	}
+
+	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
 
 	/* initialize prescaler */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
@@ -181,13 +177,10 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 
 	omap_wdt_disable(wdev);
 
-	if (cpu_is_omap16xx())
-		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+	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 */
-	}
+
+	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
 #else
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
@@ -304,10 +297,10 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	wdev->mem = mem;
 
 	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;
+		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "armwdt_ck");
+		if (IS_ERR(wdev->mpu_wdt_fck)) {
+			ret = PTR_ERR(wdev->mpu_wdt_fck);
+			wdev->mpu_wdt_fck = NULL;
 			goto err_clk;
 		}
 	}
@@ -351,13 +344,10 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdev);
 
 	/* enable clocks for register access */
-	if (cpu_is_omap16xx())
-		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+	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 */
-	}
+
+	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
 
 	omap_wdt_disable(wdev);
 	omap_wdt_adjust_timeout(timer_margin);
@@ -379,13 +369,9 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
 	/* disable clocks since we don't need them now */
-	if (cpu_is_omap16xx())
-		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+	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 */
-	}
+	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
 
 	omap_wdt_dev = pdev;
 
@@ -399,8 +385,6 @@ 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)
@@ -436,11 +420,6 @@ static int omap_wdt_remove(struct platform_device *pdev)
 	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;
-- 
1.5.4.3

[PATCH 2/6] OMAP: omap_wdt: Fix interface clock existence check.

[Atal Shargorodsky]

When enabling/disabling the iterface clock, the question
to be asked is if this clock exists rather than to
decide about it's existance by retrieving the chip version.
It can be done only once at init time.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 5b72c7c..05dc55a 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -145,7 +145,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_omap24xx() || cpu_is_omap34xx())
+	if (wdev->mpu_wdt_ick)
 		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
 
 	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
@@ -177,7 +177,7 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 
 	omap_wdt_disable(wdev);
 
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (wdev->mpu_wdt_ick)
 		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
 
 	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
@@ -344,7 +344,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdev);
 
 	/* enable clocks for register access */
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (wdev->mpu_wdt_ick)
 		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
 
 	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
@@ -369,7 +369,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
 	/* disable clocks since we don't need them now */
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (wdev->mpu_wdt_ick)
 		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
 	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
 
-- 
1.5.4.3

[PATCH 3/6] OMAP: omap_wdt: Remove non-explanatory comments.

[Atal Shargorodsky]

The call to clk_disable does need a trivial
comment like /* Disable the clock */.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 05dc55a..1364d7e 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -146,9 +146,9 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 		return -EBUSY;
 
 	if (wdev->mpu_wdt_ick)
-		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
+		clk_enable(wdev->mpu_wdt_ick);
 
-	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
+	clk_enable(wdev->mpu_wdt_fck);
 
 	/* initialize prescaler */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
@@ -178,9 +178,9 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 	omap_wdt_disable(wdev);
 
 	if (wdev->mpu_wdt_ick)
-		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
+		clk_disable(wdev->mpu_wdt_ick);
 
-	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
+	clk_disable(wdev->mpu_wdt_fck);
 #else
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
@@ -345,9 +345,9 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 
 	/* enable clocks for register access */
 	if (wdev->mpu_wdt_ick)
-		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
+		clk_enable(wdev->mpu_wdt_ick);
 
-	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
+	clk_enable(wdev->mpu_wdt_fck);
 
 	omap_wdt_disable(wdev);
 	omap_wdt_adjust_timeout(timer_margin);
@@ -370,8 +370,8 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 
 	/* disable clocks since we don't need them now */
 	if (wdev->mpu_wdt_ick)
-		clk_disable(wdev->mpu_wdt_ick);	/* Disable the clock */
-	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
+		clk_disable(wdev->mpu_wdt_ick);
+	clk_disable(wdev->mpu_wdt_fck);
 
 	omap_wdt_dev = pdev;
 
-- 
1.5.4.3

[PATCH 4/6] OMAP: omap_wdt: Proper interface clock management.

[Atal Shargorodsky]

Enable interface clock only when accesssing the hw.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   47 +++++++++++++++++++++++++++++++-----------
 1 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 1364d7e..d033139 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -83,6 +83,16 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 	/* reloaded WCRR from WLDR */
 }
 
+static void omap_wdt_ick_enable(struct clk *ick, int enable)
+{
+	if (ick) {
+		if (enable)
+			clk_enable(ick);
+		else
+			clk_disable(ick);
+	}
+}
+
 static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 {
 	void __iomem *base = wdev->base;
@@ -145,9 +155,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 (wdev->mpu_wdt_ick)
-		clk_enable(wdev->mpu_wdt_ick);
-
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 	clk_enable(wdev->mpu_wdt_fck);
 
 	/* initialize prescaler */
@@ -162,6 +170,7 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 
 	omap_wdt_set_timeout(wdev);
 	omap_wdt_enable(wdev);
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 
 	return nonseekable_open(inode, file);
 }
@@ -175,10 +184,9 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 	 */
 #ifndef CONFIG_WATCHDOG_NOWAYOUT
 
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 	omap_wdt_disable(wdev);
-
-	if (wdev->mpu_wdt_ick)
-		clk_disable(wdev->mpu_wdt_ick);
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 
 	clk_disable(wdev->mpu_wdt_fck);
 #else
@@ -196,9 +204,11 @@ static ssize_t omap_wdt_write(struct file *file, const char __user *data,
 
 	/* Refresh LOAD_TIME. */
 	if (len) {
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		spin_lock(&wdt_lock);
 		omap_wdt_ping(wdev);
 		spin_unlock(&wdt_lock);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 	}
 	return len;
 }
@@ -230,15 +240,18 @@ 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:
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		spin_lock(&wdt_lock);
 		omap_wdt_ping(wdev);
 		spin_unlock(&wdt_lock);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 		return 0;
 	case WDIOC_SETTIMEOUT:
 		if (get_user(new_margin, (int __user *)arg))
 			return -EFAULT;
 		omap_wdt_adjust_timeout(new_margin);
 
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		spin_lock(&wdt_lock);
 		omap_wdt_disable(wdev);
 		omap_wdt_set_timeout(wdev);
@@ -246,6 +259,7 @@ static long omap_wdt_ioctl(struct file *file, unsigned int cmd,
 
 		omap_wdt_ping(wdev);
 		spin_unlock(&wdt_lock);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 		/* Fall */
 	case WDIOC_GETTIMEOUT:
 		return put_user(timer_margin, (int __user *)arg);
@@ -344,9 +358,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdev);
 
 	/* enable clocks for register access */
-	if (wdev->mpu_wdt_ick)
-		clk_enable(wdev->mpu_wdt_ick);
-
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 	clk_enable(wdev->mpu_wdt_fck);
 
 	omap_wdt_disable(wdev);
@@ -369,8 +381,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
 	/* disable clocks since we don't need them now */
-	if (wdev->mpu_wdt_ick)
-		clk_disable(wdev->mpu_wdt_ick);
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 	clk_disable(wdev->mpu_wdt_fck);
 
 	omap_wdt_dev = pdev;
@@ -378,6 +389,8 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	return 0;
 
 err_misc:
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+	clk_disable(wdev->mpu_wdt_fck);
 	platform_set_drvdata(pdev, NULL);
 	iounmap(wdev->base);
 
@@ -404,8 +417,11 @@ static void omap_wdt_shutdown(struct platform_device *pdev)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
-	if (wdev->omap_wdt_users)
+	if (wdev->omap_wdt_users) {
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_disable(wdev);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+	}
 }
 
 static int omap_wdt_remove(struct platform_device *pdev)
@@ -449,8 +465,11 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
-	if (wdev->omap_wdt_users)
+	if (wdev->omap_wdt_users) {
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_disable(wdev);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+	}
 
 	return 0;
 }
@@ -460,8 +479,10 @@ static int omap_wdt_resume(struct platform_device *pdev)
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
 	if (wdev->omap_wdt_users) {
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_enable(wdev);
 		omap_wdt_ping(wdev);
+		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 	}
 
 	return 0;
-- 
1.5.4.3

[PATCH 5/6] OMAP: omap_wdt: Correct manage of activation states.

[Atal Shargorodsky]

Needed to support CONFIG_WATCHDOG_NOWAYOUT option.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   40 ++++++++++++++++++++++++++--------------
 1 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index d033139..6f6a524 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -56,10 +56,13 @@ MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 static unsigned int wdt_trgr_pattern = 0x1234;
 static spinlock_t wdt_lock;
 
+#define OMAP_WDT_STATE_OPENED_BIT	1
+#define OMAP_WDT_STATE_ACTIVATED_BIT	8
+
 struct omap_wdt_dev {
 	void __iomem    *base;          /* physical */
 	struct device   *dev;
-	int             omap_wdt_users;
+	int             omap_wdt_state;
 	struct clk      *mpu_wdt_ick;
 	struct clk      *mpu_wdt_fck;
 	struct resource *mem;
@@ -152,19 +155,25 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	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)))
+	if (test_and_set_bit(OMAP_WDT_STATE_OPENED_BIT,
+			(unsigned long *)&(wdev->omap_wdt_state)))
 		return -EBUSY;
 
 	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
-	clk_enable(wdev->mpu_wdt_fck);
+	if (wdev->omap_wdt_state & (1 << OMAP_WDT_STATE_ACTIVATED_BIT))
+		omap_wdt_ping(wdev);
+	else {
+		clk_enable(wdev->mpu_wdt_fck);
 
-	/* initialize prescaler */
-	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
-		cpu_relax();
+		/* 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();
+		__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
+		while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
+			cpu_relax();
+		wdev->omap_wdt_state |= (1 << OMAP_WDT_STATE_ACTIVATED_BIT);
+	}
 
 	file->private_data = (void *) wdev;
 
@@ -189,10 +198,13 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 
 	clk_disable(wdev->mpu_wdt_fck);
+	wdev->omap_wdt_state &= ~(1 << OMAP_WDT_STATE_ACTIVATED_BIT);
 #else
+	/* Give the user application some time to recover in case of crash. */
+	omap_wdt_ping(wdev);
 	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
 #endif
-	wdev->omap_wdt_users = 0;
+	wdev->omap_wdt_state &= ~(1 << OMAP_WDT_STATE_OPENED_BIT);
 
 	return 0;
 }
@@ -307,7 +319,7 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		goto err_kzalloc;
 	}
 
-	wdev->omap_wdt_users = 0;
+	wdev->omap_wdt_state = 0;
 	wdev->mem = mem;
 
 	if (cpu_is_omap16xx()) {
@@ -417,7 +429,7 @@ static void omap_wdt_shutdown(struct platform_device *pdev)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
-	if (wdev->omap_wdt_users) {
+	if (wdev->omap_wdt_state & (1<<OMAP_WDT_STATE_ACTIVATED_BIT)) {
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_disable(wdev);
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
@@ -465,7 +477,7 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
-	if (wdev->omap_wdt_users) {
+	if (wdev->omap_wdt_state & (1<<OMAP_WDT_STATE_ACTIVATED_BIT)) {
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_disable(wdev);
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
@@ -478,7 +490,7 @@ static int omap_wdt_resume(struct platform_device *pdev)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
-	if (wdev->omap_wdt_users) {
+	if (wdev->omap_wdt_state & (1<<OMAP_WDT_STATE_ACTIVATED_BIT)) {
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_enable(wdev);
 		omap_wdt_ping(wdev);
-- 
1.5.4.3

[PATCH 6/6] OMAP: omap_wdt: Changing NOWAYOUT behavior does not require kernel reconfiguration.

[Atal Shargorodsky]

By introducing additional module parameter we make the
CONFIG_WATCHDOG_NOWAYOUT to be a mere default module behavior,
which can be overriden by providing nowayout=[01] parameter at load time.

Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   48 +++++++++++++++++++++++-------------------
 1 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 6f6a524..6e9617c 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -53,6 +53,13 @@ static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 
+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 int wdt_trgr_pattern = 0x1234;
 static spinlock_t wdt_lock;
 
@@ -188,22 +195,20 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
 {
 	struct omap_wdt_dev *wdev = file->private_data;
 
-	/*
-	 *      Shut off the timer unless NOWAYOUT is defined.
-	 */
-#ifndef CONFIG_WATCHDOG_NOWAYOUT
-
 	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
-	omap_wdt_disable(wdev);
-	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+	if (nowayout) {
+		/* Give the user application some time to recover 
+		* in case of crash. 
+		* */
+		omap_wdt_ping(wdev);
+		printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
+	} else {
+		omap_wdt_disable(wdev);
 
-	clk_disable(wdev->mpu_wdt_fck);
-	wdev->omap_wdt_state &= ~(1 << OMAP_WDT_STATE_ACTIVATED_BIT);
-#else
-	/* Give the user application some time to recover in case of crash. */
-	omap_wdt_ping(wdev);
-	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
-#endif
+		clk_disable(wdev->mpu_wdt_fck);
+		wdev->omap_wdt_state &= ~(1 << OMAP_WDT_STATE_ACTIVATED_BIT);
+	}
+	omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
 	wdev->omap_wdt_state &= ~(1 << OMAP_WDT_STATE_OPENED_BIT);
 
 	return 0;
@@ -385,9 +390,10 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_misc;
 
-	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
+	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial "
+		"timeout %d sec, nowayout is %s\n",
 		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
-		timer_margin);
+		timer_margin, (nowayout ? "on" : "off"));
 
 	/* autogate OCP interface clock */
 	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
@@ -467,12 +473,6 @@ static int omap_wdt_remove(struct platform_device *pdev)
 
 #ifdef	CONFIG_PM
 
-/* REVISIT ... not clear this is the best way to handle system suspend; and
- * it's very inappropriate for selective device suspend (e.g. suspending this
- * through sysfs rather than by stopping the watchdog daemon).  Also, this
- * may not play well enough with NOWAYOUT...
- */
-
 static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
@@ -481,6 +481,8 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state)
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_disable(wdev);
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+		clk_disable(wdev->mpu_wdt_fck);
+printk (KERN_EMERG "omap_wdt: suspend \n");
 	}
 
 	return 0;
@@ -491,10 +493,12 @@ static int omap_wdt_resume(struct platform_device *pdev)
 	struct omap_wdt_dev *wdev = platform_get_drvdata(pdev);
 
 	if (wdev->omap_wdt_state & (1<<OMAP_WDT_STATE_ACTIVATED_BIT)) {
+		clk_enable(wdev->mpu_wdt_fck);
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 1);
 		omap_wdt_enable(wdev);
 		omap_wdt_ping(wdev);
 		omap_wdt_ick_enable(wdev->mpu_wdt_ick, 0);
+printk (KERN_EMERG "omap_wdt: resume\n");
 	}
 
 	return 0;
-- 
1.5.4.3

Re: [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure.

[Tony Lindgren]

Hi,

* Atal Shargorodsky <ext-atal.shargorodsky@nokia.com> [090311 08:31]:
> No need to hold three clocks, when for each paricular platform only
> two are used - finctional and interface.
> 
> Signed-off-by: Atal Shargorodsky <ext-atal.shargorodsky@nokia.com>
> ---
>  drivers/watchdog/omap_wdt.c |   51 ++++++++++++------------------------------
>  1 files changed, 15 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index b04f650..5b72c7c 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -60,7 +60,6 @@ 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;
> @@ -146,13 +145,10 @@ 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()) {
> +	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 */
> -	}
> +
> +	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
>  
>  	/* initialize prescaler */
>  	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)

Some extra work required.. You will need to rebase at least this patch as
the big pile of clock changes by Paul and Russell will go in this coming
merge window.

Basically the code above will be cleaner with no need for cpu_is_ stuff:

clk_enable(wdev->ick);    /* Enable the interface clock */
clk_enable(wdev->fck);    /* Enable the functional clock */

I'm mirroring Russell's omap-clks3 branch in the linux-omap tree as
clks-testing branch, so please take a look at that branch.

Then see the MAINTAINERS file for "WATCHDOG DEVICE DRIVERS", so you
can find the address for Wim. Please send the series to Wim with
LKML, LAKML, and linux-omap mailing lists Cc'd. Please also mention
that these patches depend on some pending clock changes.

Or even better, maybe you can leave out the clock part from your patch
so your series applies to both the mainline kernel and clks-testing
branch with some offset warnings ;)

Regards,

Tony



> @@ -181,13 +177,10 @@ static int omap_wdt_release(struct inode *inode, struct file *file)
>  
>  	omap_wdt_disable(wdev);
>  
> -	if (cpu_is_omap16xx())
> -		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
> -
> -	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> +	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 */
> -	}
> +
> +	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
>  #else
>  	printk(KERN_CRIT "omap_wdt: Unexpected close, not stopping!\n");
>  #endif
> @@ -304,10 +297,10 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  	wdev->mem = mem;
>  
>  	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;
> +		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "armwdt_ck");
> +		if (IS_ERR(wdev->mpu_wdt_fck)) {
> +			ret = PTR_ERR(wdev->mpu_wdt_fck);
> +			wdev->mpu_wdt_fck = NULL;
>  			goto err_clk;
>  		}
>  	}
> @@ -351,13 +344,10 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, wdev);
>  
>  	/* enable clocks for register access */
> -	if (cpu_is_omap16xx())
> -		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
> -
> -	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> +	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 */
> -	}
> +
> +	clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
>  
>  	omap_wdt_disable(wdev);
>  	omap_wdt_adjust_timeout(timer_margin);
> @@ -379,13 +369,9 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
>  
>  	/* disable clocks since we don't need them now */
> -	if (cpu_is_omap16xx())
> -		clk_disable(wdev->armwdt_ck);	/* Disable the clock */
> -
> -	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> +	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 */
> -	}
> +	clk_disable(wdev->mpu_wdt_fck);	/* Disable the clock */
>  
>  	omap_wdt_dev = pdev;
>  
> @@ -399,8 +385,6 @@ 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)
> @@ -436,11 +420,6 @@ static int omap_wdt_remove(struct platform_device *pdev)
>  	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;
> -- 
> 1.5.4.3
> 
> --
> 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