Subject: [PATCH 0/7] OMAP: watchdog driver fixes.

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.


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

Enable interface clock only when accesssing the hw.

Signed-off-by: Atal Shargorodsky <[email protected]>
---
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

Subject: [PATCH 2/6] Fix interface clock existance check.

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 <[email protected]>
---
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

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

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

Signed-off-by: Atal Shargorodsky <[email protected]>
---
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

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

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

Signed-off-by: Atal Shargorodsky <[email protected]>
---
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

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

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 <[email protected]>
---
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

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

Needed to support CONFIG_WATCHDOG_NOWAYOUT option.

Signed-off-by: Atal Shargorodsky <[email protected]>
---
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

2009-03-10 23:30:57

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 0/7] OMAP: watchdog driver fixes.

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.