All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] OMAP: watchdog driver fixes.
@ 2009-03-10 16:03 Atal Shargorodsky
  2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
  2009-03-10 23:30 ` [PATCH 0/7] OMAP: watchdog driver fixes Russell King - ARM Linux
  0 siblings, 2 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux

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.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure.
  2009-03-10 16:03 [PATCH 0/7] OMAP: watchdog driver fixes Atal Shargorodsky
@ 2009-03-10 16:03 ` Atal Shargorodsky
  2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
                     ` (2 more replies)
  2009-03-10 23:30 ` [PATCH 0/7] OMAP: watchdog driver fixes Russell King - ARM Linux
  1 sibling, 3 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] Fix interface clock existance check.
  2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
@ 2009-03-10 16:03   ` Atal Shargorodsky
  2009-03-10 16:03     ` [PATCH 3/6] Remove non-explanatory comments Atal Shargorodsky
  2009-03-11 15:29     ` [PATCH 3/6] OMAP: omap_wdt: Remove non-explanatory comments Atal Shargorodsky
  2009-03-11 15:29   ` [PATCH 2/6] OMAP: omap_wdt: Fix interface clock existence check Atal Shargorodsky
  2009-03-11 16:02   ` [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure Tony Lindgren
  2 siblings, 2 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] Remove non-explanatory comments.
  2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
@ 2009-03-10 16:03     ` Atal Shargorodsky
  2009-03-10 16:03       ` [PATCH 4/6] Proper interface clock management Atal Shargorodsky
  2009-03-11 15:29       ` [PATCH 4/6] OMAP: omap_wdt: Proper interface clock management Atal Shargorodsky
  2009-03-11 15:29     ` [PATCH 3/6] OMAP: omap_wdt: Remove non-explanatory comments Atal Shargorodsky
  1 sibling, 2 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] Proper interface clock management.
  2009-03-10 16:03     ` [PATCH 3/6] Remove non-explanatory comments Atal Shargorodsky
@ 2009-03-10 16:03       ` Atal Shargorodsky
  2009-03-10 16:03         ` [PATCH 5/6] Correct manage of activation states Atal Shargorodsky
  2009-03-11 15:29         ` [PATCH 5/6] OMAP: omap_wdt: Correct manage of activation states Atal Shargorodsky
  2009-03-11 15:29       ` [PATCH 4/6] OMAP: omap_wdt: Proper interface clock management Atal Shargorodsky
  1 sibling, 2 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] Correct manage of activation states.
  2009-03-10 16:03       ` [PATCH 4/6] Proper interface clock management Atal Shargorodsky
@ 2009-03-10 16:03         ` Atal Shargorodsky
  2009-03-10 16:03           ` [PATCH 6/6] Changing NOWAYOUT behavior does not require kernel reconfiguration Atal Shargorodsky
  2009-03-11 15:29           ` [PATCH 6/6] OMAP: omap_wdt: " Atal Shargorodsky
  2009-03-11 15:29         ` [PATCH 5/6] OMAP: omap_wdt: Correct manage of activation states Atal Shargorodsky
  1 sibling, 2 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] Changing NOWAYOUT behavior does not require kernel reconfiguration.
  2009-03-10 16:03         ` [PATCH 5/6] Correct manage of activation states Atal Shargorodsky
@ 2009-03-10 16:03           ` Atal Shargorodsky
  2009-03-11 15:29           ` [PATCH 6/6] OMAP: omap_wdt: " Atal Shargorodsky
  1 sibling, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-10 16:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/7] OMAP: watchdog driver fixes.
  2009-03-10 16:03 [PATCH 0/7] OMAP: watchdog driver fixes Atal Shargorodsky
  2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
@ 2009-03-10 23:30 ` Russell King - ARM Linux
  1 sibling, 0 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2009-03-10 23:30 UTC (permalink / raw)
  To: Atal Shargorodsky; +Cc: linux-kernel

On Tue, Mar 10, 2009 at 06:03:50PM +0200, Atal Shargorodsky wrote:
> 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.

NAK for the entire patch set.  The first four patches severely clash
with my clk API work which is (in theory) queued for the next merge
window.  My patches already eliminate most of the unnecessary clk API
crap in there.

Moreover, your emails show no interaction with either the ARM mailing
lists or the OMAP mailing lists.

Please submit your patches to both the ARM and OMAP mailing lists for
review there.  Thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/6]  OMAP: omap_wdt: clocks and NOWAYOUT fixes.
@ 2009-03-11 15:29 Atal Shargorodsky
  2009-03-11 15:29 ` [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
  0 siblings, 1 reply; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, Felipe Balbi

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.




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure.
  2009-03-11 15:29 [PATCH 0/6] OMAP: omap_wdt: clocks and NOWAYOUT fixes Atal Shargorodsky
@ 2009-03-11 15:29 ` Atal Shargorodsky
  0 siblings, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] OMAP: omap_wdt: Fix interface clock existence check.
  2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
  2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
@ 2009-03-11 15:29   ` Atal Shargorodsky
  2009-03-11 16:02   ` [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure Tony Lindgren
  2 siblings, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] OMAP: omap_wdt: Remove non-explanatory comments.
  2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
  2009-03-10 16:03     ` [PATCH 3/6] Remove non-explanatory comments Atal Shargorodsky
@ 2009-03-11 15:29     ` Atal Shargorodsky
  1 sibling, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] OMAP: omap_wdt: Proper interface clock management.
  2009-03-10 16:03     ` [PATCH 3/6] Remove non-explanatory comments Atal Shargorodsky
  2009-03-10 16:03       ` [PATCH 4/6] Proper interface clock management Atal Shargorodsky
@ 2009-03-11 15:29       ` Atal Shargorodsky
  1 sibling, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] OMAP: omap_wdt: Correct manage of activation states.
  2009-03-10 16:03       ` [PATCH 4/6] Proper interface clock management Atal Shargorodsky
  2009-03-10 16:03         ` [PATCH 5/6] Correct manage of activation states Atal Shargorodsky
@ 2009-03-11 15:29         ` Atal Shargorodsky
  1 sibling, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] OMAP: omap_wdt: Changing NOWAYOUT behavior does not require kernel reconfiguration.
  2009-03-10 16:03         ` [PATCH 5/6] Correct manage of activation states Atal Shargorodsky
  2009-03-10 16:03           ` [PATCH 6/6] Changing NOWAYOUT behavior does not require kernel reconfiguration Atal Shargorodsky
@ 2009-03-11 15:29           ` Atal Shargorodsky
  1 sibling, 0 replies; 16+ messages in thread
From: Atal Shargorodsky @ 2009-03-11 15:29 UTC (permalink / raw)
  To: linux-omap; +Cc: linux-arm-kernel, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure.
  2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
  2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
  2009-03-11 15:29   ` [PATCH 2/6] OMAP: omap_wdt: Fix interface clock existence check Atal Shargorodsky
@ 2009-03-11 16:02   ` Tony Lindgren
  2 siblings, 0 replies; 16+ messages in thread
From: Tony Lindgren @ 2009-03-11 16:02 UTC (permalink / raw)
  To: Atal Shargorodsky; +Cc: linux-omap, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-03-11 16:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-10 16:03 [PATCH 0/7] OMAP: watchdog driver fixes Atal Shargorodsky
2009-03-10 16:03 ` [PATCH 1/6] Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky
2009-03-10 16:03   ` [PATCH 2/6] Fix interface clock existance check Atal Shargorodsky
2009-03-10 16:03     ` [PATCH 3/6] Remove non-explanatory comments Atal Shargorodsky
2009-03-10 16:03       ` [PATCH 4/6] Proper interface clock management Atal Shargorodsky
2009-03-10 16:03         ` [PATCH 5/6] Correct manage of activation states Atal Shargorodsky
2009-03-10 16:03           ` [PATCH 6/6] Changing NOWAYOUT behavior does not require kernel reconfiguration Atal Shargorodsky
2009-03-11 15:29           ` [PATCH 6/6] OMAP: omap_wdt: " Atal Shargorodsky
2009-03-11 15:29         ` [PATCH 5/6] OMAP: omap_wdt: Correct manage of activation states Atal Shargorodsky
2009-03-11 15:29       ` [PATCH 4/6] OMAP: omap_wdt: Proper interface clock management Atal Shargorodsky
2009-03-11 15:29     ` [PATCH 3/6] OMAP: omap_wdt: Remove non-explanatory comments Atal Shargorodsky
2009-03-11 15:29   ` [PATCH 2/6] OMAP: omap_wdt: Fix interface clock existence check Atal Shargorodsky
2009-03-11 16:02   ` [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure Tony Lindgren
2009-03-10 23:30 ` [PATCH 0/7] OMAP: watchdog driver fixes Russell King - ARM Linux
2009-03-11 15:29 [PATCH 0/6] OMAP: omap_wdt: clocks and NOWAYOUT fixes Atal Shargorodsky
2009-03-11 15:29 ` [PATCH 1/6] OMAP: omap_wdt: Remove armwdt_ck field from omap_wdt_dev structure Atal Shargorodsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.