2015-04-03 17:10:45

by Andrew Bresticker

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

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 <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: James Hogan <[email protected]>
---
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


2015-04-03 17:05:34

by Andrew Bresticker

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

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

Signed-off-by: Naidu Tellapati <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: James Hogan <[email protected]>
---
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

2015-04-03 17:05:37

by Andrew Bresticker

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

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

Signed-off-by: Andrew Bresticker <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: James Hogan <[email protected]>
---
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

2015-04-05 13:04:07

by Guenter Roeck

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

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 <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: James Hogan <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> 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);
>
>

2015-04-05 13:05:19

by Guenter Roeck

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

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 <[email protected]>
> Signed-off-by: Andrew Bresticker <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: James Hogan <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

2015-04-28 22:55:06

by Ezequiel Garcia

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



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 <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: James Hogan <[email protected]>

Reviewed-by: Ezequiel Garcia <[email protected]>
--
Ezequiel

2015-04-28 23:31:44

by Ezequiel Garcia

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



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 <[email protected]>
>> Cc: Ezequiel Garcia <[email protected]>
>> Cc: James Hogan <[email protected]>
>
> Reviewed-by: Ezequiel Garcia <[email protected]>
>

And for the three patches:

Tested-by: Ezequiel Garcia <[email protected]>

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

2015-04-29 04:10:59

by Guenter Roeck

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

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 <[email protected]>
>>> Cc: Ezequiel Garcia <[email protected]>
>>> Cc: James Hogan <[email protected]>
>>
>> Reviewed-by: Ezequiel Garcia <[email protected]>
>>
>
> And for the three patches:
>
> Tested-by: Ezequiel Garcia <[email protected]>
>
> 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

2015-04-29 12:17:41

by Ezequiel Garcia

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



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 <[email protected]>
>>>> Cc: Ezequiel Garcia <[email protected]>
>>>> Cc: James Hogan <[email protected]>
>>>
>>> Reviewed-by: Ezequiel Garcia <[email protected]>
>>>
>>
>> And for the three patches:
>>
>> Tested-by: Ezequiel Garcia <[email protected]>
>>
>> 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