On 14/06/2019 17:53:01+0000, Ken Sloat wrote: > > The call to sama5d4_wdt_init() above now explicitly stops the watchdog > > even if we want to (re)start it. I think this would be better handled with an > > else case here > > > > else > > sama5d4_wdt_stop(&wdt->wdd); > > > > So we completely remove the sama5d4_wdt_init() call then correct? > > To leave the code as it behaves today with the addition > of wdt stop/start, shouldn't we call init in the else instead? > > if (watchdog_active(&wdt->wdd)) > sama5d4_wdt_start(&wdt->wdd); > else > sama5d4_wdt_init(); > > I guess I don't really understand the purpose of having the init statement in resume > in the first place. I agree, calling this first does end up essentially resetting the wdt > it will start again if it was running before, but the count will be reset. > There is a nice comment explaining why ;) -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com