linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Walter Stoll <Walter.Stoll@duagon.com>
To: "wim@linux-watchdog.org" <wim@linux-watchdog.org>,
	"linux@roeck-us.net" <linux@roeck-us.net>
Cc: "linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>
Subject: [BUG] omap_wdt.c: Watchdog not serviced by kernel
Date: Fri, 17 Sep 2021 08:36:52 +0000	[thread overview]
Message-ID: <cb068bbba92347b2ab3190fda5d85ebf@chdua14.duagon.ads> (raw)

Effect observed
---------------

We use the watchdog timer on a AM335x controller. When U-Boot runs, we enable
the watchdog and want the kernel to service the watchdog until userspace takes
it over.

We compile the watchdog directly into the kernel and add the parameter
"omap_wdt.early_enable=1" to the kernel command line. We furthermore set
"CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED=y" in the kernel configuration.

Our expectation is, that the watchdog is serviced by the kernel as long as
userspace does not touch the device /dev/watchdog. However, this is not the
case. The watchdog always expires. It is obviously not serviced by the kernel.

We observed the effect with kernel version v5.4.138-rt62. However, we think
that the most recent kernel exhibits the same behavior because the structure of
the sources in question (see below) did not change. This also holds for the non
realtime kernel.


Root cause
----------

The CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED configuration option is not implemented
in omap_wdt.c.


Fix proposal
------------

Interestingly we found only one single driver that implements this featrue,
namely the driver from STM, see 
https://elixir.bootlin.com/linux/v5.4.138/source/drivers/watchdog/stm32_iwdg.c#L274

This makes us wonder if there might be a good reason not to implement it???

However we think this feature should be available. Our use case is to make
software updates more robust. If an updated kernel hangs for whatever reason,
then U-Boot gets the chance to boot the old one provided there is a reboot.
 
Based on the STM implementation, we created a patch (see below) which resolves
the issue. The watchdog is now correctly handled by the kernel until userspace
first accesses it. 

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 9b91882fe3c4..94e2e1b494d2 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -271,6 +271,11 @@ static int omap_wdt_probe(struct platform_device *pdev)
 	if (!early_enable)
 		omap_wdt_disable(wdev);
 
+	if (IS_ENABLED(CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED)) {
+		/* Make sure the watchdog is serviced */
+		set_bit(WDOG_HW_RUNNING, &wdev->wdog.status);
+	}
+
 	ret = watchdog_register_device(&wdev->wdog);
 	if (ret) {
 		pm_runtime_disable(wdev->dev);

             reply	other threads:[~2021-09-17  8:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17  8:36 Walter Stoll [this message]
2021-09-17 13:47 ` Guenter Roeck
2021-09-17 15:00   ` AW: " Walter Stoll
2021-09-17 15:08     ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cb068bbba92347b2ab3190fda5d85ebf@chdua14.duagon.ads \
    --to=walter.stoll@duagon.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    --subject='Re: [BUG] omap_wdt.c: Watchdog not serviced by kernel' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).