Skip to contentSkip to conversation

watchdog: intel-mid_wdt: Keep watchdog running

PATCH Superseded Andy Shevchenko 2 participants 5 messages Mar 10, 2017

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 watchdog: intel-mid_wdt: Keep watchdog running

Firmware followed by bootloader leaves watchdog running.

Keep it running in the driver.

User will not need any additional options to reboot in case of panic during boot.

---
 drivers/watchdog/intel-mid_wdt.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/watchdog/intel-mid_wdt.c +10 -2
index 45e4d02221b5..f2c66689f42c 100644
@@ -147,8 +147,16 @@ static int mid_wdt_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	/* Make sure the watchdog is not running */
-	wdt_stop(wdt_dev);
+	/*
+	 * Make sure the watchdog is serviced.
+	 *
+	 * The firmware followed by U-Boot leaves the watchdog running
+	 * with the default threshold 60 seconds. Our default timeout is
+	 * 90 seconds, but internal worker divides it by two, which is
+	 * 45 seconds and should be enough (less by 15 seconds than
+	 * threshold).
+	 */
+	set_bit(WDOG_HW_RUNNING, &wdt_dev->status);
 
 	ret = devm_watchdog_register_device(&pdev->dev, wdt_dev);
 	if (ret) {
Signature · 1 line
2.11.0
#1 lore raw
Quoted 18 lines · Andy Shevchenko

Firmware followed by bootloader leaves watchdog running.

Keep it running in the driver.

User will not need any additional options to reboot in case of panic during boot.

drivers/watchdog/intel-mid_wdt.c +4 -2
-	/* Make sure the watchdog is not running */
-	wdt_stop(wdt_dev);
+	/*
+	 * Make sure the watchdog is serviced.
+	 *
+	 * The firmware followed by U-Boot leaves the watchdog

running

(quoted diff) +1 -0
+	 * with the default threshold 60 seconds. Our default timeout

is

(quoted diff) +1 -0
+	 * 90 seconds, but internal worker divides it by two, which

is

drivers/watchdog/intel-mid_wdt.c static int mid_wdt_probe(struct platform_device *pdev) · line 159

Quoted 4 lines
drivers/watchdog/intel-mid_wdt.c +4 -0
+	 * 45 seconds and should be enough (less by 15 seconds than
+	 * threshold).
+	 */
+	set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

Giving one more thought here perhaps more robust will be to start watchdog unconditionally here?

In that case it will be no matter how it's configured in firmware and/or boot loader.

Quoted 3 lines


  ret = devm_watchdog_register_device(&pdev->dev, wdt_dev); if (ret) {

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
maintainer
#2 lore raw
Quoted 18 lines · Andy Shevchenko
Quoted 17 lines · Andy Shevchenko

Firmware followed by bootloader leaves watchdog running.

Keep it running in the driver.

User will not need any additional options to reboot in case of panic during boot.

drivers/watchdog/intel-mid_wdt.c +4 -2
-	/* Make sure the watchdog is not running */
-	wdt_stop(wdt_dev);
+	/*
+	 * Make sure the watchdog is serviced.
+	 *
+	 * The firmware followed by U-Boot leaves the watchdog

running

(quoted diff) +1 -0
+	 * with the default threshold 60 seconds. Our default timeout

is

(quoted diff) +1 -0
+	 * 90 seconds, but internal worker divides it by two, which
Quoted 9 lines

drivers/watchdog/intel-mid_wdt.c static int mid_wdt_probe(struct platform_device *pdev) · line 159

Quoted 5 lines

is

drivers/watchdog/intel-mid_wdt.c +4 -0
+	 * 45 seconds and should be enough (less by 15 seconds than
+	 * threshold).
+	 */
+	set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

Giving one more thought here perhaps more robust will be to start watchdog unconditionally here?

Yes, that would be another option, and it sounds like a good idea. Let me know if you want to do that.

Thanks,
Guenter

Quoted 10 lines

In that case it will be no matter how it's configured in firmware and/or boot loader.

Quoted 3 lines


  ret = devm_watchdog_register_device(&pdev->dev, wdt_dev); if (ret) {

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#3 lore raw
Quoted 23 lines · Guenter Roeck
Quoted 13 lines · Andy Shevchenko
Quoted 11 lines · Andy Shevchenko
drivers/watchdog/intel-mid_wdt.c +4 -0
+	/*
+	 * Make sure the watchdog is serviced.
+	 *
+	 * The firmware followed by U-Boot leaves the watchdog

running

(quoted diff) +1 -0
+	 * with the default threshold 60 seconds. Our default

timeout
is

(quoted diff) +1 -0
+	 * 90 seconds, but internal worker divides it by two,

which
is

Quoted 8 lines

drivers/watchdog/intel-mid_wdt.c static int mid_wdt_probe(struct platform_device *pdev) · line 159

Quoted 5 lines
(quoted diff) +1 -0
+	 * 45 seconds and should be enough (less by 15 seconds

than

drivers/watchdog/intel-mid_wdt.c +3 -0
+	 * threshold).
+	 */
+	set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

Giving one more thought here perhaps more robust will be to start watchdog unconditionally here?

Quoted 2 lines

Yes, that would be another option, and it sounds like a good idea. Let me know if you want to do that.

Yes, I prefer it over the rest of solutions.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
maintainer
#4 lore raw
Quoted 24 lines · Andy Shevchenko
Quoted 23 lines · Guenter Roeck
Quoted 21 lines · Andy Shevchenko
Quoted 13 lines · Andy Shevchenko
drivers/watchdog/intel-mid_wdt.c +4 -0
+	/*
+	 * Make sure the watchdog is serviced.
+	 *
+	 * The firmware followed by U-Boot leaves the watchdog

running

(quoted diff) +1 -0
+	 * with the default threshold 60 seconds. Our default

timeout
is

(quoted diff) +1 -0
+	 * 90 seconds, but internal worker divides it by two,

which
is

(quoted diff) +1 -0
+	 * 45 seconds and should be enough (less by 15 seconds

than

drivers/watchdog/intel-mid_wdt.c static int mid_wdt_probe(struct platform_device *pdev) · line 159

Quoted 3 lines
drivers/watchdog/intel-mid_wdt.c +3 -0
+	 * threshold).
+	 */
+	set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

Giving one more thought here perhaps more robust will be to start watchdog unconditionally here?

Quoted 5 lines
Quoted 2 lines

Yes, that would be another option, and it sounds like a good idea. Let me know if you want to do that.

Yes, I prefer it over the rest of solutions.

Ok, I'll wait for v3 of the patch.

Thanks,
Guenter