Re: In-reply-to: <1448056496-2335-1-git-send-email-damien.riegel@savoirfairelinux.com>

[Guenter Roeck <linux@roeck-us.net>]

On 11/23/2015 07:02 AM, Harald Geyer wrote:
> Hi,
>
> sorry for breaking the threading. Only stumbled upon this message in the
> webarchive of the Mailinglist and couldn't reconstruct full headers ...
>
> I think there is an issue with your patch:
>
> +static int watchdog_reboot_notifier(struct notifier_block *nb,
> +				    unsigned long code, void *data)
> +{
> +	struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
> +						   reboot_nb);
> +
> +	if (code == SYS_DOWN || code == SYS_HALT) {
>
> I think you want this to be (code == SYS_POWER_OFF || code == SYS_HALT)
> AFAIK SYS_DOWN is the code for a reboot, where the system should come
> back up immediatly, so probably we shouldn't disable the watchdog in
> this case, for the system might crash during going down.
>
> More importantly however we should stop the watchdog on SYS_POWER_OFF
> I think.
>

Harald,

please have a look into existing watchdog drivers. Unless I am counting wrong,
there are 31 watchdog drivers using the above code, and only 3 watchdog drivers
(gpio, it8712, scx200) using SYS_POWER_OFF || SYS_HALT. I think it makes sense
to make a majority decision when moving code from drivers into the infrastructure.

Thanks,
Guenter