Re: [PATCH 3/3] MIPS: jz4740: Move reset code to the watchdog driver

[Lars-Peter Clausen <lars@metafoo.de>]

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