watchdog: intel-mid_wdt: Keep watchdog running
PATCH 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.
--- drivers/watchdog/intel-mid_wdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/watchdog/intel-mid_wdt.c +2 -2
@@ -147,8 +147,8 @@ 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 */ +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
Guenter Roeck
maintainerHi Andy,
Quoted 2 lines · Andy Shevchenko
Firmware followed by bootloader leaves watchdog running.
Is that always the case ? If not, does it hurt if the watchdog is pinged while not active ?
Also, this assumes that its default timeout is in the default range configured by the driver, ie not (much) lower than 30 seconds.
Not objecting, but it would help to have comments in the code explaining the context in some more detail.
Thanks,
Guenter
Quoted 25 lines
Keep it running in the driver.
User will not need any additional options to reboot in case of panic.
Signed-off-by Andy Shevchenko <[email protected]>--- drivers/watchdog/intel-mid_wdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/watchdog/intel-mid_wdt.c +2
-2@@ -147,8 +147,8 @@ 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 */ +set_bit (WDOG_HW_RUNNING , &wdt_dev->status); ret =devm_watchdog_register_device (&pdev->dev, wdt_dev);if (ret) {
Quoted 7 lines · Guenter Roeck
Hi Andy,
Quoted 2 lines · Andy Shevchenko
Firmware followed by bootloader leaves watchdog running.
Is that always the case ?
Yes if no-one patched boot loader specifically to avoid this.
Quoted 5 lines
If not, does it hurt if the watchdog is pinged
while not active ?Also, this assumes that its default timeout is in the default range configured by the driver, ie not (much) lower than 30 seconds.
Correct. It is hard coded in SCU firmware (60 seconds IIRC). U-Boot for now doesn't change it.
Quoted 3 lines
Not objecting, but it would help to have comments in the code explaining
the context in some more detail.
Last time you were not satisfied by the line which stops the running watchdog [1]. I found a way how to keep it running gracefully, which I proposed to do at that time (assuming boot loader doesn't stop it) [2].
I'm confused now what is preferable way to do the things?
P.S. I suppose previously we missed the chance to do this better and I perhaps have to add Fixes: tag as well.
[1] https://www.spinics.net/lists/linux-watchdog/msg10386.html
[2] https://www.spinics.net/lists/linux-watchdog/msg10377.html
Quoted 34 lines
Thanks,
GuenterQuoted 28 lines
Keep it running in the driver.
User will not need any additional options to reboot in case of panic.
Signed-off-by Andy Shevchenko <[email protected]>---
drivers/watchdog/intel-mid_wdt.c | 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/watchdog/intel-mid_wdt.c +2
-2@@ -147,8 +147,8 @@ 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 */ +set_bit (WDOG_HW_RUNNING , &wdt_dev->status); ret = devm_watchdog_register_device(&pdev->dev, wdt_dev);if (ret) {
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Guenter Roeck
maintainerQuoted 24 lines · Andy Shevchenko
Quoted 7 lines · Guenter Roeck
Hi Andy,
Quoted 2 lines · Andy Shevchenko
Firmware followed by bootloader leaves watchdog running.
Is that always the case ?
Yes if no-one patched boot loader specifically to avoid this.
Quoted 5 lines
If not, does it hurt if the watchdog is pinged
while not active ?Also, this assumes that its default timeout is in the default range configured by the driver, ie not (much) lower than 30 seconds.
Correct. It is hard coded in SCU firmware (60 seconds IIRC). U-Boot for now doesn't change it.
Quoted 3 lines
Not objecting, but it would help to have comments in the code explaining
the context in some more detail.
Quoted 6 lines
Last time you were not satisfied by the line which stops the running watchdog [1]. I found a way how to keep it running gracefully, which I proposed to do at that time (assuming boot loader doesn't stop it) [2].
I'm confused now what is preferable way to do the things?
The suggested patch is ok. I was just asking for additional comments, along the line of 1) the SCU firmware sets the timeout to 60s, the default timeout of 30s is therefore safe, and 2) that the watchdog is by default enabled in the SCU firmware (and, if appropriate, that it doesn't get a hiccup if it is pinged if it should have been disabled for some unspecified / unknown reason).
Thanks,
Guenter
Quoted 33 lines · Guenter Roeck
Quoted 23 lines · Andy Shevchenko
Quoted 4 lines · Guenter Roeck
Quoted 1 line · Andy Shevchenko
Firmware followed by bootloader leaves watchdog running.
Is that always the case ?
Yes if no-one patched boot loader specifically to avoid this.
Quoted 6 lines
If not, does it hurt if the watchdog is pinged
while not active ?Also, this assumes that its default timeout is in the default range
configured by the driver, ie not (much) lower than 30 seconds.Correct. It is hard coded in SCU firmware (60 seconds IIRC). U-Boot for
now doesn't change it.Quoted 3 lines
Not objecting, but it would help to have comments in the code explaining
the context in some more detail.Quoted 8 lines
Last time you were not satisfied by the line which stops the running watchdog [1]. I found a way how to keep it running gracefully, which I
proposed to do at that time (assuming boot loader doesn't stop it) [2].I'm confused now what is preferable way to do the things?
Quoted 10 lines
The suggested patch is ok. I was just asking for additional comments, along the line of 1) the SCU firmware sets the timeout to 60s, the default
timeout of 30s is therefore safe, and 2) that the watchdog is by default
enabled in the SCU firmware (and, if appropriate, that it doesn't get a
hiccup if it is pinged if it should have been disabled for some unspecified
/ unknown reason).
Okay, I will send v2 with additional comments.
Thanks for clarifying!
P.S. It looks like we have to add some lines in the driver for U-Boot to make developer pay attention to the potential asynchronization.
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy