On Sun, Nov 17, 2019 at 3:44 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 11/17/19 12:03 PM, Brandon Maier wrote: > > When the 'early_enable' module_param is enabled, Linux's watchdogd > > thread does not start, causing the watchdog to eventually fire. > > > > What does early_enable have to do with watchdogd ? The early_enable doesn't directly control the watchdogd, it only controls starting the watchdog. The problem is that, if the watchdog is started, we need the watchdogd to ping the watchdog. It watchdogd is not running, then the watchdog will trip and reset the system. > Why would watchdogd not start if this flag is set ? watchdog_register_device() checks this flag, and only enables the watchdogd if it's set. See following: https://elixir.bootlin.com/linux/latest/source/drivers/watchdog/watchdog_dev.c#L1031 > > The purpose of early_enable in this driver, as I understand it, > was to force watchdogd to start within the timeout period. So > it does exactly what it is supposed to be doing. I'm not sure what you're referring to by "timeout period". But early_enable does force the watchdog to start during module insertion, that works correctly. The issue is that the driver doesn't tell the watchdog core to launch the watchdogd. This change is based off many of the other watchdog drivers. They also set the WDOG_HW_RUNNING flag if the watchdog is running. The only significant difference is that those drivers don't typically have an early_enable, they just detect if the watchdog was already running before insertion. But the end result is the same, the watchdog is running and will trigger if watchdogd isn't. > > Guenter > > > For the watchdogd to be started, the WDOG_HW_RUNNING flag must be set > > before watchdog_register_device(). > > > > Signed-off-by: Brandon Maier <brandon.maier@xxxxxxxxxxxxxxxxxxx> > > --- > > drivers/watchdog/omap_wdt.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > > index 9b91882fe3c4..ecc8592c00a5 100644 > > --- a/drivers/watchdog/omap_wdt.c > > +++ b/drivers/watchdog/omap_wdt.c > > @@ -268,8 +268,13 @@ static int omap_wdt_probe(struct platform_device *pdev) > > wdev->wdog.bootstatus = WDIOF_CARDRESET; > > } > > > > - if (!early_enable) > > + if (!early_enable) { > > omap_wdt_disable(wdev); > > + clear_bit(WDOG_HW_RUNNING, &wdev->wdog.status); > > + } else { > > + omap_wdt_start(&wdev->wdog); > > + set_bit(WDOG_HW_RUNNING, &wdev->wdog.status); > > + } > > > > ret = watchdog_register_device(&wdev->wdog); > > if (ret) { > > @@ -281,9 +286,6 @@ static int omap_wdt_probe(struct platform_device *pdev) > > readl_relaxed(wdev->base + OMAP_WATCHDOG_REV) & 0xFF, > > wdev->wdog.timeout); > > > > - if (early_enable) > > - omap_wdt_start(&wdev->wdog); > > - > > pm_runtime_put(wdev->dev); > > > > return 0; > > > Brandon
