[Ccing Guenter as he reviewed the patches] On Mon, Nov 23, 2015 at 04:02:52PM +0100, 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. Well, most of the drivers (all of them but gpio) that I changed stopped on SYS_HALT and SYS_DOWN, so they explicitely wanted to stop the watchdog on reboot. I just factorized that in watchdog core. Maybe they should not stop on reboot in the first place, but this serie does not introduce a new behaviour. Also, note that the watchdog will be stopped only if it registers for that by calling watchdog_stop_on_reboot, so there is no side effect for watchdogs that should not be stopped. > > More importantly however we should stop the watchdog on SYS_POWER_OFF > I think. > My understanding here is that if the system is powered off, the watchdog will be powered off too, so there is no need to stop it. Damien > Otherwise your series looks good to me and I look forward to rebase > my own patches on it. > > HTH, > Harald > > + if (watchdog_active(wdd)) { > + int ret; > + > + ret = wdd->ops->stop(wdd); > + if (ret) > + return NOTIFY_BAD; > + } > + } > + > + return NOTIFY_DONE; > +} > -- > If you want to support my work: > see http://friends.ccbib.org/harald/supporting/ > or donate via bitcoin to 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html