[PATCH V3 00/14] watchdog: ARM mpcore Improvements

[PATCH V3 00/14] watchdog: ARM mpcore Improvements

[Viresh Kumar]

** This still doesn't make it workable **

Hi,

ARM mpcore watchdog isn't workable and so is marked broken in the first patch of
the series. Reasons are mentioned in 01/14 and important links are shared too.

Other are basically improvements which I have written more than a year back. I
am just pushing them through as they still improve the driver (or broken
driver). They still don't fix the issues pointed out by Marc Zyngier earlier.
But as these are generic improvements I don't see why they can't be applied,
even to a still broken driver.

I don't want somebody to waste time reinventing these patches, rather they can
work on fixing the issues due to which this driver is broken.

I don't have hardware & time to fix the BROKEN part of this driver now :(

Link to the last version, nothing much changed after that. Just a resend:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/089076.html

Viresh Kumar (14):
  watchdog/mpcore_wdt: Mark it as BROKEN
  watchdog/mpcore_wdt: convert to watchdog core
  watchdog/mpcore_wdt: Fix multiline comments
  watchdog/mpcore_wdt: Arrange #includes in alphabetical order
  watchdog/mpcore_wdt: Set default heartbeat in probe instead of init
  watchdog/mpcore_wdt: convert to use module_platform_driver()
  watchdog/mpcore_wdt: Add support for dev_pm_ops interface
  watchdog/mpcore_wdt: disable wdt in suspend only if it is busy
  watchdog/mpcore_wdt: replace (__raw_)readl/writel with lighter
    *_relaxed variants
  watchdog/mpcore_wdt: Add support for WDIOC_GETBOOTSTATUS IOCTL
  watchdog/mpcore_wdt: Add clock framework support
  watchdog/mpcore_wdt: use correct clk_rate to program timeout
  watchdog/mpcore_wdt: Start registers from 0x00 instead of 0x20
  watchdog/mpcore_wdt: Add DT probing support for ARM mpcore watchdog

 arch/arm/include/asm/smp_twd.h |   7 -
 drivers/watchdog/Kconfig       |   3 +-
 drivers/watchdog/mpcore_wdt.c  | 529 ++++++++++++++++++++---------------------
 3 files changed, 256 insertions(+), 283 deletions(-)

-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Viresh Kumar]

This driver was broken since ever.

- Interrupt request doesn't use the right API: The TWD watchdog uses a per-cpu
  interrupt (usually interrupt #30), and the GIC configuration should flag it as
  such. With this setup, request_irq() should fail, and the right API is
  request_percpu_irq(), together with enable_percpu_irq()/disable_percpu_irq().

- Nothing ensures the userspace ioctl() will end-up kicking the watchdog on the
  right CPU.

Was last discussed here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/095960.html

Lets mark it broken until somebody with this hardware gets up and fixes it.

Suggested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9d03af1..c7dabe9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -223,7 +223,7 @@ config DW_WATCHDOG
 
 config MPCORE_WATCHDOG
 	tristate "MPcore watchdog"
-	depends on HAVE_ARM_TWD
+	depends on HAVE_ARM_TWD && BROKEN
 	help
 	  Watchdog timer embedded into the MPcore system.
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Guenter Roeck]

On Tue, Jun 18, 2013 at 08:50:25PM +0530, Viresh Kumar wrote:
> This driver was broken since ever.
> 
> - Interrupt request doesn't use the right API: The TWD watchdog uses a per-cpu
>   interrupt (usually interrupt #30), and the GIC configuration should flag it as
>   such. With this setup, request_irq() should fail, and the right API is
>   request_percpu_irq(), together with enable_percpu_irq()/disable_percpu_irq().
> 
> - Nothing ensures the userspace ioctl() will end-up kicking the watchdog on the
>   right CPU.
> 
> Was last discussed here:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/095960.html
> 
> Lets mark it broken until somebody with this hardware gets up and fixes it.
> 
I must be missing something. What is the point of the remaining patches in this
case ?

Guenter

> Suggested-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/watchdog/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 9d03af1..c7dabe9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -223,7 +223,7 @@ config DW_WATCHDOG
>  
>  config MPCORE_WATCHDOG
>  	tristate "MPcore watchdog"
> -	depends on HAVE_ARM_TWD
> +	depends on HAVE_ARM_TWD && BROKEN
>  	help
>  	  Watchdog timer embedded into the MPcore system.
>  
> -- 
> 1.7.12.rc2.18.g61b472e
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Marc Zyngier]

On 18/06/13 16:42, Guenter Roeck wrote:
> On Tue, Jun 18, 2013 at 08:50:25PM +0530, Viresh Kumar wrote:
>> This driver was broken since ever.
>>
>> - Interrupt request doesn't use the right API: The TWD watchdog uses a per-cpu
>>   interrupt (usually interrupt #30), and the GIC configuration should flag it as
>>   such. With this setup, request_irq() should fail, and the right API is
>>   request_percpu_irq(), together with enable_percpu_irq()/disable_percpu_irq().
>>
>> - Nothing ensures the userspace ioctl() will end-up kicking the watchdog on the
>>   right CPU.
>>
>> Was last discussed here:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/095960.html
>>
>> Lets mark it broken until somebody with this hardware gets up and fixes it.
>>
> I must be missing something. What is the point of the remaining patches in this
> case ?

Indeed. This looks like pointless churn to me, unless someone actually
picks up the driver and fixes it for good.

If nobody cares enough about it, then maybe it should be moved into
staging and eventually retired...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Olof Johansson]

On Tue, Jun 18, 2013 at 9:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 18/06/13 16:42, Guenter Roeck wrote:
>> On Tue, Jun 18, 2013 at 08:50:25PM +0530, Viresh Kumar wrote:
>>> This driver was broken since ever.
>>>
>>> - Interrupt request doesn't use the right API: The TWD watchdog uses a per-cpu
>>>   interrupt (usually interrupt #30), and the GIC configuration should flag it as
>>>   such. With this setup, request_irq() should fail, and the right API is
>>>   request_percpu_irq(), together with enable_percpu_irq()/disable_percpu_irq().
>>>
>>> - Nothing ensures the userspace ioctl() will end-up kicking the watchdog on the
>>>   right CPU.
>>>
>>> Was last discussed here:
>>>
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/095960.html
>>>
>>> Lets mark it broken until somebody with this hardware gets up and fixes it.
>>>
>> I must be missing something. What is the point of the remaining patches in this
>> case ?
>
> Indeed. This looks like pointless churn to me, unless someone actually
> picks up the driver and fixes it for good.
>
> If nobody cares enough about it, then maybe it should be moved into
> staging and eventually retired...


That was a year ago, and nobody has done anything to the driver. Just
remove it -- if someone wants to do the work later on it's easy to
revert the commit and start over.

Keeping code in the kernel but marking it BROKEN is only useful if we
think someone will fix it soon. It seems very unlikely in this case.


-Olof

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Viresh Kumar]

Wow!! So many replies, let me reply to everyone in this chain.

On 18 June 2013 22:05, Olof Johansson <olof@lixom.net> wrote:
> On Tue, Jun 18, 2013 at 9:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 18/06/13 16:42, Guenter Roeck wrote:
>>> On Tue, Jun 18, 2013 at 08:50:25PM +0530, Viresh Kumar wrote:

>>>> Lets mark it broken until somebody with this hardware gets up and fixes it.
>>>>
>>> I must be missing something. What is the point of the remaining patches in this
>>> case ?

In case somebody wakes up and tries to fix this driver, he doesn't have to
write stuff which I already wrote. That's it. This stuff was pending in my tree
for more than a year now and I wanted to get rid of it (without deleting it) :)

>> Indeed. This looks like pointless churn to me, unless someone actually
>> picks up the driver and fixes it for good.
>>
>> If nobody cares enough about it, then maybe it should be moved into
>> staging and eventually retired...
>
>
> That was a year ago, and nobody has done anything to the driver. Just
> remove it -- if someone wants to do the work later on it's easy to
> revert the commit and start over.
>
> Keeping code in the kernel but marking it BROKEN is only useful if we
> think someone will fix it soon. It seems very unlikely in this case.

I believed that this is the driver which will be used by all cortex family, i.e.
all ARM SMP platforms, isn't it? I am sure atleast the A9 family had this.

If no, then which ones are the real users of this driver/hardware?
If yes, Why isn't anybody using this?

I will send a patch that will delete this driver and will provide link to my
patches, in case somebody wants it back in future.

--
Viresh

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Marc Zyngier]

On Wed, 19 Jun 2013 08:40:25 +0530, Viresh Kumar <viresh.kumar@linaro.org>
wrote:
> Wow!! So many replies, let me reply to everyone in this chain.
> 
> On 18 June 2013 22:05, Olof Johansson <olof@lixom.net> wrote:
>> On Tue, Jun 18, 2013 at 9:11 AM, Marc Zyngier <marc.zyngier@arm.com>
>> wrote:
>>> On 18/06/13 16:42, Guenter Roeck wrote:
>>>> On Tue, Jun 18, 2013 at 08:50:25PM +0530, Viresh Kumar wrote:
> 
>>>>> Lets mark it broken until somebody with this hardware gets up and
>>>>> fixes it.
>>>>>
>>>> I must be missing something. What is the point of the remaining
>>>> patches in this
>>>> case ?
> 
> In case somebody wakes up and tries to fix this driver, he doesn't have
to
> write stuff which I already wrote. That's it. This stuff was pending in
my
> tree
> for more than a year now and I wanted to get rid of it (without deleting
> it) :)
> 
>>> Indeed. This looks like pointless churn to me, unless someone actually
>>> picks up the driver and fixes it for good.
>>>
>>> If nobody cares enough about it, then maybe it should be moved into
>>> staging and eventually retired...
>>
>>
>> That was a year ago, and nobody has done anything to the driver. Just
>> remove it -- if someone wants to do the work later on it's easy to
>> revert the commit and start over.
>>
>> Keeping code in the kernel but marking it BROKEN is only useful if we
>> think someone will fix it soon. It seems very unlikely in this case.
> 
> I believed that this is the driver which will be used by all cortex
> family, i.e.
> all ARM SMP platforms, isn't it? I am sure atleast the A9 family had
this.

ARM11, A5 and A9 in their MP configurations only.

> If no, then which ones are the real users of this driver/hardware?
> If yes, Why isn't anybody using this?

Because, as Russell mentioned, the piece of IP doesn't fit our watchdog
model at all (per-CPU watchdog???), and most SoCs/boards have a separate
watchdog anyway.

> I will send a patch that will delete this driver and will provide link
to
> my
> patches, in case somebody wants it back in future.

Thanks.

        M.
-- 
Fast, cheap, reliable. Pick two.

[PATCH V3 01/14] watchdog/mpcore_wdt: Mark it as BROKEN

[Viresh Kumar]

On 19 June 2013 13:26, Marc Zyngier <marc.zyngier@arm.com> wrote:
> ARM11, A5 and A9 in their MP configurations only.
>
>> If no, then which ones are the real users of this driver/hardware?
>> If yes, Why isn't anybody using this?
>
> Because, as Russell mentioned, the piece of IP doesn't fit our watchdog
> model at all (per-CPU watchdog???), and most SoCs/boards have a separate
> watchdog anyway.

I believe we should use mpcore watchdogs. Atleast on the platforms I have
worked (SPEAr), they don't have an external watchdog.

But yes, this would require somebody, with a A5/9/ARM11 board, no
separate watchdog and a customer requirement, to work on it :)

I will get rid of it.

Thanks.

[PATCH V3 02/14] watchdog/mpcore_wdt: convert to watchdog core

[Viresh Kumar]

This patch converts existing mpcore watchdog driver to use already in place
common infrastructure present in watchdog core. With this lot of code goes away.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/Kconfig      |   1 +
 drivers/watchdog/mpcore_wdt.c | 275 ++++++++++--------------------------------
 2 files changed, 68 insertions(+), 208 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c7dabe9..81bc1d4 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -224,6 +224,7 @@ config DW_WATCHDOG
 config MPCORE_WATCHDOG
 	tristate "MPcore watchdog"
 	depends on HAVE_ARM_TWD && BROKEN
+	select WATCHDOG_CORE
 	help
 	  Watchdog timer embedded into the MPcore system.
 
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 4f5ac40..bdbe3d8 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -25,31 +25,27 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/types.h>
-#include <linux/miscdevice.h>
 #include <linux/watchdog.h>
-#include <linux/fs.h>
 #include <linux/reboot.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
-#include <linux/uaccess.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 
 #include <asm/smp_twd.h>
 
 struct mpcore_wdt {
-	unsigned long	timer_alive;
+	struct watchdog_device wdd;
 	struct device	*dev;
 	void __iomem	*base;
+	spinlock_t	lock;
 	int		irq;
 	unsigned int	perturb;
-	char		expect_close;
 };
 
-static struct platform_device *mpcore_wdt_pdev;
-static DEFINE_SPINLOCK(wdt_lock);
-
+#define MIN_TIME	0x0001
+#define MAX_TIME	0xFFFF
 #define TIMER_MARGIN	60
 static int mpcore_margin = TIMER_MARGIN;
 module_param(mpcore_margin, int, 0);
@@ -89,17 +85,18 @@ static irqreturn_t mpcore_wdt_fire(int irq, void *arg)
 }
 
 /*
- *	mpcore_wdt_keepalive - reload the timer
+ *	mpcore_wdt_ping - reload the timer
  *
  *	Note that the spec says a DIFFERENT value must be written to the reload
  *	register each time.  The "perturb" variable deals with this by adding 1
  *	to the count every other time the function is called.
  */
-static void mpcore_wdt_keepalive(struct mpcore_wdt *wdt)
+static int mpcore_wdt_ping(struct watchdog_device *wdd)
 {
+	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
 	unsigned long count;
 
-	spin_lock(&wdt_lock);
+	spin_lock(&wdt->lock);
 	/* Assume prescale is set to 256 */
 	count =  __raw_readl(wdt->base + TWD_WDOG_COUNTER);
 	count = (0xFFFFFFFFU - count) * (HZ / 5);
@@ -108,24 +105,32 @@ static void mpcore_wdt_keepalive(struct mpcore_wdt *wdt)
 	/* Reload the counter */
 	writel(count + wdt->perturb, wdt->base + TWD_WDOG_LOAD);
 	wdt->perturb = wdt->perturb ? 0 : 1;
-	spin_unlock(&wdt_lock);
+	spin_unlock(&wdt->lock);
+
+	return 0;
 }
 
-static void mpcore_wdt_stop(struct mpcore_wdt *wdt)
+static int mpcore_wdt_stop(struct watchdog_device *wdd)
 {
-	spin_lock(&wdt_lock);
+	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	spin_lock(&wdt->lock);
 	writel(0x12345678, wdt->base + TWD_WDOG_DISABLE);
 	writel(0x87654321, wdt->base + TWD_WDOG_DISABLE);
 	writel(0x0, wdt->base + TWD_WDOG_CONTROL);
-	spin_unlock(&wdt_lock);
+	spin_unlock(&wdt->lock);
+
+	return 0;
 }
 
-static void mpcore_wdt_start(struct mpcore_wdt *wdt)
+static int mpcore_wdt_start(struct watchdog_device *wdd)
 {
+	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+
 	dev_info(wdt->dev, "enabling watchdog\n");
 
 	/* This loads the count register but does NOT start the count yet */
-	mpcore_wdt_keepalive(wdt);
+	mpcore_wdt_ping(wdd);
 
 	if (mpcore_noboot) {
 		/* Enable watchdog - prescale=256, watchdog mode=0, enable=1 */
@@ -134,167 +139,30 @@ static void mpcore_wdt_start(struct mpcore_wdt *wdt)
 		/* Enable watchdog - prescale=256, watchdog mode=1, enable=1 */
 		writel(0x0000FF09, wdt->base + TWD_WDOG_CONTROL);
 	}
-}
-
-static int mpcore_wdt_set_heartbeat(int t)
-{
-	if (t < 0x0001 || t > 0xFFFF)
-		return -EINVAL;
 
-	mpcore_margin = t;
 	return 0;
 }
 
-/*
- *	/dev/watchdog handling
- */
-static int mpcore_wdt_open(struct inode *inode, struct file *file)
-{
-	struct mpcore_wdt *wdt = platform_get_drvdata(mpcore_wdt_pdev);
-
-	if (test_and_set_bit(0, &wdt->timer_alive))
-		return -EBUSY;
-
-	if (nowayout)
-		__module_get(THIS_MODULE);
-
-	file->private_data = wdt;
-
-	/*
-	 *	Activate timer
-	 */
-	mpcore_wdt_start(wdt);
-
-	return nonseekable_open(inode, file);
-}
-
-static int mpcore_wdt_release(struct inode *inode, struct file *file)
+static int mpcore_wdt_set_heartbeat(struct watchdog_device *wdd, unsigned int t)
 {
-	struct mpcore_wdt *wdt = file->private_data;
-
-	/*
-	 *	Shut off the timer.
-	 *	Lock it in if it's a module and we set nowayout
-	 */
-	if (wdt->expect_close == 42)
-		mpcore_wdt_stop(wdt);
-	else {
-		dev_crit(wdt->dev,
-			 "unexpected close, not stopping watchdog!\n");
-		mpcore_wdt_keepalive(wdt);
-	}
-	clear_bit(0, &wdt->timer_alive);
-	wdt->expect_close = 0;
+	mpcore_margin = t;
 	return 0;
 }
 
-static ssize_t mpcore_wdt_write(struct file *file, const char *data,
-						size_t len, loff_t *ppos)
-{
-	struct mpcore_wdt *wdt = file->private_data;
-
-	/*
-	 *	Refresh the timer.
-	 */
-	if (len) {
-		if (!nowayout) {
-			size_t i;
-
-			/* In case it was set long ago */
-			wdt->expect_close = 0;
-
-			for (i = 0; i != len; i++) {
-				char c;
-
-				if (get_user(c, data + i))
-					return -EFAULT;
-				if (c == 'V')
-					wdt->expect_close = 42;
-			}
-		}
-		mpcore_wdt_keepalive(wdt);
-	}
-	return len;
-}
-
-static const struct watchdog_info ident = {
+static const struct watchdog_info mpcore_wdt_info = {
 	.options		= WDIOF_SETTIMEOUT |
 				  WDIOF_KEEPALIVEPING |
 				  WDIOF_MAGICCLOSE,
 	.identity		= "MPcore Watchdog",
 };
 
-static long mpcore_wdt_ioctl(struct file *file, unsigned int cmd,
-							unsigned long arg)
-{
-	struct mpcore_wdt *wdt = file->private_data;
-	int ret;
-	union {
-		struct watchdog_info ident;
-		int i;
-	} uarg;
-
-	if (_IOC_DIR(cmd) && _IOC_SIZE(cmd) > sizeof(uarg))
-		return -ENOTTY;
-
-	if (_IOC_DIR(cmd) & _IOC_WRITE) {
-		ret = copy_from_user(&uarg, (void __user *)arg, _IOC_SIZE(cmd));
-		if (ret)
-			return -EFAULT;
-	}
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		uarg.ident = ident;
-		ret = 0;
-		break;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		uarg.i = 0;
-		ret = 0;
-		break;
-
-	case WDIOC_SETOPTIONS:
-		ret = -EINVAL;
-		if (uarg.i & WDIOS_DISABLECARD) {
-			mpcore_wdt_stop(wdt);
-			ret = 0;
-		}
-		if (uarg.i & WDIOS_ENABLECARD) {
-			mpcore_wdt_start(wdt);
-			ret = 0;
-		}
-		break;
-
-	case WDIOC_KEEPALIVE:
-		mpcore_wdt_keepalive(wdt);
-		ret = 0;
-		break;
-
-	case WDIOC_SETTIMEOUT:
-		ret = mpcore_wdt_set_heartbeat(uarg.i);
-		if (ret)
-			break;
-
-		mpcore_wdt_keepalive(wdt);
-		/* Fall */
-	case WDIOC_GETTIMEOUT:
-		uarg.i = mpcore_margin;
-		ret = 0;
-		break;
-
-	default:
-		return -ENOTTY;
-	}
-
-	if (ret == 0 && _IOC_DIR(cmd) & _IOC_READ) {
-		ret = copy_to_user((void __user *)arg, &uarg, _IOC_SIZE(cmd));
-		if (ret)
-			ret = -EFAULT;
-	}
-	return ret;
-}
+static const struct watchdog_ops mpcore_wdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= mpcore_wdt_start,
+	.stop		= mpcore_wdt_stop,
+	.ping		= mpcore_wdt_ping,
+	.set_timeout	= mpcore_wdt_set_heartbeat,
+};
 
 /*
  *	System shutdown handler.  Turn off the watchdog if we're
@@ -305,37 +173,15 @@ static void mpcore_wdt_shutdown(struct platform_device *pdev)
 	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
 
 	if (system_state == SYSTEM_RESTART || system_state == SYSTEM_HALT)
-		mpcore_wdt_stop(wdt);
+		mpcore_wdt_stop(&wdt->wdd);
 }
 
-/*
- *	Kernel Interfaces
- */
-static const struct file_operations mpcore_wdt_fops = {
-	.owner		= THIS_MODULE,
-	.llseek		= no_llseek,
-	.write		= mpcore_wdt_write,
-	.unlocked_ioctl	= mpcore_wdt_ioctl,
-	.open		= mpcore_wdt_open,
-	.release	= mpcore_wdt_release,
-};
-
-static struct miscdevice mpcore_wdt_miscdev = {
-	.minor		= WATCHDOG_MINOR,
-	.name		= "watchdog",
-	.fops		= &mpcore_wdt_fops,
-};
-
 static int mpcore_wdt_probe(struct platform_device *pdev)
 {
 	struct mpcore_wdt *wdt;
 	struct resource *res;
 	int ret;
 
-	/* We only accept one device, and it must have an id of -1 */
-	if (pdev->id != -1)
-		return -ENODEV;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -ENODEV;
@@ -361,28 +207,39 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 	if (!wdt->base)
 		return -ENOMEM;
 
-	mpcore_wdt_miscdev.parent = &pdev->dev;
-	ret = misc_register(&mpcore_wdt_miscdev);
-	if (ret) {
-		dev_err(wdt->dev,
-			"cannot register miscdev on minor=%d (err=%d)\n",
-			WATCHDOG_MINOR, ret);
-		return ret;
-	}
+	wdt->wdd.info = &mpcore_wdt_info;
+	wdt->wdd.ops = &mpcore_wdt_ops;
+	wdt->wdd.min_timeout = MIN_TIME;
+	wdt->wdd.max_timeout = MAX_TIME;
+	spin_lock_init(&wdt->lock);
 
-	mpcore_wdt_stop(wdt);
+	watchdog_set_nowayout(&wdt->wdd, nowayout);
 	platform_set_drvdata(pdev, wdt);
-	mpcore_wdt_pdev = pdev;
+	watchdog_set_drvdata(&wdt->wdd, wdt);
+
+	mpcore_wdt_stop(&wdt->wdd);
+
+	ret = watchdog_register_device(&wdt->wdd);
+	if (ret) {
+		dev_err(wdt->dev, "watchdog_register_device() failed: %d\n",
+				ret);
+		goto err_register;
+	}
 
 	return 0;
+
+err_register:
+	watchdog_set_drvdata(&wdt->wdd, NULL);
+
+	return ret;
 }
 
 static int mpcore_wdt_remove(struct platform_device *pdev)
 {
-	misc_deregister(&mpcore_wdt_miscdev);
-
-	mpcore_wdt_pdev = NULL;
+	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
 
+	watchdog_unregister_device(&wdt->wdd);
+	watchdog_set_drvdata(&wdt->wdd, NULL);
 	return 0;
 }
 
@@ -390,16 +247,18 @@ static int mpcore_wdt_remove(struct platform_device *pdev)
 static int mpcore_wdt_suspend(struct platform_device *pdev, pm_message_t msg)
 {
 	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
-	mpcore_wdt_stop(wdt);		/* Turn the WDT off */
+	mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
+
 	return 0;
 }
 
 static int mpcore_wdt_resume(struct platform_device *pdev)
 {
 	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
-	/* re-activate timer */
-	if (test_bit(0, &wdt->timer_alive))
-		mpcore_wdt_start(wdt);
+
+	if (watchdog_active(&wdt->wdd))
+		mpcore_wdt_start(&wdt->wdd);
+
 	return 0;
 }
 #else
@@ -425,15 +284,16 @@ static struct platform_driver mpcore_wdt_driver = {
 static int __init mpcore_wdt_init(void)
 {
 	/*
-	 * Check that the margin value is within it's range;
+	 * Check that the mpcore_margin value is within it's range;
 	 * if not reset to the default
 	 */
-	if (mpcore_wdt_set_heartbeat(mpcore_margin)) {
-		mpcore_wdt_set_heartbeat(TIMER_MARGIN);
+	if (mpcore_margin < MIN_TIME || mpcore_margin > MAX_TIME) {
+		mpcore_margin = TIMER_MARGIN;
 		pr_info("mpcore_margin value must be 0 < mpcore_margin < 65536, using %d\n",
 			TIMER_MARGIN);
 	}
 
+	mpcore_wdt_set_heartbeat(NULL, mpcore_margin);
 	pr_info("MPcore Watchdog Timer: 0.1. mpcore_noboot=%d mpcore_margin=%d sec (nowayout= %d)\n",
 		mpcore_noboot, mpcore_margin, nowayout);
 
@@ -451,4 +311,3 @@ module_exit(mpcore_wdt_exit);
 MODULE_AUTHOR("ARM Limited");
 MODULE_DESCRIPTION("MPcore Watchdog Device Driver");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 03/14] watchdog/mpcore_wdt: Fix multiline comments

[Viresh Kumar]

In case of few multiline comment a 'tab' is present instead of 'space' after the
'*' in the second column. Replace tab with space here.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index bdbe3d8..6e7afe0 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -1,23 +1,21 @@
 /*
- *	Watchdog driver for the mpcore watchdog timer
+ * Watchdog driver for the mpcore watchdog timer
  *
- *	(c) Copyright 2004 ARM Limited
+ * (c) Copyright 2004 ARM Limited
  *
- *	Based on the SoftDog driver:
- *	(c) Copyright 1996 Alan Cox <alan@lxorguk.ukuu.org.uk>,
- *						All Rights Reserved.
+ * Based on the SoftDog driver:
+ * (c) Copyright 1996 Alan Cox <alan@lxorguk.ukuu.org.uk>, All Rights Reserved.
  *
- *	This program is free software; you can redistribute it and/or
- *	modify it under the terms of the GNU General Public License
- *	as published by the Free Software Foundation; either version
- *	2 of the License, or (at your option) any later version.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
  *
- *	Neither Alan Cox nor CymruNet Ltd. admit liability nor provide
- *	warranty for any of this software. This material is provided
- *	"AS-IS" and at no charge.
- *
- *	(c) Copyright 1995    Alan Cox <alan@lxorguk.ukuu.org.uk>
+ * Neither Alan Cox nor CymruNet Ltd. admit liability nor provide
+ * warranty for any of this software. This material is provided
+ * "AS-IS" and at no charge.
  *
+ * (c) Copyright 1995    Alan Cox <alan@lxorguk.ukuu.org.uk>
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -67,8 +65,8 @@ MODULE_PARM_DESC(mpcore_noboot, "MPcore watchdog action, "
 					__MODULE_STRING(ONLY_TESTING) ")");
 
 /*
- *	This is the interrupt handler.  Note that we only use this
- *	in testing mode, so don't actually do a reboot here.
+ * This is the interrupt handler.  Note that we only use this
+ * in testing mode, so don't actually do a reboot here.
  */
 static irqreturn_t mpcore_wdt_fire(int irq, void *arg)
 {
@@ -85,11 +83,11 @@ static irqreturn_t mpcore_wdt_fire(int irq, void *arg)
 }
 
 /*
- *	mpcore_wdt_ping - reload the timer
+ * mpcore_wdt_ping - reload the timer
  *
- *	Note that the spec says a DIFFERENT value must be written to the reload
- *	register each time.  The "perturb" variable deals with this by adding 1
- *	to the count every other time the function is called.
+ * Note that the spec says a DIFFERENT value must be written to the reload
+ * register each time.  The "perturb" variable deals with this by adding 1 to
+ * the count every other time the function is called.
  */
 static int mpcore_wdt_ping(struct watchdog_device *wdd)
 {
@@ -165,8 +163,8 @@ static const struct watchdog_ops mpcore_wdt_ops = {
 };
 
 /*
- *	System shutdown handler.  Turn off the watchdog if we're
- *	restarting or halting the system.
+ * System shutdown handler.  Turn off the watchdog if we're restarting or
+ * halting the system.
  */
 static void mpcore_wdt_shutdown(struct platform_device *pdev)
 {
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 04/14] watchdog/mpcore_wdt: Arrange #includes in alphabetical order

[Viresh Kumar]

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 6e7afe0..ca9afa7 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -20,16 +20,16 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/types.h>
-#include <linux/watchdog.h>
-#include <linux/reboot.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
-#include <linux/io.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
 
 #include <asm/smp_twd.h>
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 05/14] watchdog/mpcore_wdt: Set default heartbeat in probe instead of init

[Viresh Kumar]

Currently mpcore_margin is handled in mpcore_wdt_init routine, which will be
called even if we haven't added any device for watchdog.

This patch moves this code into probe routine, so that it only gets hit once we
add device for this driver.

This also uses dev_info() style print statements instead of printk()

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index ca9afa7..b699df4 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -224,6 +224,21 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 		goto err_register;
 	}
 
+	/*
+	 * Check that the mpcore_margin value is within it's range; if not reset
+	 * to the default
+	 */
+	if (mpcore_margin < MIN_TIME || mpcore_margin > MAX_TIME) {
+		mpcore_margin = TIMER_MARGIN;
+		dev_info(wdt->dev, "mpcore_margin value must be 0 < mpcore_margin < 65536, using %d\n",
+			TIMER_MARGIN);
+	}
+
+	mpcore_wdt_set_heartbeat(NULL, mpcore_margin);
+	dev_info(wdt->dev, "MPcore Watchdog Timer: 0.1. mpcore_noboot=%d "
+			"mpcore_margin=%d sec (nowayout= %d)\n", mpcore_noboot,
+			mpcore_margin, nowayout);
+
 	return 0;
 
 err_register:
@@ -281,20 +296,6 @@ static struct platform_driver mpcore_wdt_driver = {
 
 static int __init mpcore_wdt_init(void)
 {
-	/*
-	 * Check that the mpcore_margin value is within it's range;
-	 * if not reset to the default
-	 */
-	if (mpcore_margin < MIN_TIME || mpcore_margin > MAX_TIME) {
-		mpcore_margin = TIMER_MARGIN;
-		pr_info("mpcore_margin value must be 0 < mpcore_margin < 65536, using %d\n",
-			TIMER_MARGIN);
-	}
-
-	mpcore_wdt_set_heartbeat(NULL, mpcore_margin);
-	pr_info("MPcore Watchdog Timer: 0.1. mpcore_noboot=%d mpcore_margin=%d sec (nowayout= %d)\n",
-		mpcore_noboot, mpcore_margin, nowayout);
-
 	return platform_driver_register(&mpcore_wdt_driver);
 }
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 06/14] watchdog/mpcore_wdt: convert to use module_platform_driver()

[Viresh Kumar]

This patch converts the mpcore_wdt driver to use the module_platform_driver()
macro which makes the code smaller and a bit simpler.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index b699df4..5955757 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -294,18 +294,7 @@ static struct platform_driver mpcore_wdt_driver = {
 	},
 };
 
-static int __init mpcore_wdt_init(void)
-{
-	return platform_driver_register(&mpcore_wdt_driver);
-}
-
-static void __exit mpcore_wdt_exit(void)
-{
-	platform_driver_unregister(&mpcore_wdt_driver);
-}
-
-module_init(mpcore_wdt_init);
-module_exit(mpcore_wdt_exit);
+module_platform_driver(mpcore_wdt_driver);
 
 MODULE_AUTHOR("ARM Limited");
 MODULE_DESCRIPTION("MPcore Watchdog Device Driver");
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 07/14] watchdog/mpcore_wdt: Add support for dev_pm_ops interface

[Viresh Kumar]

This patch adds dev_pm_ops support for standby/slepp/hibernate.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 5955757..b4d06f1 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -257,40 +258,39 @@ static int mpcore_wdt_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int mpcore_wdt_suspend(struct platform_device *pdev, pm_message_t msg)
+static int mpcore_wdt_suspend(struct device *dev)
 {
-	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
+	struct mpcore_wdt *wdt = dev_get_drvdata(dev);
 	mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
 
 	return 0;
 }
 
-static int mpcore_wdt_resume(struct platform_device *pdev)
+static int mpcore_wdt_resume(struct device *dev)
 {
-	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
+	struct mpcore_wdt *wdt = dev_get_drvdata(dev);
 
 	if (watchdog_active(&wdt->wdd))
 		mpcore_wdt_start(&wdt->wdd);
 
 	return 0;
 }
-#else
-#define mpcore_wdt_suspend	NULL
-#define mpcore_wdt_resume	NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
+		mpcore_wdt_resume);
+
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= mpcore_wdt_remove,
-	.suspend	= mpcore_wdt_suspend,
-	.resume		= mpcore_wdt_resume,
 	.shutdown	= mpcore_wdt_shutdown,
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
+		.pm	= &mpcore_wdt_dev_pm_ops,
 	},
 };
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 08/14] watchdog/mpcore_wdt: disable wdt in suspend only if it is busy

[Viresh Kumar]

We don't need to call mpcore_wdt_stop() in suspend, if wdt is not BUSY. So, call
mpcore_wdt_stop() conditionally in suspend.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index b4d06f1..75cc518 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -261,7 +261,9 @@ static int mpcore_wdt_remove(struct platform_device *pdev)
 static int mpcore_wdt_suspend(struct device *dev)
 {
 	struct mpcore_wdt *wdt = dev_get_drvdata(dev);
-	mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
+
+	if (watchdog_active(&wdt->wdd))
+		mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
 
 	return 0;
 }
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 09/14] watchdog/mpcore_wdt: replace (__raw_)readl/writel with lighter *_relaxed variants

[Viresh Kumar]

readl/writel versions for ARMV6+ contain memory barrier instruction for
synchronizing DMA buffers. These are not required at least on this module.  So
use lighter _relaxed variants.

Also, __raw_* variants don't have take care about endianess, so replace them too
with _relaxed variants.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 75cc518..d8917ad 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -74,10 +74,10 @@ static irqreturn_t mpcore_wdt_fire(int irq, void *arg)
 	struct mpcore_wdt *wdt = arg;
 
 	/* Check it really was our interrupt */
-	if (readl(wdt->base + TWD_WDOG_INTSTAT)) {
+	if (readl_relaxed(wdt->base + TWD_WDOG_INTSTAT)) {
 		dev_crit(wdt->dev, "Triggered - Reboot ignored\n");
 		/* Clear the interrupt on the watchdog */
-		writel(1, wdt->base + TWD_WDOG_INTSTAT);
+		writel_relaxed(1, wdt->base + TWD_WDOG_INTSTAT);
 		return IRQ_HANDLED;
 	}
 	return IRQ_NONE;
@@ -97,12 +97,12 @@ static int mpcore_wdt_ping(struct watchdog_device *wdd)
 
 	spin_lock(&wdt->lock);
 	/* Assume prescale is set to 256 */
-	count =  __raw_readl(wdt->base + TWD_WDOG_COUNTER);
+	count = readl_relaxed(wdt->base + TWD_WDOG_COUNTER);
 	count = (0xFFFFFFFFU - count) * (HZ / 5);
 	count = (count / 256) * mpcore_margin;
 
 	/* Reload the counter */
-	writel(count + wdt->perturb, wdt->base + TWD_WDOG_LOAD);
+	writel_relaxed(count + wdt->perturb, wdt->base + TWD_WDOG_LOAD);
 	wdt->perturb = wdt->perturb ? 0 : 1;
 	spin_unlock(&wdt->lock);
 
@@ -114,9 +114,9 @@ static int mpcore_wdt_stop(struct watchdog_device *wdd)
 	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
 
 	spin_lock(&wdt->lock);
-	writel(0x12345678, wdt->base + TWD_WDOG_DISABLE);
-	writel(0x87654321, wdt->base + TWD_WDOG_DISABLE);
-	writel(0x0, wdt->base + TWD_WDOG_CONTROL);
+	writel_relaxed(0x12345678, wdt->base + TWD_WDOG_DISABLE);
+	writel_relaxed(0x87654321, wdt->base + TWD_WDOG_DISABLE);
+	writel_relaxed(0x0, wdt->base + TWD_WDOG_CONTROL);
 	spin_unlock(&wdt->lock);
 
 	return 0;
@@ -133,10 +133,10 @@ static int mpcore_wdt_start(struct watchdog_device *wdd)
 
 	if (mpcore_noboot) {
 		/* Enable watchdog - prescale=256, watchdog mode=0, enable=1 */
-		writel(0x0000FF01, wdt->base + TWD_WDOG_CONTROL);
+		writel_relaxed(0x0000FF01, wdt->base + TWD_WDOG_CONTROL);
 	} else {
 		/* Enable watchdog - prescale=256, watchdog mode=1, enable=1 */
-		writel(0x0000FF09, wdt->base + TWD_WDOG_CONTROL);
+		writel_relaxed(0x0000FF09, wdt->base + TWD_WDOG_CONTROL);
 	}
 
 	return 0;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 10/14] watchdog/mpcore_wdt: Add support for WDIOC_GETBOOTSTATUS IOCTL

[Viresh Kumar]

mpcore watchdog can give last boot status, whether we got a watchdog reset or
not, via bit 0 of TWD_WDOG_RESETSTAT register. This patch adds support to return
correct status if WDIOC_GETBOOTSTATUS cmd is called with ioctl.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/include/asm/smp_twd.h | 2 ++
 drivers/watchdog/mpcore_wdt.c  | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index 7b2899c..a62bcfa 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -18,6 +18,8 @@
 #define TWD_TIMER_CONTROL_PERIODIC	(1 << 1)
 #define TWD_TIMER_CONTROL_IT_ENABLE	(1 << 2)
 
+#define TWD_WDOG_RESETSTAT_MASK		0x1
+
 #include <linux/ioport.h>
 
 struct twd_local_timer {
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index d8917ad..43f749d 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -151,6 +151,7 @@ static int mpcore_wdt_set_heartbeat(struct watchdog_device *wdd, unsigned int t)
 static const struct watchdog_info mpcore_wdt_info = {
 	.options		= WDIOF_SETTIMEOUT |
 				  WDIOF_KEEPALIVEPING |
+				  WDIOF_CARDRESET |
 				  WDIOF_MAGICCLOSE,
 	.identity		= "MPcore Watchdog",
 };
@@ -216,6 +217,9 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdt);
 	watchdog_set_drvdata(&wdt->wdd, wdt);
 
+	wdt->wdd.bootstatus = (readl_relaxed(wdt->base + TWD_WDOG_RESETSTAT) &
+			TWD_WDOG_RESETSTAT_MASK) ? WDIOF_CARDRESET : 0;
+
 	mpcore_wdt_stop(&wdt->wdd);
 
 	ret = watchdog_register_device(&wdt->wdd);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 11/14] watchdog/mpcore_wdt: Add clock framework support

[Viresh Kumar]

This patch adds in clk framework support for wdt driver. If clk_get fails for
some platform, then we continue without clk_* API's.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 47 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 43f749d..1a1a1fd 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -20,6 +20,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/clk.h>
+#include <linux/err.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -39,6 +41,7 @@ struct mpcore_wdt {
 	struct device	*dev;
 	void __iomem	*base;
 	spinlock_t	lock;
+	struct clk	*clk;
 	int		irq;
 	unsigned int	perturb;
 };
@@ -109,15 +112,23 @@ static int mpcore_wdt_ping(struct watchdog_device *wdd)
 	return 0;
 }
 
-static int mpcore_wdt_stop(struct watchdog_device *wdd)
+static void __mpcore_wdt_stop(struct mpcore_wdt *wdt)
 {
-	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
-
 	spin_lock(&wdt->lock);
 	writel_relaxed(0x12345678, wdt->base + TWD_WDOG_DISABLE);
 	writel_relaxed(0x87654321, wdt->base + TWD_WDOG_DISABLE);
 	writel_relaxed(0x0, wdt->base + TWD_WDOG_CONTROL);
 	spin_unlock(&wdt->lock);
+}
+
+static int mpcore_wdt_stop(struct watchdog_device *wdd)
+{
+	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	__mpcore_wdt_stop(wdt);
+
+	if (!IS_ERR(wdt->clk))
+		clk_disable_unprepare(wdt->clk);
 
 	return 0;
 }
@@ -125,9 +136,18 @@ static int mpcore_wdt_stop(struct watchdog_device *wdd)
 static int mpcore_wdt_start(struct watchdog_device *wdd)
 {
 	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+	int ret;
 
 	dev_info(wdt->dev, "enabling watchdog\n");
 
+	if (!IS_ERR(wdt->clk)) {
+		ret = clk_prepare_enable(wdt->clk);
+		if (ret) {
+			dev_err(wdt->dev, "clock prepare-enabled failed");
+			return ret;
+		}
+	}
+
 	/* This loads the count register but does NOT start the count yet */
 	mpcore_wdt_ping(wdd);
 
@@ -173,7 +193,7 @@ static void mpcore_wdt_shutdown(struct platform_device *pdev)
 	struct mpcore_wdt *wdt = platform_get_drvdata(pdev);
 
 	if (system_state == SYSTEM_RESTART || system_state == SYSTEM_HALT)
-		mpcore_wdt_stop(&wdt->wdd);
+		__mpcore_wdt_stop(wdt);
 }
 
 static int mpcore_wdt_probe(struct platform_device *pdev)
@@ -217,6 +237,17 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdt);
 	watchdog_set_drvdata(&wdt->wdd, wdt);
 
+	wdt->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(wdt->clk)) {
+		dev_warn(&pdev->dev, "Clock not found\n");
+	} else {
+		ret = clk_prepare_enable(wdt->clk);
+		if (ret) {
+			dev_err(wdt->dev, "clock prepare-enable failed");
+			goto err_set_drvdata;
+		}
+	}
+
 	wdt->wdd.bootstatus = (readl_relaxed(wdt->base + TWD_WDOG_RESETSTAT) &
 			TWD_WDOG_RESETSTAT_MASK) ? WDIOF_CARDRESET : 0;
 
@@ -226,7 +257,7 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(wdt->dev, "watchdog_register_device() failed: %d\n",
 				ret);
-		goto err_register;
+		goto err_set_drvdata;
 	}
 
 	/*
@@ -246,7 +277,7 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_register:
+err_set_drvdata:
 	watchdog_set_drvdata(&wdt->wdd, NULL);
 
 	return ret;
@@ -267,7 +298,7 @@ static int mpcore_wdt_suspend(struct device *dev)
 	struct mpcore_wdt *wdt = dev_get_drvdata(dev);
 
 	if (watchdog_active(&wdt->wdd))
-		mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
+		return mpcore_wdt_stop(&wdt->wdd);	/* Turn the WDT off */
 
 	return 0;
 }
@@ -277,7 +308,7 @@ static int mpcore_wdt_resume(struct device *dev)
 	struct mpcore_wdt *wdt = dev_get_drvdata(dev);
 
 	if (watchdog_active(&wdt->wdd))
-		mpcore_wdt_start(&wdt->wdd);
+		return mpcore_wdt_start(&wdt->wdd);
 
 	return 0;
 }
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 12/14] watchdog/mpcore_wdt: use correct clk_rate to program timeout

[Viresh Kumar]

Currently, mpcore wdt driver doesn't use exact clk_rate to program timeout in
hardware. This patch provides two ways of doing so:
	- either use clk_get_rate() if clk_get passes
	- use clk_rate passed via module param

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/include/asm/smp_twd.h | 10 +++++
 drivers/watchdog/mpcore_wdt.c  | 99 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 90 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index a62bcfa..a9791c5 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -13,11 +13,21 @@
 #define TWD_WDOG_RESETSTAT		0x30
 #define TWD_WDOG_DISABLE		0x34
 
+#define TWD_WDOG_LOAD_MIN		0x00000000
+#define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
+
 #define TWD_TIMER_CONTROL_ENABLE	(1 << 0)
 #define TWD_TIMER_CONTROL_ONESHOT	(0 << 1)
 #define TWD_TIMER_CONTROL_PERIODIC	(1 << 1)
 #define TWD_TIMER_CONTROL_IT_ENABLE	(1 << 2)
 
+#define TWD_WDOG_CONTROL_ENABLE		(1 << 0)
+#define TWD_WDOG_CONTROL_IRQ_ENABLE	(1 << 2)
+#define TWD_WDOG_CONTROL_WDT_MODE	(1 << 3)
+#define TWD_WDOG_CONTROL_WDT_PRESCALE(x)	((x) << 8)
+#define TWD_WDOG_CONTROL_PRESCALE_MIN	0x00
+#define TWD_WDOG_CONTROL_PRESCALE_MAX	0xFF
+
 #define TWD_WDOG_RESETSTAT_MASK		0x1
 
 #include <linux/ioport.h>
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 1a1a1fd..64878a7 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -42,7 +42,10 @@ struct mpcore_wdt {
 	void __iomem	*base;
 	spinlock_t	lock;
 	struct clk	*clk;
+	unsigned long	clk_rate;	/* In Hz */
 	int		irq;
+	unsigned int	prescale;
+	unsigned int	load_val;
 	unsigned int	perturb;
 };
 
@@ -61,6 +64,11 @@ MODULE_PARM_DESC(nowayout,
 	"Watchdog cannot be stopped once started (default="
 				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+static int clk_rate;
+module_param(clk_rate, int, 0);
+MODULE_PARM_DESC(clk_rate,
+	"Watchdog clock rate in Hz, required if clk_get() fails");
+
 #define ONLY_TESTING	0
 static int mpcore_noboot = ONLY_TESTING;
 module_param(mpcore_noboot, int, 0);
@@ -96,17 +104,10 @@ static irqreturn_t mpcore_wdt_fire(int irq, void *arg)
 static int mpcore_wdt_ping(struct watchdog_device *wdd)
 {
 	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
-	unsigned long count;
 
 	spin_lock(&wdt->lock);
-	/* Assume prescale is set to 256 */
-	count = readl_relaxed(wdt->base + TWD_WDOG_COUNTER);
-	count = (0xFFFFFFFFU - count) * (HZ / 5);
-	count = (count / 256) * mpcore_margin;
-
-	/* Reload the counter */
-	writel_relaxed(count + wdt->perturb, wdt->base + TWD_WDOG_LOAD);
-	wdt->perturb = wdt->perturb ? 0 : 1;
+	writel_relaxed(wdt->load_val + wdt->perturb, wdt->base + TWD_WDOG_LOAD);
+	wdt->perturb = !wdt->perturb;
 	spin_unlock(&wdt->lock);
 
 	return 0;
@@ -136,6 +137,7 @@ static int mpcore_wdt_stop(struct watchdog_device *wdd)
 static int mpcore_wdt_start(struct watchdog_device *wdd)
 {
 	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+	u32 val, mode;
 	int ret;
 
 	dev_info(wdt->dev, "enabling watchdog\n");
@@ -151,20 +153,70 @@ static int mpcore_wdt_start(struct watchdog_device *wdd)
 	/* This loads the count register but does NOT start the count yet */
 	mpcore_wdt_ping(wdd);
 
-	if (mpcore_noboot) {
-		/* Enable watchdog - prescale=256, watchdog mode=0, enable=1 */
-		writel_relaxed(0x0000FF01, wdt->base + TWD_WDOG_CONTROL);
-	} else {
-		/* Enable watchdog - prescale=256, watchdog mode=1, enable=1 */
-		writel_relaxed(0x0000FF09, wdt->base + TWD_WDOG_CONTROL);
-	}
+	if (mpcore_noboot)
+		mode = 0;
+	else
+		mode = TWD_WDOG_CONTROL_WDT_MODE;
+
+	spin_lock(&wdt->lock);
+
+	val = TWD_WDOG_CONTROL_WDT_PRESCALE(wdt->prescale) |
+		TWD_WDOG_CONTROL_ENABLE | mode;
+	writel_relaxed(val, wdt->base + TWD_WDOG_CONTROL);
+
+	spin_unlock(&wdt->lock);
 
 	return 0;
 }
 
-static int mpcore_wdt_set_heartbeat(struct watchdog_device *wdd, unsigned int t)
+/* binary search */
+static inline void bsearch(u32 *var, u32 var_start, u32 var_end,
+		const u64 param, const u32 timeout, u32 rate)
 {
-	mpcore_margin = t;
+	u64 tmp = 0;
+
+	/* get the lowest var value that can satisfy our requirement */
+	while (var_start < var_end) {
+		tmp = var_start;
+		tmp += var_end;
+		tmp = div_u64(tmp, 2);
+		if (timeout > div_u64((param + 1) * (tmp + 1), rate)) {
+			if (var_start == tmp)
+				break;
+			else
+				var_start = tmp;
+		} else {
+			if (var_end == tmp)
+				break;
+			else
+				var_end = tmp;
+		}
+	}
+	*var = tmp;
+}
+
+static int
+mpcore_wdt_set_heartbeat(struct watchdog_device *wdd, unsigned int timeout)
+{
+	struct mpcore_wdt *wdt = watchdog_get_drvdata(wdd);
+	unsigned int psc, rate = wdt->clk_rate;
+	u64 load = 0;
+
+	/* get appropriate value of psc and load */
+	bsearch(&psc, TWD_WDOG_CONTROL_PRESCALE_MIN,
+			TWD_WDOG_CONTROL_PRESCALE_MAX, TWD_WDOG_LOAD_MAX,
+			timeout, rate);
+	bsearch((u32 *)&load, TWD_WDOG_LOAD_MIN, TWD_WDOG_LOAD_MAX, psc,
+			timeout, rate);
+
+	spin_lock(&wdt->lock);
+	wdt->prescale = psc;
+	wdt->load_val = load;
+
+	/* roundup timeout to closest positive integer value */
+	mpcore_margin = div_u64((psc + 1) * (load + 1) + (rate / 2), rate);
+	spin_unlock(&wdt->lock);
+
 	return 0;
 }
 
@@ -240,7 +292,10 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 	wdt->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(wdt->clk)) {
 		dev_warn(&pdev->dev, "Clock not found\n");
+		wdt->clk_rate = clk_rate;
 	} else {
+		wdt->clk_rate = clk_get_rate(wdt->clk);
+
 		ret = clk_prepare_enable(wdt->clk);
 		if (ret) {
 			dev_err(wdt->dev, "clock prepare-enable failed");
@@ -253,6 +308,12 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 
 	mpcore_wdt_stop(&wdt->wdd);
 
+	if (!wdt->clk_rate) {
+		dev_err(&pdev->dev, "Clock rate can't be zero\n");
+		ret = -EINVAL;
+		goto err_put_clk;
+	}
+
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret) {
 		dev_err(wdt->dev, "watchdog_register_device() failed: %d\n",
@@ -270,7 +331,7 @@ static int mpcore_wdt_probe(struct platform_device *pdev)
 			TIMER_MARGIN);
 	}
 
-	mpcore_wdt_set_heartbeat(NULL, mpcore_margin);
+	mpcore_wdt_set_heartbeat(&wdt->wdd, mpcore_margin);
 	dev_info(wdt->dev, "MPcore Watchdog Timer: 0.1. mpcore_noboot=%d "
 			"mpcore_margin=%d sec (nowayout= %d)\n", mpcore_noboot,
 			mpcore_margin, nowayout);
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 13/14] watchdog/mpcore_wdt: Start registers from 0x00 instead of 0x20

[Viresh Kumar]

TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
contain this extra 0x20 offset, i.e. users were required to pass base address of
TWD instead of WDOG to WDOG driver.

Change this, so that users can pass base address of WDOG to WDOG driver instead
of TWD module. For this, subtract 0x20 from offsets of WDOG registers.

This could break any current users of TWD_WDOG, but i couldn't find any users of
this driver in current Linux tree. So, haven't fixed any platform code. If some
platforms are broken please report to me, so that we can get them fixed in this
patch only.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/include/asm/smp_twd.h | 19 -------------------
 drivers/watchdog/mpcore_wdt.c  | 24 +++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index a9791c5..c5d81a9 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -6,30 +6,11 @@
 #define TWD_TIMER_CONTROL		0x08
 #define TWD_TIMER_INTSTAT		0x0C
 
-#define TWD_WDOG_LOAD			0x20
-#define TWD_WDOG_COUNTER		0x24
-#define TWD_WDOG_CONTROL		0x28
-#define TWD_WDOG_INTSTAT		0x2C
-#define TWD_WDOG_RESETSTAT		0x30
-#define TWD_WDOG_DISABLE		0x34
-
-#define TWD_WDOG_LOAD_MIN		0x00000000
-#define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
-
 #define TWD_TIMER_CONTROL_ENABLE	(1 << 0)
 #define TWD_TIMER_CONTROL_ONESHOT	(0 << 1)
 #define TWD_TIMER_CONTROL_PERIODIC	(1 << 1)
 #define TWD_TIMER_CONTROL_IT_ENABLE	(1 << 2)
 
-#define TWD_WDOG_CONTROL_ENABLE		(1 << 0)
-#define TWD_WDOG_CONTROL_IRQ_ENABLE	(1 << 2)
-#define TWD_WDOG_CONTROL_WDT_MODE	(1 << 3)
-#define TWD_WDOG_CONTROL_WDT_PRESCALE(x)	((x) << 8)
-#define TWD_WDOG_CONTROL_PRESCALE_MIN	0x00
-#define TWD_WDOG_CONTROL_PRESCALE_MAX	0xFF
-
-#define TWD_WDOG_RESETSTAT_MASK		0x1
-
 #include <linux/ioport.h>
 
 struct twd_local_timer {
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 64878a7..d7ee4eb 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -34,7 +34,29 @@
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
-#include <asm/smp_twd.h>
+/*
+ * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
+ * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG must pass base
+ * address of WDOG to WDOG driver instead of TWD module.
+ */
+#define TWD_WDOG_LOAD				0x00
+#define TWD_WDOG_COUNTER			0x04
+#define TWD_WDOG_CONTROL			0x08
+#define TWD_WDOG_INTSTAT			0x0C
+#define TWD_WDOG_RESETSTAT			0x10
+#define TWD_WDOG_DISABLE			0x14
+
+#define TWD_WDOG_LOAD_MIN			0x00000000
+#define TWD_WDOG_LOAD_MAX			0xFFFFFFFF
+
+#define TWD_WDOG_CONTROL_ENABLE			(1 << 0)
+#define TWD_WDOG_CONTROL_IRQ_ENABLE		(1 << 2)
+#define TWD_WDOG_CONTROL_WDT_MODE		(1 << 3)
+#define TWD_WDOG_CONTROL_WDT_PRESCALE(x)	((x) << 8)
+#define TWD_WDOG_CONTROL_PRESCALE_MIN		0x00
+#define TWD_WDOG_CONTROL_PRESCALE_MAX		0xFF
+
+#define TWD_WDOG_RESETSTAT_MASK			0x1
 
 struct mpcore_wdt {
 	struct watchdog_device wdd;
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 14/14] watchdog/mpcore_wdt: Add DT probing support for ARM mpcore watchdog

[Viresh Kumar]

This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding
were already documented in Documentation/devicetree/bindings/arm/twd.txt.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/watchdog/mpcore_wdt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index d7ee4eb..50f5a0b 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -403,6 +404,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpcore_wdt_id_table[] = {
+	{ .compatible = "arm,cortex-a9-twd-wdt" },
+	{ .compatible = "arm,cortex-a5-twd-wdt" },
+	{ .compatible = "arm,arm11mp-twd-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
+#endif
+
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= mpcore_wdt_remove,
@@ -411,6 +422,7 @@ static struct platform_driver mpcore_wdt_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
 		.pm	= &mpcore_wdt_dev_pm_ops,
+		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
 	},
 };
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 00/14] watchdog: ARM mpcore Improvements

[Russell King - ARM Linux]

On Tue, Jun 18, 2013 at 08:50:24PM +0530, Viresh Kumar wrote:
> ARM mpcore watchdog isn't workable and so is marked broken in the first patch of
> the series. Reasons are mentioned in 01/14 and important links are shared too.

The only way this is usable is if a watchdog user binds to a particular
CPU before opening this device, and remains bound to that user.  The
irq-requesting thing is something that has changed since the driver
was originally added.

I suspect there are no users of this driver, and it should simply be
deleted.  It doesn't really fit our watchdog API.  If anyone wants to
resurect it, it's in git, your patches are in the list archives...