On 01/11/2015 02:43 AM, Maarten ter Huurne wrote:
On Saturday 10 January 2015 17:18:03 Guenter Roeck wrote:
On 01/10/2015 10:29 AM, Lars-Peter Clausen wrote:
@@ -186,9 +208,20 @@ static int jz4740_wdt_probe(struct platform_device
*pdev)>
if (ret < 0)
goto err_disable_clk;
+ drvdata->restart_handler.notifier_call = jz4740_wdt_restart;
+ drvdata->restart_handler.priority = 128;
+ ret = register_restart_handler(&drvdata->restart_handler);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot register restart handler, %d\n",
+ ret);
+ goto err_unregister_watchdog;
Are you sure you want to abort in this case ?
After all, the watchdog would still work.
That raises a similar question: what about the opposite case, where the
watchdog registration fails? If the resource acquisition part of the probe
fails, neither the watchdog nor the restart functionality is going to work,
but if the call to watchdog_register_device() fails, the restart handler
would still work.
I think this is fine, if either the watchdog or the restart handler
registration fail then the system is probably already in a rather unusable
state.
But that got me thinking, maybe instead of having each watchdog driver
register and implement its own restart handler we should maybe add this as a
functionality to the watchdog framework. Something along the lines off.
watchdog_set_timeout(wdt, wdt->min_timeout);
watchdog_start(wdt);
mdelay(wdt->min_timeout * 2000);
- Lars
