[PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Andrew Bresticker]

Since the heartbeat is statically initialized to its default value,
watchdog_init_timeout() will never look in the device-tree for a
timeout-sec value.  Instead of statically initializing heartbeat,
fall back to the default timeout value if watchdog_init_timeout()
fails.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Cc: James Hogan <james.hogan@imgtec.com>
---
Changes from v2:
 - Set timeout before calling watchdog_init_timeout
 - Don't print an error on watchdog_init_timeout failure
New for v2.
---
 drivers/watchdog/imgpdc_wdt.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index 0deaa4f..d6826a6 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -42,7 +42,7 @@
 #define PDC_WDT_MIN_TIMEOUT		1
 #define PDC_WDT_DEF_TIMEOUT		64
 
-static int heartbeat = PDC_WDT_DEF_TIMEOUT;
+static int heartbeat;
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds "
 	"(default=" __MODULE_STRING(PDC_WDT_DEF_TIMEOUT) ")");
@@ -190,15 +190,11 @@ static int pdc_wdt_probe(struct platform_device *pdev)
 	pdc_wdt->wdt_dev.info = &pdc_wdt_info;
 	pdc_wdt->wdt_dev.ops = &pdc_wdt_ops;
 	pdc_wdt->wdt_dev.max_timeout = 1 << PDC_WDT_CONFIG_DELAY_MASK;
+	pdc_wdt->wdt_dev.timeout = PDC_WDT_DEF_TIMEOUT;
 	pdc_wdt->wdt_dev.parent = &pdev->dev;
 	watchdog_set_drvdata(&pdc_wdt->wdt_dev, pdc_wdt);
 
-	ret = watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, &pdev->dev);
-	if (ret < 0) {
-		pdc_wdt->wdt_dev.timeout = pdc_wdt->wdt_dev.max_timeout;
-		dev_warn(&pdev->dev,
-			 "Initial timeout out of range! setting max timeout\n");
-	}
+	watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, &pdev->dev);
 
 	pdc_wdt_stop(&pdc_wdt->wdt_dev);
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH V3 2/3] watchdog: imgpdc: Set timeout before starting watchdog

[Andrew Bresticker]

Set up the watchdog for the specified timeout before attempting to start it.

Signed-off-by: Naidu Tellapati <naidu.tellapati@imgtec.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Cc: James Hogan <james.hogan@imgtec.com>
---
No changes from v2.
Changes from v1:
 - Moved setting of timeout to a helper as suggested by Guenter.
---
 drivers/watchdog/imgpdc_wdt.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index d6826a6..ffeb1bf 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -84,18 +84,24 @@ static int pdc_wdt_stop(struct watchdog_device *wdt_dev)
 	return 0;
 }
 
+static void __pdc_wdt_set_timeout(struct pdc_wdt_dev *wdt)
+{
+	unsigned long clk_rate = clk_get_rate(wdt->wdt_clk);
+	unsigned int val;
+
+	val = readl(wdt->base + PDC_WDT_CONFIG) & ~PDC_WDT_CONFIG_DELAY_MASK;
+	val |= order_base_2(wdt->wdt_dev.timeout * clk_rate) - 1;
+	writel(val, wdt->base + PDC_WDT_CONFIG);
+}
+
 static int pdc_wdt_set_timeout(struct watchdog_device *wdt_dev,
 			       unsigned int new_timeout)
 {
-	unsigned int val;
 	struct pdc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
-	unsigned long clk_rate = clk_get_rate(wdt->wdt_clk);
 
 	wdt->wdt_dev.timeout = new_timeout;
 
-	val = readl(wdt->base + PDC_WDT_CONFIG) & ~PDC_WDT_CONFIG_DELAY_MASK;
-	val |= order_base_2(new_timeout * clk_rate) - 1;
-	writel(val, wdt->base + PDC_WDT_CONFIG);
+	__pdc_wdt_set_timeout(wdt);
 
 	return 0;
 }
@@ -106,6 +112,8 @@ static int pdc_wdt_start(struct watchdog_device *wdt_dev)
 	unsigned int val;
 	struct pdc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
 
+	__pdc_wdt_set_timeout(wdt);
+
 	val = readl(wdt->base + PDC_WDT_CONFIG);
 	val |= PDC_WDT_CONFIG_ENABLE;
 	writel(val, wdt->base + PDC_WDT_CONFIG);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH V3 3/3] watchdog: imgpdc: Add reboot support

[Andrew Bresticker]

Register a restart handler that will restart the system by writing
to the watchdog's SOFT_RESET register.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Cc: James Hogan <james.hogan@imgtec.com>
---
No changes from v1/v2.
---
 drivers/watchdog/imgpdc_wdt.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index ffeb1bf..28c10e2 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -16,6 +16,7 @@
 #include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/watchdog.h>
 
@@ -57,6 +58,7 @@ struct pdc_wdt_dev {
 	struct clk *wdt_clk;
 	struct clk *sys_clk;
 	void __iomem *base;
+	struct notifier_block restart_handler;
 };
 
 static int pdc_wdt_keepalive(struct watchdog_device *wdt_dev)
@@ -136,6 +138,18 @@ static const struct watchdog_ops pdc_wdt_ops = {
 	.set_timeout	= pdc_wdt_set_timeout,
 };
 
+static int pdc_wdt_restart(struct notifier_block *this, unsigned long mode,
+			   void *cmd)
+{
+	struct pdc_wdt_dev *wdt = container_of(this, struct pdc_wdt_dev,
+					       restart_handler);
+
+	/* Assert SOFT_RESET */
+	writel(0x1, wdt->base + PDC_WDT_SOFT_RESET);
+
+	return NOTIFY_OK;
+}
+
 static int pdc_wdt_probe(struct platform_device *pdev)
 {
 	int ret, val;
@@ -242,6 +256,13 @@ static int pdc_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_wdt_clk;
 
+	pdc_wdt->restart_handler.notifier_call = pdc_wdt_restart;
+	pdc_wdt->restart_handler.priority = 128;
+	ret = register_restart_handler(&pdc_wdt->restart_handler);
+	if (ret)
+		dev_warn(&pdev->dev, "failed to register restart handler: %d\n",
+			 ret);
+
 	return 0;
 
 disable_wdt_clk:
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Guenter Roeck]

On 04/03/2015 10:05 AM, Andrew Bresticker wrote:
> Since the heartbeat is statically initialized to its default value,
> watchdog_init_timeout() will never look in the device-tree for a
> timeout-sec value.  Instead of statically initializing heartbeat,
> fall back to the default timeout value if watchdog_init_timeout()
> fails.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> Cc: James Hogan <james.hogan@imgtec.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes from v2:
>   - Set timeout before calling watchdog_init_timeout
>   - Don't print an error on watchdog_init_timeout failure
> New for v2.
> ---
>   drivers/watchdog/imgpdc_wdt.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
> index 0deaa4f..d6826a6 100644
> --- a/drivers/watchdog/imgpdc_wdt.c
> +++ b/drivers/watchdog/imgpdc_wdt.c
> @@ -42,7 +42,7 @@
>   #define PDC_WDT_MIN_TIMEOUT		1
>   #define PDC_WDT_DEF_TIMEOUT		64
>
> -static int heartbeat = PDC_WDT_DEF_TIMEOUT;
> +static int heartbeat;
>   module_param(heartbeat, int, 0);
>   MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds "
>   	"(default=" __MODULE_STRING(PDC_WDT_DEF_TIMEOUT) ")");
> @@ -190,15 +190,11 @@ static int pdc_wdt_probe(struct platform_device *pdev)
>   	pdc_wdt->wdt_dev.info = &pdc_wdt_info;
>   	pdc_wdt->wdt_dev.ops = &pdc_wdt_ops;
>   	pdc_wdt->wdt_dev.max_timeout = 1 << PDC_WDT_CONFIG_DELAY_MASK;
> +	pdc_wdt->wdt_dev.timeout = PDC_WDT_DEF_TIMEOUT;
>   	pdc_wdt->wdt_dev.parent = &pdev->dev;
>   	watchdog_set_drvdata(&pdc_wdt->wdt_dev, pdc_wdt);
>
> -	ret = watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, &pdev->dev);
> -	if (ret < 0) {
> -		pdc_wdt->wdt_dev.timeout = pdc_wdt->wdt_dev.max_timeout;
> -		dev_warn(&pdev->dev,
> -			 "Initial timeout out of range! setting max timeout\n");
> -	}
> +	watchdog_init_timeout(&pdc_wdt->wdt_dev, heartbeat, &pdev->dev);
>
>   	pdc_wdt_stop(&pdc_wdt->wdt_dev);
>
>

Re: [PATCH V3 2/3] watchdog: imgpdc: Set timeout before starting watchdog

[Guenter Roeck]

On 04/03/2015 10:05 AM, Andrew Bresticker wrote:
> Set up the watchdog for the specified timeout before attempting to start it.
>
> Signed-off-by: Naidu Tellapati <naidu.tellapati@imgtec.com>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> Cc: James Hogan <james.hogan@imgtec.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Ezequiel Garcia]

On 04/03/2015 02:05 PM, Andrew Bresticker wrote:
> Since the heartbeat is statically initialized to its default value,
> watchdog_init_timeout() will never look in the device-tree for a
> timeout-sec value.  Instead of statically initializing heartbeat,
> fall back to the default timeout value if watchdog_init_timeout()
> fails.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> Cc: James Hogan <james.hogan@imgtec.com>

Reviewed-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
-- 
Ezequiel

Re: [PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Ezequiel Garcia]

On 04/28/2015 07:51 PM, Ezequiel Garcia wrote:
> 
> 
> On 04/03/2015 02:05 PM, Andrew Bresticker wrote:
>> Since the heartbeat is statically initialized to its default value,
>> watchdog_init_timeout() will never look in the device-tree for a
>> timeout-sec value.  Instead of statically initializing heartbeat,
>> fall back to the default timeout value if watchdog_init_timeout()
>> fails.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>> Cc: James Hogan <james.hogan@imgtec.com>
> 
> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 

And for the three patches:

Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

Any chance these get merged soon(ishly)?
-- 
Ezequiel

Re: [PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Guenter Roeck]

On 04/28/2015 04:28 PM, Ezequiel Garcia wrote:
>
>
> On 04/28/2015 07:51 PM, Ezequiel Garcia wrote:
>>
>>
>> On 04/03/2015 02:05 PM, Andrew Bresticker wrote:
>>> Since the heartbeat is statically initialized to its default value,
>>> watchdog_init_timeout() will never look in the device-tree for a
>>> timeout-sec value.  Instead of statically initializing heartbeat,
>>> fall back to the default timeout value if watchdog_init_timeout()
>>> fails.
>>>
>>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>>> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>
>> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>
>
> And for the three patches:
>
> Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>
> Any chance these get merged soon(ishly)?
>

I added the patches to my test branch. I'll send a pull request to
Wim in time for the next commit window, so hopefully the patches
should make it into 4.2.

Guenter

Re: [PATCH V3 1/3] watchdog: imgpdc: Allow timeout to be set in device-tree

[Ezequiel Garcia]

On 04/29/2015 01:10 AM, Guenter Roeck wrote:
> On 04/28/2015 04:28 PM, Ezequiel Garcia wrote:
>>
>>
>> On 04/28/2015 07:51 PM, Ezequiel Garcia wrote:
>>>
>>>
>>> On 04/03/2015 02:05 PM, Andrew Bresticker wrote:
>>>> Since the heartbeat is statically initialized to its default value,
>>>> watchdog_init_timeout() will never look in the device-tree for a
>>>> timeout-sec value.  Instead of statically initializing heartbeat,
>>>> fall back to the default timeout value if watchdog_init_timeout()
>>>> fails.
>>>>
>>>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>>>> Cc: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>>> Cc: James Hogan <james.hogan@imgtec.com>
>>>
>>> Reviewed-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>>
>>
>> And for the three patches:
>>
>> Tested-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>
>> Any chance these get merged soon(ishly)?
>>
> 
> I added the patches to my test branch. I'll send a pull request to
> Wim in time for the next commit window, so hopefully the patches
> should make it into 4.2.
> 

OK, cool. Thanks a lot.

-- 
Ezequiel