[PATCH 3/3] watchdog: qcom: register a restart notifier
From: linux@roeck-us.net (Guenter Roeck)
Date: 2014-09-19 02:48:00
Also in:
linux-arm-msm, linux-watchdog, lkml
On 09/18/2014 03:27 PM, Josh Cartwright wrote:
quoted hunk ↗ jump to hunk
The WDT's BITE_TIME warm-reset behavior can be leveraged as a last resort mechanism for triggering chip reset. Usually, other restart methods (such as PS_HOLD) are preferrable for issuing a more complete reset of the chip. As such, keep the priority of the watchdog notifier low. Signed-off-by: Josh Cartwright <redacted> --- drivers/watchdog/qcom-wdt.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c index e9409f5..710ab43 100644 --- a/drivers/watchdog/qcom-wdt.c +++ b/drivers/watchdog/qcom-wdt.c@@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/reboot.h> #include <linux/watchdog.h> #define WDT_RST 0x0@@ -24,6 +25,7 @@ struct qcom_wdt { struct watchdog_device wdd; unsigned long freq; + struct notifier_block restart_nb; void __iomem *base; };@@ -82,6 +84,24 @@ static const struct watchdog_info qcom_wdt_info = { .identity = KBUILD_MODNAME, }; +static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb); + + /* + * Trigger watchdog bite: + * Setup BITE_TIME to be very low, and enable WDT. + */ + mutex_lock(&wdt->wdd.lock);
At this time you don't need to worry about locks. Actually, this might be dangerous if the lock happens to be taken, as it won't be released (there is no other code running anymore when this function is called).
+ writel_relaxed(0, wdt->base + WDT_EN); + writel_relaxed(1, wdt->base + WDT_RST); + writel_relaxed(0x31F3, wdt->base + WDT_BITE_TIME);
What is the magic here, ie what does 0x31F3 stand for ?
quoted hunk ↗ jump to hunk
+ writel_relaxed(1, wdt->base + WDT_EN); + mutex_unlock(&wdt->wdd.lock); + return NOTIFY_DONE; +} + static int qcom_watchdog_probe(struct platform_device *pdev) { struct qcom_wdt *wdt;@@ -121,6 +141,17 @@ static int qcom_watchdog_probe(struct platform_device *pdev) return ret; } + /* + * WDT restart notifier has priority 0 (use as a last resort) + */ + wdt->restart_nb.notifier_call = qcom_wdt_restart; + ret = register_restart_handler(&wdt->restart_nb); + if (ret) { + dev_err(&pdev->dev, "failed to setup restart handler\n"); + watchdog_unregister_device(&wdt->wdd); + return ret;
Sure you want to return an error here ? The watchdog itself is still working, and this is supposed to be a restart method of last resort. Causing the driver to fail loading because it can not register its restart handler seems to be a bit aggressive. Thanks, Guenter
+ } + return 0; }